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

feat: oauth2 - add RFC 8707 resource indicators and audience validation#18575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletionsCLAUDE.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -89,6 +89,10 @@ Read [cursor rules](.cursorrules).
- Format: `{number}_{description}.{up|down}.sql`
- Number must be unique and sequential
- Always include both up and down migrations
- **Use helper scripts**:
- `./coderd/database/migrations/create_migration.sh "migration name"` - Creates new migration files
- `./coderd/database/migrations/fix_migration_numbers.sh` - Renumbers migrations to avoid conflicts
- `./coderd/database/migrations/create_fixture.sh "fixture name"` - Creates test fixtures for migrations

2. **Update database queries**:
- MUST DO! Any changes to database - adding queries, modifying queries should be done in the `coderd/database/queries/*.sql` files
Expand DownExpand Up@@ -125,6 +129,29 @@ Read [cursor rules](.cursorrules).
4. Run `make gen` again
5. Run `make lint` to catch any remaining issues

### In-Memory Database Testing

When adding new database fields:

- **CRITICAL**: Update `coderd/database/dbmem/dbmem.go` in-memory implementations
- The `Insert*` functions must include ALL new fields, not just basic ones
- Common issue: Tests pass with real database but fail with in-memory database due to missing field mappings
- Always verify in-memory database functions match the real database schema after migrations

Example pattern:

```go
// In dbmem.go - ensure ALL fields are included
code := database.OAuth2ProviderAppCode{
ID: arg.ID,
CreatedAt: arg.CreatedAt,
// ... existing fields ...
ResourceUri: arg.ResourceUri, // New field
CodeChallenge: arg.CodeChallenge, // New field
CodeChallengeMethod: arg.CodeChallengeMethod, // New field
}
```

## Architecture

### Core Components
Expand DownExpand Up@@ -209,6 +236,12 @@ When working on OAuth2 provider features:
- Avoid dependency on referer headers for security decisions
- Support proper state parameter validation

6. **RFC 8707 Resource Indicators**:
- Store resource parameters in database for server-side validation (opaque tokens)
- Validate resource consistency between authorization and token requests
- Support audience validation in refresh token flows
- Resource parameter is optional but must be consistent when provided

### OAuth2 Error Handling Pattern

```go
Expand DownExpand Up@@ -265,3 +298,6 @@ Always run the full test suite after OAuth2 changes:
4. **Missing newlines** - Ensure files end with newline character
5. **Tests passing locally but failing in CI** - Check if `dbmem` implementation needs updating
6. **OAuth2 endpoints returning wrong error format** - Ensure OAuth2 endpoints return RFC 6749 compliant errors
7. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go`
8. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly
9. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields
3 changes: 3 additions & 0 deletionscoderd/coderd.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -781,6 +781,7 @@ func New(options *Options) *API {
Optional: false,
SessionTokenFunc: nil, // Default behavior
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
Logger: options.Logger,
})
// Same as above but it redirects to the login page.
apiKeyMiddlewareRedirect := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
Expand All@@ -791,6 +792,7 @@ func New(options *Options) *API {
Optional: false,
SessionTokenFunc: nil, // Default behavior
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
Logger: options.Logger,
})
// Same as the first but it's optional.
apiKeyMiddlewareOptional := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
Expand All@@ -801,6 +803,7 @@ func New(options *Options) *API {
Optional: true,
SessionTokenFunc: nil, // Default behavior
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
Logger: options.Logger,
})

workspaceAgentInfo := httpmw.ExtractWorkspaceAgentAndLatestBuild(httpmw.ExtractWorkspaceAgentAndLatestBuildConfig{
Expand Down
26 changes: 16 additions & 10 deletionscoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2181,19 +2181,29 @@ func (q *querier) GetOAuth2ProviderAppSecretsByAppID(ctx context.Context, appID
return q.db.GetOAuth2ProviderAppSecretsByAppID(ctx, appID)
}

func (q *querier)GetOAuth2ProviderAppTokenByPrefix(ctx context.Context,hashPrefix []byte) (database.OAuth2ProviderAppToken, error) {
token, err := q.db.GetOAuth2ProviderAppTokenByPrefix(ctx,hashPrefix)
func (q *querier)GetOAuth2ProviderAppTokenByAPIKeyID(ctx context.Context,apiKeyID string) (database.OAuth2ProviderAppToken, error) {
token, err := q.db.GetOAuth2ProviderAppTokenByAPIKeyID(ctx,apiKeyID)
if err != nil {
return database.OAuth2ProviderAppToken{}, err
}
// The user ID is on the API key so that has to be fetched.
key, err := q.db.GetAPIKeyByID(ctx, token.APIKeyID)

if err := q.authorizeContext(ctx, policy.ActionRead, token.RBACObject()); err != nil {
return database.OAuth2ProviderAppToken{}, err
}

return token, nil
}

func (q *querier) GetOAuth2ProviderAppTokenByPrefix(ctx context.Context, hashPrefix []byte) (database.OAuth2ProviderAppToken, error) {
token, err := q.db.GetOAuth2ProviderAppTokenByPrefix(ctx, hashPrefix)
if err != nil {
return database.OAuth2ProviderAppToken{}, err
}
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceOauth2AppCodeToken.WithOwner(key.UserID.String())); err != nil {

if err := q.authorizeContext(ctx, policy.ActionRead, token.RBACObject()); err != nil {
return database.OAuth2ProviderAppToken{}, err
}

return token, nil
}

Expand DownExpand Up@@ -3646,11 +3656,7 @@ func (q *querier) InsertOAuth2ProviderAppSecret(ctx context.Context, arg databas
}

func (q *querier) InsertOAuth2ProviderAppToken(ctx context.Context, arg database.InsertOAuth2ProviderAppTokenParams) (database.OAuth2ProviderAppToken, error) {
key, err := q.db.GetAPIKeyByID(ctx, arg.APIKeyID)
if err != nil {
return database.OAuth2ProviderAppToken{}, err
}
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceOauth2AppCodeToken.WithOwner(key.UserID.String())); err != nil {
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceOauth2AppCodeToken.WithOwner(arg.UserID.String())); err != nil {
return database.OAuth2ProviderAppToken{}, err
}
return q.db.InsertOAuth2ProviderAppToken(ctx, arg)
Expand Down
24 changes: 21 additions & 3 deletionscoderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5195,12 +5195,11 @@ func (s *MethodTestSuite) TestOAuth2ProviderApps() {
_ = dbgen.OAuth2ProviderAppToken(s.T(), db, database.OAuth2ProviderAppToken{
AppSecretID: secret.ID,
APIKeyID: key.ID,
UserID: user.ID,
HashPrefix: []byte(fmt.Sprintf("%d", i)),
})
}
expectedApp := app
expectedApp.CreatedAt = createdAt
expectedApp.UpdatedAt = createdAt
check.Args(user.ID).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()), policy.ActionRead).Returns([]database.GetOAuth2ProviderAppsByUserIDRow{
{
OAuth2ProviderApp: expectedApp,
Expand DownExpand Up@@ -5363,6 +5362,7 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() {
check.Args(database.InsertOAuth2ProviderAppTokenParams{
AppSecretID: secret.ID,
APIKeyID: key.ID,
UserID: user.ID,
}).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()), policy.ActionCreate)
}))
s.Run("GetOAuth2ProviderAppTokenByPrefix", s.Subtest(func(db database.Store, check *expects) {
Expand All@@ -5377,8 +5377,25 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() {
token := dbgen.OAuth2ProviderAppToken(s.T(), db, database.OAuth2ProviderAppToken{
AppSecretID: secret.ID,
APIKeyID: key.ID,
UserID: user.ID,
})
check.Args(token.HashPrefix).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()), policy.ActionRead)
check.Args(token.HashPrefix).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()).WithID(token.ID), policy.ActionRead).Returns(token)
}))
s.Run("GetOAuth2ProviderAppTokenByAPIKeyID", s.Subtest(func(db database.Store, check *expects) {
user := dbgen.User(s.T(), db, database.User{})
key, _ := dbgen.APIKey(s.T(), db, database.APIKey{
UserID: user.ID,
})
app := dbgen.OAuth2ProviderApp(s.T(), db, database.OAuth2ProviderApp{})
secret := dbgen.OAuth2ProviderAppSecret(s.T(), db, database.OAuth2ProviderAppSecret{
AppID: app.ID,
})
token := dbgen.OAuth2ProviderAppToken(s.T(), db, database.OAuth2ProviderAppToken{
AppSecretID: secret.ID,
APIKeyID: key.ID,
UserID: user.ID,
})
check.Args(token.APIKeyID).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()).WithID(token.ID), policy.ActionRead).Returns(token)
}))
s.Run("DeleteOAuth2ProviderAppTokensByAppAndUserID", s.Subtest(func(db database.Store, check *expects) {
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
Expand All@@ -5394,6 +5411,7 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() {
_ = dbgen.OAuth2ProviderAppToken(s.T(), db, database.OAuth2ProviderAppToken{
AppSecretID: secret.ID,
APIKeyID: key.ID,
UserID: user.ID,
HashPrefix: []byte(fmt.Sprintf("%d", i)),
})
}
Expand Down
1 change: 1 addition & 0 deletionscoderd/database/dbgen/dbgen.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1190,6 +1190,7 @@ func OAuth2ProviderAppToken(t testing.TB, db database.Store, seed database.OAuth
RefreshHash: takeFirstSlice(seed.RefreshHash, []byte("hashed-secret")),
AppSecretID: takeFirst(seed.AppSecretID, uuid.New()),
APIKeyID: takeFirst(seed.APIKeyID, uuid.New().String()),
UserID: takeFirst(seed.UserID, uuid.New()),
Audience: seed.Audience,
})
require.NoError(t, err, "insert oauth2 app token")
Expand Down
54 changes: 35 additions & 19 deletionscoderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4054,6 +4054,19 @@ func (q *FakeQuerier) GetOAuth2ProviderAppSecretsByAppID(_ context.Context, appI
return []database.OAuth2ProviderAppSecret{}, sql.ErrNoRows
}

func (q *FakeQuerier) GetOAuth2ProviderAppTokenByAPIKeyID(_ context.Context, apiKeyID string) (database.OAuth2ProviderAppToken, error) {
q.mutex.Lock()
defer q.mutex.Unlock()

for _, token := range q.oauth2ProviderAppTokens {
if token.APIKeyID == apiKeyID {
return token, nil
}
}

return database.OAuth2ProviderAppToken{}, sql.ErrNoRows
}

func (q *FakeQuerier) GetOAuth2ProviderAppTokenByPrefix(_ context.Context, hashPrefix []byte) (database.OAuth2ProviderAppToken, error) {
q.mutex.Lock()
defer q.mutex.Unlock()
Expand DownExpand Up@@ -4099,13 +4112,8 @@ func (q *FakeQuerier) GetOAuth2ProviderAppsByUserID(_ context.Context, userID uu
}
if len(tokens) > 0 {
rows = append(rows, database.GetOAuth2ProviderAppsByUserIDRow{
OAuth2ProviderApp: database.OAuth2ProviderApp{
CallbackURL: app.CallbackURL,
ID: app.ID,
Icon: app.Icon,
Name: app.Name,
},
TokenCount: int64(len(tokens)),
OAuth2ProviderApp: app,
TokenCount: int64(len(tokens)),
})
}
}
Expand DownExpand Up@@ -8918,12 +8926,15 @@ func (q *FakeQuerier) InsertOAuth2ProviderApp(_ context.Context, arg database.In

//nolint:gosimple // Go wants database.OAuth2ProviderApp(arg), but we cannot be sure the structs will remain identical.
app := database.OAuth2ProviderApp{
ID: arg.ID,
CreatedAt: arg.CreatedAt,
UpdatedAt: arg.UpdatedAt,
Name: arg.Name,
Icon: arg.Icon,
CallbackURL: arg.CallbackURL,
ID: arg.ID,
CreatedAt: arg.CreatedAt,
UpdatedAt: arg.UpdatedAt,
Name: arg.Name,
Icon: arg.Icon,
CallbackURL: arg.CallbackURL,
RedirectUris: arg.RedirectUris,
ClientType: arg.ClientType,
DynamicallyRegistered: arg.DynamicallyRegistered,
}
q.oauth2ProviderApps = append(q.oauth2ProviderApps, app)

Expand DownExpand Up@@ -9008,6 +9019,8 @@ func (q *FakeQuerier) InsertOAuth2ProviderAppToken(_ context.Context, arg databa
RefreshHash: arg.RefreshHash,
APIKeyID: arg.APIKeyID,
AppSecretID: arg.AppSecretID,
UserID: arg.UserID,
Audience: arg.Audience,
}
q.oauth2ProviderAppTokens = append(q.oauth2ProviderAppTokens, token)
return token, nil
Expand DownExpand Up@@ -10790,12 +10803,15 @@ func (q *FakeQuerier) UpdateOAuth2ProviderAppByID(_ context.Context, arg databas
for index, app := range q.oauth2ProviderApps {
if app.ID == arg.ID {
newApp := database.OAuth2ProviderApp{
ID: arg.ID,
CreatedAt: app.CreatedAt,
UpdatedAt: arg.UpdatedAt,
Name: arg.Name,
Icon: arg.Icon,
CallbackURL: arg.CallbackURL,
ID: arg.ID,
CreatedAt: app.CreatedAt,
UpdatedAt: arg.UpdatedAt,
Name: arg.Name,
Icon: arg.Icon,
CallbackURL: arg.CallbackURL,
RedirectUris: arg.RedirectUris,
ClientType: arg.ClientType,
DynamicallyRegistered: arg.DynamicallyRegistered,
}
q.oauth2ProviderApps[index] = newApp
return newApp, nil
Expand Down
7 changes: 7 additions & 0 deletionscoderd/database/dbmetrics/querymetrics.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

15 changes: 15 additions & 0 deletionscoderd/database/dbmock/dbmock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

8 changes: 7 additions & 1 deletioncoderd/database/dump.sql
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

1 change: 1 addition & 0 deletionscoderd/database/foreign_key_constraint.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
-- Remove the denormalized user_id column from oauth2_provider_app_tokens
ALTER TABLE oauth2_provider_app_tokens
DROP CONSTRAINT IF EXISTS fk_oauth2_provider_app_tokens_user_id;

ALTER TABLE oauth2_provider_app_tokens
DROP COLUMN IF EXISTS user_id;
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
-- Add user_id column to oauth2_provider_app_tokens for performance optimization
-- This eliminates the need to join with api_keys table for authorization checks
ALTER TABLE oauth2_provider_app_tokens
ADD COLUMN user_id uuid;

-- Backfill existing records with user_id from the associated api_key
UPDATE oauth2_provider_app_tokens
SET user_id = api_keys.user_id
FROM api_keys
WHERE oauth2_provider_app_tokens.api_key_id = api_keys.id;

-- Make user_id NOT NULL after backfilling
ALTER TABLE oauth2_provider_app_tokens
ALTER COLUMN user_id SET NOT NULL;

-- Add foreign key constraint to maintain referential integrity
ALTER TABLE oauth2_provider_app_tokens
ADD CONSTRAINT fk_oauth2_provider_app_tokens_user_id
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE;

COMMENT ON COLUMN oauth2_provider_app_tokens.user_id IS 'Denormalized user ID for performance optimization in authorization checks';
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp