opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: add oauth2 scope columns and single-use delete queries (#28007)

Browse Source 
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.

<details>
<summary>End-to-end scope enforcement flow (green marks what this PR
touches)</summary>

```mermaid
flowchart TD
    subgraph authorize["/oauth2/authorize"]
        AZ1["ShowAuthorizePage (GET)<br/>renders consent page"]
        AZ2["ProcessAuthorize (POST)<br/>scope parsed, then discarded"]
        Q1["InsertOAuth2ProviderAppCode<br/>gains a Scope param"]
        AZ1 --> AZ2 --> Q1
    end

    Q1 --> CODES[("oauth2_provider_app_codes<br/>new column: scope text NOT NULL")]

    subgraph codegrant["POST /oauth2/token, grant_type=authorization_code"]
        G1["authorizationCodeGrant"]
        Q2["GetOAuth2ProviderAppCodeByPrefix<br/>now returns Scope"]
        Q4["DeleteOAuth2ProviderAppCodeByIDReturningRow<br/>added, no caller yet"]
        G2["apikey.Generate + UserRBACSubject<br/>hardcoded to full access"]
        G1 --> Q2 --> G2
        G1 -.-> Q4
    end

    CODES --> G1
    G2 --> Q3

    Q3["InsertOAuth2ProviderAppToken<br/>gains a Scope param"]
    Q3 --> TOKENS[("oauth2_provider_app_tokens<br/>new column: scope text NOT NULL")]

    subgraph refresh["POST /oauth2/token, grant_type=refresh_token"]
        G3["refreshTokenGrant"]
        Q5["GetOAuth2ProviderAppTokenByPrefix<br/>now returns Scope"]
        Q6["DeleteAPIKeyByIDReturningRow<br/>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.

</details>

<details>
<summary>Suggested reading order</summary>

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`.

</details>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bobby Ho
2026-08-13 09:58:51 -07:00
committed by GitHub
co-authored by Claude Opus 5
parent 8d4d0b35dd
commit 990d24dc42
13 changed files with 161 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/queries.sql.go
Generated
+24 -10
View File
@@ -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
}