mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat!: add admin-controlled dynamic client registration toggle (#27316)
`POST /oauth2/register` (RFC 7591 Dynamic Client Registration) has exactly one gate today: `ExperimentOAuth2`, a static, process-lifetime flag that wraps the entire `/oauth2/*` route tree as an all-or-nothing switch. That flag is scheduled for removal at GA, which would leave DCR with zero admin control at all once it is gone. Add a persistent, DCR-specific `oauth2_dcr_enabled` deployment setting, independent of the experiment system, so admin control over DCR survives GA. `POST /oauth2/register` checks the flag and rejects new registrations with an RFC 7591-shaped `403` when disabled; discovery metadata (`GET /.well-known/oauth-authorization-server`) conditionally omits `registration_endpoint`. A new audited `GET`/`PUT /api/v2/oauth2-provider/settings` endpoint lets an owner toggle it live, no restart required. The setting defaults to disabled, matching the canonical design proposal; disabling only stops new self-registrations, clients that already registered continue to authorize and exchange tokens normally. Address issue described in [ENG-3056](https://linear.app/codercom/issue/ENG-3056/oauth2-dcr-admin-configurable-enabledisable). ## Where this sits in the request path ```mermaid sequenceDiagram autonumber participant A as Admin participant S as coderd participant DB as site_configs<br/>(oauth2_dcr_enabled) participant C as OAuth2/MCP Client Note over A,S: Admin toggles DCR (new) A->>S: PUT /api/v2/oauth2-provider/settings<br/>{dynamic_client_registration_enabled: false} S->>S: authorizeContext(ActionUpdate, ResourceDeploymentConfig) S->>DB: UPSERT oauth2_dcr_enabled = false S-->>A: 200 OK (audited) Note over C,S: Client discovery + registration afterward C->>S: GET /.well-known/oauth-authorization-server S->>DB: GetOAuth2DCREnabled (system ctx, every request, no cache) DB-->>S: false S-->>C: 200 metadata, registration_endpoint omitted C->>S: POST /oauth2/register S->>DB: GetOAuth2DCREnabled (system ctx, every request, no cache) DB-->>S: false S-->>C: 403 invalid_request,<br/>"Dynamic client registration is disabled" Note over C,S: A client that registered before the change is unaffected C->>S: GET /oauth2/authorize?client_id=... Note over S: no DCR-enabled check on this path S-->>C: 200 (proceeds normally) C->>S: PUT/DELETE /oauth2/clients/{client_id} (RFC 7592 self-management) Note over S: no DCR-enabled check on this path either S-->>C: 200 (proceeds normally) ``` ## Files changed: manual vs. generated Reviewers should focus on the **manual** files. The **generated** ones are `make gen` output that follows mechanically from the manual changes and don't need direct review. <details> <summary><b>Manual files (26)</b> — click to expand, grouped the same way as "Suggested review order" below</summary> **1. Database** | File | What changed | |---|---| | `coderd/database/queries/siteconfig.sql` | New `GetOAuth2DCREnabled`/`UpsertOAuth2DCREnabled` query pair on the existing generic `site_configs` table. No schema change. | | `coderd/database/dbauthz/dbauthz.go` | RBAC check (`rbac.ResourceDeploymentConfig`) on the two new query methods; extends the `subjectSystemOAuth2` system-actor role with read-only `ResourceDeploymentConfig` access, needed so the public discovery/registration endpoints can read the flag via `dbauthz.AsSystemOAuth2`. | | `coderd/database/dbauthz/dbauthz_test.go` | RBAC assertion coverage for `GetOAuth2DCREnabled`/`UpsertOAuth2DCREnabled` in the method-coverage test suite. | **2. Request gating (the actual feature)** | File | What changed | |---|---| | `coderd/oauth2provider/registration.go` | The actual gate: `CreateDynamicClientRegistration` reads the flag first and returns an RFC 7591-shaped `403` when disabled (defaults disabled if never configured). | | `coderd/oauth2provider/registration_test.go` | New unit test, `TestCreateDynamicClientRegistration_DCREnabled`: calls the handler directly (no HTTP server), covering enabled / explicitly disabled / never-configured. | | `coderd/oauth2provider/metadata.go` | `GetAuthorizationServerMetadata` conditionally omits `registration_endpoint` from discovery metadata when DCR is disabled. | | `coderd/oauth2provider/metadata_test.go` | New unit test, `TestGetAuthorizationServerMetadata_DCREnabled`: same three states, for the discovery handler. | **3. Admin settings endpoint** | File | What changed | |---|---| | `codersdk/oauth2.go` | New `OAuth2ProviderSettings` SDK type plus `Client.OAuth2ProviderSettings`/`PutOAuth2ProviderSettings` methods. | | `coderd/oauth2.go` | New `oauth2ProviderSettings`/`putOAuth2ProviderSettings` admin handlers (audited via `audit.InitRequest`); updates the `GetAuthorizationServerMetadata` call site to pass `api.Database`. | | `coderd/coderd.go` | Registers `GET`/`PUT /api/v2/oauth2-provider/settings`. | | `coderd/oauth2_provider_settings_test.go` | New test file: admin `GET`/`PUT` round-trip, default-disabled-before-any-`PUT`, and `403` for a non-owner on both `GET` and `PUT`. | **4. Audit wiring** | File | What changed | |---|---| | `coderd/database/types.go` | New `database.OAuth2ProviderSettings` audit-only struct (mirrors `NotificationsSettings`). | | `coderd/audit/diff.go` | Adds the new struct to the `Auditable` type union. | | `coderd/audit/request.go` | Adds the new struct to all four dispatch switches (`ResourceTarget`, `ResourceID`, `ResourceType`, `ResourceRequiresOrgID`). | | `codersdk/audit.go` | New API-facing `ResourceTypeOAuth2ProviderSettings` constant and its `FriendlyString` case. | | `enterprise/audit/table.go` | Field-level audit action map (`ActionTrack`/`ActionIgnore`) for the new struct. | | `coderd/database/migrations/000546_audit_oauth2_provider_settings.up.sql` | Adds `oauth2_provider_settings` to the `resource_type` Postgres enum, required for the audit wiring above (`resource_type` is a real enum, not a Go-only value). | | `coderd/database/migrations/000546_audit_oauth2_provider_settings.down.sql` | No-op (`ALTER TYPE ... ADD VALUE` can't be reverted). | **5. Test-suite ripple from the disabled-by-default flip** | File | What changed | |---|---| | `coderd/oauth2provider/oauth2providertest/helpers.go` | New shared test helper, `EnableDCR`, since DCR now defaults to disabled and many pre-existing tests need it turned on to register a client. | | `coderd/oauth2_test.go` | Adds `TestOAuth2DynamicClientRegistrationDisabled` (registers a client, disables DCR, verifies new registration is rejected while the existing client's self-management, authorize, and token exchange all keep working); calls `EnableDCR` in every pre-existing test that registers a client. | | `coderd/oauth2_error_compliance_test.go` | Calls `EnableDCR` in every test that registers a client, so RFC-error-format assertions aren't masked by the new disabled-by-default gate. | | `coderd/oauth2_metadata_validation_test.go` | Same: `EnableDCR` added to every registration-dependent test. | | `coderd/oauth2_security_test.go` | Same. | | `coderd/oauth2provider/validation_test.go` | Same (near-duplicate of `oauth2_metadata_validation_test.go` in a different package). | | `coderd/oauth2provider/provider_test.go` | Same. | | `coderd/mcp/mcp_e2e_test.go` | Same, for the MCP end-to-end dynamic-registration flow test. | </details> <details> <summary><b>Generated files (12)</b> — from <code>make gen</code>, no need to review directly</summary> `coderd/apidoc/docs.go`, `coderd/apidoc/swagger.json`, `coderd/database/dbmetrics/querymetrics.go`, `coderd/database/dbmock/dbmock.go`, `coderd/database/dump.sql`, `coderd/database/models.go`, `coderd/database/querier.go`, `coderd/database/queries.sql.go`, `docs/admin/security/audit-logs.md`, `docs/reference/api/enterprise.md`, `docs/reference/api/schemas.md`, `site/src/api/typesGenerated.ts`. </details> ## Suggested review order ### 1. Database Establishes the persisted setting and its RBAC rule; everything else builds on `GetOAuth2DCREnabled`/`UpsertOAuth2DCREnabled`. 1. `coderd/database/queries/siteconfig.sql` — the two new queries. Same boolean-encoding pattern as the existing `oauth2_github_default_eligible` key right above them in the same file. 2. `coderd/database/dbauthz/dbauthz.go` — the RBAC wrapper for those two queries, plus the `subjectSystemOAuth2` role extension (search this file for `ResourceDeploymentConfig`, it appears in both spots). 3. `coderd/database/dbauthz/dbauthz_test.go` — asserts the RBAC checks from (2) actually fire. ### 2. Request gating (the actual feature) Where `POST /oauth2/register` and discovery metadata change behavior. 1. `coderd/oauth2provider/registration.go` — the primary gate. Read this first; it's the feature. 2. `coderd/oauth2provider/registration_test.go` — its new unit test, exercising the gate's three states directly against the handler. 3. `coderd/oauth2provider/metadata.go` — the same gating pattern applied to the discovery `GET` endpoint. 4. `coderd/oauth2provider/metadata_test.go` — its new unit test. ### 3. Admin settings endpoint How an owner flips the setting live. 1. `codersdk/oauth2.go` — the `OAuth2ProviderSettings` SDK type and `Client` methods first; this is the public contract everything below implements against. 2. `coderd/oauth2.go` — the `GET`/`PUT` handlers themselves. 3. `coderd/coderd.go` — route registration, to see where those handlers get wired in. 4. `coderd/oauth2_provider_settings_test.go` — round-trip and permission tests. ### 4. Audit wiring Plumbing required so step 3's `PUT` is auditable; mechanical except for (3). 1. `coderd/database/types.go` — the audit-only struct; everything else in this layer exists to plumb it through. 2. `coderd/audit/diff.go` — adds it to the `Auditable` type union (the compiler enforces this one). 3. `coderd/audit/request.go` — the four dispatch switches; the one part of this layer worth reading closely. 4. `codersdk/audit.go` — the API-facing resource type constant. 5. `enterprise/audit/table.go` — the field-action map. 6. `coderd/database/migrations/000546_audit_oauth2_provider_settings.{up,down}.sql` — read last; a consequence of needing a new `resource_type` enum value for (1)-(5), not a design decision of its own. ### 5. Test-suite ripple from the disabled-by-default flip 1. `coderd/oauth2provider/oauth2providertest/helpers.go` — the new `EnableDCR` helper. Read first to understand the fix pattern before seeing it applied repeatedly. 2. `coderd/oauth2_test.go` — next, since it also contains the new `TestOAuth2DynamicClientRegistrationDisabled`, not just `EnableDCR` call sites. 3. The rest, in any order, they're mechanical repeats of the same one-line addition: `coderd/oauth2_error_compliance_test.go`, `coderd/oauth2_metadata_validation_test.go`, `coderd/oauth2_security_test.go`, `coderd/oauth2provider/validation_test.go`, `coderd/oauth2provider/provider_test.go`, `coderd/mcp/mcp_e2e_test.go`. ## Explicitly out of scope Per the design proposal: rate limiting on `POST /oauth2/register` (tracked separately), retroactively affecting already-registered clients when DCR is disabled (this only gates new self-registration), and an Initial Access Token requirement (a separate, follow-up ticket).
This commit is contained in:
Generated
+72
@@ -4648,6 +4648,68 @@ const docTemplate = `{
|
||||
]
|
||||
}
|
||||
},
|
||||
"/api/v2/oauth2-provider/settings": {
|
||||
"get": {
|
||||
"produces": [
|
||||
"application/json"
|
||||
],
|
||||
"tags": [
|
||||
"Enterprise"
|
||||
],
|
||||
"summary": "Get OAuth2 provider settings.",
|
||||
"operationId": "get-oauth2-provider-settings",
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.OAuth2ProviderSettings"
|
||||
}
|
||||
}
|
||||
},
|
||||
"security": [
|
||||
{
|
||||
"CoderSessionToken": []
|
||||
}
|
||||
]
|
||||
},
|
||||
"put": {
|
||||
"consumes": [
|
||||
"application/json"
|
||||
],
|
||||
"produces": [
|
||||
"application/json"
|
||||
],
|
||||
"tags": [
|
||||
"Enterprise"
|
||||
],
|
||||
"summary": "Update OAuth2 provider settings.",
|
||||
"operationId": "update-oauth2-provider-settings",
|
||||
"parameters": [
|
||||
{
|
||||
"description": "OAuth2 provider settings request",
|
||||
"name": "request",
|
||||
"in": "body",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.OAuth2ProviderSettings"
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.OAuth2ProviderSettings"
|
||||
}
|
||||
}
|
||||
},
|
||||
"security": [
|
||||
{
|
||||
"CoderSessionToken": []
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"/api/v2/organizations": {
|
||||
"get": {
|
||||
"produces": [
|
||||
@@ -21930,6 +21992,14 @@ const docTemplate = `{
|
||||
"OAuth2ProviderResponseTypeToken"
|
||||
]
|
||||
},
|
||||
"codersdk.OAuth2ProviderSettings": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"dynamic_client_registration_enabled": {
|
||||
"type": "boolean"
|
||||
}
|
||||
}
|
||||
},
|
||||
"codersdk.OAuth2TokenEndpointAuthMethod": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
@@ -23734,6 +23804,7 @@ const docTemplate = `{
|
||||
"health_settings",
|
||||
"notifications_settings",
|
||||
"prebuilds_settings",
|
||||
"oauth2_provider_settings",
|
||||
"workspace_proxy",
|
||||
"organization",
|
||||
"oauth2_provider_app",
|
||||
@@ -23771,6 +23842,7 @@ const docTemplate = `{
|
||||
"ResourceTypeHealthSettings",
|
||||
"ResourceTypeNotificationsSettings",
|
||||
"ResourceTypePrebuildsSettings",
|
||||
"ResourceTypeOAuth2ProviderSettings",
|
||||
"ResourceTypeWorkspaceProxy",
|
||||
"ResourceTypeOrganization",
|
||||
"ResourceTypeOAuth2ProviderApp",
|
||||
|
||||
Generated
+62
@@ -4107,6 +4107,58 @@
|
||||
]
|
||||
}
|
||||
},
|
||||
"/api/v2/oauth2-provider/settings": {
|
||||
"get": {
|
||||
"produces": ["application/json"],
|
||||
"tags": ["Enterprise"],
|
||||
"summary": "Get OAuth2 provider settings.",
|
||||
"operationId": "get-oauth2-provider-settings",
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.OAuth2ProviderSettings"
|
||||
}
|
||||
}
|
||||
},
|
||||
"security": [
|
||||
{
|
||||
"CoderSessionToken": []
|
||||
}
|
||||
]
|
||||
},
|
||||
"put": {
|
||||
"consumes": ["application/json"],
|
||||
"produces": ["application/json"],
|
||||
"tags": ["Enterprise"],
|
||||
"summary": "Update OAuth2 provider settings.",
|
||||
"operationId": "update-oauth2-provider-settings",
|
||||
"parameters": [
|
||||
{
|
||||
"description": "OAuth2 provider settings request",
|
||||
"name": "request",
|
||||
"in": "body",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.OAuth2ProviderSettings"
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.OAuth2ProviderSettings"
|
||||
}
|
||||
}
|
||||
},
|
||||
"security": [
|
||||
{
|
||||
"CoderSessionToken": []
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"/api/v2/organizations": {
|
||||
"get": {
|
||||
"produces": ["application/json"],
|
||||
@@ -20010,6 +20062,14 @@
|
||||
"OAuth2ProviderResponseTypeToken"
|
||||
]
|
||||
},
|
||||
"codersdk.OAuth2ProviderSettings": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"dynamic_client_registration_enabled": {
|
||||
"type": "boolean"
|
||||
}
|
||||
}
|
||||
},
|
||||
"codersdk.OAuth2TokenEndpointAuthMethod": {
|
||||
"type": "string",
|
||||
"enum": ["client_secret_basic", "client_secret_post", "none"],
|
||||
@@ -21744,6 +21804,7 @@
|
||||
"health_settings",
|
||||
"notifications_settings",
|
||||
"prebuilds_settings",
|
||||
"oauth2_provider_settings",
|
||||
"workspace_proxy",
|
||||
"organization",
|
||||
"oauth2_provider_app",
|
||||
@@ -21781,6 +21842,7 @@
|
||||
"ResourceTypeHealthSettings",
|
||||
"ResourceTypeNotificationsSettings",
|
||||
"ResourceTypePrebuildsSettings",
|
||||
"ResourceTypeOAuth2ProviderSettings",
|
||||
"ResourceTypeWorkspaceProxy",
|
||||
"ResourceTypeOrganization",
|
||||
"ResourceTypeOAuth2ProviderApp",
|
||||
|
||||
@@ -25,6 +25,7 @@ type Auditable interface {
|
||||
database.OAuth2ProviderApp |
|
||||
database.OAuth2ProviderAppSecret |
|
||||
database.PrebuildsSettings |
|
||||
database.OAuth2ProviderSettings |
|
||||
database.CustomRole |
|
||||
database.AuditableOrganizationMember |
|
||||
database.Organization |
|
||||
|
||||
@@ -112,6 +112,8 @@ func ResourceTarget[T Auditable](tgt T) string {
|
||||
return "" // no target?
|
||||
case database.PrebuildsSettings:
|
||||
return "" // no target?
|
||||
case database.OAuth2ProviderSettings:
|
||||
return "" // no target?
|
||||
case database.OAuth2ProviderApp:
|
||||
return typed.Name
|
||||
case database.OAuth2ProviderAppSecret:
|
||||
@@ -200,6 +202,9 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
|
||||
case database.PrebuildsSettings:
|
||||
// Artificial ID for auditing purposes
|
||||
return typed.ID
|
||||
case database.OAuth2ProviderSettings:
|
||||
// Artificial ID for auditing purposes
|
||||
return typed.ID
|
||||
case database.OAuth2ProviderApp:
|
||||
return typed.ID
|
||||
case database.OAuth2ProviderAppSecret:
|
||||
@@ -273,6 +278,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
|
||||
return database.ResourceTypeNotificationsSettings
|
||||
case database.PrebuildsSettings:
|
||||
return database.ResourceTypePrebuildsSettings
|
||||
case database.OAuth2ProviderSettings:
|
||||
return database.ResourceTypeOauth2ProviderSettings
|
||||
case database.OAuth2ProviderApp:
|
||||
return database.ResourceTypeOauth2ProviderApp
|
||||
case database.OAuth2ProviderAppSecret:
|
||||
@@ -349,6 +356,9 @@ func ResourceRequiresOrgID[T Auditable]() bool {
|
||||
case database.PrebuildsSettings:
|
||||
// Artificial ID for auditing purposes
|
||||
return false
|
||||
case database.OAuth2ProviderSettings:
|
||||
// Artificial ID for auditing purposes
|
||||
return false
|
||||
case database.OAuth2ProviderApp:
|
||||
return false
|
||||
case database.OAuth2ProviderAppSecret:
|
||||
|
||||
@@ -2111,6 +2111,10 @@ func New(options *Options) *API {
|
||||
})
|
||||
})
|
||||
})
|
||||
r.Route("/settings", func(r chi.Router) {
|
||||
r.Get("/", api.oauth2ProviderSettings)
|
||||
r.Put("/", api.putOAuth2ProviderSettings)
|
||||
})
|
||||
})
|
||||
r.Route("/notifications", func(r chi.Router) {
|
||||
r.Use(apiKeyMiddleware)
|
||||
|
||||
@@ -557,6 +557,11 @@ var (
|
||||
// Minimal read permissions that might be needed for OAuth2 operations
|
||||
rbac.ResourceUser.Type: {policy.ActionRead},
|
||||
rbac.ResourceOrganization.Type: {policy.ActionRead},
|
||||
|
||||
// Read-only access to check the DCR enabled/disabled
|
||||
// deployment setting from the public discovery and
|
||||
// registration endpoints.
|
||||
rbac.ResourceDeploymentConfig.Type: {policy.ActionRead},
|
||||
}),
|
||||
User: []rbac.Permission{},
|
||||
ByOrgID: map[string]rbac.OrgPermissions{},
|
||||
@@ -4171,6 +4176,13 @@ func (q *querier) GetNotificationsSettings(ctx context.Context) (string, error)
|
||||
return q.db.GetNotificationsSettings(ctx)
|
||||
}
|
||||
|
||||
func (q *querier) GetOAuth2DCREnabled(ctx context.Context) (bool, error) {
|
||||
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceDeploymentConfig); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return q.db.GetOAuth2DCREnabled(ctx)
|
||||
}
|
||||
|
||||
func (q *querier) GetOAuth2GithubDefaultEligible(ctx context.Context) (bool, error) {
|
||||
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceDeploymentConfig); err != nil {
|
||||
return false, err
|
||||
@@ -9084,6 +9096,13 @@ func (q *querier) UpsertNotificationsSettings(ctx context.Context, value string)
|
||||
return q.db.UpsertNotificationsSettings(ctx, value)
|
||||
}
|
||||
|
||||
func (q *querier) UpsertOAuth2DCREnabled(ctx context.Context, enabled bool) error {
|
||||
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
|
||||
return err
|
||||
}
|
||||
return q.db.UpsertOAuth2DCREnabled(ctx, enabled)
|
||||
}
|
||||
|
||||
func (q *querier) UpsertOAuth2GithubDefaultEligible(ctx context.Context, eligible bool) error {
|
||||
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
|
||||
return err
|
||||
|
||||
@@ -5652,6 +5652,14 @@ func (s *MethodTestSuite) TestSystemFunctions() {
|
||||
dbm.EXPECT().UpsertTelemetryItem(gomock.Any(), arg).Return(nil).AnyTimes()
|
||||
check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
|
||||
}))
|
||||
s.Run("GetOAuth2DCREnabled", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
|
||||
dbm.EXPECT().GetOAuth2DCREnabled(gomock.Any()).Return(false, sql.ErrNoRows).AnyTimes()
|
||||
check.Args().Asserts(rbac.ResourceDeploymentConfig, policy.ActionRead).Errors(sql.ErrNoRows)
|
||||
}))
|
||||
s.Run("UpsertOAuth2DCREnabled", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
|
||||
dbm.EXPECT().UpsertOAuth2DCREnabled(gomock.Any(), true).Return(nil).AnyTimes()
|
||||
check.Args(true).Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
|
||||
}))
|
||||
s.Run("GetOAuth2GithubDefaultEligible", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
|
||||
dbm.EXPECT().GetOAuth2GithubDefaultEligible(gomock.Any()).Return(false, sql.ErrNoRows).AnyTimes()
|
||||
check.Args().Asserts(rbac.ResourceDeploymentConfig, policy.ActionRead).Errors(sql.ErrNoRows)
|
||||
|
||||
+16
@@ -2457,6 +2457,14 @@ func (m queryMetricsStore) GetNotificationsSettings(ctx context.Context) (string
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) GetOAuth2DCREnabled(ctx context.Context) (bool, error) {
|
||||
start := time.Now()
|
||||
r0, r1 := m.s.GetOAuth2DCREnabled(ctx)
|
||||
m.queryLatencies.WithLabelValues("GetOAuth2DCREnabled").Observe(time.Since(start).Seconds())
|
||||
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetOAuth2DCREnabled").Inc()
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) GetOAuth2GithubDefaultEligible(ctx context.Context) (bool, error) {
|
||||
start := time.Now()
|
||||
r0, r1 := m.s.GetOAuth2GithubDefaultEligible(ctx)
|
||||
@@ -6497,6 +6505,14 @@ func (m queryMetricsStore) UpsertNotificationsSettings(ctx context.Context, valu
|
||||
return r0
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) UpsertOAuth2DCREnabled(ctx context.Context, enabled bool) error {
|
||||
start := time.Now()
|
||||
r0 := m.s.UpsertOAuth2DCREnabled(ctx, enabled)
|
||||
m.queryLatencies.WithLabelValues("UpsertOAuth2DCREnabled").Observe(time.Since(start).Seconds())
|
||||
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpsertOAuth2DCREnabled").Inc()
|
||||
return r0
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) UpsertOAuth2GithubDefaultEligible(ctx context.Context, eligible bool) error {
|
||||
start := time.Now()
|
||||
r0 := m.s.UpsertOAuth2GithubDefaultEligible(ctx, eligible)
|
||||
|
||||
Generated
+29
@@ -4558,6 +4558,21 @@ func (mr *MockStoreMockRecorder) GetNotificationsSettings(ctx any) *gomock.Call
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationsSettings", reflect.TypeOf((*MockStore)(nil).GetNotificationsSettings), ctx)
|
||||
}
|
||||
|
||||
// GetOAuth2DCREnabled mocks base method.
|
||||
func (m *MockStore) GetOAuth2DCREnabled(ctx context.Context) (bool, error) {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "GetOAuth2DCREnabled", ctx)
|
||||
ret0, _ := ret[0].(bool)
|
||||
ret1, _ := ret[1].(error)
|
||||
return ret0, ret1
|
||||
}
|
||||
|
||||
// GetOAuth2DCREnabled indicates an expected call of GetOAuth2DCREnabled.
|
||||
func (mr *MockStoreMockRecorder) GetOAuth2DCREnabled(ctx any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOAuth2DCREnabled", reflect.TypeOf((*MockStore)(nil).GetOAuth2DCREnabled), ctx)
|
||||
}
|
||||
|
||||
// GetOAuth2GithubDefaultEligible mocks base method.
|
||||
func (m *MockStore) GetOAuth2GithubDefaultEligible(ctx context.Context) (bool, error) {
|
||||
m.ctrl.T.Helper()
|
||||
@@ -12153,6 +12168,20 @@ func (mr *MockStoreMockRecorder) UpsertNotificationsSettings(ctx, value any) *go
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertNotificationsSettings", reflect.TypeOf((*MockStore)(nil).UpsertNotificationsSettings), ctx, value)
|
||||
}
|
||||
|
||||
// UpsertOAuth2DCREnabled mocks base method.
|
||||
func (m *MockStore) UpsertOAuth2DCREnabled(ctx context.Context, enabled bool) error {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "UpsertOAuth2DCREnabled", ctx, enabled)
|
||||
ret0, _ := ret[0].(error)
|
||||
return ret0
|
||||
}
|
||||
|
||||
// UpsertOAuth2DCREnabled indicates an expected call of UpsertOAuth2DCREnabled.
|
||||
func (mr *MockStoreMockRecorder) UpsertOAuth2DCREnabled(ctx, enabled any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertOAuth2DCREnabled", reflect.TypeOf((*MockStore)(nil).UpsertOAuth2DCREnabled), ctx, enabled)
|
||||
}
|
||||
|
||||
// UpsertOAuth2GithubDefaultEligible mocks base method.
|
||||
func (m *MockStore) UpsertOAuth2GithubDefaultEligible(ctx context.Context, eligible bool) error {
|
||||
m.ctrl.T.Helper()
|
||||
|
||||
Generated
+2
-1
@@ -596,7 +596,8 @@ CREATE TYPE resource_type AS ENUM (
|
||||
'group_ai_budget',
|
||||
'user_skill',
|
||||
'ai_gateway_key',
|
||||
'user_ai_budget_override'
|
||||
'user_ai_budget_override',
|
||||
'oauth2_provider_settings'
|
||||
);
|
||||
|
||||
CREATE TYPE shareable_workspace_owners AS ENUM (
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
-- No-op, enum values can't be dropped.
|
||||
@@ -0,0 +1,2 @@
|
||||
ALTER TYPE resource_type
|
||||
ADD VALUE IF NOT EXISTS 'oauth2_provider_settings';
|
||||
@@ -1800,6 +1800,71 @@ func TestMigration000555LegacyNoneLoginToPassword(t *testing.T) {
|
||||
require.Equal(t, "password", gotLoginType)
|
||||
}
|
||||
|
||||
// TestMigration000558AuditOAuth2ProviderSettingsEnumInSingleTxn reproduces
|
||||
// the production upgrade path, where every pending migration in a deploy
|
||||
// runs inside a single transaction (see pgTxnDriver). 000558 adds
|
||||
// 'oauth2_provider_settings' to the resource_type enum via ALTER TYPE ...
|
||||
// ADD VALUE. Postgres forbids using an enum value added by ADD VALUE within
|
||||
// the same transaction that added it, so this confirms the audit write path
|
||||
// (a separate transaction, exactly like commitAudit() performs it for a real
|
||||
// PUT to the DCR settings endpoint) can use the new value immediately after
|
||||
// the migration transaction commits, and that pre-existing audit data from
|
||||
// before the upgrade survives untouched.
|
||||
func TestMigration000558AuditOAuth2ProviderSettingsEnumInSingleTxn(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
sqlDB := testSQLDB(t)
|
||||
ctx := testutil.Context(t, testutil.WaitSuperLong)
|
||||
|
||||
// Apply everything through 557 and commit, simulating a deployment
|
||||
// that was already running the previous release, with real
|
||||
// pre-existing audit data, before the upgrade that adds 558.
|
||||
applyMigrationsInTxn(ctx, t, sqlDB, 1, 557)
|
||||
|
||||
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||
preUpgradeLogID := uuid.New()
|
||||
_, err := sqlDB.ExecContext(ctx, `
|
||||
INSERT INTO audit_logs (
|
||||
id, time, user_id, organization_id, resource_type, resource_id,
|
||||
resource_target, action, diff, status_code, additional_fields,
|
||||
request_id, resource_icon
|
||||
) VALUES (
|
||||
$1, $2, $3, $4, 'oauth2_provider_app', $5, 'pre-upgrade-app',
|
||||
'write', '{}', 200, '{}', $6, ''
|
||||
)`,
|
||||
preUpgradeLogID, now, uuid.New(), uuid.New(), uuid.New(), uuid.New(),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Apply 558 in the same single transaction production uses for the
|
||||
// whole pending batch.
|
||||
applyMigrationsInTxn(ctx, t, sqlDB, 558, 558)
|
||||
|
||||
// Pre-existing audit data survives the upgrade untouched.
|
||||
var resourceTarget string
|
||||
err = sqlDB.QueryRowContext(ctx,
|
||||
`SELECT resource_target FROM audit_logs WHERE id = $1`, preUpgradeLogID,
|
||||
).Scan(&resourceTarget)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "pre-upgrade-app", resourceTarget)
|
||||
|
||||
// The new enum value is usable immediately after the migration
|
||||
// transaction commits, in a separate transaction, exactly like a real
|
||||
// PUT to the DCR settings endpoint would write it.
|
||||
_, err = sqlDB.ExecContext(ctx, `
|
||||
INSERT INTO audit_logs (
|
||||
id, time, user_id, organization_id, resource_type, resource_id,
|
||||
resource_target, action, diff, status_code, additional_fields,
|
||||
request_id, resource_icon
|
||||
) VALUES (
|
||||
$1, $2, $3, $4, 'oauth2_provider_settings', $5, '', 'write',
|
||||
'{}', 200, '{}', $6, ''
|
||||
)`,
|
||||
uuid.New(), now, uuid.New(), uuid.New(), uuid.New(), uuid.New(),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestMigration000498SoftDeleteStaleWorkspaceAgents(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Generated
+4
-1
@@ -3531,6 +3531,7 @@ const (
|
||||
ResourceTypeUserSkill ResourceType = "user_skill"
|
||||
ResourceTypeAIGatewayKey ResourceType = "ai_gateway_key"
|
||||
ResourceTypeUserAIBudgetOverride ResourceType = "user_ai_budget_override"
|
||||
ResourceTypeOauth2ProviderSettings ResourceType = "oauth2_provider_settings"
|
||||
)
|
||||
|
||||
func (e *ResourceType) Scan(src interface{}) error {
|
||||
@@ -3604,7 +3605,8 @@ func (e ResourceType) Valid() bool {
|
||||
ResourceTypeGroupAIBudget,
|
||||
ResourceTypeUserSkill,
|
||||
ResourceTypeAIGatewayKey,
|
||||
ResourceTypeUserAIBudgetOverride:
|
||||
ResourceTypeUserAIBudgetOverride,
|
||||
ResourceTypeOauth2ProviderSettings:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
@@ -3647,6 +3649,7 @@ func AllResourceTypeValues() []ResourceType {
|
||||
ResourceTypeUserSkill,
|
||||
ResourceTypeAIGatewayKey,
|
||||
ResourceTypeUserAIBudgetOverride,
|
||||
ResourceTypeOauth2ProviderSettings,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Generated
+2
@@ -668,6 +668,7 @@ type sqlcQuerier interface {
|
||||
GetNotificationTemplateByID(ctx context.Context, id uuid.UUID) (NotificationTemplate, error)
|
||||
GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error)
|
||||
GetNotificationsSettings(ctx context.Context) (string, error)
|
||||
GetOAuth2DCREnabled(ctx context.Context) (bool, error)
|
||||
GetOAuth2GithubDefaultEligible(ctx context.Context) (bool, error)
|
||||
// RFC 7591/7592 Dynamic Client Registration queries
|
||||
GetOAuth2ProviderAppByClientID(ctx context.Context, id uuid.UUID) (OAuth2ProviderApp, error)
|
||||
@@ -1644,6 +1645,7 @@ type sqlcQuerier interface {
|
||||
// Insert or update notification report generator logs with recent activity.
|
||||
UpsertNotificationReportGeneratorLog(ctx context.Context, arg UpsertNotificationReportGeneratorLogParams) error
|
||||
UpsertNotificationsSettings(ctx context.Context, value string) error
|
||||
UpsertOAuth2DCREnabled(ctx context.Context, enabled bool) error
|
||||
UpsertOAuth2GithubDefaultEligible(ctx context.Context, eligible bool) error
|
||||
UpsertPrebuildsSettings(ctx context.Context, value string) error
|
||||
UpsertProvisionerDaemon(ctx context.Context, arg UpsertProvisionerDaemonParams) (ProvisionerDaemon, error)
|
||||
|
||||
Generated
+36
@@ -25424,6 +25424,20 @@ func (q *sqlQuerier) GetNotificationsSettings(ctx context.Context) (string, erro
|
||||
return notifications_settings, err
|
||||
}
|
||||
|
||||
const getOAuth2DCREnabled = `-- name: GetOAuth2DCREnabled :one
|
||||
SELECT COALESCE(
|
||||
(SELECT value = 'true' FROM site_configs WHERE key = 'oauth2_dcr_enabled'),
|
||||
false
|
||||
)::bool
|
||||
`
|
||||
|
||||
func (q *sqlQuerier) GetOAuth2DCREnabled(ctx context.Context) (bool, error) {
|
||||
row := q.db.QueryRowContext(ctx, getOAuth2DCREnabled)
|
||||
var column_1 bool
|
||||
err := row.Scan(&column_1)
|
||||
return column_1, err
|
||||
}
|
||||
|
||||
const getOAuth2GithubDefaultEligible = `-- name: GetOAuth2GithubDefaultEligible :one
|
||||
SELECT
|
||||
CASE
|
||||
@@ -25817,6 +25831,28 @@ func (q *sqlQuerier) UpsertNotificationsSettings(ctx context.Context, value stri
|
||||
return err
|
||||
}
|
||||
|
||||
const upsertOAuth2DCREnabled = `-- name: UpsertOAuth2DCREnabled :exec
|
||||
INSERT INTO site_configs (key, value)
|
||||
VALUES (
|
||||
'oauth2_dcr_enabled',
|
||||
CASE
|
||||
WHEN $1::bool THEN 'true'
|
||||
ELSE 'false'
|
||||
END
|
||||
)
|
||||
ON CONFLICT (key) DO UPDATE
|
||||
SET value = CASE
|
||||
WHEN $1::bool THEN 'true'
|
||||
ELSE 'false'
|
||||
END
|
||||
WHERE site_configs.key = 'oauth2_dcr_enabled'
|
||||
`
|
||||
|
||||
func (q *sqlQuerier) UpsertOAuth2DCREnabled(ctx context.Context, enabled bool) error {
|
||||
_, err := q.db.ExecContext(ctx, upsertOAuth2DCREnabled, enabled)
|
||||
return err
|
||||
}
|
||||
|
||||
const upsertOAuth2GithubDefaultEligible = `-- name: UpsertOAuth2GithubDefaultEligible :exec
|
||||
INSERT INTO site_configs (key, value)
|
||||
VALUES (
|
||||
|
||||
@@ -120,6 +120,28 @@ SET value = CASE
|
||||
END
|
||||
WHERE site_configs.key = 'oauth2_github_default_eligible';
|
||||
|
||||
-- name: GetOAuth2DCREnabled :one
|
||||
SELECT COALESCE(
|
||||
(SELECT value = 'true' FROM site_configs WHERE key = 'oauth2_dcr_enabled'),
|
||||
false
|
||||
)::bool;
|
||||
|
||||
-- name: UpsertOAuth2DCREnabled :exec
|
||||
INSERT INTO site_configs (key, value)
|
||||
VALUES (
|
||||
'oauth2_dcr_enabled',
|
||||
CASE
|
||||
WHEN sqlc.arg(enabled)::bool THEN 'true'
|
||||
ELSE 'false'
|
||||
END
|
||||
)
|
||||
ON CONFLICT (key) DO UPDATE
|
||||
SET value = CASE
|
||||
WHEN sqlc.arg(enabled)::bool THEN 'true'
|
||||
ELSE 'false'
|
||||
END
|
||||
WHERE site_configs.key = 'oauth2_dcr_enabled';
|
||||
|
||||
-- name: UpsertWebpushVAPIDKeys :exec
|
||||
INSERT INTO site_configs (key, value)
|
||||
VALUES
|
||||
|
||||
@@ -44,6 +44,11 @@ type PrebuildsSettings struct {
|
||||
ReconciliationPaused bool `db:"reconciliation_paused" json:"reconciliation_paused"`
|
||||
}
|
||||
|
||||
type OAuth2ProviderSettings struct {
|
||||
ID uuid.UUID `db:"id" json:"id"`
|
||||
DynamicClientRegistrationEnabled bool `db:"dynamic_client_registration_enabled" json:"dynamic_client_registration_enabled"`
|
||||
}
|
||||
|
||||
type Actions []policy.Action
|
||||
|
||||
func (a *Actions) Scan(src interface{}) error {
|
||||
|
||||
@@ -30,6 +30,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbfake"
|
||||
mcpserver "github.com/coder/coder/v2/coderd/mcp"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/codersdk/toolsdk"
|
||||
@@ -461,6 +462,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
|
||||
t.Cleanup(func() { closer.Close() })
|
||||
|
||||
_ = coderdtest.CreateFirstUser(t, coderClient)
|
||||
oauth2providertest.EnableDCR(t, coderClient)
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
|
||||
+97
-1
@@ -3,7 +3,15 @@ package coderd
|
||||
import (
|
||||
"net/http"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/audit"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/httpapi"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
// @Summary Get OAuth2 applications.
|
||||
@@ -180,7 +188,7 @@ func (api *API) revokeOAuth2Token() http.HandlerFunc {
|
||||
// @Success 200 {object} codersdk.OAuth2AuthorizationServerMetadata
|
||||
// @Router /.well-known/oauth-authorization-server [get]
|
||||
func (api *API) oauth2AuthorizationServerMetadata() http.HandlerFunc {
|
||||
return oauth2provider.GetAuthorizationServerMetadata(api.AccessURL)
|
||||
return oauth2provider.GetAuthorizationServerMetadata(api.Database, api.AccessURL)
|
||||
}
|
||||
|
||||
// @Summary OAuth2 protected resource metadata.
|
||||
@@ -205,6 +213,94 @@ func (api *API) postOAuth2ClientRegistration() http.HandlerFunc {
|
||||
return oauth2provider.CreateDynamicClientRegistration(api.Database, api.AccessURL, api.Auditor.Load(), api.Logger)
|
||||
}
|
||||
|
||||
// @Summary Get OAuth2 provider settings.
|
||||
// @ID get-oauth2-provider-settings
|
||||
// @Security CoderSessionToken
|
||||
// @Produce json
|
||||
// @Tags Enterprise
|
||||
// @Success 200 {object} codersdk.OAuth2ProviderSettings
|
||||
// @Router /api/v2/oauth2-provider/settings [get]
|
||||
func (api *API) oauth2ProviderSettings(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
enabled, err := api.Database.GetOAuth2DCREnabled(ctx)
|
||||
if err != nil {
|
||||
if rbac.IsUnauthorizedError(err) {
|
||||
httpapi.Forbidden(rw)
|
||||
return
|
||||
}
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
httpapi.Write(ctx, rw, http.StatusOK, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(enabled),
|
||||
})
|
||||
}
|
||||
|
||||
// @Summary Update OAuth2 provider settings.
|
||||
// @ID update-oauth2-provider-settings
|
||||
// @Security CoderSessionToken
|
||||
// @Accept json
|
||||
// @Produce json
|
||||
// @Tags Enterprise
|
||||
// @Param request body codersdk.OAuth2ProviderSettings true "OAuth2 provider settings request"
|
||||
// @Success 200 {object} codersdk.OAuth2ProviderSettings
|
||||
// @Router /api/v2/oauth2-provider/settings [put]
|
||||
func (api *API) putOAuth2ProviderSettings(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
var req codersdk.OAuth2ProviderSettings
|
||||
if !httpapi.Read(ctx, rw, r, &req) {
|
||||
return
|
||||
}
|
||||
|
||||
aReq, commitAudit := audit.InitRequest[database.OAuth2ProviderSettings](rw, &audit.RequestParams{
|
||||
Audit: *api.Auditor.Load(),
|
||||
Log: api.Logger,
|
||||
Request: r,
|
||||
Action: database.AuditActionWrite,
|
||||
})
|
||||
defer commitAudit()
|
||||
|
||||
var resolvedEnabled bool
|
||||
err := api.Database.InTx(func(tx database.Store) error {
|
||||
oldEnabled, err := tx.GetOAuth2DCREnabled(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
aReq.Old = database.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: oldEnabled,
|
||||
}
|
||||
|
||||
// A nil field means the caller omitted it, leave the current value
|
||||
// alone rather than overwrite it with a decoded zero value. This
|
||||
// matters once a second field lands in this struct: an older client
|
||||
// that only knows about this field must not silently clear a newer
|
||||
// one it never sent.
|
||||
resolvedEnabled = oldEnabled
|
||||
if req.DynamicClientRegistrationEnabled != nil {
|
||||
resolvedEnabled = *req.DynamicClientRegistrationEnabled
|
||||
if err := tx.UpsertOAuth2DCREnabled(ctx, resolvedEnabled); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
aReq.New = database.OAuth2ProviderSettings{
|
||||
ID: uuid.New(),
|
||||
DynamicClientRegistrationEnabled: resolvedEnabled,
|
||||
}
|
||||
return nil
|
||||
}, &database.TxOptions{TxIdentifier: "update_oauth2_provider_settings"})
|
||||
if err != nil {
|
||||
if rbac.IsUnauthorizedError(err) {
|
||||
httpapi.Forbidden(rw)
|
||||
return
|
||||
}
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
httpapi.Write(ctx, rw, http.StatusOK, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(resolvedEnabled),
|
||||
})
|
||||
}
|
||||
|
||||
// @Summary Get OAuth2 client configuration (RFC 7592)
|
||||
// @ID get-oauth2-client-configuration
|
||||
// @Accept json
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@@ -29,6 +30,7 @@ func TestOAuth2ErrorResponseFormat(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// Make a request that will definitely fail
|
||||
@@ -152,6 +154,7 @@ func TestOAuth2RegistrationErrorCodes(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// Create a copy of the request with a unique client name
|
||||
@@ -214,6 +217,7 @@ func TestOAuth2ManagementErrorCodes(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// First register a valid client to use for management tests
|
||||
@@ -286,6 +290,7 @@ func TestOAuth2ErrorResponseStructure(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// Make a request that will generate an error
|
||||
@@ -335,6 +340,7 @@ func TestOAuth2ErrorHTTPHeaders(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// Make a request that will fail
|
||||
@@ -360,6 +366,7 @@ func TestOAuth2SpecificErrorScenarios(t *testing.T) {
|
||||
// coderd server. Sub-tests that don't need one just ignore it.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("MissingRequiredFields", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@@ -21,6 +22,7 @@ func TestOAuth2ClientMetadataValidation(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps with unique client names.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("RedirectURIValidation", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
@@ -468,6 +470,7 @@ func TestOAuth2ClientNameValidation(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -545,6 +548,7 @@ func TestOAuth2ClientScopeValidation(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -632,6 +636,7 @@ func TestOAuth2ClientMetadataDefaults(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
@@ -675,6 +680,7 @@ func TestOAuth2ClientMetadataEdgeCases(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps with unique client names.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("ExtremelyLongRedirectURI", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
package coderd_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
func TestOAuth2ProviderSettings(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("DefaultDisabled", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
|
||||
settings, err := client.OAuth2ProviderSettings(ctx)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, settings.DynamicClientRegistrationEnabled, "GET must always return a concrete value")
|
||||
require.False(t, *settings.DynamicClientRegistrationEnabled)
|
||||
})
|
||||
|
||||
t.Run("RoundTrip", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
|
||||
updated, err := client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(false),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, updated.DynamicClientRegistrationEnabled)
|
||||
require.False(t, *updated.DynamicClientRegistrationEnabled)
|
||||
|
||||
settings, err := client.OAuth2ProviderSettings(ctx)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, settings.DynamicClientRegistrationEnabled)
|
||||
require.False(t, *settings.DynamicClientRegistrationEnabled)
|
||||
|
||||
updated, err = client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(true),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, updated.DynamicClientRegistrationEnabled)
|
||||
require.True(t, *updated.DynamicClientRegistrationEnabled)
|
||||
})
|
||||
|
||||
t.Run("OmittedFieldLeavesValueUnchanged", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
|
||||
// Establish a known, non-default value first so a subsequent
|
||||
// omitted-field PUT has something to preserve or wrongly clear.
|
||||
_, err := client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(true),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// A PUT with the field omitted (nil) must leave the current value
|
||||
// alone rather than decode to false and silently disable it. This
|
||||
// guards the fix for https://github.com/coder/coder/pull/27316#issuecomment-5086163278:
|
||||
// once a second field lands in this struct, an older client that only
|
||||
// knows about this field would otherwise always send its zero value.
|
||||
updated, err := client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: nil,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, updated.DynamicClientRegistrationEnabled, "response must always reflect the resolved value, never echo back nil")
|
||||
require.True(t, *updated.DynamicClientRegistrationEnabled, "omitted field must not have reset the value to false")
|
||||
|
||||
settings, err := client.OAuth2ProviderSettings(ctx)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, settings.DynamicClientRegistrationEnabled)
|
||||
require.True(t, *settings.DynamicClientRegistrationEnabled, "value must remain unchanged in the database")
|
||||
})
|
||||
|
||||
t.Run("PermissionDenied", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
do func(ctx context.Context, client *codersdk.Client) error
|
||||
}{
|
||||
{
|
||||
name: "Get",
|
||||
do: func(ctx context.Context, client *codersdk.Client) error {
|
||||
_, err := client.OAuth2ProviderSettings(ctx)
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Put",
|
||||
do: func(ctx context.Context, client *codersdk.Client) error {
|
||||
_, err := client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(false),
|
||||
})
|
||||
return err
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
firstUser := coderdtest.CreateFirstUser(t, client)
|
||||
anotherClient, _ := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
|
||||
err := tt.do(ctx, anotherClient)
|
||||
var sdkError *codersdk.Error
|
||||
require.Error(t, err)
|
||||
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
|
||||
require.Equal(t, http.StatusForbidden, sdkError.StatusCode())
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
@@ -21,6 +22,7 @@ func TestOAuth2ClientIsolation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
@@ -108,6 +110,7 @@ func TestOAuth2RegistrationTokenSecurity(t *testing.T) {
|
||||
// independent OAuth2 apps with unique client names.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("InvalidTokenFormats", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
@@ -209,6 +212,7 @@ func TestOAuth2PrivilegeEscalation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := t.Context()
|
||||
|
||||
// Register a basic client
|
||||
@@ -243,6 +247,7 @@ func TestOAuth2PrivilegeEscalation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := t.Context()
|
||||
|
||||
// Test valid custom schemes per RFC 7591/8252
|
||||
@@ -316,6 +321,7 @@ func TestOAuth2InformationDisclosure(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
@@ -399,6 +405,7 @@ func TestOAuth2ConcurrentSecurityOperations(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtime"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/coderd/userpassword"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
@@ -1345,6 +1346,7 @@ func TestOAuth2DynamicClientRegistration(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("BasicRegistration", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
@@ -1440,12 +1442,97 @@ func TestOAuth2DynamicClientRegistration(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestOAuth2DynamicClientRegistrationDisabled verifies the admin DCR
|
||||
// enabled/disabled toggle: new registrations are rejected while a client
|
||||
// that registered before DCR was disabled keeps working (RFC 7592
|
||||
// self-management, authorization, and token exchange are unaffected).
|
||||
func TestOAuth2DynamicClientRegistrationDisabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
// Register a client while DCR is still enabled.
|
||||
regResp, err := client.PostOAuth2ClientRegistration(ctx, codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{oauth2providertest.TestRedirectURI},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(false),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("NewRegistrationRejected", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
_, err := client.PostOAuth2ClientRegistration(ctx, codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{oauth2providertest.TestRedirectURI},
|
||||
})
|
||||
require.Error(t, err)
|
||||
var sdkError *codersdk.Error
|
||||
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
|
||||
require.Equal(t, http.StatusForbidden, sdkError.StatusCode())
|
||||
})
|
||||
|
||||
t.Run("DiscoveryOmitsRegistrationEndpoint", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
res, err := client.Request(ctx, http.MethodGet, "/.well-known/oauth-authorization-server", nil)
|
||||
require.NoError(t, err)
|
||||
defer res.Body.Close()
|
||||
require.Equal(t, http.StatusOK, res.StatusCode)
|
||||
|
||||
var metadata codersdk.OAuth2AuthorizationServerMetadata
|
||||
require.NoError(t, json.NewDecoder(res.Body).Decode(&metadata))
|
||||
require.Empty(t, metadata.RegistrationEndpoint)
|
||||
})
|
||||
|
||||
t.Run("ExistingClientSelfManagementUnaffected", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
config, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, regResp.RegistrationAccessToken)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, regResp.ClientID, config.ClientID)
|
||||
})
|
||||
|
||||
t.Run("ExistingClientAuthorizeAndTokenUnaffected", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
codeVerifier, codeChallenge := oauth2providertest.GeneratePKCE(t)
|
||||
state := oauth2providertest.GenerateState(t)
|
||||
|
||||
code := oauth2providertest.AuthorizeOAuth2App(t, client, client.URL.String(), oauth2providertest.AuthorizeParams{
|
||||
ClientID: regResp.ClientID,
|
||||
ResponseType: "code",
|
||||
RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
State: state,
|
||||
CodeChallenge: codeChallenge,
|
||||
CodeChallengeMethod: "S256",
|
||||
})
|
||||
require.NotEmpty(t, code)
|
||||
|
||||
token := oauth2providertest.ExchangeCodeForToken(t, client.URL.String(), oauth2providertest.TokenExchangeParams{
|
||||
GrantType: "authorization_code",
|
||||
Code: code,
|
||||
ClientID: regResp.ClientID,
|
||||
ClientSecret: regResp.ClientSecret,
|
||||
CodeVerifier: codeVerifier,
|
||||
RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
})
|
||||
require.NotEmpty(t, token.AccessToken)
|
||||
})
|
||||
}
|
||||
|
||||
// TestOAuth2ClientConfiguration tests RFC 7592 client configuration management
|
||||
func TestOAuth2ClientConfiguration(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
// Helper to register a client
|
||||
registerClient := func(t *testing.T) (string, string, string) {
|
||||
@@ -1570,6 +1657,7 @@ func TestOAuth2RegistrationAccessToken(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("ValidToken", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -4,27 +4,43 @@ import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbauthz"
|
||||
"github.com/coder/coder/v2/coderd/httpapi"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
// GetAuthorizationServerMetadata returns an http.HandlerFunc that handles GET /.well-known/oauth-authorization-server
|
||||
func GetAuthorizationServerMetadata(accessURL *url.URL) http.HandlerFunc {
|
||||
func GetAuthorizationServerMetadata(db database.Store, accessURL *url.URL) http.HandlerFunc {
|
||||
return func(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
|
||||
// This is queried on every request rather than cached, for the
|
||||
// same reason as the registration endpoint: discovery is not
|
||||
// expected to be a hot path, and a flood of requests should be
|
||||
// mitigated with rate limiting or firewalling, not a cache.
|
||||
//nolint:gocritic // Public discovery endpoint, no authenticated actor to authorize against.
|
||||
dcrEnabled, err := db.GetOAuth2DCREnabled(dbauthz.AsSystemOAuth2(ctx))
|
||||
if err != nil {
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
|
||||
metadata := codersdk.OAuth2AuthorizationServerMetadata{
|
||||
Issuer: accessURL.String(),
|
||||
AuthorizationEndpoint: accessURL.JoinPath("/oauth2/authorize").String(),
|
||||
TokenEndpoint: accessURL.JoinPath("/oauth2/tokens").String(),
|
||||
RegistrationEndpoint: accessURL.JoinPath("/oauth2/register").String(), // RFC 7591
|
||||
RevocationEndpoint: accessURL.JoinPath("/oauth2/revoke").String(), // RFC 7009
|
||||
RevocationEndpoint: accessURL.JoinPath("/oauth2/revoke").String(), // RFC 7009
|
||||
ResponseTypesSupported: []codersdk.OAuth2ProviderResponseType{codersdk.OAuth2ProviderResponseTypeCode},
|
||||
GrantTypesSupported: []codersdk.OAuth2ProviderGrantType{codersdk.OAuth2ProviderGrantTypeAuthorizationCode, codersdk.OAuth2ProviderGrantTypeRefreshToken},
|
||||
CodeChallengeMethodsSupported: []codersdk.OAuth2PKCECodeChallengeMethod{codersdk.OAuth2PKCECodeChallengeMethodS256},
|
||||
ScopesSupported: rbac.ExternalScopeNames(),
|
||||
TokenEndpointAuthMethodsSupported: []codersdk.OAuth2TokenEndpointAuthMethod{codersdk.OAuth2TokenEndpointAuthMethodClientSecretBasic, codersdk.OAuth2TokenEndpointAuthMethodClientSecretPost},
|
||||
}
|
||||
if dcrEnabled {
|
||||
metadata.RegistrationEndpoint = accessURL.JoinPath("/oauth2/register").String() // RFC 7591
|
||||
}
|
||||
httpapi.Write(ctx, rw, http.StatusOK, metadata)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,13 +2,19 @@ package oauth2provider_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@@ -40,6 +46,72 @@ func TestOAuth2AuthorizationServerMetadata(t *testing.T) {
|
||||
require.Equal(t, rbac.ExternalScopeNames(), metadata.ScopesSupported)
|
||||
}
|
||||
|
||||
// TestGetAuthorizationServerMetadata_DCREnabled is a focused unit test on
|
||||
// the discovery handler itself, bypassing the full coderdtest HTTP server.
|
||||
// It verifies the dynamic-client-registration-enabled gate: registration_endpoint
|
||||
// is advertised once an admin explicitly enables DCR, omitted when explicitly
|
||||
// disabled, and omitted by default when the setting has never been
|
||||
// configured.
|
||||
func TestGetAuthorizationServerMetadata_DCREnabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
accessURL, err := url.Parse("https://oauth2-metadata-dcr-test.example.com")
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
// configureDCR is nil for "never configured".
|
||||
configureDCR *bool
|
||||
wantRegistrationEndpoint bool
|
||||
}{
|
||||
{
|
||||
name: "EnabledAdvertisesRegistrationEndpoint",
|
||||
configureDCR: ptr.Ref(true),
|
||||
wantRegistrationEndpoint: true,
|
||||
},
|
||||
{
|
||||
name: "DisabledOmitsRegistrationEndpoint",
|
||||
configureDCR: ptr.Ref(false),
|
||||
wantRegistrationEndpoint: false,
|
||||
},
|
||||
{
|
||||
name: "NeverConfiguredDefaultsToOmitted",
|
||||
configureDCR: nil,
|
||||
wantRegistrationEndpoint: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
if tt.configureDCR != nil {
|
||||
err := db.UpsertOAuth2DCREnabled(ctx, *tt.configureDCR)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
handler := oauth2provider.GetAuthorizationServerMetadata(db, accessURL)
|
||||
|
||||
r := httptest.NewRequest(http.MethodGet, "/.well-known/oauth-authorization-server", nil).WithContext(ctx)
|
||||
rw := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(rw, r)
|
||||
require.Equal(t, http.StatusOK, rw.Code)
|
||||
|
||||
var metadata codersdk.OAuth2AuthorizationServerMetadata
|
||||
require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &metadata))
|
||||
|
||||
if tt.wantRegistrationEndpoint {
|
||||
require.NotEmpty(t, metadata.RegistrationEndpoint)
|
||||
} else {
|
||||
require.Empty(t, metadata.RegistrationEndpoint)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestOAuth2ProtectedResourceMetadata(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/oauth2"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@@ -76,6 +77,20 @@ func CreateTestOAuth2App(t *testing.T, client *codersdk.Client) (*codersdk.OAuth
|
||||
return &app, secret.ClientSecretFull
|
||||
}
|
||||
|
||||
// EnableDCR turns on dynamic client registration for the deployment.
|
||||
// DCR defaults to disabled, so any test that registers a client via
|
||||
// POST /oauth2/register must call this first. The caller-provided client
|
||||
// must have owner-level permissions.
|
||||
func EnableDCR(t *testing.T, client *codersdk.Client) {
|
||||
t.Helper()
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
_, err := client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{
|
||||
DynamicClientRegistrationEnabled: ptr.Ref(true),
|
||||
})
|
||||
require.NoError(t, err, "failed to enable dynamic client registration")
|
||||
}
|
||||
|
||||
// GeneratePKCE generates a random PKCE code verifier and challenge
|
||||
func GeneratePKCE(t *testing.T) (verifier, challenge string) {
|
||||
t.Helper()
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@@ -177,6 +178,7 @@ func TestOAuth2ClientRegistrationValidation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
validURIs := []string{
|
||||
@@ -223,6 +225,7 @@ func TestOAuth2ClientRegistrationValidation(t *testing.T) {
|
||||
// Create new client for each sub-test to avoid shared state issues
|
||||
subClient := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, subClient)
|
||||
oauth2providertest.EnableDCR(t, subClient)
|
||||
subCtx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
req := codersdk.OAuth2ClientRegistrationRequest{
|
||||
@@ -242,6 +245,7 @@ func TestOAuth2ClientRegistrationValidation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
req := codersdk.OAuth2ClientRegistrationRequest{
|
||||
@@ -260,6 +264,7 @@ func TestOAuth2ClientRegistrationValidation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
req := codersdk.OAuth2ClientRegistrationRequest{
|
||||
@@ -278,6 +283,7 @@ func TestOAuth2ClientRegistrationValidation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
req := codersdk.OAuth2ClientRegistrationRequest{
|
||||
@@ -296,6 +302,7 @@ func TestOAuth2ClientRegistrationValidation(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
req := codersdk.OAuth2ClientRegistrationRequest{
|
||||
|
||||
@@ -29,6 +29,25 @@ import (
|
||||
func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, auditor *audit.Auditor, logger slog.Logger) http.HandlerFunc {
|
||||
return func(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
|
||||
// This is queried on every request rather than cached, since
|
||||
// registration is expected to happen rarely, not on a hot path. A
|
||||
// misconfigured or misbehaving client flooding this endpoint is a
|
||||
// rate-limiting or firewalling problem to solve at the deployment
|
||||
// level, not a reason to cache this flag.
|
||||
//nolint:gocritic // Public registration endpoint, no authenticated actor to authorize against.
|
||||
dcrEnabled, err := db.GetOAuth2DCREnabled(dbauthz.AsSystemOAuth2(ctx))
|
||||
if err != nil {
|
||||
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
|
||||
"server_error", "Failed to check registration availability")
|
||||
return
|
||||
}
|
||||
if !dcrEnabled {
|
||||
writeOAuth2RegistrationError(ctx, rw, http.StatusForbidden,
|
||||
"invalid_request", "Dynamic client registration is disabled on this deployment")
|
||||
return
|
||||
}
|
||||
|
||||
aReq, commitAudit := audit.InitRequest[database.OAuth2ProviderApp](rw, &audit.RequestParams{
|
||||
Audit: *auditor,
|
||||
Log: logger,
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
package oauth2provider_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"cdr.dev/slog/v3/sloggers/slogtest"
|
||||
"github.com/coder/coder/v2/coderd/audit"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider"
|
||||
"github.com/coder/coder/v2/coderd/tracing"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
// TestCreateDynamicClientRegistration_DCREnabled is a focused unit test on
|
||||
// the RFC 7591 handler itself, bypassing the full coderdtest HTTP server. It
|
||||
// verifies the dynamic-client-registration-enabled gate: registration
|
||||
// succeeds once an admin explicitly enables DCR, is rejected with 403 when
|
||||
// explicitly disabled, and defaults to disabled when the setting has never
|
||||
// been configured.
|
||||
func TestCreateDynamicClientRegistration_DCREnabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
accessURL, err := url.Parse("https://oauth2-registration-dcr-test.example.com")
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
// configureDCR is nil for "never configured".
|
||||
configureDCR *bool
|
||||
wantStatus int
|
||||
}{
|
||||
{
|
||||
name: "EnabledAllowsRegistration",
|
||||
configureDCR: ptr.Ref(true),
|
||||
wantStatus: http.StatusCreated,
|
||||
},
|
||||
{
|
||||
name: "DisabledRejectsRegistration",
|
||||
configureDCR: ptr.Ref(false),
|
||||
wantStatus: http.StatusForbidden,
|
||||
},
|
||||
{
|
||||
name: "NeverConfiguredDefaultsToDisabled",
|
||||
configureDCR: nil,
|
||||
wantStatus: http.StatusForbidden,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
if tt.configureDCR != nil {
|
||||
err := db.UpsertOAuth2DCREnabled(ctx, *tt.configureDCR)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
logger := slogtest.Make(t, nil)
|
||||
auditor := audit.NewNop()
|
||||
// audit.InitRequest requires the ResponseWriter to be a
|
||||
// *tracing.StatusWriter, which normally comes from the
|
||||
// middleware chain in coderd.go; wrap it here to match.
|
||||
handler := tracing.StatusWriterMiddleware(oauth2provider.CreateDynamicClientRegistration(db, accessURL, &auditor, logger))
|
||||
|
||||
req := codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{"https://example.com/callback"},
|
||||
}
|
||||
body, err := json.Marshal(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/oauth2/register", bytes.NewReader(body)).WithContext(ctx)
|
||||
r.Header.Set("Content-Type", "application/json")
|
||||
rw := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(rw, r)
|
||||
require.Equal(t, tt.wantStatus, rw.Code)
|
||||
|
||||
if tt.wantStatus != http.StatusForbidden {
|
||||
return
|
||||
}
|
||||
|
||||
var errResp map[string]string
|
||||
require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &errResp))
|
||||
require.Equal(t, "invalid_request", errResp["error"])
|
||||
require.Contains(t, errResp["error_description"], "disabled")
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@@ -21,6 +22,7 @@ func TestOAuth2ClientMetadataValidation(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps with unique client names.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("RedirectURIValidation", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
@@ -468,6 +470,7 @@ func TestOAuth2ClientNameValidation(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -545,6 +548,7 @@ func TestOAuth2ClientScopeValidation(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -632,6 +636,7 @@ func TestOAuth2ClientMetadataDefaults(t *testing.T) {
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
@@ -675,6 +680,7 @@ func TestOAuth2ClientMetadataEdgeCases(t *testing.T) {
|
||||
// Single instance shared across all sub-tests. Each registers independent OAuth2 apps with unique client names.
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
|
||||
t.Run("ExtremelyLongRedirectURI", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
Reference in New Issue
Block a user