Commit f00c577d authored by Małgorzata Ksionek's avatar Małgorzata Ksionek

Introduce changes from code review

parent eff3b3c0
...@@ -90,7 +90,7 @@ func TestLfsAuthenticateRequests(t *testing.T) { ...@@ -90,7 +90,7 @@ func TestLfsAuthenticateRequests(t *testing.T) {
}, },
}, },
{ {
Path: "/api/v4/internal/allowed/secure", Path: "/api/v4/internal/allowed",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body) b, err := ioutil.ReadAll(r.Body)
defer r.Body.Close() defer r.Body.Close()
......
...@@ -22,7 +22,7 @@ func TestCustomReceivePack(t *testing.T) { ...@@ -22,7 +22,7 @@ func TestCustomReceivePack(t *testing.T) {
requests := []testserver.TestRequestHandler{ requests := []testserver.TestRequestHandler{
{ {
Path: "/api/v4/internal/allowed/secure", Path: "/api/v4/internal/allowed",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body) b, err := ioutil.ReadAll(r.Body)
require.NoError(t, err) require.NoError(t, err)
......
...@@ -24,7 +24,7 @@ var ( ...@@ -24,7 +24,7 @@ var (
func setup(t *testing.T) (*Command, *bytes.Buffer, *bytes.Buffer, func()) { func setup(t *testing.T) (*Command, *bytes.Buffer, *bytes.Buffer, func()) {
requests := []testserver.TestRequestHandler{ requests := []testserver.TestRequestHandler{
{ {
Path: "/api/v4/internal/allowed/secure", Path: "/api/v4/internal/allowed",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body) b, err := ioutil.ReadAll(r.Body)
require.NoError(t, err) require.NoError(t, err)
......
...@@ -8,6 +8,7 @@ import ( ...@@ -8,6 +8,7 @@ import (
"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs"
"gitlab.com/gitlab-org/gitlab-shell/go/internal/config" "gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet" "gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet"
"gitlab.com/gitlab-org/gitlab-shell/go/internal/sshenv"
) )
const ( const (
...@@ -26,6 +27,7 @@ type Request struct { ...@@ -26,6 +27,7 @@ type Request struct {
Protocol string `json:"protocol"` Protocol string `json:"protocol"`
KeyId string `json:"key_id,omitempty"` KeyId string `json:"key_id,omitempty"`
Username string `json:"username,omitempty"` Username string `json:"username,omitempty"`
CheckIp string `json:"check_ip,omitempty"`
} }
type Gitaly struct { type Gitaly struct {
...@@ -80,7 +82,9 @@ func (c *Client) Verify(args *commandargs.Shell, action commandargs.CommandType, ...@@ -80,7 +82,9 @@ func (c *Client) Verify(args *commandargs.Shell, action commandargs.CommandType,
request.KeyId = args.GitlabKeyId request.KeyId = args.GitlabKeyId
} }
response, err := c.client.Post("/allowed/secure", request) request.CheckIp = sshenv.LocalAddr()
response, err := c.client.Post("/allowed", request)
if err != nil { if err != nil {
return nil, err return nil, err
} }
......
...@@ -157,7 +157,7 @@ func setup(t *testing.T) (*Client, func()) { ...@@ -157,7 +157,7 @@ func setup(t *testing.T) (*Client, func()) {
requests := []testserver.TestRequestHandler{ requests := []testserver.TestRequestHandler{
{ {
Path: "/api/v4/internal/allowed/secure", Path: "/api/v4/internal/allowed",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body) b, err := ioutil.ReadAll(r.Body)
require.NoError(t, err) require.NoError(t, err)
......
...@@ -10,7 +10,6 @@ import ( ...@@ -10,7 +10,6 @@ import (
"strings" "strings"
"gitlab.com/gitlab-org/gitlab-shell/go/internal/config" "gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
"gitlab.com/gitlab-org/gitlab-shell/go/internal/sshenv"
) )
const ( const (
...@@ -110,10 +109,6 @@ func (c *GitlabClient) DoRequest(method, path string, data interface{}) (*http.R ...@@ -110,10 +109,6 @@ func (c *GitlabClient) DoRequest(method, path string, data interface{}) (*http.R
request.Header.Set(secretHeaderName, encodedSecret) request.Header.Set(secretHeaderName, encodedSecret)
request.Header.Add("Content-Type", "application/json") request.Header.Add("Content-Type", "application/json")
ipAddr := sshenv.LocalAddr()
if ipAddr != "" {
request.Header.Add("X-Forwarded-For", ipAddr)
}
request.Close = true request.Close = true
......
...@@ -50,20 +50,6 @@ func TestClients(t *testing.T) { ...@@ -50,20 +50,6 @@ func TestClients(t *testing.T) {
fmt.Fprint(w, r.Header.Get(secretHeaderName)) fmt.Fprint(w, r.Header.Get(secretHeaderName))
}, },
}, },
{
Path: "/api/v4/internal/with_ip",
Handler: func(w http.ResponseWriter, r *http.Request) {
header := r.Header.Get("X-Forwarded-For")
require.Equal(t, "127.0.0.1", header)
},
},
{
Path: "/api/v4/internal/with_empty_ip",
Handler: func(w http.ResponseWriter, r *http.Request) {
header := r.Header.Get("X-Forwarded-For")
require.Equal(t, "", header)
},
},
{ {
Path: "/api/v4/internal/error", Path: "/api/v4/internal/error",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
...@@ -233,49 +219,3 @@ func testAuthenticationHeader(t *testing.T, client *GitlabClient) { ...@@ -233,49 +219,3 @@ func testAuthenticationHeader(t *testing.T, client *GitlabClient) {
assert.Equal(t, "sssh, it's a secret", string(header)) assert.Equal(t, "sssh, it's a secret", string(header))
}) })
} }
func testXForwardedForHeader(t *testing.T, client *GitlabClient) {
t.Run("X-Forwarded-For for GET", func(t *testing.T) {
cleanup, err := testhelper.Setenv("SSH_CONNECTION", "127.0.0.1 0")
require.NoError(t, err)
defer cleanup()
response, err := client.Get("/with_ip")
require.NoError(t, err)
require.NotNil(t, response)
response.Body.Close()
})
t.Run("X-Forwarded-For for POST", func(t *testing.T) {
data := map[string]string{"key": "value"}
cleanup, err := testhelper.Setenv("SSH_CONNECTION", "127.0.0.1 0")
require.NoError(t, err)
defer cleanup()
response, err := client.Post("/with_ip", data)
require.NoError(t, err)
require.NotNil(t, response)
response.Body.Close()
})
}
func testEmptyForwardedForHeader(t *testing.T, client *GitlabClient) {
t.Run("X-Forwarded-For empty for GET", func(t *testing.T) {
response, err := client.Get("/with_empty_ip")
require.NoError(t, err)
require.NotNil(t, response)
response.Body.Close()
})
t.Run("X-Forwarded-For empty for POST", func(t *testing.T) {
data := map[string]string{"key": "value"}
response, err := client.Post("/with_empty_ip", data)
require.NoError(t, err)
require.NotNil(t, response)
response.Body.Close()
})
}
...@@ -11,5 +11,5 @@ func LocalAddr() string { ...@@ -11,5 +11,5 @@ func LocalAddr() string {
if address != "" { if address != "" {
return strings.Fields(address)[0] return strings.Fields(address)[0]
} }
return address return ""
} }
...@@ -13,7 +13,7 @@ import ( ...@@ -13,7 +13,7 @@ import (
func BuildDisallowedByApiHandlers(t *testing.T) []testserver.TestRequestHandler { func BuildDisallowedByApiHandlers(t *testing.T) []testserver.TestRequestHandler {
requests := []testserver.TestRequestHandler{ requests := []testserver.TestRequestHandler{
{ {
Path: "/api/v4/internal/allowed/secure", Path: "/api/v4/internal/allowed",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
body := map[string]interface{}{ body := map[string]interface{}{
"status": false, "status": false,
...@@ -31,7 +31,7 @@ func BuildDisallowedByApiHandlers(t *testing.T) []testserver.TestRequestHandler ...@@ -31,7 +31,7 @@ func BuildDisallowedByApiHandlers(t *testing.T) []testserver.TestRequestHandler
func BuildAllowedWithGitalyHandlers(t *testing.T, gitalyAddress string) []testserver.TestRequestHandler { func BuildAllowedWithGitalyHandlers(t *testing.T, gitalyAddress string) []testserver.TestRequestHandler {
requests := []testserver.TestRequestHandler{ requests := []testserver.TestRequestHandler{
{ {
Path: "/api/v4/internal/allowed/secure", Path: "/api/v4/internal/allowed",
Handler: func(w http.ResponseWriter, r *http.Request) { Handler: func(w http.ResponseWriter, r *http.Request) {
body := map[string]interface{}{ body := map[string]interface{}{
"status": true, "status": true,
......
...@@ -77,7 +77,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength ...@@ -77,7 +77,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
end end
if @command == GIT_RECEIVE_PACK_COMMAND && access_status.custom_action? if @command == GIT_RECEIVE_PACK_COMMAND && access_status.custom_action?
# If the response from /api/v4/allowed/secure is a HTTP 300, we need to perform # If the response from /api/v4/allowed is a HTTP 300, we need to perform
# a Custom Action and therefore should return and not call process_cmd() # a Custom Action and therefore should return and not call process_cmd()
# #
return process_custom_action(access_status) return process_custom_action(access_status)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment