From f4c519016bdd75e82114fc93c9f78ab293e60946 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 14 Sep 2022 20:31:59 +0000 Subject: [PATCH 1/9] feat: add apikey scopes --- coderd/authorize.go | 10 ++-- coderd/coderdtest/authtest.go | 12 +++-- coderd/database/databasefake/databasefake.go | 1 + coderd/database/dump.sql | 8 ++- .../migrations/000050_apikey_scope.down.sql | 6 +++ .../migrations/000050_apikey_scope.up.sql | 6 +++ coderd/database/modelmethods.go | 31 +++++++++++ coderd/database/models.go | 20 +++++++ coderd/database/queries.sql.go | 14 +++-- coderd/database/queries/apikeys.sql | 5 +- coderd/httpmw/apikey.go | 29 +++++++--- coderd/httpmw/apikey_test.go | 53 +++++++++++++++++++ coderd/httpmw/authorize_test.go | 3 +- coderd/httpmw/organizationparam_test.go | 1 + coderd/httpmw/templateparam_test.go | 1 + coderd/httpmw/templateversionparam_test.go | 1 + coderd/httpmw/userparam_test.go | 1 + coderd/httpmw/workspaceagentparam_test.go | 1 + coderd/httpmw/workspacebuildparam_test.go | 1 + coderd/httpmw/workspaceparam_test.go | 2 + coderd/members.go | 2 +- .../prometheusmetrics_test.go | 5 ++ coderd/provisionerjobs_internal_test.go | 1 + coderd/rbac/authz.go | 36 +++++++++---- coderd/rbac/authz_internal_test.go | 21 ++++---- coderd/rbac/builtin_test.go | 18 ++++++- coderd/rbac/partial.go | 49 +++++++++++++++-- coderd/rbac/scopes.go | 45 ++++++++++++++++ coderd/roles.go | 7 +-- coderd/telemetry/telemetry_test.go | 1 + coderd/users.go | 3 +- 31 files changed, 341 insertions(+), 53 deletions(-) create mode 100644 coderd/database/migrations/000050_apikey_scope.down.sql create mode 100644 coderd/database/migrations/000050_apikey_scope.up.sql create mode 100644 coderd/rbac/scopes.go diff --git a/coderd/authorize.go b/coderd/authorize.go index 55310cee78755..b21f6a19fcffe 100644 --- a/coderd/authorize.go +++ b/coderd/authorize.go @@ -11,14 +11,15 @@ import ( ) func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) { - roles := httpmw.AuthorizationUserRoles(r) - objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, action, objects) + roles := httpmw.UserAuthorization(r) + objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, objects) if err != nil { // Log the error as Filter should not be erroring. h.Logger.Error(r.Context(), "filter failed", slog.Error(err), slog.F("user_id", roles.ID), slog.F("username", roles.Username), + slog.F("scope", roles.Scope), slog.F("route", r.URL.Path), slog.F("action", action), ) @@ -55,8 +56,8 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec // return // } func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool { - roles := httpmw.AuthorizationUserRoles(r) - err := h.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject()) + roles := httpmw.UserAuthorization(r) + err := h.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, object.RBACObject()) if err != nil { // Log the errors for debugging internalError := new(rbac.UnauthorizedError) @@ -70,6 +71,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r slog.F("roles", roles.Roles), slog.F("user_id", roles.ID), slog.F("username", roles.Username), + slog.F("scope", roles.Scope), slog.F("route", r.URL.Path), slog.F("action", action), slog.F("object", object), diff --git a/coderd/coderdtest/authtest.go b/coderd/coderdtest/authtest.go index 6eb3df8ac6bc5..2af292a0e3daa 100644 --- a/coderd/coderdtest/authtest.go +++ b/coderd/coderdtest/authtest.go @@ -518,6 +518,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck type authCall struct { SubjectID string Roles []string + Scope rbac.Scope Action rbac.Action Object rbac.Object } @@ -527,21 +528,25 @@ type recordingAuthorizer struct { AlwaysReturn error } -func (r *recordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error { +var _ rbac.Authorizer = (*recordingAuthorizer)(nil) + +func (r *recordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.Scope, action rbac.Action, object rbac.Object) error { r.Called = &authCall{ SubjectID: subjectID, Roles: roleNames, + Scope: scope, Action: action, Object: object, } return r.AlwaysReturn } -func (r *recordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { +func (r *recordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.Scope, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { return &fakePreparedAuthorizer{ Original: r, SubjectID: subjectID, Roles: roles, + Scope: scope, Action: action, }, nil } @@ -554,9 +559,10 @@ type fakePreparedAuthorizer struct { Original *recordingAuthorizer SubjectID string Roles []string + Scope rbac.Scope Action rbac.Action } func (f *fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error { - return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Action, object) + return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Scope, f.Action, object) } diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 76c0d278b8abe..9f518a43c3872 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1564,6 +1564,7 @@ func (q *fakeQuerier) InsertAPIKey(_ context.Context, arg database.InsertAPIKeyP UpdatedAt: arg.UpdatedAt, LastUsed: arg.LastUsed, LoginType: arg.LoginType, + Scope: arg.Scope, } q.apiKeys = append(q.apiKeys, key) return key, nil diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 4b91a6bdd5f08..e496b34eb30fd 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1,5 +1,10 @@ -- Code generated by 'make coderd/database/generate'. DO NOT EDIT. +CREATE TYPE api_key_scope AS ENUM ( + 'any', + 'devurls' +); + CREATE TYPE audit_action AS ENUM ( 'create', 'write', @@ -109,7 +114,8 @@ CREATE TABLE api_keys ( updated_at timestamp with time zone NOT NULL, login_type login_type NOT NULL, lifetime_seconds bigint DEFAULT 86400 NOT NULL, - ip_address inet DEFAULT '0.0.0.0'::inet NOT NULL + ip_address inet DEFAULT '0.0.0.0'::inet NOT NULL, + scope api_key_scope DEFAULT 'any'::public.api_key_scope NOT NULL ); CREATE TABLE audit_logs ( diff --git a/coderd/database/migrations/000050_apikey_scope.down.sql b/coderd/database/migrations/000050_apikey_scope.down.sql new file mode 100644 index 0000000000000..73506f8e8dd68 --- /dev/null +++ b/coderd/database/migrations/000050_apikey_scope.down.sql @@ -0,0 +1,6 @@ +-- Avoid "upgrading" devurl keys to fully fledged API keys. +DELETE FROM api_keys WHERE scope != 'any'; + +ALTER TABLE api_keys DROP COLUMN scope; + +DROP TYPE api_key_scope; diff --git a/coderd/database/migrations/000050_apikey_scope.up.sql b/coderd/database/migrations/000050_apikey_scope.up.sql new file mode 100644 index 0000000000000..73eafabb40992 --- /dev/null +++ b/coderd/database/migrations/000050_apikey_scope.up.sql @@ -0,0 +1,6 @@ +CREATE TYPE api_key_scope AS ENUM ( + 'any', + 'devurls' +); + +ALTER TABLE api_keys ADD COLUMN scope api_key_scope NOT NULL DEFAULT 'any'; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 6df4d67716f7d..ba46c95a6fc57 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -1,9 +1,40 @@ package database import ( + "golang.org/x/xerrors" + "github.com/coder/coder/coderd/rbac" ) +var validAPIKeyScopes map[string]ApiKeyScope + +func init() { + validAPIKeyScopes = make(map[string]ApiKeyScope) + for _, scope := range []ApiKeyScope{ApiKeyScopeAny, ApiKeyScopeDevurls} { + validAPIKeyScopes[string(scope)] = scope + } +} + +func ToAPIKeyScope(v string) (ApiKeyScope, error) { + scope, ok := validAPIKeyScopes[v] + if !ok { + return ApiKeyScope(""), xerrors.Errorf("invalid token scope: %s", v) + } + + return scope, nil +} + +func (s ApiKeyScope) ToRBAC() rbac.Scope { + switch { + case s == ApiKeyScopeAny: + return rbac.ScopeAny + case s == ApiKeyScopeDevurls: + return rbac.ScopeDevURLs + default: + panic("developer error: unknown scope type " + string(s)) + } +} + func (t Template) RBACObject() rbac.Object { return rbac.ResourceTemplate.InOrg(t.OrganizationID) } diff --git a/coderd/database/models.go b/coderd/database/models.go index c850b011bdffb..ad95e7fbd3972 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -14,6 +14,25 @@ import ( "github.com/tabbed/pqtype" ) +type ApiKeyScope string + +const ( + ApiKeyScopeAny ApiKeyScope = "any" + ApiKeyScopeDevurls ApiKeyScope = "devurls" +) + +func (e *ApiKeyScope) Scan(src interface{}) error { + switch s := src.(type) { + case []byte: + *e = ApiKeyScope(s) + case string: + *e = ApiKeyScope(s) + default: + return fmt.Errorf("unsupported scan type for ApiKeyScope: %T", src) + } + return nil +} + type AuditAction string const ( @@ -324,6 +343,7 @@ type APIKey struct { LoginType LoginType `db:"login_type" json:"login_type"` LifetimeSeconds int64 `db:"lifetime_seconds" json:"lifetime_seconds"` IPAddress pqtype.Inet `db:"ip_address" json:"ip_address"` + Scope ApiKeyScope `db:"scope" json:"scope"` } type AgentStat struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 88dd2091a7718..6a009d34114ce 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -128,7 +128,7 @@ func (q *sqlQuerier) DeleteAPIKeyByID(ctx context.Context, id string) error { const getAPIKeyByID = `-- name: GetAPIKeyByID :one SELECT - id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address + id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope FROM api_keys WHERE @@ -151,12 +151,13 @@ func (q *sqlQuerier) GetAPIKeyByID(ctx context.Context, id string) (APIKey, erro &i.LoginType, &i.LifetimeSeconds, &i.IPAddress, + &i.Scope, ) return i, err } const getAPIKeysLastUsedAfter = `-- name: GetAPIKeysLastUsedAfter :many -SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address FROM api_keys WHERE last_used > $1 +SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope FROM api_keys WHERE last_used > $1 ` func (q *sqlQuerier) GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Time) ([]APIKey, error) { @@ -179,6 +180,7 @@ func (q *sqlQuerier) GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time. &i.LoginType, &i.LifetimeSeconds, &i.IPAddress, + &i.Scope, ); err != nil { return nil, err } @@ -205,7 +207,8 @@ INSERT INTO expires_at, created_at, updated_at, - login_type + login_type, + scope ) VALUES ($1, @@ -214,7 +217,7 @@ VALUES WHEN 0 THEN 86400 ELSE $2::bigint END - , $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address + , $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope ` type InsertAPIKeyParams struct { @@ -228,6 +231,7 @@ type InsertAPIKeyParams struct { CreatedAt time.Time `db:"created_at" json:"created_at"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` LoginType LoginType `db:"login_type" json:"login_type"` + Scope ApiKeyScope `db:"scope" json:"scope"` } func (q *sqlQuerier) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) { @@ -242,6 +246,7 @@ func (q *sqlQuerier) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) ( arg.CreatedAt, arg.UpdatedAt, arg.LoginType, + arg.Scope, ) var i APIKey err := row.Scan( @@ -255,6 +260,7 @@ func (q *sqlQuerier) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) ( &i.LoginType, &i.LifetimeSeconds, &i.IPAddress, + &i.Scope, ) return i, err } diff --git a/coderd/database/queries/apikeys.sql b/coderd/database/queries/apikeys.sql index 22ce2e6057f3e..7ee97b3beaa79 100644 --- a/coderd/database/queries/apikeys.sql +++ b/coderd/database/queries/apikeys.sql @@ -23,7 +23,8 @@ INSERT INTO expires_at, created_at, updated_at, - login_type + login_type, + scope ) VALUES (@id, @@ -32,7 +33,7 @@ VALUES WHEN 0 THEN 86400 ELSE @lifetime_seconds::bigint END - , @hashed_secret, @ip_address, @user_id, @last_used, @expires_at, @created_at, @updated_at, @login_type) RETURNING *; + , @hashed_secret, @ip_address, @user_id, @last_used, @expires_at, @created_at, @updated_at, @login_type, @scope) RETURNING *; -- name: UpdateAPIKeyByID :exec UPDATE diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 65c156b53173a..b9c27cf0ddf02 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -35,16 +35,23 @@ func APIKey(r *http.Request) database.APIKey { } // User roles are the 'subject' field of Authorize() -type userRolesKey struct{} +type userAuthKey struct{} -// AuthorizationUserRoles returns the roles used for authorization. -// Comes from the ExtractAPIKey handler. -func AuthorizationUserRoles(r *http.Request) database.GetAuthorizationUserRolesRow { - userRoles, ok := r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow) +type Authorization struct { + ID uuid.UUID + Username string + Roles []string + Scope database.ApiKeyScope +} + +// UserAuthorization returns the roles and scope used for authorization. Depends +// on the ExtractAPIKey handler. +func UserAuthorization(r *http.Request) Authorization { + auth, ok := r.Context().Value(userAuthKey{}).(Authorization) if !ok { - panic("developer error: user roles middleware not provided") + panic("developer error: ExtractAPIKey middleware not provided") } - return userRoles + return auth } // OAuth2Configs is a collection of configurations for OAuth-based authentication. @@ -324,7 +331,13 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool ctx := r.Context() ctx = context.WithValue(ctx, apiKeyContextKey{}, key) - ctx = context.WithValue(ctx, userRolesKey{}, roles) + ctx = context.WithValue(ctx, userAuthKey{}, Authorization{ + ID: key.UserID, + Username: roles.Username, + Roles: roles.Roles, + Scope: key.Scope, + }) + next.ServeHTTP(rw, r.WithContext(ctx)) }) } diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index fc7bf92af781b..cf7c7d9e20b77 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" @@ -145,6 +146,7 @@ func TestAPIKey(t *testing.T) { ID: id, HashedSecret: hashed[:], UserID: user.ID, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -170,6 +172,7 @@ func TestAPIKey(t *testing.T) { HashedSecret: hashed[:], UserID: user.ID, LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -196,6 +199,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -215,6 +219,49 @@ func TestAPIKey(t *testing.T) { require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt) }) + t.Run("ValidWithScope", func(t *testing.T) { + t.Parallel() + var ( + db = databasefake.New() + id, secret = randomAPIKeyParts() + hashed = sha256.Sum256([]byte(secret)) + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) + ) + r.AddCookie(&http.Cookie{ + Name: codersdk.SessionTokenKey, + Value: fmt.Sprintf("%s-%s", id, secret), + }) + + yeah, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{ + ID: id, + UserID: user.ID, + HashedSecret: hashed[:], + ExpiresAt: database.Now().AddDate(0, 0, 1), + LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeDevurls, + }) + require.NoError(t, err) + + fmt.Printf("%+v\n", yeah) + + httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + // Checks that it exists on the context! + apiKey := httpmw.APIKey(r) + fmt.Printf("%+v\n", apiKey) + assert.Equal(t, database.ApiKeyScopeDevurls, apiKey.Scope) + + httpapi.Write(rw, http.StatusOK, codersdk.Response{ + Message: "it worked!", + }) + })).ServeHTTP(rw, r) + + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + }) + t.Run("QueryParameter", func(t *testing.T) { t.Parallel() var ( @@ -235,6 +282,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -268,6 +316,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -301,6 +350,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().Add(time.Minute), UserID: user.ID, LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -334,6 +384,7 @@ func TestAPIKey(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) @@ -373,6 +424,7 @@ func TestAPIKey(t *testing.T) { LoginType: database.LoginTypeGithub, LastUsed: database.Now(), UserID: user.ID, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) _, err = db.InsertUserLink(r.Context(), database.InsertUserLinkParams{ @@ -425,6 +477,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index 32e2742ccd47f..f8f99c89bb5fb 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -87,7 +87,7 @@ func TestExtractUserRoles(t *testing.T) { httpmw.ExtractAPIKey(db, &httpmw.OAuth2Configs{}, false), ) rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) { - roles := httpmw.AuthorizationUserRoles(r) + roles := httpmw.UserAuthorization(r) require.ElementsMatch(t, user.ID, roles.ID) require.ElementsMatch(t, expRoles, roles.Roles) }) @@ -124,6 +124,7 @@ func addUser(t *testing.T, db database.Store, roles ...string) (database.User, s LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/httpmw/organizationparam_test.go b/coderd/httpmw/organizationparam_test.go index 0da000802879f..d15e1dc3c8a88 100644 --- a/coderd/httpmw/organizationparam_test.go +++ b/coderd/httpmw/organizationparam_test.go @@ -51,6 +51,7 @@ func TestOrganizationParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, chi.NewRouteContext())) diff --git a/coderd/httpmw/templateparam_test.go b/coderd/httpmw/templateparam_test.go index 1e936b403ee5a..055ffbc4b1e08 100644 --- a/coderd/httpmw/templateparam_test.go +++ b/coderd/httpmw/templateparam_test.go @@ -51,6 +51,7 @@ func TestTemplateParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/httpmw/templateversionparam_test.go b/coderd/httpmw/templateversionparam_test.go index 6c28cb743f6ff..1070ef2529015 100644 --- a/coderd/httpmw/templateversionparam_test.go +++ b/coderd/httpmw/templateversionparam_test.go @@ -51,6 +51,7 @@ func TestTemplateVersionParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/httpmw/userparam_test.go b/coderd/httpmw/userparam_test.go index 0b5e6013c38d2..0e274da5876df 100644 --- a/coderd/httpmw/userparam_test.go +++ b/coderd/httpmw/userparam_test.go @@ -45,6 +45,7 @@ func TestUserParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/httpmw/workspaceagentparam_test.go b/coderd/httpmw/workspaceagentparam_test.go index 435308bd3d4c9..476ea224d423a 100644 --- a/coderd/httpmw/workspaceagentparam_test.go +++ b/coderd/httpmw/workspaceagentparam_test.go @@ -51,6 +51,7 @@ func TestWorkspaceAgentParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/httpmw/workspacebuildparam_test.go b/coderd/httpmw/workspacebuildparam_test.go index c0028347ed3d7..61ccd7ced4958 100644 --- a/coderd/httpmw/workspacebuildparam_test.go +++ b/coderd/httpmw/workspacebuildparam_test.go @@ -51,6 +51,7 @@ func TestWorkspaceBuildParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/httpmw/workspaceparam_test.go b/coderd/httpmw/workspaceparam_test.go index 7dfaa55724996..82502f7691418 100644 --- a/coderd/httpmw/workspaceparam_test.go +++ b/coderd/httpmw/workspaceparam_test.go @@ -54,6 +54,7 @@ func TestWorkspaceParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) @@ -359,6 +360,7 @@ func setupWorkspaceWithAgents(t testing.TB, cfg setupConfig) (database.Store, *h LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) diff --git a/coderd/members.go b/coderd/members.go index d3b83f5db85d3..d270d6682cf75 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -21,7 +21,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) { organization := httpmw.OrganizationParam(r) member := httpmw.OrganizationMemberParam(r) apiKey := httpmw.APIKey(r) - actorRoles := httpmw.AuthorizationUserRoles(r) + actorRoles := httpmw.UserAuthorization(r) if apiKey.UserID == member.UserID { httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index e85c5399e93af..3531a94dfa25b 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -38,6 +38,7 @@ func TestActiveUsers(t *testing.T) { _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), + Scope: database.ApiKeyScopeAny, }) return db }, @@ -49,12 +50,14 @@ func TestActiveUsers(t *testing.T) { _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), + Scope: database.ApiKeyScopeAny, }) // Because this API key hasn't been used in the past hour, this shouldn't // add to the user count. _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now().Add(-2 * time.Hour), + Scope: database.ApiKeyScopeAny, }) return db }, @@ -66,10 +69,12 @@ func TestActiveUsers(t *testing.T) { _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), + Scope: database.ApiKeyScopeAny, }) _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), + Scope: database.ApiKeyScopeAny, }) return db }, diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go index 67004661d9583..e557ec0ed6524 100644 --- a/coderd/provisionerjobs_internal_test.go +++ b/coderd/provisionerjobs_internal_test.go @@ -77,6 +77,7 @@ func TestProvisionerJobLogs_Unit(t *testing.T) { UserID: userID, ExpiresAt: time.Now().Add(5 * time.Hour), LoginType: database.LoginTypePassword, + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) _, err = fDB.InsertUser(ctx, database.InsertUserParams{ diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index d124aae29bc24..ff04d88d5cf6e 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -10,8 +10,8 @@ import ( ) type Authorizer interface { - ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error - PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) + ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, action Action, object Object) error + PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, action Action, objectType string) (PreparedAuthorized, error) } type PreparedAuthorized interface { @@ -21,7 +21,7 @@ type PreparedAuthorized interface { // Filter takes in a list of objects, and will filter the list removing all // the elements the subject does not have permission for. All objects must be // of the same type. -func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objects []O) ([]O, error) { +func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, scope Scope, action Action, objects []O) ([]O, error) { if len(objects) == 0 { // Nothing to filter return objects, nil @@ -29,7 +29,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub objectType := objects[0].RBACObject().Type filtered := make([]O, 0) - prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, action, objectType) + prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, scope, action, objectType) if err != nil { return nil, xerrors.Errorf("prepare: %w", err) } @@ -53,6 +53,8 @@ type RegoAuthorizer struct { query rego.PreparedEvalQuery } +var _ Authorizer = (*RegoAuthorizer)(nil) + // Load the policy from policy.rego in this directory. // //go:embed policy.rego @@ -81,13 +83,27 @@ type authSubject struct { // ByRoleName will expand all roleNames into roles before calling Authorize(). // This is the function intended to be used outside this package. // The role is fetched from the builtin map located in memory. -func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error { +func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, action Action, object Object) error { roles, err := RolesByNames(roleNames) if err != nil { return err } - return a.Authorize(ctx, subjectID, roles, action, object) + err = a.Authorize(ctx, subjectID, roles, action, object) + if err != nil { + return err + } + + // If the scope isn't "any", we need to check with the scope's role as well. + if scope != ScopeAny { + scopeRole := builtinScopes[scope] + err = a.Authorize(ctx, subjectID, []Role{scopeRole}, action, object) + if err != nil { + return err + } + } + + return nil } // Authorize allows passing in custom Roles. @@ -116,8 +132,8 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [ // Prepare will partially execute the rego policy leaving the object fields unknown (except for the type). // This will vastly speed up performance if batch authorization on the same type of objects is needed. -func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*PartialAuthorizer, error) { - auth, err := newPartialAuthorizer(ctx, subjectID, roles, action, objectType) +func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, scope Scope, action Action, objectType string) (*PartialAuthorizer, error) { + auth, err := newPartialAuthorizer(ctx, subjectID, roles, scope, action, objectType) if err != nil { return nil, xerrors.Errorf("new partial authorizer: %w", err) } @@ -125,11 +141,11 @@ func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Rol return auth, nil } -func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) { +func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, action Action, objectType string) (PreparedAuthorized, error) { roles, err := RolesByNames(roleNames) if err != nil { return nil, err } - return a.Prepare(ctx, subjectID, roles, action, objectType) + return a.Prepare(ctx, subjectID, roles, scope, action, objectType) } diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index b91130d0f4def..0530bb3a1ae08 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -13,7 +13,6 @@ import ( "github.com/coder/coder/testutil" ) -// subject is required because rego needs type subject struct { UserID string `json:"id"` // For the unit test we want to pass in the roles directly, instead of just @@ -42,7 +41,7 @@ func TestFilterError(t *testing.T) { auth, err := NewAuthorizer() require.NoError(t, err) - _, err = Filter(context.Background(), auth, uuid.NewString(), []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace}) + _, err = Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAny, ActionRead, []Object{ResourceUser, ResourceWorkspace}) require.ErrorContains(t, err, "object types must be uniform") } @@ -158,7 +157,8 @@ func TestFilter(t *testing.T) { var allowedCount int for i, obj := range localObjects { obj.Type = tc.ObjectType - err := auth.ByRoleName(ctx, tc.SubjectID, tc.Roles, ActionRead, obj.RBACObject()) + // TODO: scopes + err := auth.ByRoleName(ctx, tc.SubjectID, tc.Roles, ScopeAny, ActionRead, obj.RBACObject()) obj.Allowed = err == nil if err == nil { allowedCount++ @@ -167,7 +167,7 @@ func TestFilter(t *testing.T) { } // Run by filter - list, err := Filter(ctx, auth, tc.SubjectID, tc.Roles, tc.Action, localObjects) + list, err := Filter(ctx, auth, tc.SubjectID, tc.Roles, ScopeAny, tc.Action, localObjects) require.NoError(t, err) require.Equal(t, allowedCount, len(list), "expected number of allowed") for _, obj := range list { @@ -666,23 +666,24 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes assert.Error(t, authError, "expected unauthorized") } - partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, a, c.resource.Type) + // TODO: scopes + partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, ScopeAny, a, c.resource.Type) require.NoError(t, err, "make prepared authorizer") // Also check the rego policy can form a valid partial query result. // This ensures we can convert the queries into SQL WHERE clauses in the future. // If this function returns 'Support' sections, then we cannot convert the query into SQL. - if len(partialAuthz.partialQueries.Support) > 0 { - d, _ := json.Marshal(partialAuthz.input) + if len(partialAuthz.mainAuthorizer.partialQueries.Support) > 0 { + d, _ := json.Marshal(partialAuthz.mainAuthorizer.input) t.Logf("input: %s", string(d)) - for _, q := range partialAuthz.partialQueries.Queries { + for _, q := range partialAuthz.mainAuthorizer.partialQueries.Queries { t.Logf("query: %+v", q.String()) } - for _, s := range partialAuthz.partialQueries.Support { + for _, s := range partialAuthz.mainAuthorizer.partialQueries.Support { t.Logf("support: %+v", s.String()) } } - require.Equal(t, 0, len(partialAuthz.partialQueries.Support), "expected 0 support rules") + require.Equal(t, 0, len(partialAuthz.mainAuthorizer.partialQueries.Support), "expected 0 support rules") partialErr := partialAuthz.Authorize(ctx, c.resource) if authError != nil { diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index d357271943b8b..b5204c5037dd9 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -33,28 +33,33 @@ func BenchmarkRBACFilter(b *testing.B) { Name string Roles []string UserID uuid.UUID + Scope rbac.Scope }{ { Name: "NoRoles", Roles: []string{}, UserID: users[0], + Scope: rbac.ScopeAny, }, { Name: "Admin", // Give some extra roles that an admin might have Roles: []string{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, UserID: users[0], + Scope: rbac.ScopeAny, }, { Name: "OrgAdmin", Roles: []string{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgAdmin(orgs[0]), rbac.RoleMember()}, UserID: users[0], + Scope: rbac.ScopeAny, }, { Name: "OrgMember", // Member of 2 orgs Roles: []string{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgMember(orgs[1]), rbac.RoleMember()}, UserID: users[0], + Scope: rbac.ScopeAny, }, { Name: "ManyRoles", @@ -66,6 +71,14 @@ func BenchmarkRBACFilter(b *testing.B) { rbac.RoleMember(), }, UserID: users[0], + Scope: rbac.ScopeAny, + }, + { + Name: "AdminWithScope", + // Give some extra roles that an admin might have + Roles: []string{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, + UserID: users[0], + Scope: rbac.ScopeDevURLs, }, } @@ -77,7 +90,7 @@ func BenchmarkRBACFilter(b *testing.B) { b.Run(c.Name, func(b *testing.B) { objects := benchmarkSetup(orgs, users, b.N) b.ResetTimer() - allowed, err := rbac.Filter(context.Background(), authorizer, c.UserID.String(), c.Roles, rbac.ActionRead, objects) + allowed, err := rbac.Filter(context.Background(), authorizer, c.UserID.String(), c.Roles, c.Scope, rbac.ActionRead, objects) require.NoError(b, err) var _ = allowed }) @@ -335,7 +348,8 @@ func TestRolePermissions(t *testing.T) { for _, subj := range subjs { delete(remainingSubjs, subj.Name) msg := fmt.Sprintf("%s as %q doing %q on %q", c.Name, subj.Name, action, c.Resource.Type) - err := auth.ByRoleName(context.Background(), subj.UserID, subj.Roles, action, c.Resource) + // TODO: scopey + err := auth.ByRoleName(context.Background(), subj.UserID, subj.Roles, rbac.ScopeAny, action, c.Resource) if result { assert.NoError(t, err, fmt.Sprintf("Should pass: %s", msg)) } else { diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index 8ff0f1d17593f..fd693fb0069e6 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -9,6 +9,49 @@ import ( ) type PartialAuthorizer struct { + // mainAuthorizer is used for the user's roles. It is always not-nil. + mainAuthorizer *subPartialAuthorizer + // scopeAuthorizer is used for the API key scope. It may be nil. + scopeAuthorizer *subPartialAuthorizer +} + +var _ PreparedAuthorized = (*PartialAuthorizer)(nil) + +func (pa *PartialAuthorizer) Authorize(ctx context.Context, object Object) error { + err := pa.mainAuthorizer.Authorize(ctx, object) + if err != nil { + return err + } + + if pa.scopeAuthorizer != nil { + return pa.scopeAuthorizer.Authorize(ctx, object) + } + + return nil +} + +func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, scope Scope, action Action, objectType string) (*PartialAuthorizer, error) { + pAuth, err := newSubPartialAuthorizer(ctx, subjectID, roles, action, objectType) + if err != nil { + return nil, err + } + + var scopeAuth *subPartialAuthorizer + if scope != ScopeAny { + scopeRole := builtinScopes[scope] + scopeAuth, err = newSubPartialAuthorizer(ctx, subjectID, []Role{scopeRole}, action, objectType) + if err != nil { + return nil, err + } + } + + return &PartialAuthorizer{ + mainAuthorizer: pAuth, + scopeAuthorizer: scopeAuth, + }, nil +} + +type subPartialAuthorizer struct { // partialQueries is mainly used for unit testing to assert our rego policy // can always be compressed into a set of queries. partialQueries *rego.PartialQueries @@ -23,7 +66,7 @@ type PartialAuthorizer struct { alwaysTrue bool } -func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*PartialAuthorizer, error) { +func newSubPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*subPartialAuthorizer, error) { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, @@ -50,7 +93,7 @@ func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, a return nil, xerrors.Errorf("prepare: %w", err) } - pAuth := &PartialAuthorizer{ + pAuth := &subPartialAuthorizer{ partialQueries: partialQueries, preparedQueries: []rego.PreparedEvalQuery{}, input: input, @@ -82,7 +125,7 @@ func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, a } // Authorize authorizes a single object using the partially prepared queries. -func (a PartialAuthorizer) Authorize(ctx context.Context, object Object) error { +func (a subPartialAuthorizer) Authorize(ctx context.Context, object Object) error { if a.alwaysTrue { return nil } diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go new file mode 100644 index 0000000000000..1f3875aa569ee --- /dev/null +++ b/coderd/rbac/scopes.go @@ -0,0 +1,45 @@ +package rbac + +import ( + "fmt" + + "golang.org/x/xerrors" +) + +type Scope string + +const ( + ScopeAny Scope = "any" + ScopeDevURLs Scope = "devurls" +) + +var builtinScopes map[Scope]Role = map[Scope]Role{ + // ScopeAny is a special scope that allows access to all resources. During + // authorize checks it is usually not used directly and skips scope checks. + ScopeAny: { + Name: fmt.Sprintf("Scope_%s", ScopeAny), + DisplayName: "Any operation", + Site: permissions(map[Object][]Action{ + ResourceWildcard: {WildcardSymbol}, + }), + Org: map[string][]Permission{}, + User: []Permission{}, + }, + + // TODO: fill in the site permissions + ScopeDevURLs: { + Name: fmt.Sprintf("Scope_%s", ScopeDevURLs), + DisplayName: "Ability to connect to devurls", + Site: permissions(map[Object][]Action{}), + Org: map[string][]Permission{}, + User: []Permission{}, + }, +} + +func ScopeRole(scope Scope) (Role, error) { + role, ok := builtinScopes[scope] + if !ok { + return Role{}, xerrors.Errorf("no scope named %q", scope) + } + return role, nil +} diff --git a/coderd/roles.go b/coderd/roles.go index 3370d2248b99b..cfac554cde0dc 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -13,7 +13,7 @@ import ( // assignableSiteRoles returns all site wide roles that can be assigned. func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) { - actorRoles := httpmw.AuthorizationUserRoles(r) + actorRoles := httpmw.UserAuthorization(r) if !api.Authorize(r, rbac.ActionRead, rbac.ResourceRoleAssignment) { httpapi.Forbidden(rw) return @@ -26,7 +26,7 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) { // assignableSiteRoles returns all site wide roles that can be assigned. func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { organization := httpmw.OrganizationParam(r) - actorRoles := httpmw.AuthorizationUserRoles(r) + actorRoles := httpmw.UserAuthorization(r) if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) { httpapi.Forbidden(rw) @@ -39,6 +39,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) { user := httpmw.UserParam(r) + apiKey := httpmw.APIKey(r) if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser) { httpapi.ResourceNotFound(rw) @@ -69,7 +70,7 @@ func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) { if v.Object.OwnerID == "me" { v.Object.OwnerID = roles.ID.String() } - err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.Action(v.Action), + err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, apiKey.Scope.ToRBAC(), rbac.Action(v.Action), rbac.Object{ Owner: v.Object.OwnerID, OrgID: v.Object.OrganizationID, diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index 4e78b19ec8c54..0d2af1f659090 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -34,6 +34,7 @@ func TestTelemetry(t *testing.T) { _, err := db.InsertAPIKey(ctx, database.InsertAPIKeyParams{ ID: uuid.NewString(), LastUsed: database.Now(), + Scope: database.ApiKeyScopeAny, }) require.NoError(t, err) _, err = db.InsertParameterSchema(ctx, database.InsertParameterSchemaParams{ diff --git a/coderd/users.go b/coderd/users.go index 0447130343c0e..d473f24c467ee 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -696,7 +696,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { var ( // User is the user to modify. user = httpmw.UserParam(r) - actorRoles = httpmw.AuthorizationUserRoles(r) + actorRoles = httpmw.UserAuthorization(r) apiKey = httpmw.APIKey(r) aReq, commitAudit = audit.InitRequest[database.User](rw, &audit.RequestParams{ Features: api.FeaturesService, @@ -1073,6 +1073,7 @@ func (api *API) createAPIKey(r *http.Request, params createAPIKeyParams) (*http. UpdatedAt: database.Now(), HashedSecret: hashed[:], LoginType: params.LoginType, + Scope: database.ApiKeyScopeAny, }) if err != nil { return nil, xerrors.Errorf("insert API key: %w", err) From db015efaf5f2569b7f4d922ef26a65891690d582 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 15 Sep 2022 18:19:00 +0000 Subject: [PATCH 2/9] devurls -> application_connect --- coderd/database/dump.sql | 2 +- .../database/migrations/000050_apikey_scope.up.sql | 2 +- coderd/database/modelmethods.go | 14 +++++--------- coderd/database/models.go | 4 ++-- coderd/httpmw/apikey_test.go | 4 ++-- coderd/rbac/builtin_test.go | 2 +- coderd/rbac/scopes.go | 10 +++++----- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index e496b34eb30fd..cc459ae173ef0 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2,7 +2,7 @@ CREATE TYPE api_key_scope AS ENUM ( 'any', - 'devurls' + 'application_connect' ); CREATE TYPE audit_action AS ENUM ( diff --git a/coderd/database/migrations/000050_apikey_scope.up.sql b/coderd/database/migrations/000050_apikey_scope.up.sql index 73eafabb40992..62f47a8935ab2 100644 --- a/coderd/database/migrations/000050_apikey_scope.up.sql +++ b/coderd/database/migrations/000050_apikey_scope.up.sql @@ -1,6 +1,6 @@ CREATE TYPE api_key_scope AS ENUM ( 'any', - 'devurls' + 'application_connect' ); ALTER TABLE api_keys ADD COLUMN scope api_key_scope NOT NULL DEFAULT 'any'; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index ba46c95a6fc57..d4ec26ae4c495 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -6,13 +6,9 @@ import ( "github.com/coder/coder/coderd/rbac" ) -var validAPIKeyScopes map[string]ApiKeyScope - -func init() { - validAPIKeyScopes = make(map[string]ApiKeyScope) - for _, scope := range []ApiKeyScope{ApiKeyScopeAny, ApiKeyScopeDevurls} { - validAPIKeyScopes[string(scope)] = scope - } +var validAPIKeyScopes = map[string]ApiKeyScope{ + string(ApiKeyScopeAny): ApiKeyScopeAny, + string(ApiKeyScopeApplicationConnect): ApiKeyScopeApplicationConnect, } func ToAPIKeyScope(v string) (ApiKeyScope, error) { @@ -28,8 +24,8 @@ func (s ApiKeyScope) ToRBAC() rbac.Scope { switch { case s == ApiKeyScopeAny: return rbac.ScopeAny - case s == ApiKeyScopeDevurls: - return rbac.ScopeDevURLs + case s == ApiKeyScopeApplicationConnect: + return rbac.ScopeApplicationConnect default: panic("developer error: unknown scope type " + string(s)) } diff --git a/coderd/database/models.go b/coderd/database/models.go index ad95e7fbd3972..a341086e05155 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -17,8 +17,8 @@ import ( type ApiKeyScope string const ( - ApiKeyScopeAny ApiKeyScope = "any" - ApiKeyScopeDevurls ApiKeyScope = "devurls" + ApiKeyScopeAny ApiKeyScope = "any" + ApiKeyScopeApplicationConnect ApiKeyScope = "application_connect" ) func (e *ApiKeyScope) Scan(src interface{}) error { diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index cf7c7d9e20b77..3a328a75c4aca 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -240,7 +240,7 @@ func TestAPIKey(t *testing.T) { HashedSecret: hashed[:], ExpiresAt: database.Now().AddDate(0, 0, 1), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeDevurls, + Scope: database.ApiKeyScopeApplicationConnect, }) require.NoError(t, err) @@ -250,7 +250,7 @@ func TestAPIKey(t *testing.T) { // Checks that it exists on the context! apiKey := httpmw.APIKey(r) fmt.Printf("%+v\n", apiKey) - assert.Equal(t, database.ApiKeyScopeDevurls, apiKey.Scope) + assert.Equal(t, database.ApiKeyScopeApplicationConnect, apiKey.Scope) httpapi.Write(rw, http.StatusOK, codersdk.Response{ Message: "it worked!", diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index b5204c5037dd9..b3b1d25198b80 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -78,7 +78,7 @@ func BenchmarkRBACFilter(b *testing.B) { // Give some extra roles that an admin might have Roles: []string{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, UserID: users[0], - Scope: rbac.ScopeDevURLs, + Scope: rbac.ScopeApplicationConnect, }, } diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 1f3875aa569ee..774b503f73748 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -9,8 +9,8 @@ import ( type Scope string const ( - ScopeAny Scope = "any" - ScopeDevURLs Scope = "devurls" + ScopeAny Scope = "any" + ScopeApplicationConnect Scope = "application_connect" ) var builtinScopes map[Scope]Role = map[Scope]Role{ @@ -27,9 +27,9 @@ var builtinScopes map[Scope]Role = map[Scope]Role{ }, // TODO: fill in the site permissions - ScopeDevURLs: { - Name: fmt.Sprintf("Scope_%s", ScopeDevURLs), - DisplayName: "Ability to connect to devurls", + ScopeApplicationConnect: { + Name: fmt.Sprintf("Scope_%s", ScopeApplicationConnect), + DisplayName: "Ability to connect to applications", Site: permissions(map[Object][]Action{}), Org: map[string][]Permission{}, User: []Permission{}, From ee3b3c9bd0436a931f1279ddb3325415b4a3d6f9 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 15 Sep 2022 19:00:23 +0000 Subject: [PATCH 3/9] chore: add rbac scope tests --- coderd/rbac/authz_internal_test.go | 82 +++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 0530bb3a1ae08..c08ae467be8a3 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -3,6 +3,7 @@ package rbac import ( "context" "encoding/json" + "fmt" "testing" "github.com/google/uuid" @@ -19,6 +20,7 @@ type subject struct { // by name. This allows us to test custom roles that do not exist in the product, // but test edge cases of the implementation. Roles []Role `json:"roles"` + Scope Scope `json:"scope"` } type fakeObject struct { @@ -74,6 +76,7 @@ func TestFilter(t *testing.T) { SubjectID string Roles []string Action Action + Scope Scope ObjectType string }{ { @@ -138,6 +141,13 @@ func TestFilter(t *testing.T) { ObjectType: ResourceOrganization.Type, Action: ActionRead, }, + { + Name: "ScopeApplicationConnect", + SubjectID: userIDs[0].String(), + Roles: []string{RoleOrgMember(orgIDs[0]), "auditor", RoleOwner(), RoleMember()}, + ObjectType: ResourceWorkspace.Type, + Action: ActionRead, + }, } for _, tc := range testCases { @@ -153,12 +163,17 @@ func TestFilter(t *testing.T) { auth, err := NewAuthorizer() require.NoError(t, err, "new auth") + scope := ScopeAny + if tc.Scope != "" { + scope = tc.Scope + } + // Run auth 1 by 1 var allowedCount int for i, obj := range localObjects { obj.Type = tc.ObjectType // TODO: scopes - err := auth.ByRoleName(ctx, tc.SubjectID, tc.Roles, ScopeAny, ActionRead, obj.RBACObject()) + err := auth.ByRoleName(ctx, tc.SubjectID, tc.Roles, scope, ActionRead, obj.RBACObject()) obj.Allowed = err == nil if err == nil { allowedCount++ @@ -167,7 +182,7 @@ func TestFilter(t *testing.T) { } // Run by filter - list, err := Filter(ctx, auth, tc.SubjectID, tc.Roles, ScopeAny, tc.Action, localObjects) + list, err := Filter(ctx, auth, tc.SubjectID, tc.Roles, scope, tc.Action, localObjects) require.NoError(t, err) require.Equal(t, allowedCount, len(list), "expected number of allowed") for _, obj := range list { @@ -614,6 +629,49 @@ func TestAuthorizeLevels(t *testing.T) { })) } +func TestAuthorizeScope(t *testing.T) { + t.Parallel() + + defOrg := uuid.New() + unusedID := uuid.New() + user := subject{ + UserID: "me", + Roles: []Role{ + must(RoleByName(RoleOwner())), + must(RoleByName(RoleMember())), + }, + } + + user.Scope = ScopeApplicationConnect + testAuthorize(t, "ScopeApplicationConnect", user, []authTestCase{ + // Org + me + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(defOrg), actions: allActions(), allow: false}, + + {resource: ResourceWorkspace.WithOwner(user.UserID), actions: allActions(), allow: false}, + + {resource: ResourceWorkspace.All(), actions: allActions(), allow: false}, + + // Other org + me + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(unusedID), actions: allActions(), allow: false}, + + // Other org + other user + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: allActions(), allow: false}, + + {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, + + // Other org + other use + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(unusedID), actions: allActions(), allow: false}, + + {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, + + // TODO: add allowed cases when scope application_connect actually has + // permissions + }) +} + // cases applies a given function to all test cases. This makes generalities easier to create. func cases(opt func(c authTestCase) authTestCase, cases []authTestCase) []authTestCase { if opt == nil { @@ -636,13 +694,26 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes authorizer, err := NewAuthorizer() require.NoError(t, err) for _, cases := range sets { - for _, c := range cases { - t.Run(name, func(t *testing.T) { + for i, c := range cases { + c := c + caseName := fmt.Sprintf("%s/%d", name, i) + t.Run(caseName, func(t *testing.T) { t.Parallel() for _, a := range c.actions { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) t.Cleanup(cancel) + + scope := ScopeAny + if subject.Scope != "" { + scope = subject.Scope + } + authError := authorizer.Authorize(ctx, subject.UserID, subject.Roles, a, c.resource) + if authError == nil && scope != ScopeAny { + scopeRole := builtinScopes[scope] + authError = authorizer.Authorize(ctx, subject.UserID, []Role{scopeRole}, a, c.resource) + } + // Logging only if authError != nil { var uerr *UnauthorizedError @@ -666,8 +737,7 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes assert.Error(t, authError, "expected unauthorized") } - // TODO: scopes - partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, ScopeAny, a, c.resource.Type) + partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, scope, a, c.resource.Type) require.NoError(t, err, "make prepared authorizer") // Also check the rego policy can form a valid partial query result. From 9c06467b6097db1e71747af23168b5914f7ef367 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 15 Sep 2022 19:13:37 +0000 Subject: [PATCH 4/9] feat: add WorkspaceApplicationConnect permission --- coderd/coderdtest/authtest.go | 6 ++++-- coderd/database/modelmethods.go | 4 ++++ coderd/httpmw/apikey_test.go | 5 +---- coderd/rbac/builtin_test.go | 10 ++++++++++ coderd/rbac/object.go | 9 +++++++++ coderd/rbac/scopes.go | 9 +++++---- coderd/workspaceapps.go | 4 ++-- 7 files changed, 35 insertions(+), 12 deletions(-) diff --git a/coderd/coderdtest/authtest.go b/coderd/coderdtest/authtest.go index 2af292a0e3daa..2ba404d7c4254 100644 --- a/coderd/coderdtest/authtest.go +++ b/coderd/coderdtest/authtest.go @@ -163,6 +163,8 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { // Some quick reused objects workspaceRBACObj := rbac.ResourceWorkspace.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) workspaceExecObj := rbac.ResourceWorkspaceExecution.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) + applicationConnectObj := rbac.ResourceWorkspaceApplicationConnect.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) + // skipRoutes allows skipping routes from being checked. skipRoutes := map[string]string{ "POST:/api/v2/users/logout": "Logging out deletes the API Key for other routes", @@ -408,11 +410,11 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { assertAllHTTPMethods("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}/*", RouteCheck{ AssertAction: rbac.ActionCreate, - AssertObject: workspaceExecObj, + AssertObject: applicationConnectObj, }) assertAllHTTPMethods("/@{user}/{workspace_and_agent}/apps/{workspaceapp}/*", RouteCheck{ AssertAction: rbac.ActionCreate, - AssertObject: workspaceExecObj, + AssertObject: applicationConnectObj, }) return skipRoutes, assertRoute diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index d4ec26ae4c495..9c14039e97c11 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -48,6 +48,10 @@ func (w Workspace) ExecutionRBAC() rbac.Object { return rbac.ResourceWorkspaceExecution.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) } +func (w Workspace) ApplicationConnectRBAC() rbac.Object { + return rbac.ResourceWorkspaceApplicationConnect.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) +} + func (m OrganizationMember) RBACObject() rbac.Object { return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID) } diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index 3a328a75c4aca..8e06935dc6765 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -234,7 +234,7 @@ func TestAPIKey(t *testing.T) { Value: fmt.Sprintf("%s-%s", id, secret), }) - yeah, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{ + _, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{ ID: id, UserID: user.ID, HashedSecret: hashed[:], @@ -244,12 +244,9 @@ func TestAPIKey(t *testing.T) { }) require.NoError(t, err) - fmt.Printf("%+v\n", yeah) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { // Checks that it exists on the context! apiKey := httpmw.APIKey(r) - fmt.Printf("%+v\n", apiKey) assert.Equal(t, database.ApiKeyScopeApplicationConnect, apiKey.Scope) httpapi.Write(rw, http.StatusOK, codersdk.Response{ diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index b3b1d25198b80..fb1727a3c52e0 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -197,6 +197,16 @@ func TestRolePermissions(t *testing.T) { false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, }, }, + { + Name: "MyWorkspaceInOrgAppConnect", + // When creating the WithID won't be set, but it does not change the result. + Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, + Resource: rbac.ResourceWorkspaceApplicationConnect.InOrg(orgID).WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]authSubject{ + true: {owner, orgAdmin, orgMemberMe}, + false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, + }, + }, { Name: "Templates", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 45d084ea42313..f56804b774cc5 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -31,6 +31,15 @@ var ( Type: "workspace_execution", } + // ResourceWorkspaceApplicationConnect CRUD. Org + User owner + // create = connect to an application + // read = ? + // update = ? + // delete = ? + ResourceWorkspaceApplicationConnect = Object{ + Type: "application_connect", + } + // ResourceAuditLog // read = access audit log ResourceAuditLog = Object{ diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 774b503f73748..642717410fa71 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -26,13 +26,14 @@ var builtinScopes map[Scope]Role = map[Scope]Role{ User: []Permission{}, }, - // TODO: fill in the site permissions ScopeApplicationConnect: { Name: fmt.Sprintf("Scope_%s", ScopeApplicationConnect), DisplayName: "Ability to connect to applications", - Site: permissions(map[Object][]Action{}), - Org: map[string][]Permission{}, - User: []Permission{}, + Site: permissions(map[Object][]Action{ + ResourceWorkspaceApplicationConnect: {ActionCreate}, + }), + Org: map[string][]Permission{}, + User: []Permission{}, }, } diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index c82ed680a3258..db638a0c874e2 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -25,7 +25,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) workspace := httpmw.WorkspaceParam(r) agent := httpmw.WorkspaceAgentParam(r) - if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { + if !api.Authorize(r, rbac.ActionCreate, workspace.ApplicationConnectRBAC()) { httpapi.ResourceNotFound(rw) return } @@ -125,7 +125,7 @@ type proxyApplication struct { } func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.ResponseWriter, r *http.Request) { - if !api.Authorize(r, rbac.ActionCreate, proxyApp.Workspace.ExecutionRBAC()) { + if !api.Authorize(r, rbac.ActionCreate, proxyApp.Workspace.ApplicationConnectRBAC()) { httpapi.ResourceNotFound(rw) return } From 6bd2398be60a39d408a46a7d60d9ec6cfb010b96 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Fri, 16 Sep 2022 17:27:54 +0000 Subject: [PATCH 5/9] fixup! Merge branch 'main' into dean/token-scopes --- coderd/database/modelmethods.go | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 9c14039e97c11..ca44250305e46 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -1,30 +1,14 @@ package database import ( - "golang.org/x/xerrors" - "github.com/coder/coder/coderd/rbac" ) -var validAPIKeyScopes = map[string]ApiKeyScope{ - string(ApiKeyScopeAny): ApiKeyScopeAny, - string(ApiKeyScopeApplicationConnect): ApiKeyScopeApplicationConnect, -} - -func ToAPIKeyScope(v string) (ApiKeyScope, error) { - scope, ok := validAPIKeyScopes[v] - if !ok { - return ApiKeyScope(""), xerrors.Errorf("invalid token scope: %s", v) - } - - return scope, nil -} - func (s ApiKeyScope) ToRBAC() rbac.Scope { - switch { - case s == ApiKeyScopeAny: + switch s { + case ApiKeyScopeAny: return rbac.ScopeAny - case s == ApiKeyScopeApplicationConnect: + case ApiKeyScopeApplicationConnect: return rbac.ScopeApplicationConnect default: panic("developer error: unknown scope type " + string(s)) From a87d6e16b20a08da678efab6639c7c9ea38c883b Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 19 Sep 2022 16:27:10 +0000 Subject: [PATCH 6/9] chore: rename ApiKeyScope with sqlc and move migrations code --- cli/resetpassword.go | 3 ++- cli/server.go | 3 ++- coderd/database/db_test.go | 19 +++++++++++++++- coderd/database/dump.sql | 4 ++-- coderd/database/gen/dump/main.go | 4 ++-- .../migrations/000050_apikey_scope.down.sql | 2 +- .../migrations/000050_apikey_scope.up.sql | 4 ++-- coderd/database/{ => migrations}/migrate.go | 6 ++--- .../database/{ => migrations}/migrate_test.go | 18 +++++++-------- coderd/database/modelmethods.go | 8 +++---- coderd/database/models.go | 16 +++++++------- coderd/database/postgres/postgres.go | 4 ++-- coderd/database/queries.sql.go | 2 +- coderd/database/sqlc.yaml | 3 +++ coderd/httpmw/apikey.go | 2 +- coderd/httpmw/apikey_test.go | 22 +++++++++---------- coderd/httpmw/authorize_test.go | 2 +- coderd/httpmw/organizationparam_test.go | 2 +- coderd/httpmw/templateparam_test.go | 2 +- coderd/httpmw/templateversionparam_test.go | 2 +- coderd/httpmw/userparam_test.go | 2 +- coderd/httpmw/workspaceagentparam_test.go | 2 +- coderd/httpmw/workspacebuildparam_test.go | 2 +- coderd/httpmw/workspaceparam_test.go | 4 ++-- .../prometheusmetrics_test.go | 10 ++++----- coderd/provisionerjobs_internal_test.go | 2 +- coderd/rbac/authz.go | 2 +- coderd/rbac/authz_internal_test.go | 8 +++---- coderd/rbac/builtin_test.go | 12 +++++----- coderd/rbac/partial.go | 2 +- coderd/rbac/scopes.go | 10 ++++----- coderd/telemetry/telemetry_test.go | 2 +- coderd/users.go | 2 +- scripts/migrate-ci/main.go | 4 ++-- 34 files changed, 107 insertions(+), 85 deletions(-) rename coderd/database/{ => migrations}/migrate.go (97%) rename coderd/database/{ => migrations}/migrate_test.go (87%) diff --git a/cli/resetpassword.go b/cli/resetpassword.go index e33483243fa0b..496559113a143 100644 --- a/cli/resetpassword.go +++ b/cli/resetpassword.go @@ -10,6 +10,7 @@ import ( "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/coderd/userpassword" ) @@ -35,7 +36,7 @@ func resetPassword() *cobra.Command { return xerrors.Errorf("ping postgres: %w", err) } - err = database.EnsureClean(sqlDB) + err = migrations.EnsureClean(sqlDB) if err != nil { return xerrors.Errorf("database needs migration: %w", err) } diff --git a/cli/server.go b/cli/server.go index 2a6418ac734e9..12f8b46339d55 100644 --- a/cli/server.go +++ b/cli/server.go @@ -53,6 +53,7 @@ import ( "github.com/coder/coder/coderd/autobuild/executor" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/databasefake" + "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/coderd/devtunnel" "github.com/coder/coder/coderd/gitsshkey" "github.com/coder/coder/coderd/prometheusmetrics" @@ -430,7 +431,7 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command { if err != nil { return xerrors.Errorf("ping postgres: %w", err) } - err = database.MigrateUp(sqlDB) + err = migrations.MigrateUp(sqlDB) if err != nil { return xerrors.Errorf("migrate up: %w", err) } diff --git a/coderd/database/db_test.go b/coderd/database/db_test.go index 3ac9ac0519f3a..25dca9b43a1fe 100644 --- a/coderd/database/db_test.go +++ b/coderd/database/db_test.go @@ -4,12 +4,15 @@ package database_test import ( "context" + "database/sql" "testing" "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/migrations" + "github.com/coder/coder/coderd/database/postgres" ) func TestNestedInTx(t *testing.T) { @@ -20,7 +23,7 @@ func TestNestedInTx(t *testing.T) { uid := uuid.New() sqlDB := testSQLDB(t) - err := database.MigrateUp(sqlDB) + err := migrations.MigrateUp(sqlDB) require.NoError(t, err, "migrations") db := database.New(sqlDB) @@ -48,3 +51,17 @@ func TestNestedInTx(t *testing.T) { require.NoError(t, err, "user exists") require.Equal(t, uid, user.ID, "user id expected") } + +func testSQLDB(t testing.TB) *sql.DB { + t.Helper() + + connection, closeFn, err := postgres.Open() + require.NoError(t, err) + t.Cleanup(closeFn) + + db, err := sql.Open("postgres", connection) + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + return db +} diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index cc459ae173ef0..81557a9022d00 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1,7 +1,7 @@ -- Code generated by 'make coderd/database/generate'. DO NOT EDIT. CREATE TYPE api_key_scope AS ENUM ( - 'any', + 'all', 'application_connect' ); @@ -115,7 +115,7 @@ CREATE TABLE api_keys ( login_type login_type NOT NULL, lifetime_seconds bigint DEFAULT 86400 NOT NULL, ip_address inet DEFAULT '0.0.0.0'::inet NOT NULL, - scope api_key_scope DEFAULT 'any'::public.api_key_scope NOT NULL + scope api_key_scope DEFAULT 'all'::public.api_key_scope NOT NULL ); CREATE TABLE audit_logs ( diff --git a/coderd/database/gen/dump/main.go b/coderd/database/gen/dump/main.go index 43c694b36a959..6be8237f77894 100644 --- a/coderd/database/gen/dump/main.go +++ b/coderd/database/gen/dump/main.go @@ -9,7 +9,7 @@ import ( "path/filepath" "runtime" - "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/coderd/database/postgres" ) @@ -25,7 +25,7 @@ func main() { panic(err) } - err = database.MigrateUp(db) + err = migrations.MigrateUp(db) if err != nil { panic(err) } diff --git a/coderd/database/migrations/000050_apikey_scope.down.sql b/coderd/database/migrations/000050_apikey_scope.down.sql index 73506f8e8dd68..507093ff8dab2 100644 --- a/coderd/database/migrations/000050_apikey_scope.down.sql +++ b/coderd/database/migrations/000050_apikey_scope.down.sql @@ -1,5 +1,5 @@ -- Avoid "upgrading" devurl keys to fully fledged API keys. -DELETE FROM api_keys WHERE scope != 'any'; +DELETE FROM api_keys WHERE scope != 'all'; ALTER TABLE api_keys DROP COLUMN scope; diff --git a/coderd/database/migrations/000050_apikey_scope.up.sql b/coderd/database/migrations/000050_apikey_scope.up.sql index 62f47a8935ab2..e75a79e569dd5 100644 --- a/coderd/database/migrations/000050_apikey_scope.up.sql +++ b/coderd/database/migrations/000050_apikey_scope.up.sql @@ -1,6 +1,6 @@ CREATE TYPE api_key_scope AS ENUM ( - 'any', + 'all', 'application_connect' ); -ALTER TABLE api_keys ADD COLUMN scope api_key_scope NOT NULL DEFAULT 'any'; +ALTER TABLE api_keys ADD COLUMN scope api_key_scope NOT NULL DEFAULT 'all'; diff --git a/coderd/database/migrate.go b/coderd/database/migrations/migrate.go similarity index 97% rename from coderd/database/migrate.go rename to coderd/database/migrations/migrate.go index 88b4bf5e3b725..67ac5948f2c9b 100644 --- a/coderd/database/migrate.go +++ b/coderd/database/migrations/migrate.go @@ -1,4 +1,4 @@ -package database +package migrations import ( "context" @@ -14,12 +14,12 @@ import ( "golang.org/x/xerrors" ) -//go:embed migrations/*.sql +//go:embed *.sql var migrations embed.FS func migrateSetup(db *sql.DB) (source.Driver, *migrate.Migrate, error) { ctx := context.Background() - sourceDriver, err := iofs.New(migrations, "migrations") + sourceDriver, err := iofs.New(migrations, ".") if err != nil { return nil, nil, xerrors.Errorf("create iofs: %w", err) } diff --git a/coderd/database/migrate_test.go b/coderd/database/migrations/migrate_test.go similarity index 87% rename from coderd/database/migrate_test.go rename to coderd/database/migrations/migrate_test.go index a8f739c275c36..43e2b7797df74 100644 --- a/coderd/database/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -1,6 +1,6 @@ //go:build linux -package database_test +package migrations_test import ( "database/sql" @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/goleak" - "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/coderd/database/postgres" ) @@ -33,7 +33,7 @@ func TestMigrate(t *testing.T) { db := testSQLDB(t) - err := database.MigrateUp(db) + err := migrations.MigrateUp(db) require.NoError(t, err) }) @@ -42,10 +42,10 @@ func TestMigrate(t *testing.T) { db := testSQLDB(t) - err := database.MigrateUp(db) + err := migrations.MigrateUp(db) require.NoError(t, err) - err = database.MigrateUp(db) + err = migrations.MigrateUp(db) require.NoError(t, err) }) @@ -54,13 +54,13 @@ func TestMigrate(t *testing.T) { db := testSQLDB(t) - err := database.MigrateUp(db) + err := migrations.MigrateUp(db) require.NoError(t, err) - err = database.MigrateDown(db) + err = migrations.MigrateDown(db) require.NoError(t, err) - err = database.MigrateUp(db) + err = migrations.MigrateUp(db) require.NoError(t, err) }) } @@ -120,7 +120,7 @@ func TestCheckLatestVersion(t *testing.T) { }) } - err := database.CheckLatestVersion(driver, tc.currentVersion) + err := migrations.CheckLatestVersion(driver, tc.currentVersion) var errMessage string if err != nil { errMessage = err.Error() diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index ca44250305e46..f6e28bd5dc824 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -4,11 +4,11 @@ import ( "github.com/coder/coder/coderd/rbac" ) -func (s ApiKeyScope) ToRBAC() rbac.Scope { +func (s APIKeyScope) ToRBAC() rbac.Scope { switch s { - case ApiKeyScopeAny: - return rbac.ScopeAny - case ApiKeyScopeApplicationConnect: + case APIKeyScopeAll: + return rbac.ScopeAll + case APIKeyScopeApplicationConnect: return rbac.ScopeApplicationConnect default: panic("developer error: unknown scope type " + string(s)) diff --git a/coderd/database/models.go b/coderd/database/models.go index a341086e05155..b5d48bf6c0c32 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -14,21 +14,21 @@ import ( "github.com/tabbed/pqtype" ) -type ApiKeyScope string +type APIKeyScope string const ( - ApiKeyScopeAny ApiKeyScope = "any" - ApiKeyScopeApplicationConnect ApiKeyScope = "application_connect" + APIKeyScopeAll APIKeyScope = "all" + APIKeyScopeApplicationConnect APIKeyScope = "application_connect" ) -func (e *ApiKeyScope) Scan(src interface{}) error { +func (e *APIKeyScope) Scan(src interface{}) error { switch s := src.(type) { case []byte: - *e = ApiKeyScope(s) + *e = APIKeyScope(s) case string: - *e = ApiKeyScope(s) + *e = APIKeyScope(s) default: - return fmt.Errorf("unsupported scan type for ApiKeyScope: %T", src) + return fmt.Errorf("unsupported scan type for APIKeyScope: %T", src) } return nil } @@ -343,7 +343,7 @@ type APIKey struct { LoginType LoginType `db:"login_type" json:"login_type"` LifetimeSeconds int64 `db:"lifetime_seconds" json:"lifetime_seconds"` IPAddress pqtype.Inet `db:"ip_address" json:"ip_address"` - Scope ApiKeyScope `db:"scope" json:"scope"` + Scope APIKeyScope `db:"scope" json:"scope"` } type AgentStat struct { diff --git a/coderd/database/postgres/postgres.go b/coderd/database/postgres/postgres.go index a1637cfb0261c..b5af1cc50aa70 100644 --- a/coderd/database/postgres/postgres.go +++ b/coderd/database/postgres/postgres.go @@ -14,7 +14,7 @@ import ( "github.com/ory/dockertest/v3/docker" "golang.org/x/xerrors" - "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/cryptorand" ) @@ -143,7 +143,7 @@ func Open() (string, func(), error) { return retryErr } - err = database.MigrateUp(db) + err = migrations.MigrateUp(db) if err != nil { retryErr = xerrors.Errorf("migrate db: %w", err) // Only try to migrate once. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 6a009d34114ce..d033a901e7805 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -231,7 +231,7 @@ type InsertAPIKeyParams struct { CreatedAt time.Time `db:"created_at" json:"created_at"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` LoginType LoginType `db:"login_type" json:"login_type"` - Scope ApiKeyScope `db:"scope" json:"scope"` + Scope APIKeyScope `db:"scope" json:"scope"` } func (q *sqlQuerier) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) { diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 800b94983488d..579264d8a499a 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -18,6 +18,9 @@ packages: rename: api_key: APIKey + api_key_scope: APIKeyScope + api_key_scope_all: APIKeyScopeAll + api_key_scope_application_connect: APIKeyScopeApplicationConnect avatar_url: AvatarURL login_type_oidc: LoginTypeOIDC oauth_access_token: OAuthAccessToken diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index b9c27cf0ddf02..3d11ba98493b1 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -41,7 +41,7 @@ type Authorization struct { ID uuid.UUID Username string Roles []string - Scope database.ApiKeyScope + Scope database.APIKeyScope } // UserAuthorization returns the roles and scope used for authorization. Depends diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index 8e06935dc6765..e59065db668d3 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -146,7 +146,7 @@ func TestAPIKey(t *testing.T) { ID: id, HashedSecret: hashed[:], UserID: user.ID, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -172,7 +172,7 @@ func TestAPIKey(t *testing.T) { HashedSecret: hashed[:], UserID: user.ID, LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -199,7 +199,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -240,14 +240,14 @@ func TestAPIKey(t *testing.T) { HashedSecret: hashed[:], ExpiresAt: database.Now().AddDate(0, 0, 1), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeApplicationConnect, + Scope: database.APIKeyScopeApplicationConnect, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { // Checks that it exists on the context! apiKey := httpmw.APIKey(r) - assert.Equal(t, database.ApiKeyScopeApplicationConnect, apiKey.Scope) + assert.Equal(t, database.APIKeyScopeApplicationConnect, apiKey.Scope) httpapi.Write(rw, http.StatusOK, codersdk.Response{ Message: "it worked!", @@ -279,7 +279,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -313,7 +313,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -347,7 +347,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().Add(time.Minute), UserID: user.ID, LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) @@ -381,7 +381,7 @@ func TestAPIKey(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) @@ -421,7 +421,7 @@ func TestAPIKey(t *testing.T) { LoginType: database.LoginTypeGithub, LastUsed: database.Now(), UserID: user.ID, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) _, err = db.InsertUserLink(r.Context(), database.InsertUserLinkParams{ @@ -474,7 +474,7 @@ func TestAPIKey(t *testing.T) { ExpiresAt: database.Now().AddDate(0, 0, 1), UserID: user.ID, LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index f8f99c89bb5fb..e1ac548092f8b 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -124,7 +124,7 @@ func addUser(t *testing.T, db database.Store, roles ...string) (database.User, s LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/httpmw/organizationparam_test.go b/coderd/httpmw/organizationparam_test.go index d15e1dc3c8a88..58816c3b783fe 100644 --- a/coderd/httpmw/organizationparam_test.go +++ b/coderd/httpmw/organizationparam_test.go @@ -51,7 +51,7 @@ func TestOrganizationParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, chi.NewRouteContext())) diff --git a/coderd/httpmw/templateparam_test.go b/coderd/httpmw/templateparam_test.go index 055ffbc4b1e08..7cef1dd7801af 100644 --- a/coderd/httpmw/templateparam_test.go +++ b/coderd/httpmw/templateparam_test.go @@ -51,7 +51,7 @@ func TestTemplateParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/httpmw/templateversionparam_test.go b/coderd/httpmw/templateversionparam_test.go index 1070ef2529015..1eefc677f542f 100644 --- a/coderd/httpmw/templateversionparam_test.go +++ b/coderd/httpmw/templateversionparam_test.go @@ -51,7 +51,7 @@ func TestTemplateVersionParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/httpmw/userparam_test.go b/coderd/httpmw/userparam_test.go index 0e274da5876df..fbb66af173ba8 100644 --- a/coderd/httpmw/userparam_test.go +++ b/coderd/httpmw/userparam_test.go @@ -45,7 +45,7 @@ func TestUserParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/httpmw/workspaceagentparam_test.go b/coderd/httpmw/workspaceagentparam_test.go index 476ea224d423a..ae22582d615b4 100644 --- a/coderd/httpmw/workspaceagentparam_test.go +++ b/coderd/httpmw/workspaceagentparam_test.go @@ -51,7 +51,7 @@ func TestWorkspaceAgentParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/httpmw/workspacebuildparam_test.go b/coderd/httpmw/workspacebuildparam_test.go index 61ccd7ced4958..97cf08b16122e 100644 --- a/coderd/httpmw/workspacebuildparam_test.go +++ b/coderd/httpmw/workspacebuildparam_test.go @@ -51,7 +51,7 @@ func TestWorkspaceBuildParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/httpmw/workspaceparam_test.go b/coderd/httpmw/workspaceparam_test.go index 82502f7691418..5c36363f6a1b4 100644 --- a/coderd/httpmw/workspaceparam_test.go +++ b/coderd/httpmw/workspaceparam_test.go @@ -54,7 +54,7 @@ func TestWorkspaceParam(t *testing.T) { LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) @@ -360,7 +360,7 @@ func setupWorkspaceWithAgents(t testing.TB, cfg setupConfig) (database.Store, *h LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 3531a94dfa25b..b18d5ba4cadf1 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -38,7 +38,7 @@ func TestActiveUsers(t *testing.T) { _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) return db }, @@ -50,14 +50,14 @@ func TestActiveUsers(t *testing.T) { _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) // Because this API key hasn't been used in the past hour, this shouldn't // add to the user count. _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now().Add(-2 * time.Hour), - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) return db }, @@ -69,12 +69,12 @@ func TestActiveUsers(t *testing.T) { _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) _, _ = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ UserID: uuid.New(), LastUsed: database.Now(), - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) return db }, diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go index e557ec0ed6524..9a58a27193d4d 100644 --- a/coderd/provisionerjobs_internal_test.go +++ b/coderd/provisionerjobs_internal_test.go @@ -77,7 +77,7 @@ func TestProvisionerJobLogs_Unit(t *testing.T) { UserID: userID, ExpiresAt: time.Now().Add(5 * time.Hour), LoginType: database.LoginTypePassword, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) _, err = fDB.InsertUser(ctx, database.InsertUserParams{ diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index ff04d88d5cf6e..9d886a4f8b5d6 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -95,7 +95,7 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa } // If the scope isn't "any", we need to check with the scope's role as well. - if scope != ScopeAny { + if scope != ScopeAll { scopeRole := builtinScopes[scope] err = a.Authorize(ctx, subjectID, []Role{scopeRole}, action, object) if err != nil { diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index c08ae467be8a3..ec3408dd8cd0d 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -43,7 +43,7 @@ func TestFilterError(t *testing.T) { auth, err := NewAuthorizer() require.NoError(t, err) - _, err = Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAny, ActionRead, []Object{ResourceUser, ResourceWorkspace}) + _, err = Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAll, ActionRead, []Object{ResourceUser, ResourceWorkspace}) require.ErrorContains(t, err, "object types must be uniform") } @@ -163,7 +163,7 @@ func TestFilter(t *testing.T) { auth, err := NewAuthorizer() require.NoError(t, err, "new auth") - scope := ScopeAny + scope := ScopeAll if tc.Scope != "" { scope = tc.Scope } @@ -703,13 +703,13 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) t.Cleanup(cancel) - scope := ScopeAny + scope := ScopeAll if subject.Scope != "" { scope = subject.Scope } authError := authorizer.Authorize(ctx, subject.UserID, subject.Roles, a, c.resource) - if authError == nil && scope != ScopeAny { + if authError == nil && scope != ScopeAll { scopeRole := builtinScopes[scope] authError = authorizer.Authorize(ctx, subject.UserID, []Role{scopeRole}, a, c.resource) } diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index fb1727a3c52e0..2616466c39e1e 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -39,27 +39,27 @@ func BenchmarkRBACFilter(b *testing.B) { Name: "NoRoles", Roles: []string{}, UserID: users[0], - Scope: rbac.ScopeAny, + Scope: rbac.ScopeAll, }, { Name: "Admin", // Give some extra roles that an admin might have Roles: []string{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, UserID: users[0], - Scope: rbac.ScopeAny, + Scope: rbac.ScopeAll, }, { Name: "OrgAdmin", Roles: []string{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgAdmin(orgs[0]), rbac.RoleMember()}, UserID: users[0], - Scope: rbac.ScopeAny, + Scope: rbac.ScopeAll, }, { Name: "OrgMember", // Member of 2 orgs Roles: []string{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgMember(orgs[1]), rbac.RoleMember()}, UserID: users[0], - Scope: rbac.ScopeAny, + Scope: rbac.ScopeAll, }, { Name: "ManyRoles", @@ -71,7 +71,7 @@ func BenchmarkRBACFilter(b *testing.B) { rbac.RoleMember(), }, UserID: users[0], - Scope: rbac.ScopeAny, + Scope: rbac.ScopeAll, }, { Name: "AdminWithScope", @@ -359,7 +359,7 @@ func TestRolePermissions(t *testing.T) { delete(remainingSubjs, subj.Name) msg := fmt.Sprintf("%s as %q doing %q on %q", c.Name, subj.Name, action, c.Resource.Type) // TODO: scopey - err := auth.ByRoleName(context.Background(), subj.UserID, subj.Roles, rbac.ScopeAny, action, c.Resource) + err := auth.ByRoleName(context.Background(), subj.UserID, subj.Roles, rbac.ScopeAll, action, c.Resource) if result { assert.NoError(t, err, fmt.Sprintf("Should pass: %s", msg)) } else { diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index fd693fb0069e6..6c8352f2beea6 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -37,7 +37,7 @@ func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, s } var scopeAuth *subPartialAuthorizer - if scope != ScopeAny { + if scope != ScopeAll { scopeRole := builtinScopes[scope] scopeAuth, err = newSubPartialAuthorizer(ctx, subjectID, []Role{scopeRole}, action, objectType) if err != nil { diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 642717410fa71..9f5268f2cb735 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -9,16 +9,16 @@ import ( type Scope string const ( - ScopeAny Scope = "any" + ScopeAll Scope = "all" ScopeApplicationConnect Scope = "application_connect" ) var builtinScopes map[Scope]Role = map[Scope]Role{ - // ScopeAny is a special scope that allows access to all resources. During + // ScopeAll is a special scope that allows access to all resources. During // authorize checks it is usually not used directly and skips scope checks. - ScopeAny: { - Name: fmt.Sprintf("Scope_%s", ScopeAny), - DisplayName: "Any operation", + ScopeAll: { + Name: fmt.Sprintf("Scope_%s", ScopeAll), + DisplayName: "All operations", Site: permissions(map[Object][]Action{ ResourceWildcard: {WildcardSymbol}, }), diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index 0d2af1f659090..ddfccf68100e9 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -34,7 +34,7 @@ func TestTelemetry(t *testing.T) { _, err := db.InsertAPIKey(ctx, database.InsertAPIKeyParams{ ID: uuid.NewString(), LastUsed: database.Now(), - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) require.NoError(t, err) _, err = db.InsertParameterSchema(ctx, database.InsertParameterSchemaParams{ diff --git a/coderd/users.go b/coderd/users.go index d473f24c467ee..53e67937b87c3 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1073,7 +1073,7 @@ func (api *API) createAPIKey(r *http.Request, params createAPIKeyParams) (*http. UpdatedAt: database.Now(), HashedSecret: hashed[:], LoginType: params.LoginType, - Scope: database.ApiKeyScopeAny, + Scope: database.APIKeyScopeAll, }) if err != nil { return nil, xerrors.Errorf("insert API key: %w", err) diff --git a/scripts/migrate-ci/main.go b/scripts/migrate-ci/main.go index c3bac6c10d5d5..0f9c1845fc3b3 100644 --- a/scripts/migrate-ci/main.go +++ b/scripts/migrate-ci/main.go @@ -4,7 +4,7 @@ import ( "database/sql" "fmt" - "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/cryptorand" ) @@ -34,7 +34,7 @@ func main() { } defer target.Close() - err = database.MigrateUp(target) + err = migrations.MigrateUp(target) if err != nil { panic(err) } From f7707a8b270a51c77148db60617cefb8e3942c13 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 19 Sep 2022 16:39:49 +0000 Subject: [PATCH 7/9] chore: migrations.MigrateUp -> migrations.Up --- cli/server.go | 2 +- coderd/database/db_test.go | 2 +- coderd/database/gen/dump/main.go | 2 +- coderd/database/migrations/migrate.go | 16 ++++++++-------- coderd/database/migrations/migrate_test.go | 12 ++++++------ coderd/database/postgres/postgres.go | 2 +- scripts/migrate-ci/main.go | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/cli/server.go b/cli/server.go index 19b3379a82308..d0657b112fb27 100644 --- a/cli/server.go +++ b/cli/server.go @@ -431,7 +431,7 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command { if err != nil { return xerrors.Errorf("ping postgres: %w", err) } - err = migrations.MigrateUp(sqlDB) + err = migrations.Up(sqlDB) if err != nil { return xerrors.Errorf("migrate up: %w", err) } diff --git a/coderd/database/db_test.go b/coderd/database/db_test.go index 25dca9b43a1fe..bf0afc31f3119 100644 --- a/coderd/database/db_test.go +++ b/coderd/database/db_test.go @@ -23,7 +23,7 @@ func TestNestedInTx(t *testing.T) { uid := uuid.New() sqlDB := testSQLDB(t) - err := migrations.MigrateUp(sqlDB) + err := migrations.Up(sqlDB) require.NoError(t, err, "migrations") db := database.New(sqlDB) diff --git a/coderd/database/gen/dump/main.go b/coderd/database/gen/dump/main.go index 6be8237f77894..6025ace128718 100644 --- a/coderd/database/gen/dump/main.go +++ b/coderd/database/gen/dump/main.go @@ -25,7 +25,7 @@ func main() { panic(err) } - err = migrations.MigrateUp(db) + err = migrations.Up(db) if err != nil { panic(err) } diff --git a/coderd/database/migrations/migrate.go b/coderd/database/migrations/migrate.go index 67ac5948f2c9b..1501f5f6c4e30 100644 --- a/coderd/database/migrations/migrate.go +++ b/coderd/database/migrations/migrate.go @@ -17,7 +17,7 @@ import ( //go:embed *.sql var migrations embed.FS -func migrateSetup(db *sql.DB) (source.Driver, *migrate.Migrate, error) { +func setup(db *sql.DB) (source.Driver, *migrate.Migrate, error) { ctx := context.Background() sourceDriver, err := iofs.New(migrations, ".") if err != nil { @@ -45,9 +45,9 @@ func migrateSetup(db *sql.DB) (source.Driver, *migrate.Migrate, error) { return sourceDriver, m, nil } -// MigrateUp runs SQL migrations to ensure the database schema is up-to-date. -func MigrateUp(db *sql.DB) (retErr error) { - _, m, err := migrateSetup(db) +// Up runs SQL migrations to ensure the database schema is up-to-date. +func Up(db *sql.DB) (retErr error) { + _, m, err := setup(db) if err != nil { return xerrors.Errorf("migrate setup: %w", err) } @@ -76,9 +76,9 @@ func MigrateUp(db *sql.DB) (retErr error) { return nil } -// MigrateDown runs all down SQL migrations. -func MigrateDown(db *sql.DB) error { - _, m, err := migrateSetup(db) +// Down runs all down SQL migrations. +func Down(db *sql.DB) error { + _, m, err := setup(db) if err != nil { return xerrors.Errorf("migrate setup: %w", err) } @@ -100,7 +100,7 @@ func MigrateDown(db *sql.DB) error { // applied, without making any changes to the database. If not, returns a // non-nil error. func EnsureClean(db *sql.DB) error { - sourceDriver, m, err := migrateSetup(db) + sourceDriver, m, err := setup(db) if err != nil { return xerrors.Errorf("migrate setup: %w", err) } diff --git a/coderd/database/migrations/migrate_test.go b/coderd/database/migrations/migrate_test.go index 43e2b7797df74..ece3be596922c 100644 --- a/coderd/database/migrations/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -33,7 +33,7 @@ func TestMigrate(t *testing.T) { db := testSQLDB(t) - err := migrations.MigrateUp(db) + err := migrations.Up(db) require.NoError(t, err) }) @@ -42,10 +42,10 @@ func TestMigrate(t *testing.T) { db := testSQLDB(t) - err := migrations.MigrateUp(db) + err := migrations.Up(db) require.NoError(t, err) - err = migrations.MigrateUp(db) + err = migrations.Up(db) require.NoError(t, err) }) @@ -54,13 +54,13 @@ func TestMigrate(t *testing.T) { db := testSQLDB(t) - err := migrations.MigrateUp(db) + err := migrations.Up(db) require.NoError(t, err) - err = migrations.MigrateDown(db) + err = migrations.Down(db) require.NoError(t, err) - err = migrations.MigrateUp(db) + err = migrations.Up(db) require.NoError(t, err) }) } diff --git a/coderd/database/postgres/postgres.go b/coderd/database/postgres/postgres.go index b5af1cc50aa70..89b6d8dfb9da3 100644 --- a/coderd/database/postgres/postgres.go +++ b/coderd/database/postgres/postgres.go @@ -143,7 +143,7 @@ func Open() (string, func(), error) { return retryErr } - err = migrations.MigrateUp(db) + err = migrations.Up(db) if err != nil { retryErr = xerrors.Errorf("migrate db: %w", err) // Only try to migrate once. diff --git a/scripts/migrate-ci/main.go b/scripts/migrate-ci/main.go index 0f9c1845fc3b3..636067cf8dd9e 100644 --- a/scripts/migrate-ci/main.go +++ b/scripts/migrate-ci/main.go @@ -34,7 +34,7 @@ func main() { } defer target.Close() - err = migrations.MigrateUp(target) + err = migrations.Up(target) if err != nil { panic(err) } From 49059d1f7196807dfbeb78f733aa6471811c4bd8 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 19 Sep 2022 17:28:27 +0000 Subject: [PATCH 8/9] chore: improve rbac scope tests --- coderd/rbac/authz.go | 6 ++++- coderd/rbac/authz_internal_test.go | 41 ++++++++---------------------- coderd/rbac/partial.go | 6 ++++- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index f544a07b0f00c..ee7ddb25cae60 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -106,7 +106,11 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa // If the scope isn't "any", we need to check with the scope's role as well. if scope != ScopeAll { - scopeRole := builtinScopes[scope] + scopeRole, err := ScopeRole(scope) + if err != nil { + return err + } + err = a.Authorize(ctx, subjectID, []Role{scopeRole}, action, object) if err != nil { return err diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index ec3408dd8cd0d..1c516fbf19824 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -20,7 +20,6 @@ type subject struct { // by name. This allows us to test custom roles that do not exist in the product, // but test edge cases of the implementation. Roles []Role `json:"roles"` - Scope Scope `json:"scope"` } type fakeObject struct { @@ -172,7 +171,6 @@ func TestFilter(t *testing.T) { var allowedCount int for i, obj := range localObjects { obj.Type = tc.ObjectType - // TODO: scopes err := auth.ByRoleName(ctx, tc.SubjectID, tc.Roles, scope, ActionRead, obj.RBACObject()) obj.Allowed = err == nil if err == nil { @@ -636,39 +634,26 @@ func TestAuthorizeScope(t *testing.T) { unusedID := uuid.New() user := subject{ UserID: "me", - Roles: []Role{ - must(RoleByName(RoleOwner())), - must(RoleByName(RoleMember())), - }, + Roles: []Role{}, } - user.Scope = ScopeApplicationConnect - testAuthorize(t, "ScopeApplicationConnect", user, []authTestCase{ - // Org + me + user.Roles = []Role{must(ScopeRole(ScopeApplicationConnect))} + testAuthorize(t, "Admin_ScopeApplicationConnect", user, []authTestCase{ {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(defOrg), actions: allActions(), allow: false}, - {resource: ResourceWorkspace.WithOwner(user.UserID), actions: allActions(), allow: false}, - {resource: ResourceWorkspace.All(), actions: allActions(), allow: false}, - - // Other org + me {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(unusedID), actions: allActions(), allow: false}, - - // Other org + other user {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: allActions(), allow: false}, - {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, - - // Other org + other use {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(unusedID), actions: allActions(), allow: false}, - {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, - // TODO: add allowed cases when scope application_connect actually has - // permissions + // Allowed by scope: + {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: true}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionCreate}, allow: true}, }) } @@ -696,6 +681,9 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes for _, cases := range sets { for i, c := range cases { c := c + if c.resource.Type != "application_connect" { + continue + } caseName := fmt.Sprintf("%s/%d", name, i) t.Run(caseName, func(t *testing.T) { t.Parallel() @@ -703,16 +691,7 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) t.Cleanup(cancel) - scope := ScopeAll - if subject.Scope != "" { - scope = subject.Scope - } - authError := authorizer.Authorize(ctx, subject.UserID, subject.Roles, a, c.resource) - if authError == nil && scope != ScopeAll { - scopeRole := builtinScopes[scope] - authError = authorizer.Authorize(ctx, subject.UserID, []Role{scopeRole}, a, c.resource) - } // Logging only if authError != nil { @@ -737,7 +716,7 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes assert.Error(t, authError, "expected unauthorized") } - partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, scope, a, c.resource.Type) + partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, ScopeAll, a, c.resource.Type) require.NoError(t, err, "make prepared authorizer") // Also check the rego policy can form a valid partial query result. diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index 0a57d9c59f289..59e68c202d94b 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -46,7 +46,11 @@ func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, s var scopeAuth *subPartialAuthorizer if scope != ScopeAll { - scopeRole := builtinScopes[scope] + scopeRole, err := ScopeRole(scope) + if err != nil { + return nil, xerrors.Errorf("unknown scope %q", scope) + } + scopeAuth, err = newSubPartialAuthorizer(ctx, subjectID, []Role{scopeRole}, action, objectType) if err != nil { return nil, err From 2b37954002d28f3f0cc373b8ea0cfebaeb90e8e7 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 19 Sep 2022 17:30:53 +0000 Subject: [PATCH 9/9] fixup! chore: improve rbac scope tests --- coderd/rbac/authz_internal_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 1c516fbf19824..24bb5a90ea468 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -732,7 +732,19 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes t.Logf("support: %+v", s.String()) } } - require.Equal(t, 0, len(partialAuthz.mainAuthorizer.partialQueries.Support), "expected 0 support rules") + if partialAuthz.scopeAuthorizer != nil { + if len(partialAuthz.scopeAuthorizer.partialQueries.Support) > 0 { + d, _ := json.Marshal(partialAuthz.scopeAuthorizer.input) + t.Logf("scope input: %s", string(d)) + for _, q := range partialAuthz.scopeAuthorizer.partialQueries.Queries { + t.Logf("scope query: %+v", q.String()) + } + for _, s := range partialAuthz.scopeAuthorizer.partialQueries.Support { + t.Logf("scope support: %+v", s.String()) + } + } + require.Equal(t, 0, len(partialAuthz.scopeAuthorizer.partialQueries.Support), "expected 0 support rules in scope authorizer") + } partialErr := partialAuthz.Authorize(ctx, c.resource) if authError != nil {