Skip to content

Commit 4309188

Browse files
Vasilii IakliushinEmma Park
authored and
GitLab
committed
Merge branch 'fix-deploy-key-audit-event' into 'main'
Pass key_id to git_audit_event endpoint for deploy key auth See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/1411 Merged-by: Vasilii Iakliushin <viakliushin@gitlab.com> Approved-by: Vasilii Iakliushin <viakliushin@gitlab.com> Reviewed-by: Vasilii Iakliushin <viakliushin@gitlab.com> Reviewed-by: GitLab Duo <gitlab-duo@gitlab.com> Co-authored-by: Emma Park <epark@gitlab.com>
2 parents 195365f + 40a78c5 commit 4309188

4 files changed

Lines changed: 128 additions & 47 deletions

File tree

‎internal/command/gitauditevent/audit.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,12 @@ func Audit(ctx context.Context, args *commandargs.Shell, c *config.Config, respo
3333
return
3434
}
3535

36-
errOnlyLog = gitAuditClient.Audit(ctx, response.Username, args, response.Repo, packfileStats)
36+
errOnlyLog = gitAuditClient.Audit(ctx, gitauditevent.AuditParams{
37+
Username: response.Username,
38+
KeyID: response.KeyID,
39+
Repo: response.Repo,
40+
PackfileStats: packfileStats,
41+
}, args)
3742
if errOnlyLog != nil {
3843
slog.ErrorContext(ctx, fmt.Sprintf("failed to audit git event: %v", errOnlyLog))
3944
return

‎internal/command/gitauditevent/audit_test.go‎

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,41 +20,64 @@ import (
2020
var (
2121
testUsername = "gitlab-shell"
2222
testRepo = "project-1"
23+
testKeyID = 123
2324
)
2425

2526
func TestGitAudit(t *testing.T) {
26-
called := false
27-
28-
requests := []testserver.TestRequestHandler{
29-
{
30-
Path: "/api/v4/internal/shellhorse/git_audit_event",
31-
Handler: func(w http.ResponseWriter, r *http.Request) {
32-
called = true
33-
34-
body, err := io.ReadAll(r.Body)
35-
assert.NoError(t, err)
36-
defer r.Body.Close()
37-
38-
var request *gitauditevent.Request
39-
assert.NoError(t, json.Unmarshal(body, &request))
40-
assert.Equal(t, testUsername, request.Username)
41-
assert.Equal(t, testRepo, request.Repo)
42-
43-
w.WriteHeader(http.StatusOK)
44-
},
45-
},
27+
tests := []struct {
28+
name string
29+
keyID int
30+
expectKeyID bool
31+
}{
32+
{name: "with deploy key", keyID: testKeyID, expectKeyID: true},
33+
{name: "without deploy key", keyID: 0, expectKeyID: false},
4634
}
4735

48-
args := &commandargs.Shell{
49-
CommandType: commandargs.UploadArchive,
50-
Env: sshenv.Env{RemoteAddr: "18.245.0.42"},
51-
}
36+
for _, tt := range tests {
37+
t.Run(tt.name, func(t *testing.T) {
38+
called := false
39+
requests := []testserver.TestRequestHandler{{
40+
Path: "/api/v4/internal/shellhorse/git_audit_event",
41+
Handler: func(w http.ResponseWriter, r *http.Request) {
42+
called = true
43+
44+
body, err := io.ReadAll(r.Body)
45+
assert.NoError(t, err)
46+
defer r.Body.Close()
47+
48+
var rawJSON map[string]interface{}
49+
assert.NoError(t, json.Unmarshal(body, &rawJSON))
50+
_, hasKeyID := rawJSON["key_id"]
51+
assert.Equal(t, tt.expectKeyID, hasKeyID)
52+
53+
if tt.expectKeyID {
54+
keyIDFloat, ok := rawJSON["key_id"].(float64)
55+
assert.True(t, ok, "key_id should be a number")
56+
assert.Equal(t, tt.keyID, int(keyIDFloat))
57+
}
5258

53-
url := testserver.StartSocketHTTPServer(t, requests)
54-
Audit(context.Background(), args, &config.Config{GitlabURL: url}, &accessverifier.Response{
55-
Username: testUsername,
56-
Repo: testRepo,
57-
}, nil)
59+
var request *gitauditevent.Request
60+
assert.NoError(t, json.Unmarshal(body, &request))
61+
assert.Equal(t, testUsername, request.Username)
62+
assert.Equal(t, testRepo, request.Repo)
5863

59-
require.True(t, called)
64+
w.WriteHeader(http.StatusOK)
65+
},
66+
}}
67+
68+
args := &commandargs.Shell{
69+
CommandType: commandargs.UploadArchive,
70+
Env: sshenv.Env{RemoteAddr: "18.245.0.42"},
71+
}
72+
73+
url := testserver.StartSocketHTTPServer(t, requests)
74+
Audit(context.Background(), args, &config.Config{GitlabURL: url}, &accessverifier.Response{
75+
Username: testUsername,
76+
Repo: testRepo,
77+
KeyID: tt.keyID,
78+
}, nil)
79+
80+
require.True(t, called)
81+
})
82+
}
6083
}

‎internal/gitlabnet/gitauditevent/client.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,29 @@ type Request struct {
3636
Protocol string `json:"protocol"`
3737
Repo string `json:"gl_repository"`
3838
Username string `json:"username"`
39+
KeyID int `json:"key_id,omitempty"`
3940
PackfileStats *pb.PackfileNegotiationStatistics `json:"packfile_stats,omitempty"`
4041
CheckIP string `json:"check_ip,omitempty"`
4142
Changes string `json:"changes"`
4243
}
4344

45+
// AuditParams contains parameters for sending an audit event.
46+
type AuditParams struct {
47+
Username string
48+
KeyID int
49+
Repo string
50+
PackfileStats *pb.PackfileNegotiationStatistics
51+
}
52+
4453
// Audit sends an audit event to the GitLab API.
45-
func (c *Client) Audit(ctx context.Context, username string, args *commandargs.Shell, repo string, packfileStats *pb.PackfileNegotiationStatistics) error {
54+
func (c *Client) Audit(ctx context.Context, params AuditParams, args *commandargs.Shell) error {
4655
request := &Request{
4756
Action: args.CommandType,
48-
Repo: repo,
57+
Repo: params.Repo,
4958
Protocol: "ssh",
50-
Username: username,
51-
PackfileStats: packfileStats,
59+
Username: params.Username,
60+
KeyID: params.KeyID,
61+
PackfileStats: params.PackfileStats,
5262
CheckIP: gitlabnet.ParseIP(args.Env.RemoteAddr),
5363
Changes: "_any",
5464
}

‎internal/gitlabnet/gitauditevent/client_test.go‎

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
var (
2020
testUsername = "gitlab-shell"
21+
testKeyID = 123
2122
testRepo = "gitlab-org/gitlab-shell"
2223
testPackfileWants int64 = 100
2324
testPackfileHaves int64 = 100
@@ -28,26 +29,57 @@ var (
2829
)
2930

3031
func TestAudit(t *testing.T) {
31-
client := setup(t, http.StatusOK)
32+
tests := []struct {
33+
name string
34+
keyID int
35+
expectKeyID bool
36+
}{
37+
{
38+
name: "with key_id",
39+
keyID: testKeyID,
40+
expectKeyID: true,
41+
},
42+
{
43+
name: "without key_id",
44+
keyID: 0,
45+
expectKeyID: false,
46+
},
47+
}
3248

33-
err := client.Audit(context.Background(), testUsername, testArgs, testRepo, &pb.PackfileNegotiationStatistics{
34-
Wants: testPackfileWants,
35-
Haves: testPackfileHaves,
36-
})
37-
require.NoError(t, err)
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
client := setup(t, http.StatusOK, tt.keyID, tt.expectKeyID)
52+
53+
err := client.Audit(context.Background(), AuditParams{
54+
Username: testUsername,
55+
KeyID: tt.keyID,
56+
Repo: testRepo,
57+
PackfileStats: &pb.PackfileNegotiationStatistics{
58+
Wants: testPackfileWants,
59+
Haves: testPackfileHaves,
60+
},
61+
}, testArgs)
62+
require.NoError(t, err)
63+
})
64+
}
3865
}
3966

4067
func TestAuditFailed(t *testing.T) {
41-
client := setup(t, http.StatusBadRequest)
68+
client := setup(t, http.StatusBadRequest, testKeyID, true)
4269

43-
err := client.Audit(context.Background(), testUsername, testArgs, testRepo, &pb.PackfileNegotiationStatistics{
44-
Wants: testPackfileWants,
45-
Haves: testPackfileHaves,
46-
})
70+
err := client.Audit(context.Background(), AuditParams{
71+
Username: testUsername,
72+
KeyID: testKeyID,
73+
Repo: testRepo,
74+
PackfileStats: &pb.PackfileNegotiationStatistics{
75+
Wants: testPackfileWants,
76+
Haves: testPackfileHaves,
77+
},
78+
}, testArgs)
4779
require.Error(t, err)
4880
}
4981

50-
func setup(t *testing.T, responseStatus int) *Client {
82+
func setup(t *testing.T, responseStatus int, keyID int, expectKeyID bool) *Client {
5183
requests := []testserver.TestRequestHandler{
5284
{
5385
Path: uri,
@@ -56,9 +88,20 @@ func setup(t *testing.T, responseStatus int) *Client {
5688
assert.NoError(t, err)
5789
defer r.Body.Close()
5890

91+
// Check if key_id is present/absent in raw JSON
92+
var rawJSON map[string]interface{}
93+
assert.NoError(t, json.Unmarshal(body, &rawJSON))
94+
_, hasKeyID := rawJSON["key_id"]
95+
if expectKeyID {
96+
assert.True(t, hasKeyID, "key_id should be present in JSON")
97+
} else {
98+
assert.False(t, hasKeyID, "key_id should not be present in JSON")
99+
}
100+
59101
var request *Request
60102
assert.NoError(t, json.Unmarshal(body, &request))
61103
assert.Equal(t, testUsername, request.Username)
104+
assert.Equal(t, keyID, request.KeyID)
62105
assert.Equal(t, testArgs.Env.RemoteAddr, request.CheckIP)
63106
assert.Equal(t, testArgs.CommandType, request.Action)
64107
assert.Equal(t, testRepo, request.Repo)

0 commit comments

Comments
 (0)