Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfb90065

Browse files
committed
feat(oauth2): add RFC 8707 resource indicators and audience validation
Implements RFC 8707 Resource Indicators for OAuth2 provider to enable properaudience validation and token binding for multi-tenant scenarios.Key changes:- Add resource parameter support to authorization and token endpoints- Implement server-side audience validation for opaque tokens- Add database fields: ResourceUri (codes) and Audience (tokens)- Add comprehensive resource parameter validation logic- Add cross-resource audience validation in API middleware- Add extensive test coverage for RFC 8707 scenarios- Enhance PKCE implementation with timing attack protectionThis enables OAuth2 clients to specify target resource servers and preventstoken abuse across different Coder deployments through proper audience binding.Change-Id: I3924cb2139e837e3ac0b0bd40a5aeb59637ebc1bSigned-off-by: Thomas Kosiewski <tk@coder.com>
1 parent018694a commitfb90065

File tree

12 files changed

+551
-14
lines changed

12 files changed

+551
-14
lines changed

‎CLAUDE.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,29 @@ Read [cursor rules](.cursorrules).
120120
4.Run`make gen` again
121121
5.Run`make lint` to catch any remaining issues
122122

123+
###In-MemoryDatabaseTesting
124+
125+
When adding new database fields:
126+
127+
- **CRITICAL**:Update`coderd/database/dbmem/dbmem.go` in-memory implementations
128+
-The`Insert*` functions must includeALL new fields, not just basic ones
129+
-Common issue:Tests pass with real database but fail with in-memory database due to missing field mappings
130+
-Always verify in-memory database functions match the real database schema after migrations
131+
132+
Example pattern:
133+
134+
```go
135+
// In dbmem.go - ensure ALL fields are included
136+
code := database.OAuth2ProviderAppCode{
137+
ID: arg.ID,
138+
CreatedAt: arg.CreatedAt,
139+
// ... existing fields ...
140+
ResourceUri: arg.ResourceUri, // New field
141+
CodeChallenge: arg.CodeChallenge, // New field
142+
CodeChallengeMethod: arg.CodeChallengeMethod, // New field
143+
}
144+
```
145+
123146
##Architecture
124147

125148
###CoreComponents
@@ -204,6 +227,12 @@ When working on OAuth2 provider features:
204227
-Avoid dependency on referer headersfor security decisions
205228
-Support proper state parameter validation
206229

230+
6. **RFC8707ResourceIndicators**:
231+
-Store resource parameters in databasefor server-sidevalidation (opaque tokens)
232+
-Validate resource consistency between authorization and token requests
233+
-Support audience validation in refresh token flows
234+
-Resource parameter is optional but must be consistent when provided
235+
207236
###OAuth2ErrorHandlingPattern
208237

209238
```go
@@ -258,3 +287,6 @@ Always run the full test suite after OAuth2 changes:
258287
2. **"package should be X_test"** -Use`package_test` namingfor test files
259288
3. **SQLtype errors** -Use`sql.Null*` typesfor nullable fields
260289
4. **Missing newlines** -Ensure files end with newline character
290+
5. **OAuth2 tests failing but scripts working** -Check in-memory database implementations in`dbmem.go`
291+
6. **Resource indicator validation failing** -Ensure database stores and retrieves resource parameters correctly
292+
7. **PKCE tests failing** -Verify both authorization code storage and token exchange handlePKCE fields

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2165,6 +2165,25 @@ func (q *querier) GetOAuth2ProviderAppSecretsByAppID(ctx context.Context, appID
21652165
returnq.db.GetOAuth2ProviderAppSecretsByAppID(ctx,appID)
21662166
}
21672167

2168+
func (q*querier)GetOAuth2ProviderAppTokenByAPIKeyID(ctx context.Context,apiKeyIDstring) (database.OAuth2ProviderAppToken,error) {
2169+
token,err:=q.db.GetOAuth2ProviderAppTokenByAPIKeyID(ctx,apiKeyID)
2170+
iferr!=nil {
2171+
return database.OAuth2ProviderAppToken{},err
2172+
}
2173+
2174+
// Get the associated API key to check ownership
2175+
apiKey,err:=q.db.GetAPIKeyByID(ctx,token.APIKeyID)
2176+
iferr!=nil {
2177+
return database.OAuth2ProviderAppToken{},err
2178+
}
2179+
2180+
iferr:=q.authorizeContext(ctx,policy.ActionRead,rbac.ResourceOauth2AppCodeToken.WithOwner(apiKey.UserID.String()));err!=nil {
2181+
return database.OAuth2ProviderAppToken{},err
2182+
}
2183+
2184+
returntoken,nil
2185+
}
2186+
21682187
func (q*querier)GetOAuth2ProviderAppTokenByPrefix(ctx context.Context,hashPrefix []byte) (database.OAuth2ProviderAppToken,error) {
21692188
token,err:=q.db.GetOAuth2ProviderAppTokenByPrefix(ctx,hashPrefix)
21702189
iferr!=nil {

‎coderd/database/dbmem/dbmem.go

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4050,6 +4050,19 @@ func (q *FakeQuerier) GetOAuth2ProviderAppSecretsByAppID(_ context.Context, appI
40504050
return []database.OAuth2ProviderAppSecret{},sql.ErrNoRows
40514051
}
40524052

4053+
func (q*FakeQuerier)GetOAuth2ProviderAppTokenByAPIKeyID(_ context.Context,apiKeyIDstring) (database.OAuth2ProviderAppToken,error) {
4054+
q.mutex.Lock()
4055+
deferq.mutex.Unlock()
4056+
4057+
for_,token:=rangeq.oauth2ProviderAppTokens {
4058+
iftoken.APIKeyID==apiKeyID {
4059+
returntoken,nil
4060+
}
4061+
}
4062+
4063+
return database.OAuth2ProviderAppToken{},sql.ErrNoRows
4064+
}
4065+
40534066
func (q*FakeQuerier)GetOAuth2ProviderAppTokenByPrefix(_ context.Context,hashPrefix []byte) (database.OAuth2ProviderAppToken,error) {
40544067
q.mutex.Lock()
40554068
deferq.mutex.Unlock()
@@ -8938,13 +8951,16 @@ func (q *FakeQuerier) InsertOAuth2ProviderAppCode(_ context.Context, arg databas
89388951
for_,app:=rangeq.oauth2ProviderApps {
89398952
ifapp.ID==arg.AppID {
89408953
code:= database.OAuth2ProviderAppCode{
8941-
ID:arg.ID,
8942-
CreatedAt:arg.CreatedAt,
8943-
ExpiresAt:arg.ExpiresAt,
8944-
SecretPrefix:arg.SecretPrefix,
8945-
HashedSecret:arg.HashedSecret,
8946-
UserID:arg.UserID,
8947-
AppID:arg.AppID,
8954+
ID:arg.ID,
8955+
CreatedAt:arg.CreatedAt,
8956+
ExpiresAt:arg.ExpiresAt,
8957+
SecretPrefix:arg.SecretPrefix,
8958+
HashedSecret:arg.HashedSecret,
8959+
UserID:arg.UserID,
8960+
AppID:arg.AppID,
8961+
ResourceUri:arg.ResourceUri,
8962+
CodeChallenge:arg.CodeChallenge,
8963+
CodeChallengeMethod:arg.CodeChallengeMethod,
89488964
}
89498965
q.oauth2ProviderAppCodes=append(q.oauth2ProviderAppCodes,code)
89508966
returncode,nil

‎coderd/database/dbmetrics/querymetrics.go

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

‎coderd/database/dbmock/dbmock.go

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

‎coderd/database/querier.go

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

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/oauth2.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ INSERT INTO oauth2_provider_app_tokens (
136136
-- name: GetOAuth2ProviderAppTokenByPrefix :one
137137
SELECT*FROM oauth2_provider_app_tokensWHERE hash_prefix= $1;
138138

139+
-- name: GetOAuth2ProviderAppTokenByAPIKeyID :one
140+
SELECT*FROM oauth2_provider_app_tokensWHERE api_key_id= $1;
141+
139142
-- name: GetOAuth2ProviderAppsByUserID :many
140143
SELECT
141144
COUNT(DISTINCToauth2_provider_app_tokens.id)as token_count,

‎coderd/httpmw/apikey.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,16 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
240240
})
241241
}
242242

243+
// Validate OAuth2 provider app token audience (RFC 8707) if applicable
244+
ifkey.LoginType==database.LoginTypeOAuth2ProviderApp {
245+
iferr:=validateOAuth2ProviderAppTokenAudience(ctx,cfg.DB,*key,r);err!=nil {
246+
returnoptionalWrite(http.StatusForbidden, codersdk.Response{
247+
Message:"Token audience mismatch",
248+
Detail:err.Error(),
249+
})
250+
}
251+
}
252+
243253
// We only check OIDC stuff if we have a valid APIKey. An expired key means we don't trust the requestor
244254
// really is the user whose key they have, and so we shouldn't be doing anything on their behalf including possibly
245255
// refreshing the OIDC token.
@@ -446,6 +456,47 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
446456
returnkey,&actor,true
447457
}
448458

459+
// validateOAuth2ProviderAppTokenAudience validates that an OAuth2 provider app token
460+
// is being used with the correct audience/resource server (RFC 8707).
461+
funcvalidateOAuth2ProviderAppTokenAudience(ctx context.Context,db database.Store,key database.APIKey,r*http.Request)error {
462+
// Get the OAuth2 provider app token to check its audience
463+
//nolint:gocritic // System needs to access token for audience validation
464+
token,err:=db.GetOAuth2ProviderAppTokenByAPIKeyID(dbauthz.AsSystemRestricted(ctx),key.ID)
465+
iferr!=nil {
466+
returnxerrors.Errorf("failed to get OAuth2 token: %w",err)
467+
}
468+
469+
// If no audience is set, allow the request (for backward compatibility)
470+
if!token.Audience.Valid||token.Audience.String=="" {
471+
returnnil
472+
}
473+
474+
// Extract the expected audience from the request
475+
expectedAudience:=extractExpectedAudience(r)
476+
477+
// Validate that the token's audience matches the expected audience
478+
iftoken.Audience.String!=expectedAudience {
479+
returnxerrors.Errorf("token audience %q does not match expected audience %q",
480+
token.Audience.String,expectedAudience)
481+
}
482+
483+
returnnil
484+
}
485+
486+
// extractExpectedAudience determines the expected audience for the current request.
487+
// This should match the resource parameter used during authorization.
488+
funcextractExpectedAudience(r*http.Request)string {
489+
// For MCP compliance, the audience should be the canonical URI of the resource server
490+
// This typically matches the access URL of the Coder deployment
491+
scheme:="https"
492+
ifr.TLS==nil {
493+
scheme="http"
494+
}
495+
496+
// Use the Host header to construct the canonical audience URI
497+
returnfmt.Sprintf("%s://%s",scheme,r.Host)
498+
}
499+
449500
// UserRBACSubject fetches a user's rbac.Subject from the database. It pulls all roles from both
450501
// site and organization scopes. It also pulls the groups, and the user's status.
451502
funcUserRBACSubject(ctx context.Context,db database.Store,userID uuid.UUID,scope rbac.ExpandableScope) (rbac.Subject, database.UserStatus,error) {

‎coderd/identityprovider/authorize.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ func extractAuthorizeParams(r *http.Request, callbackURL *url.URL) (authorizePar
4444
codeChallenge:p.String(vals,"","code_challenge"),
4545
codeChallengeMethod:p.String(vals,"","code_challenge_method"),
4646
}
47+
// Validate resource indicator syntax (RFC 8707): must be absolute URI without fragment
48+
ifparams.resource!="" {
49+
ifu,err:=url.Parse(params.resource);err!=nil||u.Scheme==""||u.Fragment!="" {
50+
p.Errors=append(p.Errors, codersdk.ValidationError{
51+
Field:"resource",
52+
Detail:"must be an absolute URI without fragment",
53+
})
54+
}
55+
}
4756

4857
p.ErrorExcessParams(vals)
4958
iflen(p.Errors)>0 {

‎coderd/identityprovider/tokens.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ var (
3333
errBadToken=xerrors.New("Invalid token")
3434
// errInvalidPKCE means the PKCE verification failed.
3535
errInvalidPKCE=xerrors.New("invalid code_verifier")
36+
// errInvalidResource means the resource parameter validation failed.
37+
errInvalidResource=xerrors.New("invalid resource parameter")
3638
)
3739

3840
// OAuth2Error represents an OAuth2-compliant error response.
@@ -87,6 +89,15 @@ func extractTokenParams(r *http.Request, callbackURL *url.URL) (tokenParams, []c
8789
codeVerifier:p.String(vals,"","code_verifier"),
8890
resource:p.String(vals,"","resource"),
8991
}
92+
// Validate resource parameter syntax (RFC 8707): must be absolute URI without fragment
93+
ifparams.resource!="" {
94+
ifu,err:=url.Parse(params.resource);err!=nil||u.Scheme==""||u.Fragment!="" {
95+
p.Errors=append(p.Errors, codersdk.ValidationError{
96+
Field:"resource",
97+
Detail:"must be an absolute URI without fragment",
98+
})
99+
}
100+
}
90101

91102
p.ErrorExcessParams(vals)
92103
iflen(p.Errors)>0 {
@@ -153,6 +164,10 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerF
153164
writeOAuth2Error(ctx,rw,http.StatusBadRequest,"invalid_grant","The PKCE code verifier is invalid")
154165
return
155166
}
167+
iferrors.Is(err,errInvalidResource) {
168+
writeOAuth2Error(ctx,rw,http.StatusBadRequest,"invalid_target","The resource parameter is invalid")
169+
return
170+
}
156171
iferrors.Is(err,errBadToken) {
157172
writeOAuth2Error(ctx,rw,http.StatusBadRequest,"invalid_grant","The refresh token is invalid or expired")
158173
return
@@ -229,6 +244,20 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
229244
}
230245
}
231246

247+
// Verify resource parameter consistency (RFC 8707)
248+
ifdbCode.ResourceUri.Valid&&dbCode.ResourceUri.String!="" {
249+
// Resource was specified during authorization - it must match in token request
250+
ifparams.resource=="" {
251+
return oauth2.Token{},errInvalidResource
252+
}
253+
ifparams.resource!=dbCode.ResourceUri.String {
254+
return oauth2.Token{},errInvalidResource
255+
}
256+
}elseifparams.resource!="" {
257+
// Resource was not specified during authorization but is now provided
258+
return oauth2.Token{},errInvalidResource
259+
}
260+
232261
// Generate a refresh token.
233262
refreshToken,err:=GenerateSecret()
234263
iferr!=nil {
@@ -335,6 +364,14 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
335364
return oauth2.Token{},errBadToken
336365
}
337366

367+
// Verify resource parameter consistency for refresh tokens (RFC 8707)
368+
ifparams.resource!="" {
369+
// If resource is provided in refresh request, it must match the original token's audience
370+
if!dbToken.Audience.Valid||dbToken.Audience.String!=params.resource {
371+
return oauth2.Token{},errInvalidResource
372+
}
373+
}
374+
338375
// Grab the user roles so we can perform the refresh as the user.
339376
//nolint:gocritic // There is no user yet so we must use the system.
340377
prevKey,err:=db.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx),dbToken.APIKeyID)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp