- Notifications
You must be signed in to change notification settings - Fork1.1k
chore: refactor /insights API routes for testing#21051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Draft
spikecurtis wants to merge1 commit intomainChoose a base branch fromspike/coderd-api-test-refactor
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
+280 −123
Draft
Changes fromall commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
8 changes: 4 additions & 4 deletionsMakefile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
3 changes: 2 additions & 1 deletioncoderd/apikey.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
79 changes: 0 additions & 79 deletionscoderd/authorize.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
20 changes: 13 additions & 7 deletionscoderd/coderd.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
4 changes: 0 additions & 4 deletionscoderd/coderd_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
29 changes: 29 additions & 0 deletionscoderd/coderdtest/database.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package coderdtest | ||
| import ( | ||
| "sync/atomic" | ||
| "testing" | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "go.uber.org/mock/gomock" | ||
| "cdr.dev/slog" | ||
| "github.com/coder/coder/v2/coderd/database" | ||
| "github.com/coder/coder/v2/coderd/database/dbauthz" | ||
| "github.com/coder/coder/v2/coderd/database/dbmock" | ||
| "github.com/coder/coder/v2/coderd/rbac" | ||
| ) | ||
| func MockedDatabaseWithAuthz(t testing.TB, logger slog.Logger) (*gomock.Controller, *dbmock.MockStore, database.Store, rbac.Authorizer) { | ||
| ctrl := gomock.NewController(t) | ||
| mDB := dbmock.NewMockStore(ctrl) | ||
| auth := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry()) | ||
| accessControlStore := &atomic.Pointer[dbauthz.AccessControlStore]{} | ||
| var acs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{} | ||
| accessControlStore.Store(&acs) | ||
| // dbauthz will call Wrappers() to check for wrapped databases | ||
| mDB.EXPECT().Wrappers().Return([]string{}).AnyTimes() | ||
| authDB := dbauthz.New(mDB, auth, logger, accessControlStore) | ||
| return ctrl, mDB, authDB, auth | ||
| } |
14 changes: 14 additions & 0 deletionscoderd/coderdtest/subjects.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package coderdtest | ||
| import "github.com/coder/coder/v2/coderd/rbac" | ||
| func OwnerSubject() rbac.Subject { | ||
| return rbac.Subject{ | ||
| FriendlyName: "coderdtest-owner", | ||
| Email: "owner@coderd.test", | ||
| Type: rbac.SubjectTypeUser, | ||
| ID: "coderdtest-owner-id", | ||
| Roles: rbac.RoleIdentifiers{rbac.RoleOwner()}, | ||
| Scope: rbac.ScopeAll, | ||
| }.WithCachedASTValue() | ||
| } |
2 changes: 1 addition & 1 deletioncoderd/coderdtest/swagger_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
91 changes: 91 additions & 0 deletionscoderd/httpauthz/httpauthz.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| package httpauthz | ||
| import ( | ||
| "net/http" | ||
| "golang.org/x/xerrors" | ||
| "cdr.dev/slog" | ||
| "github.com/coder/coder/v2/coderd/httpmw" | ||
| "github.com/coder/coder/v2/coderd/rbac" | ||
| "github.com/coder/coder/v2/coderd/rbac/policy" | ||
| ) | ||
| // AuthorizationFilter takes a list of objects and returns the filtered list of | ||
| // objects that the user is authorized to perform the given action on. | ||
| // This is faster than calling Authorize() on each object. | ||
| func AuthorizationFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action policy.Action, objects []O) ([]O, error) { | ||
| roles := httpmw.UserAuthorization(r.Context()) | ||
| objects, err := rbac.Filter(r.Context(), h.Authorizer, roles, action, objects) | ||
| if err != nil { | ||
| // Log the error as Filter should not be erroring. | ||
| h.Logger.Error(r.Context(), "authorization filter failed", | ||
| slog.Error(err), | ||
| slog.F("user_id", roles.ID), | ||
| slog.F("username", roles), | ||
| slog.F("roles", roles.SafeRoleNames()), | ||
| slog.F("scope", roles.SafeScopeName()), | ||
| slog.F("route", r.URL.Path), | ||
| slog.F("action", action), | ||
| ) | ||
| return nil, err | ||
| } | ||
| return objects, nil | ||
| } | ||
| type HTTPAuthorizer struct { | ||
| Authorizer rbac.Authorizer | ||
| Logger slog.Logger | ||
| } | ||
| // AuthorizeSQLFilter returns an authorization filter that can used in a | ||
| // SQL 'WHERE' clause. If the filter is used, the resulting rows returned | ||
| // from postgres are already authorized, and the caller does not need to | ||
| // call 'Authorize()' on the returned objects. | ||
| // Note the authorization is only for the given action and object type. | ||
| func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action policy.Action, objectType string) (rbac.PreparedAuthorized, error) { | ||
| roles := httpmw.UserAuthorization(r.Context()) | ||
| prepared, err := h.Authorizer.Prepare(r.Context(), roles, action, objectType) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("prepare filter: %w", err) | ||
| } | ||
| return prepared, nil | ||
| } | ||
| // Authorize will return false if the user is not authorized to do the action. | ||
| // This function will log appropriately, but the caller must return an | ||
| // error to the api client. | ||
| // Eg: | ||
| // | ||
| //if !h.Authorize(...) { | ||
| //httpapi.Forbidden(rw) | ||
| //return | ||
| //} | ||
| func (h *HTTPAuthorizer) Authorize(r *http.Request, action policy.Action, object rbac.Objecter) bool { | ||
| roles := httpmw.UserAuthorization(r.Context()) | ||
| err := h.Authorizer.Authorize(r.Context(), roles, action, object.RBACObject()) | ||
| if err != nil { | ||
| // Log the errors for debugging | ||
| internalError := new(rbac.UnauthorizedError) | ||
| logger := h.Logger | ||
| if xerrors.As(err, internalError) { | ||
| logger = h.Logger.With(slog.F("internal_error", internalError.Internal())) | ||
| } | ||
| // Log information for debugging. This will be very helpful | ||
| // in the early days | ||
| logger.Warn(r.Context(), "requester is not authorized to access the object", | ||
| slog.F("roles", roles.SafeRoleNames()), | ||
| slog.F("actor_id", roles.ID), | ||
| slog.F("actor_name", roles), | ||
| slog.F("scope", roles.SafeScopeName()), | ||
| slog.F("route", r.URL.Path), | ||
| slog.F("action", action), | ||
| slog.F("object", object), | ||
| ) | ||
| return false | ||
| } | ||
| return true | ||
| } |
27 changes: 27 additions & 0 deletionscoderd/insightsapi/api.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package insightsapi | ||
| import ( | ||
| "cdr.dev/slog" | ||
| "github.com/coder/coder/v2/coderd/database" | ||
| "github.com/coder/coder/v2/coderd/httpauthz" | ||
| ) | ||
| typeAPIstruct { | ||
| logger slog.Logger | ||
| authorizer*httpauthz.HTTPAuthorizer | ||
| database database.Store | ||
| } | ||
| funcNewAPI( | ||
| logger slog.Logger, | ||
| db database.Store, | ||
| authorizer*httpauthz.HTTPAuthorizer, | ||
| )*API { | ||
| a:=&API{ | ||
| logger:logger.Named("insightsapi"), | ||
| authorizer:authorizer, | ||
| database:db, | ||
| } | ||
| returna | ||
| } |
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.