From 990d24dc421f1680e09c53001c61ca7130c117cb Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Thu, 13 Aug 2026 09:58:51 -0700 Subject: [PATCH] feat: add oauth2 scope columns and single-use delete queries (#28007) OAuth2 tokens issued by Coder ignore scope entirely. The authorize endpoint parses the `scope` parameter and then discards it, and both grant paths mint API keys with full API access regardless of what the client requested or what the app's allowlist permits. There is also nowhere to put a negotiated scope: nothing carries one from the authorize step to the token it produces. Schema and query groundwork for that pipeline. No behavior change on its own. - Migration `000569` adds a `scope` column to `oauth2_provider_app_codes` and `oauth2_provider_app_tokens`, so a negotiated scope can travel from a code to the token it is exchanged for, and from a token to its refreshed successor. - Existing rows are backfilled to `coder:all`, then both columns become NOT NULL with a non-empty CHECK. Every OAuth2 key is unrestricted in fact today, so the backfill only writes that down, and a caller that omits the column now fails instead of silently issuing full access. - Adds `DeleteOAuth2ProviderAppCodeByIDReturningRow` and `DeleteAPIKeyByIDReturningRow`, which return `sql.ErrNoRows` when the row is already gone. Postgres serializes concurrent deletes on the row lock, so exactly one caller gets a row back, which is what will let the grant paths enforce single use of a code or refresh token without a read-then-write race. - No callers yet. The existing blind deletes and all of their call sites are untouched, and codes and tokens record `coder:all` until a later phase negotiates a real value. Phase 1 of [PLAT-470](https://linear.app/codercom/issue/PLAT-470), tracked as [PLAT-478](https://linear.app/codercom/issue/PLAT-478/phase-1-schema-and-queries). Scope validation at authorize, applying the negotiated scope in the code grant, and refresh narrowing follow as separate PRs. Verified locally: `make gen` and `make lint` clean, the migrations suite passes both up and down, and dbauthz's `TestMethodTestSuite` passes.
End-to-end scope enforcement flow (green marks what this PR touches) ```mermaid flowchart TD subgraph authorize["/oauth2/authorize"] AZ1["ShowAuthorizePage (GET)
renders consent page"] AZ2["ProcessAuthorize (POST)
scope parsed, then discarded"] Q1["InsertOAuth2ProviderAppCode
gains a Scope param"] AZ1 --> AZ2 --> Q1 end Q1 --> CODES[("oauth2_provider_app_codes
new column: scope text NOT NULL")] subgraph codegrant["POST /oauth2/token, grant_type=authorization_code"] G1["authorizationCodeGrant"] Q2["GetOAuth2ProviderAppCodeByPrefix
now returns Scope"] Q4["DeleteOAuth2ProviderAppCodeByIDReturningRow
added, no caller yet"] G2["apikey.Generate + UserRBACSubject
hardcoded to full access"] G1 --> Q2 --> G2 G1 -.-> Q4 end CODES --> G1 G2 --> Q3 Q3["InsertOAuth2ProviderAppToken
gains a Scope param"] Q3 --> TOKENS[("oauth2_provider_app_tokens
new column: scope text NOT NULL")] subgraph refresh["POST /oauth2/token, grant_type=refresh_token"] G3["refreshTokenGrant"] Q5["GetOAuth2ProviderAppTokenByPrefix
now returns Scope"] Q6["DeleteAPIKeyByIDReturningRow
added, no caller yet"] G3 --> Q5 G3 -.-> Q6 end TOKENS --> G3 Q5 --> Q3 subgraph enforce["Every authenticated API request"] E1["httpmw ExtractAPIKey"] --> E2["APIKey.ScopeSet()"] --> E3["UserRBACSubject"] --> E4["dbauthz authorize"] end TOKENS --> E1 classDef changed fill:#c8e6c9,stroke:#2e7d32,stroke-width:2px,color:#1b3c1e classDef dormant fill:#c8e6c9,stroke:#2e7d32,stroke-width:2px,stroke-dasharray:5 3,color:#1b3c1e class Q1,Q2,Q3,Q5,CODES,TOKENS changed class Q4,Q6 dormant ``` Solid green is added or changed here. Dashed green exists but has no caller yet. Everything else is unchanged, including the enforcement engine at the bottom, which already reads a key's scopes correctly and only needs real data fed into it.
Suggested reading order Most of the diff is generated. `dump.sql`, `models.go`, `querier.go`, `queries.sql.go`, `check_constraint.go`, and the dbmock and dbmetrics packages all come from `make gen`. 1. `migrations/000569_oauth2_scope_columns.{up,down}.sql`: additive column, backfill, NOT NULL, CHECK, and a `COMMENT ON COLUMN` on each. 2. `queries/oauth2.sql` and `queries/apikeys.sql`: `scope` added to both insert column lists, plus the two new returning-row deletes alongside the untouched originals. The `Get...ByPrefix` selects needed no edit, since they are `SELECT *`. 3. `dbauthz/dbauthz.go`: hand-written wrappers for the two new queries, each fetching by ID, authorizing delete against the fetched object, then delegating. The generic `deleteQ` helper does not fit, since it requires the delete to return only `error`. 4. `oauth2provider/authorize.go` and `oauth2provider/tokens.go`: the only production changes, all behavior-neutral. 5. `dbgen/dbgen.go` and `dbauthz/dbauthz_test.go`: seed threading, plus a case per new query. `MethodTestSuite` fails with "Method never called" for anything untested. Neither type needs to become auditable, which `make lint` confirms by not erroring on `enterprise/audit/table.go`.
--------- Co-authored-by: Claude Opus 5 (1M context) --- coderd/database/check_constraint.go | 2 + coderd/database/dbauthz/dbauthz_test.go | 2 + coderd/database/dbgen/dbgen.go | 2 + coderd/database/dump.sql | 12 +++- .../000569_oauth2_scope_columns.down.sql | 3 + .../000569_oauth2_scope_columns.up.sql | 30 +++++++++ coderd/database/models.go | 4 ++ coderd/database/querier_test.go | 64 +++++++++++++++++++ coderd/database/queries.sql.go | 34 +++++++--- coderd/database/queries/oauth2.sql | 12 ++-- coderd/oauth2_test.go | 2 + coderd/oauth2provider/authorize.go | 5 ++ coderd/oauth2provider/tokens.go | 5 ++ 13 files changed, 161 insertions(+), 16 deletions(-) create mode 100644 coderd/database/migrations/000569_oauth2_scope_columns.down.sql create mode 100644 coderd/database/migrations/000569_oauth2_scope_columns.up.sql diff --git a/coderd/database/check_constraint.go b/coderd/database/check_constraint.go index 268009cd29..0402e4b8ee 100644 --- a/coderd/database/check_constraint.go +++ b/coderd/database/check_constraint.go @@ -44,6 +44,8 @@ const ( CheckMcpServerConfigsAuthTypeCheck CheckConstraint = "mcp_server_configs_auth_type_check" // mcp_server_configs CheckMcpServerConfigsAvailabilityCheck CheckConstraint = "mcp_server_configs_availability_check" // mcp_server_configs CheckMcpServerConfigsTransportCheck CheckConstraint = "mcp_server_configs_transport_check" // mcp_server_configs + CheckOauth2ProviderAppCodesScopeNotEmpty CheckConstraint = "oauth2_provider_app_codes_scope_not_empty" // oauth2_provider_app_codes + CheckOauth2ProviderAppTokensScopeNotEmpty CheckConstraint = "oauth2_provider_app_tokens_scope_not_empty" // oauth2_provider_app_tokens CheckOauth2ProviderAppsClientTypeCheck CheckConstraint = "oauth2_provider_apps_client_type_check" // oauth2_provider_apps CheckMaxProvisionerLogsLength CheckConstraint = "max_provisioner_logs_length" // provisioner_jobs CheckNatsPortValidTcp CheckConstraint = "nats_port_valid_tcp" // replicas diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 86dc818f54..a9de44c460 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -6013,6 +6013,7 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppCodes() { check.Args(database.InsertOAuth2ProviderAppCodeParams{ AppID: app.ID, UserID: user.ID, + Scope: string(database.ApiKeyScopeCoderAll), }).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()), policy.ActionCreate) })) s.Run("DeleteOAuth2ProviderAppCodeByID", s.Subtest(func(db database.Store, check *expects) { @@ -6057,6 +6058,7 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() { AppSecretID: uuid.NullUUID{UUID: secret.ID, Valid: true}, APIKeyID: key.ID, UserID: user.ID, + Scope: string(database.ApiKeyScopeCoderAll), }).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()), policy.ActionCreate) })) s.Run("GetOAuth2ProviderAppTokenByPrefix", s.Subtest(func(db database.Store, check *expects) { diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 10cfe4dddf..ff20709991 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -1784,6 +1784,7 @@ func OAuth2ProviderAppCode(t testing.TB, db database.Store, seed database.OAuth2 CodeChallengeMethod: seed.CodeChallengeMethod, StateHash: seed.StateHash, RedirectUri: seed.RedirectUri, + Scope: takeFirst(seed.Scope, string(database.ApiKeyScopeCoderAll)), }) require.NoError(t, err, "insert oauth2 app code") return code @@ -1805,6 +1806,7 @@ func OAuth2ProviderAppToken(t testing.TB, db database.Store, seed database.OAuth APIKeyID: takeFirst(seed.APIKeyID, uuid.New().String()), UserID: takeFirst(seed.UserID, uuid.New()), Audience: seed.Audience, + Scope: takeFirst(seed.Scope, string(database.ApiKeyScopeCoderAll)), }) require.NoError(t, err, "insert oauth2 app token") return token diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 79e987cc08..d7595115c9 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2596,7 +2596,9 @@ CREATE TABLE oauth2_provider_app_codes ( code_challenge text, code_challenge_method text, state_hash text, - redirect_uri text + redirect_uri text, + scope text NOT NULL, + CONSTRAINT oauth2_provider_app_codes_scope_not_empty CHECK ((scope <> ''::text)) ); COMMENT ON TABLE oauth2_provider_app_codes IS 'Codes are meant to be exchanged for access tokens.'; @@ -2611,6 +2613,8 @@ COMMENT ON COLUMN oauth2_provider_app_codes.state_hash IS 'SHA-256 hash of the O COMMENT ON COLUMN oauth2_provider_app_codes.redirect_uri IS 'The redirect_uri provided during authorization, to be verified during token exchange (RFC 6749 §4.1.3).'; +COMMENT ON COLUMN oauth2_provider_app_codes.scope IS 'Space-separated scope negotiated at authorization time, drawn from the api_key_scope vocabulary. Always set; coder:all records an unrestricted grant.'; + CREATE TABLE oauth2_provider_app_secrets ( id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -2633,7 +2637,9 @@ CREATE TABLE oauth2_provider_app_tokens ( api_key_id text NOT NULL, audience text, user_id uuid NOT NULL, - app_id uuid NOT NULL + app_id uuid NOT NULL, + scope text NOT NULL, + CONSTRAINT oauth2_provider_app_tokens_scope_not_empty CHECK ((scope <> ''::text)) ); COMMENT ON COLUMN oauth2_provider_app_tokens.refresh_hash IS 'Refresh tokens provide a way to refresh an access token (API key). An expired API key can be refreshed if this token is not yet expired, meaning this expiry can outlive an API key.'; @@ -2644,6 +2650,8 @@ COMMENT ON COLUMN oauth2_provider_app_tokens.user_id IS 'Denormalized user ID fo COMMENT ON COLUMN oauth2_provider_app_tokens.app_id IS 'Denormalized app ID so ownership checks (e.g. revocation) do not need to join through app_secret_id, which is NULL for public clients.'; +COMMENT ON COLUMN oauth2_provider_app_tokens.scope IS 'Space-separated scope granted to this token, drawn from the api_key_scope vocabulary. Always set; coder:all records an unrestricted grant. Later phases will narrow this on refresh and never widen it.'; + CREATE TABLE oauth2_provider_apps ( id uuid NOT NULL, created_at timestamp with time zone NOT NULL, diff --git a/coderd/database/migrations/000569_oauth2_scope_columns.down.sql b/coderd/database/migrations/000569_oauth2_scope_columns.down.sql new file mode 100644 index 0000000000..cc1658b160 --- /dev/null +++ b/coderd/database/migrations/000569_oauth2_scope_columns.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE oauth2_provider_app_codes DROP COLUMN scope; + +ALTER TABLE oauth2_provider_app_tokens DROP COLUMN scope; diff --git a/coderd/database/migrations/000569_oauth2_scope_columns.up.sql b/coderd/database/migrations/000569_oauth2_scope_columns.up.sql new file mode 100644 index 0000000000..2cfb8d8315 --- /dev/null +++ b/coderd/database/migrations/000569_oauth2_scope_columns.up.sql @@ -0,0 +1,30 @@ +-- The scope negotiated at /oauth2/authorize travels with the grant itself: +-- recorded on the code when it is issued, then carried onto the token it is +-- exchanged for, so a refresh can be narrowed against what was actually +-- granted rather than against the app's current allowlist. +-- +-- Existing rows are unrestricted in fact rather than by omission, since +-- apikey.Generate mints every OAuth2 access key with the coder:all scope. +-- The backfill writes that down. Both columns are then NOT NULL with no +-- default, so a grant's authority is always stated explicitly and a caller +-- that omits the column fails instead of silently issuing full access. + +ALTER TABLE oauth2_provider_app_codes ADD COLUMN scope text; + +ALTER TABLE oauth2_provider_app_tokens ADD COLUMN scope text; + +UPDATE oauth2_provider_app_codes SET scope = 'coder:all' WHERE scope IS NULL; + +UPDATE oauth2_provider_app_tokens SET scope = 'coder:all' WHERE scope IS NULL; + +ALTER TABLE oauth2_provider_app_codes + ALTER COLUMN scope SET NOT NULL, + ADD CONSTRAINT oauth2_provider_app_codes_scope_not_empty CHECK (scope <> ''); + +ALTER TABLE oauth2_provider_app_tokens + ALTER COLUMN scope SET NOT NULL, + ADD CONSTRAINT oauth2_provider_app_tokens_scope_not_empty CHECK (scope <> ''); + +COMMENT ON COLUMN oauth2_provider_app_codes.scope IS 'Space-separated scope negotiated at authorization time, drawn from the api_key_scope vocabulary. Always set; coder:all records an unrestricted grant.'; + +COMMENT ON COLUMN oauth2_provider_app_tokens.scope IS 'Space-separated scope granted to this token, drawn from the api_key_scope vocabulary. Always set; coder:all records an unrestricted grant. Later phases will narrow this on refresh and never widen it.'; diff --git a/coderd/database/models.go b/coderd/database/models.go index a68b7e54bc..c80a23665c 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -5583,6 +5583,8 @@ type OAuth2ProviderAppCode struct { StateHash sql.NullString `db:"state_hash" json:"state_hash"` // The redirect_uri provided during authorization, to be verified during token exchange (RFC 6749 §4.1.3). RedirectUri sql.NullString `db:"redirect_uri" json:"redirect_uri"` + // Space-separated scope negotiated at authorization time, drawn from the api_key_scope vocabulary. Always set; coder:all records an unrestricted grant. + Scope string `db:"scope" json:"scope"` } type OAuth2ProviderAppSecret struct { @@ -5611,6 +5613,8 @@ type OAuth2ProviderAppToken struct { UserID uuid.UUID `db:"user_id" json:"user_id"` // Denormalized app ID so ownership checks (e.g. revocation) do not need to join through app_secret_id, which is NULL for public clients. AppID uuid.UUID `db:"app_id" json:"app_id"` + // Space-separated scope granted to this token, drawn from the api_key_scope vocabulary. Always set; coder:all records an unrestricted grant. Later phases will narrow this on refresh and never widen it. + Scope string `db:"scope" json:"scope"` } type Organization struct { diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 084e61602c..44fc384b61 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -18882,6 +18882,70 @@ func TestGetActiveUsersAuthorizationRolesParity(t *testing.T) { } } +func TestOAuth2ProviderScopeNotEmpty(t *testing.T) { + t.Parallel() + if testing.Short() { + t.SkipNow() + } + + // An unrestricted grant is recorded as an explicit sentinel rather than as + // an absent value, so an insert that fails to carry the negotiated scope + // forward is rejected instead of silently issuing full access. + t.Run("Code", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitLong) + + user := dbgen.User(t, db, database.User{}) + app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{}) + + _, err := db.InsertOAuth2ProviderAppCode(ctx, database.InsertOAuth2ProviderAppCodeParams{ + ID: uuid.New(), + CreatedAt: dbtime.Now(), + ExpiresAt: dbtime.Now().Add(time.Minute), + SecretPrefix: []byte("prefix"), + HashedSecret: []byte("hashed-secret"), + AppID: app.ID, + UserID: user.ID, + ResourceUri: sql.NullString{}, + CodeChallenge: sql.NullString{}, + CodeChallengeMethod: sql.NullString{}, + StateHash: sql.NullString{}, + RedirectUri: sql.NullString{}, + Scope: "", + }) + require.True(t, database.IsCheckViolation(err, database.CheckOauth2ProviderAppCodesScopeNotEmpty), + "empty scope must be rejected, got %v", err) + }) + + t.Run("Token", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitLong) + + user := dbgen.User(t, db, database.User{}) + app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{}) + secret := dbgen.OAuth2ProviderAppSecret(t, db, database.OAuth2ProviderAppSecret{AppID: app.ID}) + key, _ := dbgen.APIKey(t, db, database.APIKey{UserID: user.ID}) + + _, err := db.InsertOAuth2ProviderAppToken(ctx, database.InsertOAuth2ProviderAppTokenParams{ + ID: uuid.New(), + CreatedAt: dbtime.Now(), + ExpiresAt: dbtime.Now().Add(time.Minute), + HashPrefix: []byte("prefix"), + RefreshHash: []byte("hashed-secret"), + AppID: app.ID, + AppSecretID: uuid.NullUUID{UUID: secret.ID, Valid: true}, + APIKeyID: key.ID, + UserID: user.ID, + Audience: sql.NullString{}, + Scope: "", + }) + require.True(t, database.IsCheckViolation(err, database.CheckOauth2ProviderAppTokensScopeNotEmpty), + "empty scope must be rejected, got %v", err) + }) +} + func TestGetAIModelPrices(t *testing.T) { t.Parallel() diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index bb57a04bb3..e15e05da2c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -19055,7 +19055,7 @@ func (q *sqlQuerier) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) } const getOAuth2ProviderAppCodeByID = `-- name: GetOAuth2ProviderAppCodeByID :one -SELECT id, created_at, expires_at, secret_prefix, hashed_secret, user_id, app_id, resource_uri, code_challenge, code_challenge_method, state_hash, redirect_uri FROM oauth2_provider_app_codes WHERE id = $1 +SELECT id, created_at, expires_at, secret_prefix, hashed_secret, user_id, app_id, resource_uri, code_challenge, code_challenge_method, state_hash, redirect_uri, scope FROM oauth2_provider_app_codes WHERE id = $1 ` func (q *sqlQuerier) GetOAuth2ProviderAppCodeByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderAppCode, error) { @@ -19074,12 +19074,13 @@ func (q *sqlQuerier) GetOAuth2ProviderAppCodeByID(ctx context.Context, id uuid.U &i.CodeChallengeMethod, &i.StateHash, &i.RedirectUri, + &i.Scope, ) return i, err } const getOAuth2ProviderAppCodeByPrefix = `-- name: GetOAuth2ProviderAppCodeByPrefix :one -SELECT id, created_at, expires_at, secret_prefix, hashed_secret, user_id, app_id, resource_uri, code_challenge, code_challenge_method, state_hash, redirect_uri FROM oauth2_provider_app_codes WHERE secret_prefix = $1 +SELECT id, created_at, expires_at, secret_prefix, hashed_secret, user_id, app_id, resource_uri, code_challenge, code_challenge_method, state_hash, redirect_uri, scope FROM oauth2_provider_app_codes WHERE secret_prefix = $1 ` func (q *sqlQuerier) GetOAuth2ProviderAppCodeByPrefix(ctx context.Context, secretPrefix []byte) (OAuth2ProviderAppCode, error) { @@ -19098,6 +19099,7 @@ func (q *sqlQuerier) GetOAuth2ProviderAppCodeByPrefix(ctx context.Context, secre &i.CodeChallengeMethod, &i.StateHash, &i.RedirectUri, + &i.Scope, ) return i, err } @@ -19176,7 +19178,7 @@ func (q *sqlQuerier) GetOAuth2ProviderAppSecretsByAppID(ctx context.Context, app } const getOAuth2ProviderAppTokenByAPIKeyID = `-- name: GetOAuth2ProviderAppTokenByAPIKeyID :one -SELECT id, created_at, expires_at, hash_prefix, refresh_hash, app_secret_id, api_key_id, audience, user_id, app_id FROM oauth2_provider_app_tokens WHERE api_key_id = $1 +SELECT id, created_at, expires_at, hash_prefix, refresh_hash, app_secret_id, api_key_id, audience, user_id, app_id, scope FROM oauth2_provider_app_tokens WHERE api_key_id = $1 ` func (q *sqlQuerier) GetOAuth2ProviderAppTokenByAPIKeyID(ctx context.Context, apiKeyID string) (OAuth2ProviderAppToken, error) { @@ -19193,12 +19195,13 @@ func (q *sqlQuerier) GetOAuth2ProviderAppTokenByAPIKeyID(ctx context.Context, ap &i.Audience, &i.UserID, &i.AppID, + &i.Scope, ) return i, err } const getOAuth2ProviderAppTokenByPrefix = `-- name: GetOAuth2ProviderAppTokenByPrefix :one -SELECT id, created_at, expires_at, hash_prefix, refresh_hash, app_secret_id, api_key_id, audience, user_id, app_id FROM oauth2_provider_app_tokens WHERE hash_prefix = $1 +SELECT id, created_at, expires_at, hash_prefix, refresh_hash, app_secret_id, api_key_id, audience, user_id, app_id, scope FROM oauth2_provider_app_tokens WHERE hash_prefix = $1 ` func (q *sqlQuerier) GetOAuth2ProviderAppTokenByPrefix(ctx context.Context, hashPrefix []byte) (OAuth2ProviderAppToken, error) { @@ -19215,6 +19218,7 @@ func (q *sqlQuerier) GetOAuth2ProviderAppTokenByPrefix(ctx context.Context, hash &i.Audience, &i.UserID, &i.AppID, + &i.Scope, ) return i, err } @@ -19506,7 +19510,8 @@ INSERT INTO oauth2_provider_app_codes ( code_challenge, code_challenge_method, state_hash, - redirect_uri + redirect_uri, + scope ) VALUES( $1, $2, @@ -19519,8 +19524,9 @@ INSERT INTO oauth2_provider_app_codes ( $9, $10, $11, - $12 -) RETURNING id, created_at, expires_at, secret_prefix, hashed_secret, user_id, app_id, resource_uri, code_challenge, code_challenge_method, state_hash, redirect_uri + $12, + $13 +) RETURNING id, created_at, expires_at, secret_prefix, hashed_secret, user_id, app_id, resource_uri, code_challenge, code_challenge_method, state_hash, redirect_uri, scope ` type InsertOAuth2ProviderAppCodeParams struct { @@ -19536,6 +19542,7 @@ type InsertOAuth2ProviderAppCodeParams struct { CodeChallengeMethod sql.NullString `db:"code_challenge_method" json:"code_challenge_method"` StateHash sql.NullString `db:"state_hash" json:"state_hash"` RedirectUri sql.NullString `db:"redirect_uri" json:"redirect_uri"` + Scope string `db:"scope" json:"scope"` } func (q *sqlQuerier) InsertOAuth2ProviderAppCode(ctx context.Context, arg InsertOAuth2ProviderAppCodeParams) (OAuth2ProviderAppCode, error) { @@ -19552,6 +19559,7 @@ func (q *sqlQuerier) InsertOAuth2ProviderAppCode(ctx context.Context, arg Insert arg.CodeChallengeMethod, arg.StateHash, arg.RedirectUri, + arg.Scope, ) var i OAuth2ProviderAppCode err := row.Scan( @@ -19567,6 +19575,7 @@ func (q *sqlQuerier) InsertOAuth2ProviderAppCode(ctx context.Context, arg Insert &i.CodeChallengeMethod, &i.StateHash, &i.RedirectUri, + &i.Scope, ) return i, err } @@ -19631,7 +19640,8 @@ INSERT INTO oauth2_provider_app_tokens ( app_secret_id, api_key_id, user_id, - audience + audience, + scope ) VALUES( $1, $2, @@ -19642,8 +19652,9 @@ INSERT INTO oauth2_provider_app_tokens ( $7, $8, $9, - $10 -) RETURNING id, created_at, expires_at, hash_prefix, refresh_hash, app_secret_id, api_key_id, audience, user_id, app_id + $10, + $11 +) RETURNING id, created_at, expires_at, hash_prefix, refresh_hash, app_secret_id, api_key_id, audience, user_id, app_id, scope ` type InsertOAuth2ProviderAppTokenParams struct { @@ -19657,6 +19668,7 @@ type InsertOAuth2ProviderAppTokenParams struct { APIKeyID string `db:"api_key_id" json:"api_key_id"` UserID uuid.UUID `db:"user_id" json:"user_id"` Audience sql.NullString `db:"audience" json:"audience"` + Scope string `db:"scope" json:"scope"` } func (q *sqlQuerier) InsertOAuth2ProviderAppToken(ctx context.Context, arg InsertOAuth2ProviderAppTokenParams) (OAuth2ProviderAppToken, error) { @@ -19671,6 +19683,7 @@ func (q *sqlQuerier) InsertOAuth2ProviderAppToken(ctx context.Context, arg Inser arg.APIKeyID, arg.UserID, arg.Audience, + arg.Scope, ) var i OAuth2ProviderAppToken err := row.Scan( @@ -19684,6 +19697,7 @@ func (q *sqlQuerier) InsertOAuth2ProviderAppToken(ctx context.Context, arg Inser &i.Audience, &i.UserID, &i.AppID, + &i.Scope, ) return i, err } diff --git a/coderd/database/queries/oauth2.sql b/coderd/database/queries/oauth2.sql index f9272b69ea..52a2031fbf 100644 --- a/coderd/database/queries/oauth2.sql +++ b/coderd/database/queries/oauth2.sql @@ -137,7 +137,8 @@ INSERT INTO oauth2_provider_app_codes ( code_challenge, code_challenge_method, state_hash, - redirect_uri + redirect_uri, + scope ) VALUES( $1, $2, @@ -150,7 +151,8 @@ INSERT INTO oauth2_provider_app_codes ( $9, $10, $11, - $12 + $12, + $13 ) RETURNING *; -- name: DeleteOAuth2ProviderAppCodeByID :exec @@ -170,7 +172,8 @@ INSERT INTO oauth2_provider_app_tokens ( app_secret_id, api_key_id, user_id, - audience + audience, + scope ) VALUES( $1, $2, @@ -181,7 +184,8 @@ INSERT INTO oauth2_provider_app_tokens ( $7, $8, $9, - $10 + $10, + $11 ) RETURNING *; -- name: GetOAuth2ProviderAppTokenByPrefix :one diff --git a/coderd/oauth2_test.go b/coderd/oauth2_test.go index d2a78bc225..e63df79a3c 100644 --- a/coderd/oauth2_test.go +++ b/coderd/oauth2_test.go @@ -442,6 +442,7 @@ func TestOAuth2ProviderTokenExchange(t *testing.T) { HashedSecret: []byte(hashedCode), AppID: apps.Default.ID, UserID: user.ID, + Scope: string(database.ApiKeyScopeCoderAll), }) return err }, @@ -737,6 +738,7 @@ func TestOAuth2ProviderTokenRefresh(t *testing.T) { AppSecretID: uuid.NullUUID{UUID: secret.ID, Valid: true}, APIKeyID: newKey.ID, UserID: user.ID, + Scope: string(database.ApiKeyScopeCoderAll), }) require.NoError(t, err) diff --git a/coderd/oauth2provider/authorize.go b/coderd/oauth2provider/authorize.go index 8b0e9cceec..d9396c2085 100644 --- a/coderd/oauth2provider/authorize.go +++ b/coderd/oauth2provider/authorize.go @@ -271,6 +271,11 @@ func ProcessAuthorize(db database.Store) http.HandlerFunc { CodeChallengeMethod: sql.NullString{String: params.codeChallengeMethod, Valid: params.codeChallengeMethod != ""}, StateHash: hashOAuth2State(params.state), RedirectUri: sql.NullString{String: params.redirectURL.String(), Valid: params.redirectURIProvided}, + // Scope negotiation lands in a later phase. Until the + // requested scope is validated against the app's allowlist, + // persisting it here would store unvalidated client input, so + // the code records an unrestricted grant. + Scope: string(database.ApiKeyScopeCoderAll), }) if err != nil { return xerrors.Errorf("insert oauth2 authorization code: %w", err) diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go index fc9527f418..1beedf8cc3 100644 --- a/coderd/oauth2provider/tokens.go +++ b/coderd/oauth2provider/tokens.go @@ -436,6 +436,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database APIKeyID: newKey.ID, UserID: dbCode.UserID, Audience: dbCode.ResourceUri, + Scope: dbCode.Scope, }) if err != nil { return xerrors.Errorf("insert oauth2 refresh token: %w", err) @@ -560,6 +561,10 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut APIKeyID: newKey.ID, UserID: dbToken.UserID, Audience: dbToken.Audience, + // RFC 6749 §6: a refresh with no scope parameter is granted the + // originally granted scope. Later phases narrow this against + // req.Scope; they never widen it. + Scope: dbToken.Scope, }) if err != nil { return xerrors.Errorf("insert oauth2 refresh token: %w", err)