Skip to content

Commit 08e728b

Browse files
EmyrkEmyrk
authored
chore: implement organization scoped audit log requests (coder#13663)
* chore: add organization_id filter to audit logs * chore: implement organization scoped audit log requests
1 parent 20e59e0 commit 08e728b

File tree

13 files changed

+123
-25
lines changed

13 files changed

+123
-25
lines changed

‎coderd/apidoc/docs.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/audit.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/coder/coder/v2/coderd/audit"
1919
"github.com/coder/coder/v2/coderd/database"
2020
"github.com/coder/coder/v2/coderd/database/db2sdk"
21+
"github.com/coder/coder/v2/coderd/database/dbauthz"
2122
"github.com/coder/coder/v2/coderd/httpapi"
2223
"github.com/coder/coder/v2/coderd/httpmw"
2324
"github.com/coder/coder/v2/coderd/searchquery"
@@ -61,6 +62,10 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
6162
}
6263

6364
dblogs, err := api.Database.GetAuditLogsOffset(ctx, filter)
65+
if dbauthz.IsNotAuthorizedError(err) {
66+
httpapi.Forbidden(rw)
67+
return
68+
}
6469
if err != nil {
6570
httpapi.InternalServerError(rw, err)
6671
return
@@ -139,6 +144,9 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
139144
if len(params.AdditionalFields) == 0 {
140145
params.AdditionalFields = json.RawMessage("{}")
141146
}
147+
if params.OrganizationID == uuid.Nil {
148+
params.OrganizationID = uuid.New()
149+
}
142150

143151
_, err = api.Database.InsertAuditLog(ctx, database.InsertAuditLogParams{
144152
ID: uuid.New(),
@@ -155,7 +163,7 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
155163
AdditionalFields: params.AdditionalFields,
156164
RequestID: uuid.Nil, // no request ID to attach this to
157165
ResourceIcon: "",
158-
OrganizationID: uuid.New(),
166+
OrganizationID: params.OrganizationID,
159167
})
160168
if err != nil {
161169
httpapi.InternalServerError(rw, err)

‎coderd/audit_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"net/http"
78
"strconv"
89
"testing"
910
"time"
1011

1112
"github.com/google/uuid"
1213
"github.com/stretchr/testify/require"
1314

15+
"cdr.dev/slog/sloggers/slogtest"
1416
"github.com/coder/coder/v2/coderd/audit"
1517
"github.com/coder/coder/v2/coderd/coderdtest"
1618
"github.com/coder/coder/v2/coderd/database"
@@ -135,6 +137,54 @@ func TestAuditLogs(t *testing.T) {
135137
require.Equal(t, auditLogs.AuditLogs[0].ResourceLink, fmt.Sprintf("/@%s/%s/builds/%s",
136138
workspace.OwnerName, workspace.Name, buildNumberString))
137139
})
140+
141+
t.Run("Organization", func(t *testing.T) {
142+
t.Parallel()
143+
144+
logger := slogtest.Make(t, &slogtest.Options{
145+
IgnoreErrors: true,
146+
})
147+
ctx := context.Background()
148+
client := coderdtest.New(t, &coderdtest.Options{
149+
Logger: &logger,
150+
})
151+
owner := coderdtest.CreateFirstUser(t, client)
152+
orgAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID))
153+
154+
err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
155+
ResourceID: owner.UserID,
156+
OrganizationID: owner.OrganizationID,
157+
})
158+
require.NoError(t, err)
159+
160+
// Add an extra audit log in another organization
161+
err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
162+
ResourceID: owner.UserID,
163+
OrganizationID: uuid.New(),
164+
})
165+
require.NoError(t, err)
166+
167+
// Fetching audit logs without an organization selector should fail
168+
_, err = orgAdmin.AuditLogs(ctx, codersdk.AuditLogsRequest{
169+
Pagination: codersdk.Pagination{
170+
Limit: 5,
171+
},
172+
})
173+
var sdkError *codersdk.Error
174+
require.Error(t, err)
175+
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
176+
require.Equal(t, http.StatusForbidden, sdkError.StatusCode())
177+
178+
// Using the organization selector allows the org admin to fetch audit logs
179+
alogs, err := orgAdmin.AuditLogs(ctx, codersdk.AuditLogsRequest{
180+
SearchQuery: fmt.Sprintf("organization_id:%s", owner.OrganizationID.String()),
181+
Pagination: codersdk.Pagination{
182+
Limit: 5,
183+
},
184+
})
185+
require.NoError(t, err)
186+
require.Len(t, alogs.AuditLogs, 1)
187+
})
138188
}
139189

140190
func TestAuditLogsFilter(t *testing.T) {

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,12 +1200,21 @@ func (q *querier) GetApplicationName(ctx context.Context) (string, error) {
12001200
}
12011201

12021202
func (q *querier) GetAuditLogsOffset(ctx context.Context, arg database.GetAuditLogsOffsetParams) ([]database.GetAuditLogsOffsetRow, error) {
1203-
// To optimize audit logs, we only check the global audit log permission once.
1204-
// This is because we expect a large unbounded set of audit logs, and applying a SQL
1205-
// filter would slow down the query for no benefit.
1206-
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceAuditLog); err != nil {
1203+
// To optimize the authz checks for audit logs, do not run an authorize
1204+
// check on each individual audit log row. In practice, audit logs are either
1205+
// fetched from a global or an organization scope.
1206+
// Applying a SQL filter would slow down the query for no benefit on how this query is
1207+
// actually used.
1208+
1209+
object := rbac.ResourceAuditLog
1210+
if arg.OrganizationID != uuid.Nil {
1211+
object = object.InOrg(arg.OrganizationID)
1212+
}
1213+
1214+
if err := q.authorizeContext(ctx, policy.ActionRead, object); err != nil {
12071215
return nil, err
12081216
}
1217+
12091218
return q.db.GetAuditLogsOffset(ctx, arg)
12101219
}
12111220

‎coderd/database/dbmem/dbmem.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,6 +1928,9 @@ func (q *FakeQuerier) GetAuditLogsOffset(_ context.Context, arg database.GetAudi
19281928
arg.Offset--
19291929
continue
19301930
}
1931+
if arg.OrganizationID != uuid.Nil && arg.OrganizationID != alog.OrganizationID {
1932+
continue
1933+
}
19311934
if arg.Action != "" && !strings.Contains(string(alog.Action), arg.Action) {
19321935
continue
19331936
}

‎coderd/database/queries.sql.go

Lines changed: 24 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/auditlogs.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ WHERE
5959
resource_id = @resource_id
6060
ELSE true
6161
END
62+
-- Filter organization_id
63+
AND CASE
64+
WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN
65+
audit_logs.organization_id = @organization_id
66+
ELSE true
67+
END
6268
-- Filter by resource_target
6369
AND CASE
6470
WHEN @resource_target :: text != '' THEN

‎coderd/members.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,11 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
176176
apiKey = httpmw.APIKey(r)
177177
auditor = api.Auditor.Load()
178178
aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationMember](rw, &audit.RequestParams{
179-
Audit: *auditor,
180-
Log: api.Logger,
181-
Request: r,
182-
Action: database.AuditActionWrite,
179+
OrganizationID: organization.ID,
180+
Audit: *auditor,
181+
Log: api.Logger,
182+
Request: r,
183+
Action: database.AuditActionWrite,
183184
})
184185
)
185186
aReq.Old = member.OrganizationMember.Auditable(member.Username)

‎coderd/searchquery/search.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func AuditLogs(query string) (database.GetAuditLogsOffsetParams, []codersdk.Vali
3030
const dateLayout = "2006-01-02"
3131
parser := httpapi.NewQueryParamParser()
3232
filter := database.GetAuditLogsOffsetParams{
33+
OrganizationID: parser.UUID(values, uuid.Nil, "organization_id"),
3334
ResourceID: parser.UUID(values, uuid.Nil, "resource_id"),
3435
ResourceTarget: parser.String(values, "", "resource_target"),
3536
Username: parser.String(values, "", "username"),

‎codersdk/audit.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ type CreateTestAuditLogRequest struct {
161161
AdditionalFields json.RawMessage `json:"additional_fields,omitempty"`
162162
Time time.Time `json:"time,omitempty" format:"date-time"`
163163
BuildReason BuildReason `json:"build_reason,omitempty" enums:"autostart,autostop,initiator"`
164+
OrganizationID uuid.UUID `json:"organization_id,omitempty" format:"uuid"`
164165
}
165166

166167
// AuditLogs retrieves audit logs from the given page.

‎docs/api/schemas.md

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎site/src/api/typesGenerated.ts

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)