From ef05fd87fc561f78b57f4cebe12b53f8cc0c44e3 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Thu, 13 Oct 2022 16:25:44 -0500 Subject: [PATCH 1/4] feat: audit git ssh key regeneration --- coderd/database/databasefake/databasefake.go | 8 ++--- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 18 ++++++++--- coderd/database/queries/gitsshkeys.sql | 6 ++-- coderd/gitsshkey.go | 34 +++++++++++++------- coderd/gitsshkey_test.go | 8 +++++ 6 files changed, 54 insertions(+), 22 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index bdb4c8e0f0e40..da75ce5562244 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -2658,21 +2658,21 @@ func (q *fakeQuerier) GetGitSSHKey(_ context.Context, userID uuid.UUID) (databas return database.GitSSHKey{}, sql.ErrNoRows } -func (q *fakeQuerier) UpdateGitSSHKey(_ context.Context, arg database.UpdateGitSSHKeyParams) error { +func (q *fakeQuerier) UpdateGitSSHKey(_ context.Context, arg database.UpdateGitSSHKeyParams) (database.GitSSHKey, error) { q.mutex.Lock() defer q.mutex.Unlock() for index, key := range q.gitSSHKey { - if key.UserID.String() != arg.UserID.String() { + if key.UserID != arg.UserID { continue } key.UpdatedAt = arg.UpdatedAt key.PrivateKey = arg.PrivateKey key.PublicKey = arg.PublicKey q.gitSSHKey[index] = key - return nil + return key, nil } - return sql.ErrNoRows + return database.GitSSHKey{}, sql.ErrNoRows } func (q *fakeQuerier) InsertGroupMember(_ context.Context, arg database.InsertGroupMemberParams) error { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index b58f6abbccfb8..d7f6d1bfceee8 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -147,7 +147,7 @@ type sqlcQuerier interface { ParameterValue(ctx context.Context, id uuid.UUID) (ParameterValue, error) ParameterValues(ctx context.Context, arg ParameterValuesParams) ([]ParameterValue, error) UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDParams) error - UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) error + UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) (GitSSHKey, error) UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDParams) (Group, error) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRolesParams) (OrganizationMember, error) UpdateProvisionerDaemonByID(ctx context.Context, arg UpdateProvisionerDaemonByIDParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2ff1805cd47e1..7d804b3f80496 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -779,7 +779,7 @@ func (q *sqlQuerier) InsertGitSSHKey(ctx context.Context, arg InsertGitSSHKeyPar return i, err } -const updateGitSSHKey = `-- name: UpdateGitSSHKey :exec +const updateGitSSHKey = `-- name: UpdateGitSSHKey :one UPDATE gitsshkeys SET @@ -788,6 +788,8 @@ SET public_key = $4 WHERE user_id = $1 +RETURNING + user_id, created_at, updated_at, private_key, public_key ` type UpdateGitSSHKeyParams struct { @@ -797,14 +799,22 @@ type UpdateGitSSHKeyParams struct { PublicKey string `db:"public_key" json:"public_key"` } -func (q *sqlQuerier) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) error { - _, err := q.db.ExecContext(ctx, updateGitSSHKey, +func (q *sqlQuerier) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) (GitSSHKey, error) { + row := q.db.QueryRowContext(ctx, updateGitSSHKey, arg.UserID, arg.UpdatedAt, arg.PrivateKey, arg.PublicKey, ) - return err + var i GitSSHKey + err := row.Scan( + &i.UserID, + &i.CreatedAt, + &i.UpdatedAt, + &i.PrivateKey, + &i.PublicKey, + ) + return i, err } const deleteGroupByID = `-- name: DeleteGroupByID :exec diff --git a/coderd/database/queries/gitsshkeys.sql b/coderd/database/queries/gitsshkeys.sql index 1fe9c97fa16ff..4365e3349bd7e 100644 --- a/coderd/database/queries/gitsshkeys.sql +++ b/coderd/database/queries/gitsshkeys.sql @@ -18,7 +18,7 @@ FROM WHERE user_id = $1; --- name: UpdateGitSSHKey :exec +-- name: UpdateGitSSHKey :one UPDATE gitsshkeys SET @@ -26,7 +26,9 @@ SET private_key = $3, public_key = $4 WHERE - user_id = $1; + user_id = $1 +RETURNING + *; -- name: DeleteGitSSHKey :exec DELETE FROM diff --git a/coderd/gitsshkey.go b/coderd/gitsshkey.go index c65600e0d3990..357f5b2e44dab 100644 --- a/coderd/gitsshkey.go +++ b/coderd/gitsshkey.go @@ -3,6 +3,7 @@ package coderd import ( "net/http" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/gitsshkey" "github.com/coder/coder/coderd/httpapi" @@ -12,14 +13,32 @@ import ( ) func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - user := httpmw.UserParam(r) + var ( + ctx = r.Context() + user = httpmw.UserParam(r) + auditor = api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.GitSSHKey](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) + ) + defer commitAudit() if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) { httpapi.ResourceNotFound(rw) return } + oldKey, err := api.Database.GetGitSSHKey(ctx, user.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + aReq.Old = oldKey + privateKey, publicKey, err := gitsshkey.Generate(api.SSHKeygenAlgorithm) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -29,7 +48,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) { return } - err = api.Database.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ + newKey, err := api.Database.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ UserID: user.ID, UpdatedAt: database.Now(), PrivateKey: privateKey, @@ -43,14 +62,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) { return } - newKey, err := api.Database.GetGitSSHKey(ctx, user.ID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching user's git SSH key.", - Detail: err.Error(), - }) - return - } + aReq.New = newKey httpapi.Write(ctx, rw, http.StatusOK, codersdk.GitSSHKey{ UserID: newKey.UserID, diff --git a/coderd/gitsshkey_test.go b/coderd/gitsshkey_test.go index d5edc6c10fcd5..b1a60b6039c60 100644 --- a/coderd/gitsshkey_test.go +++ b/coderd/gitsshkey_test.go @@ -5,9 +5,12 @@ import ( "testing" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/gitsshkey" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" @@ -73,8 +76,10 @@ func TestGitSSHKey(t *testing.T) { }) t.Run("Regenerate", func(t *testing.T) { t.Parallel() + auditor := audit.NewMock() client := coderdtest.New(t, &coderdtest.Options{ SSHKeygenAlgorithm: gitsshkey.AlgorithmEd25519, + Auditor: auditor, }) res := coderdtest.CreateFirstUser(t, client) @@ -89,6 +94,9 @@ func TestGitSSHKey(t *testing.T) { require.GreaterOrEqual(t, key2.UpdatedAt, key1.UpdatedAt) require.NotEmpty(t, key2.PublicKey) require.NotEqual(t, key2.PublicKey, key1.PublicKey) + + require.Len(t, auditor.AuditLogs, 1) + assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[0].Action) }) } From 58704565eeb74d9e769613b3bcf3fe6dea94b0cb Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Fri, 14 Oct 2022 15:42:21 -0500 Subject: [PATCH 2/4] fixup! feat: audit git ssh key regeneration --- coderd/audit.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/coderd/audit.go b/coderd/audit.go index ea1acdb1b121f..f76a6565bce77 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -221,10 +221,18 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog { } func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { - return fmt.Sprintf("{user} %s %s {target}", + str := fmt.Sprintf("{user} %s %s", codersdk.AuditAction(alog.Action).FriendlyString(), codersdk.ResourceType(alog.ResourceType).FriendlyString(), ) + + // We don't display the name for git ssh keys. It's fairly long and doesn't + // make too much sense to display. + if alog.ResourceType != database.ResourceTypeGitSshKey { + str += " {target}" + } + + return str } // auditSearchQuery takes a query string and returns the auditLog filter. From e765ffc0f1f0d74582dee116380432c2eca5a507 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Fri, 14 Oct 2022 15:43:14 -0500 Subject: [PATCH 3/4] add git ssh keys back to audit log docs --- docs/admin/audit-logs.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index ec749caabb4a4..f883525b4ac86 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -7,6 +7,7 @@ their deployment. We track **create, update and delete** events for the following resources: +- Git SSH Keys - Template - TemplateVersion - Workspace From aa96afcebaf61e0836b2bb3428f42bf7248f5478 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Fri, 14 Oct 2022 15:46:37 -0500 Subject: [PATCH 4/4] fixup! add git ssh keys back to audit log docs --- docs/admin/audit-logs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index f883525b4ac86..a227ff6511ca8 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -7,7 +7,7 @@ their deployment. We track **create, update and delete** events for the following resources: -- Git SSH Keys +- GitSSHKey - Template - TemplateVersion - Workspace