From fbac602456c9277b88db947289fcdda39a949c41 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 28 Jul 2026 16:59:33 -0700 Subject: [PATCH] feat!: add admin-controlled dynamic client registration toggle (#27316) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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
(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
{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,
"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.
Manual files (26) — click to expand, grouped the same way as "Suggested review order" below **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. |
Generated files (12) — from make gen, no need to review directly `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`.
## 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). --- cli/oauth2provider.go | 96 +++++++++++++ cli/oauth2provider_test.go | 82 +++++++++++ cli/root.go | 1 + cli/testdata/coder_--help.golden | 103 +++++++------- .../coder_oauth2-provider_--help.golden | 24 ++++ .../coder_oauth2-provider_dcr_--help.golden | 13 ++ ..._oauth2-provider_dcr_disable_--help.golden | 9 ++ ...r_oauth2-provider_dcr_enable_--help.golden | 9 ++ coderd/apidoc/docs.go | 72 ++++++++++ coderd/apidoc/swagger.json | 62 ++++++++ coderd/audit/diff.go | 1 + coderd/audit/request.go | 10 ++ coderd/coderd.go | 4 + coderd/database/dbauthz/dbauthz.go | 19 +++ coderd/database/dbauthz/dbauthz_test.go | 8 ++ coderd/database/dbmetrics/querymetrics.go | 16 +++ coderd/database/dbmock/dbmock.go | 29 ++++ coderd/database/dump.sql | 3 +- ...58_audit_oauth2_provider_settings.down.sql | 1 + ...0558_audit_oauth2_provider_settings.up.sql | 2 + coderd/database/migrations/migrate_test.go | 65 +++++++++ coderd/database/models.go | 5 +- coderd/database/querier.go | 2 + coderd/database/queries.sql.go | 36 +++++ coderd/database/queries/siteconfig.sql | 22 +++ coderd/database/types.go | 5 + coderd/mcp/mcp_e2e_test.go | 2 + coderd/oauth2.go | 98 ++++++++++++- coderd/oauth2_error_compliance_test.go | 7 + coderd/oauth2_metadata_validation_test.go | 6 + coderd/oauth2_provider_settings_test.go | 133 ++++++++++++++++++ coderd/oauth2_security_test.go | 7 + coderd/oauth2_test.go | 88 ++++++++++++ coderd/oauth2provider/metadata.go | 22 ++- coderd/oauth2provider/metadata_test.go | 72 ++++++++++ .../oauth2providertest/helpers.go | 15 ++ coderd/oauth2provider/provider_test.go | 7 + coderd/oauth2provider/registration.go | 19 +++ coderd/oauth2provider/registration_test.go | 99 +++++++++++++ coderd/oauth2provider/validation_test.go | 6 + codersdk/audit.go | 35 ++--- codersdk/oauth2.go | 41 ++++++ docs/admin/integrations/oauth2-provider.md | 30 ++++ docs/admin/security/audit-logs.md | 1 + docs/manifest.json | 20 +++ docs/reference/api/enterprise.md | 77 ++++++++++ docs/reference/api/schemas.md | 20 ++- docs/reference/cli/index.md | 1 + docs/reference/cli/oauth2-provider.md | 31 ++++ docs/reference/cli/oauth2-provider_dcr.md | 17 +++ .../cli/oauth2-provider_dcr_disable.md | 10 ++ .../cli/oauth2-provider_dcr_enable.md | 10 ++ enterprise/audit/table.go | 4 + .../oauth2providersettings_audit_test.go | 101 +++++++++++++ site/src/api/typesGenerated.ts | 18 +++ 55 files changed, 1620 insertions(+), 76 deletions(-) create mode 100644 cli/oauth2provider.go create mode 100644 cli/oauth2provider_test.go create mode 100644 cli/testdata/coder_oauth2-provider_--help.golden create mode 100644 cli/testdata/coder_oauth2-provider_dcr_--help.golden create mode 100644 cli/testdata/coder_oauth2-provider_dcr_disable_--help.golden create mode 100644 cli/testdata/coder_oauth2-provider_dcr_enable_--help.golden create mode 100644 coderd/database/migrations/000558_audit_oauth2_provider_settings.down.sql create mode 100644 coderd/database/migrations/000558_audit_oauth2_provider_settings.up.sql create mode 100644 coderd/oauth2_provider_settings_test.go create mode 100644 coderd/oauth2provider/registration_test.go create mode 100644 docs/reference/cli/oauth2-provider.md create mode 100644 docs/reference/cli/oauth2-provider_dcr.md create mode 100644 docs/reference/cli/oauth2-provider_dcr_disable.md create mode 100644 docs/reference/cli/oauth2-provider_dcr_enable.md create mode 100644 enterprise/coderd/oauth2providersettings_audit_test.go diff --git a/cli/oauth2provider.go b/cli/oauth2provider.go new file mode 100644 index 0000000000..e6bbe809ed --- /dev/null +++ b/cli/oauth2provider.go @@ -0,0 +1,96 @@ +package cli + +import ( + "fmt" + + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/serpent" +) + +func (r *RootCmd) oauth2Provider() *serpent.Command { + cmd := &serpent.Command{ + Use: "oauth2-provider", + Short: "Manage Coder OAuth2 provider settings", + Long: "Administrators can use these commands to change OAuth2 provider settings.\n" + FormatExamples( + Example{ + Description: "Enable dynamic client registration (RFC 7591), allowing OAuth2/MCP clients to self-register without an admin creating an app first", + Command: "coder oauth2-provider dcr enable", + }, + Example{ + Description: "Disable dynamic client registration. Clients that already registered are unaffected; only new self-registration attempts are rejected", + Command: "coder oauth2-provider dcr disable", + }, + ), + Handler: func(inv *serpent.Invocation) error { + return inv.Command.HelpHandler(inv) + }, + Children: []*serpent.Command{ + r.oauth2ProviderDCR(), + }, + } + return cmd +} + +func (r *RootCmd) oauth2ProviderDCR() *serpent.Command { + cmd := &serpent.Command{ + Use: "dcr", + Short: "Manage OAuth2 dynamic client registration (RFC 7591)", + Handler: func(inv *serpent.Invocation) error { + return inv.Command.HelpHandler(inv) + }, + Children: []*serpent.Command{ + r.oauth2ProviderDCRToggle(dcrToggleEnable), + r.oauth2ProviderDCRToggle(dcrToggleDisable), + }, + } + return cmd +} + +// dcrToggleAction distinguishes the "enable" and "disable" subcommands of +// `coder oauth2-provider dcr`, which are otherwise identical. +type dcrToggleAction int + +const ( + dcrToggleDisable dcrToggleAction = iota + dcrToggleEnable +) + +func (r *RootCmd) oauth2ProviderDCRToggle(action dcrToggleAction) *serpent.Command { + enabled := action == dcrToggleEnable + use, short, verb := "disable", "Disable OAuth2 dynamic client registration", "disable" + if enabled { + use, short, verb = "enable", "Enable OAuth2 dynamic client registration", "enable" + } + + cmd := &serpent.Command{ + Use: use, + Short: short, + Middleware: serpent.Chain( + serpent.RequireNArgs(0), + ), + Handler: func(inv *serpent.Invocation) error { + client, err := r.InitClient(inv) + if err != nil { + return err + } + + _, err = client.PutOAuth2ProviderSettings(inv.Context(), codersdk.OAuth2ProviderSettings{ + DynamicClientRegistrationEnabled: ptr.Ref(enabled), + }) + if err != nil { + return xerrors.Errorf("unable to %s dynamic client registration: %w", verb, err) + } + + state := "disabled" + if enabled { + state = "enabled" + } + _, _ = fmt.Fprintf(inv.Stderr, "Dynamic client registration is now %s.\n", state) + return nil + }, + } + return cmd +} diff --git a/cli/oauth2provider_test.go b/cli/oauth2provider_test.go new file mode 100644 index 0000000000..99bfa6161b --- /dev/null +++ b/cli/oauth2provider_test.go @@ -0,0 +1,82 @@ +package cli_test + +import ( + "bytes" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/cli/clitest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" +) + +func TestOAuth2ProviderDCR(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + command string + expectValue bool + expectMsg string + }{ + { + name: "Enable", + command: "enable", + expectValue: true, + expectMsg: "Dynamic client registration is now enabled.", + }, + { + name: "Disable", + command: "disable", + expectValue: false, + expectMsg: "Dynamic client registration is now disabled.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + inv, root := clitest.New(t, "oauth2-provider", "dcr", tt.command) + clitest.SetupConfig(t, client, root) + + var buf bytes.Buffer + inv.Stderr = &buf + err := inv.Run() + require.NoError(t, err) + assert.Contains(t, buf.String(), tt.expectMsg) + + 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.Equal(t, tt.expectValue, *settings.DynamicClientRegistrationEnabled) + }) + } +} + +func TestOAuth2ProviderDCR_RegularUser(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + owner := coderdtest.CreateFirstUser(t, client) + anotherClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + inv, root := clitest.New(t, "oauth2-provider", "dcr", "enable") + clitest.SetupConfig(t, anotherClient, root) + + var buf bytes.Buffer + inv.Stderr = &buf + err := inv.Run() + var sdkError *codersdk.Error + require.Error(t, err) + require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") + assert.Equal(t, http.StatusForbidden, sdkError.StatusCode()) +} diff --git a/cli/root.go b/cli/root.go index fc20141dc1..ecc2c25f94 100644 --- a/cli/root.go +++ b/cli/root.go @@ -109,6 +109,7 @@ func (r *RootCmd) CoreSubcommands() []*serpent.Command { r.logout(), r.netcheck(), r.notifications(), + r.oauth2Provider(), r.organizations(), r.portForward(), r.publickey(), diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden index cb667c3a5c..ef24f351e7 100644 --- a/cli/testdata/coder_--help.golden +++ b/cli/testdata/coder_--help.golden @@ -14,57 +14,58 @@ USAGE: $ coder templates init SUBCOMMANDS: - autoupdate Toggle auto-update policy for a workspace - completion Install or update shell completion scripts for the - detected or chosen shell. - config-ssh Add an SSH Host entry for your workspaces "ssh - workspace.coder" - create Create a workspace - delete Delete a workspace - dotfiles Personalize your workspace by applying a canonical - dotfiles repository - external-auth Manage external authentication - favorite Add a workspace to your favorites - list List workspaces - login Authenticate with Coder deployment - logout Unauthenticate your local session - logs View logs for a workspace - netcheck Print network debug information for DERP and STUN - notifications Manage Coder notifications - open Open a workspace - organizations Organization related commands - ping Ping a workspace - port-forward Forward ports from a workspace to the local machine. For - reverse port forwarding, use "coder ssh -R". - provisioner View and manage provisioner daemons and jobs - publickey Output your Coder public key used for Git operations - rename Rename a workspace - reset-password Directly connect to the database to reset a user's - password - restart Restart a workspace - schedule Schedule automated start and stop times for workspaces - secret Manage secrets - server Start a Coder server - show Display details of a workspace's resources and agents - speedtest Run upload and download tests from your machine to a - workspace - ssh Start a shell into a workspace or run a command - start Start a workspace - stat Show resource usage for the current workspace. - state Manually manage Terraform state to fix broken workspaces - stop Stop a workspace - support Commands for troubleshooting issues with a Coder - deployment. - task Manage tasks - templates Manage templates - tokens Manage personal access tokens - unfavorite Remove a workspace from your favorites - update Will update and start a given workspace if it is out of - date. If the workspace is already running, it will be - stopped first. - users Manage users - version Show coder version - whoami Fetch authenticated user info for Coder deployment + autoupdate Toggle auto-update policy for a workspace + completion Install or update shell completion scripts for the + detected or chosen shell. + config-ssh Add an SSH Host entry for your workspaces "ssh + workspace.coder" + create Create a workspace + delete Delete a workspace + dotfiles Personalize your workspace by applying a canonical + dotfiles repository + external-auth Manage external authentication + favorite Add a workspace to your favorites + list List workspaces + login Authenticate with Coder deployment + logout Unauthenticate your local session + logs View logs for a workspace + netcheck Print network debug information for DERP and STUN + notifications Manage Coder notifications + oauth2-provider Manage Coder OAuth2 provider settings + open Open a workspace + organizations Organization related commands + ping Ping a workspace + port-forward Forward ports from a workspace to the local machine. For + reverse port forwarding, use "coder ssh -R". + provisioner View and manage provisioner daemons and jobs + publickey Output your Coder public key used for Git operations + rename Rename a workspace + reset-password Directly connect to the database to reset a user's + password + restart Restart a workspace + schedule Schedule automated start and stop times for workspaces + secret Manage secrets + server Start a Coder server + show Display details of a workspace's resources and agents + speedtest Run upload and download tests from your machine to a + workspace + ssh Start a shell into a workspace or run a command + start Start a workspace + stat Show resource usage for the current workspace. + state Manually manage Terraform state to fix broken workspaces + stop Stop a workspace + support Commands for troubleshooting issues with a Coder + deployment. + task Manage tasks + templates Manage templates + tokens Manage personal access tokens + unfavorite Remove a workspace from your favorites + update Will update and start a given workspace if it is out of + date. If the workspace is already running, it will be + stopped first. + users Manage users + version Show coder version + whoami Fetch authenticated user info for Coder deployment GLOBAL OPTIONS: Global options are applied to all commands. They can be set using environment diff --git a/cli/testdata/coder_oauth2-provider_--help.golden b/cli/testdata/coder_oauth2-provider_--help.golden new file mode 100644 index 0000000000..9bebb2ed07 --- /dev/null +++ b/cli/testdata/coder_oauth2-provider_--help.golden @@ -0,0 +1,24 @@ +coder v0.0.0-devel + +USAGE: + coder oauth2-provider + + Manage Coder OAuth2 provider settings + + Administrators can use these commands to change OAuth2 provider settings. + - Enable dynamic client registration (RFC 7591), allowing OAuth2/MCP clients + to + self-register without an admin creating an app first: + + $ coder oauth2-provider dcr enable + + - Disable dynamic client registration. Clients that already registered are + unaffected; only new self-registration attempts are rejected: + + $ coder oauth2-provider dcr disable + +SUBCOMMANDS: + dcr Manage OAuth2 dynamic client registration (RFC 7591) + +——— +Run `coder --help` for a list of global options. diff --git a/cli/testdata/coder_oauth2-provider_dcr_--help.golden b/cli/testdata/coder_oauth2-provider_dcr_--help.golden new file mode 100644 index 0000000000..5727571887 --- /dev/null +++ b/cli/testdata/coder_oauth2-provider_dcr_--help.golden @@ -0,0 +1,13 @@ +coder v0.0.0-devel + +USAGE: + coder oauth2-provider dcr + + Manage OAuth2 dynamic client registration (RFC 7591) + +SUBCOMMANDS: + disable Disable OAuth2 dynamic client registration + enable Enable OAuth2 dynamic client registration + +——— +Run `coder --help` for a list of global options. diff --git a/cli/testdata/coder_oauth2-provider_dcr_disable_--help.golden b/cli/testdata/coder_oauth2-provider_dcr_disable_--help.golden new file mode 100644 index 0000000000..dbab555fd1 --- /dev/null +++ b/cli/testdata/coder_oauth2-provider_dcr_disable_--help.golden @@ -0,0 +1,9 @@ +coder v0.0.0-devel + +USAGE: + coder oauth2-provider dcr disable + + Disable OAuth2 dynamic client registration + +——— +Run `coder --help` for a list of global options. diff --git a/cli/testdata/coder_oauth2-provider_dcr_enable_--help.golden b/cli/testdata/coder_oauth2-provider_dcr_enable_--help.golden new file mode 100644 index 0000000000..b429ebf660 --- /dev/null +++ b/cli/testdata/coder_oauth2-provider_dcr_enable_--help.golden @@ -0,0 +1,9 @@ +coder v0.0.0-devel + +USAGE: + coder oauth2-provider dcr enable + + Enable OAuth2 dynamic client registration + +——— +Run `coder --help` for a list of global options. diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 006b6e3d38..e42335cbb5 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -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", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 901c851ba6..c10bf3620d 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -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", diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index c28ec8f7cb..374f74dd7a 100644 --- a/coderd/audit/diff.go +++ b/coderd/audit/diff.go @@ -25,6 +25,7 @@ type Auditable interface { database.OAuth2ProviderApp | database.OAuth2ProviderAppSecret | database.PrebuildsSettings | + database.OAuth2ProviderSettings | database.CustomRole | database.AuditableOrganizationMember | database.Organization | diff --git a/coderd/audit/request.go b/coderd/audit/request.go index 88671316e7..4b213968bd 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -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: diff --git a/coderd/coderd.go b/coderd/coderd.go index b01efaf9cd..146916fb55 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -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) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 5b8036958c..03f8ce5cac 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -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 diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index ef0e45eaeb..25d2622de1 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -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) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index db91a9772b..c56b38190b 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -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) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index a038a74957..fff45208fc 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -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() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 545cba9d22..c167dd3beb 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -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 ( diff --git a/coderd/database/migrations/000558_audit_oauth2_provider_settings.down.sql b/coderd/database/migrations/000558_audit_oauth2_provider_settings.down.sql new file mode 100644 index 0000000000..35020b349f --- /dev/null +++ b/coderd/database/migrations/000558_audit_oauth2_provider_settings.down.sql @@ -0,0 +1 @@ +-- No-op, enum values can't be dropped. diff --git a/coderd/database/migrations/000558_audit_oauth2_provider_settings.up.sql b/coderd/database/migrations/000558_audit_oauth2_provider_settings.up.sql new file mode 100644 index 0000000000..c578509311 --- /dev/null +++ b/coderd/database/migrations/000558_audit_oauth2_provider_settings.up.sql @@ -0,0 +1,2 @@ +ALTER TYPE resource_type + ADD VALUE IF NOT EXISTS 'oauth2_provider_settings'; diff --git a/coderd/database/migrations/migrate_test.go b/coderd/database/migrations/migrate_test.go index fdabc3a7df..def839c96f 100644 --- a/coderd/database/migrations/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -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() diff --git a/coderd/database/models.go b/coderd/database/models.go index 56fb2e1e1f..b3c598a2ed 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -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, } } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index f82ef6ea14..703cd40755 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -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) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 454c720a27..b86e7cb57c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -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 ( diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 3eb3aacaf0..fbadbef223 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -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 diff --git a/coderd/database/types.go b/coderd/database/types.go index f543288c04..22c18cc7c0 100644 --- a/coderd/database/types.go +++ b/coderd/database/types.go @@ -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 { diff --git a/coderd/mcp/mcp_e2e_test.go b/coderd/mcp/mcp_e2e_test.go index 633c68582a..ab48450a07 100644 --- a/coderd/mcp/mcp_e2e_test.go +++ b/coderd/mcp/mcp_e2e_test.go @@ -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() diff --git a/coderd/oauth2.go b/coderd/oauth2.go index 8523b42f8e..2e083eeca6 100644 --- a/coderd/oauth2.go +++ b/coderd/oauth2.go @@ -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 diff --git a/coderd/oauth2_error_compliance_test.go b/coderd/oauth2_error_compliance_test.go index 86553973e0..5900e9ee1e 100644 --- a/coderd/oauth2_error_compliance_test.go +++ b/coderd/oauth2_error_compliance_test.go @@ -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() diff --git a/coderd/oauth2_metadata_validation_test.go b/coderd/oauth2_metadata_validation_test.go index d880973ce1..01b2143f5a 100644 --- a/coderd/oauth2_metadata_validation_test.go +++ b/coderd/oauth2_metadata_validation_test.go @@ -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() diff --git a/coderd/oauth2_provider_settings_test.go b/coderd/oauth2_provider_settings_test.go new file mode 100644 index 0000000000..d6b66c9207 --- /dev/null +++ b/coderd/oauth2_provider_settings_test.go @@ -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()) + }) + } + }) +} diff --git a/coderd/oauth2_security_test.go b/coderd/oauth2_security_test.go index 47190cd2bf..17c092fd7a 100644 --- a/coderd/oauth2_security_test.go +++ b/coderd/oauth2_security_test.go @@ -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() diff --git a/coderd/oauth2_test.go b/coderd/oauth2_test.go index 9831067ff2..55491fcc21 100644 --- a/coderd/oauth2_test.go +++ b/coderd/oauth2_test.go @@ -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() diff --git a/coderd/oauth2provider/metadata.go b/coderd/oauth2provider/metadata.go index 53481a35d4..6b98dcb7bc 100644 --- a/coderd/oauth2provider/metadata.go +++ b/coderd/oauth2provider/metadata.go @@ -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) } } diff --git a/coderd/oauth2provider/metadata_test.go b/coderd/oauth2provider/metadata_test.go index 27f7ef5e31..4edf0f00da 100644 --- a/coderd/oauth2provider/metadata_test.go +++ b/coderd/oauth2provider/metadata_test.go @@ -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() diff --git a/coderd/oauth2provider/oauth2providertest/helpers.go b/coderd/oauth2provider/oauth2providertest/helpers.go index 59b0c38f7f..ff3d7321db 100644 --- a/coderd/oauth2provider/oauth2providertest/helpers.go +++ b/coderd/oauth2provider/oauth2providertest/helpers.go @@ -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() diff --git a/coderd/oauth2provider/provider_test.go b/coderd/oauth2provider/provider_test.go index 2a95438dcc..7d18def6b1 100644 --- a/coderd/oauth2provider/provider_test.go +++ b/coderd/oauth2provider/provider_test.go @@ -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{ diff --git a/coderd/oauth2provider/registration.go b/coderd/oauth2provider/registration.go index fa41023e74..c0d297f6ea 100644 --- a/coderd/oauth2provider/registration.go +++ b/coderd/oauth2provider/registration.go @@ -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, diff --git a/coderd/oauth2provider/registration_test.go b/coderd/oauth2provider/registration_test.go new file mode 100644 index 0000000000..f23e82dbf7 --- /dev/null +++ b/coderd/oauth2provider/registration_test.go @@ -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") + }) + } +} diff --git a/coderd/oauth2provider/validation_test.go b/coderd/oauth2provider/validation_test.go index 9367079ea6..2bb442ab3c 100644 --- a/coderd/oauth2provider/validation_test.go +++ b/coderd/oauth2provider/validation_test.go @@ -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() diff --git a/codersdk/audit.go b/codersdk/audit.go index 6193b06810..b4e8914623 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -14,22 +14,23 @@ import ( type ResourceType string const ( - ResourceTypeTemplate ResourceType = "template" - ResourceTypeTemplateVersion ResourceType = "template_version" - ResourceTypeUser ResourceType = "user" - ResourceTypeWorkspace ResourceType = "workspace" - ResourceTypeWorkspaceBuild ResourceType = "workspace_build" - ResourceTypeGitSSHKey ResourceType = "git_ssh_key" - ResourceTypeAPIKey ResourceType = "api_key" - ResourceTypeGroup ResourceType = "group" - ResourceTypeLicense ResourceType = "license" - ResourceTypeConvertLogin ResourceType = "convert_login" - ResourceTypeHealthSettings ResourceType = "health_settings" - ResourceTypeNotificationsSettings ResourceType = "notifications_settings" - ResourceTypePrebuildsSettings ResourceType = "prebuilds_settings" - ResourceTypeWorkspaceProxy ResourceType = "workspace_proxy" - ResourceTypeOrganization ResourceType = "organization" - ResourceTypeOAuth2ProviderApp ResourceType = "oauth2_provider_app" + ResourceTypeTemplate ResourceType = "template" + ResourceTypeTemplateVersion ResourceType = "template_version" + ResourceTypeUser ResourceType = "user" + ResourceTypeWorkspace ResourceType = "workspace" + ResourceTypeWorkspaceBuild ResourceType = "workspace_build" + ResourceTypeGitSSHKey ResourceType = "git_ssh_key" + ResourceTypeAPIKey ResourceType = "api_key" + ResourceTypeGroup ResourceType = "group" + ResourceTypeLicense ResourceType = "license" + ResourceTypeConvertLogin ResourceType = "convert_login" + ResourceTypeHealthSettings ResourceType = "health_settings" + ResourceTypeNotificationsSettings ResourceType = "notifications_settings" + ResourceTypePrebuildsSettings ResourceType = "prebuilds_settings" + ResourceTypeOAuth2ProviderSettings ResourceType = "oauth2_provider_settings" + ResourceTypeWorkspaceProxy ResourceType = "workspace_proxy" + ResourceTypeOrganization ResourceType = "organization" + ResourceTypeOAuth2ProviderApp ResourceType = "oauth2_provider_app" // nolint:gosec // This is not a secret. ResourceTypeOAuth2ProviderAppSecret ResourceType = "oauth2_provider_app_secret" ResourceTypeCustomRole ResourceType = "custom_role" @@ -90,6 +91,8 @@ func (r ResourceType) FriendlyString() string { return "notifications_settings" case ResourceTypePrebuildsSettings: return "prebuilds_settings" + case ResourceTypeOAuth2ProviderSettings: + return "oauth2 provider settings" case ResourceTypeOAuth2ProviderApp: return "oauth2 app" case ResourceTypeOAuth2ProviderAppSecret: diff --git a/codersdk/oauth2.go b/codersdk/oauth2.go index 3f0db4d75c..5bec8cf717 100644 --- a/codersdk/oauth2.go +++ b/codersdk/oauth2.go @@ -185,6 +185,47 @@ func (c *Client) DeleteOAuth2ProviderAppSecret(ctx context.Context, appID uuid.U return nil } +// OAuth2ProviderSettings controls deployment-wide OAuth2 provider behavior. +// +// DynamicClientRegistrationEnabled is a pointer so a PUT can omit it to leave +// the current value unchanged, rather than a decoded zero value silently +// resetting it to false. This matters once a second field lands in this +// struct (e.g. a future initial-access-token requirement): a client built +// against an older, single-field version of this struct would otherwise +// always encode the newer field's zero value, silently clearing it on every +// unrelated update. GET always returns a non-nil value. +type OAuth2ProviderSettings struct { + DynamicClientRegistrationEnabled *bool `json:"dynamic_client_registration_enabled,omitempty"` +} + +// OAuth2ProviderSettings retrieves the deployment-wide OAuth2 provider settings. +func (c *Client) OAuth2ProviderSettings(ctx context.Context) (OAuth2ProviderSettings, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/oauth2-provider/settings", nil) + if err != nil { + return OAuth2ProviderSettings{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return OAuth2ProviderSettings{}, ReadBodyAsError(res) + } + var settings OAuth2ProviderSettings + return settings, json.NewDecoder(res.Body).Decode(&settings) +} + +// PutOAuth2ProviderSettings modifies the deployment-wide OAuth2 provider settings. +func (c *Client) PutOAuth2ProviderSettings(ctx context.Context, settings OAuth2ProviderSettings) (OAuth2ProviderSettings, error) { + res, err := c.Request(ctx, http.MethodPut, "/api/v2/oauth2-provider/settings", settings) + if err != nil { + return OAuth2ProviderSettings{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return OAuth2ProviderSettings{}, ReadBodyAsError(res) + } + var updated OAuth2ProviderSettings + return updated, json.NewDecoder(res.Body).Decode(&updated) +} + type OAuth2ProviderGrantType string // OAuth2ProviderGrantType values (RFC 6749). diff --git a/docs/admin/integrations/oauth2-provider.md b/docs/admin/integrations/oauth2-provider.md index 499126d630..4411b17c58 100644 --- a/docs/admin/integrations/oauth2-provider.md +++ b/docs/admin/integrations/oauth2-provider.md @@ -67,6 +67,36 @@ curl -X POST \ "$CODER_URL/api/v2/oauth2-provider/apps/$APP_ID/secrets" ``` +## Dynamic Client Registration + +Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) lets a client register itself against `/oauth2/register` instead of an admin creating the application manually. It's **disabled by default**; an owner must turn it on before any client can self-register. + +Check or change the setting with the CLI: + +```sh +coder oauth2-provider dcr enable +coder oauth2-provider dcr disable +``` + +Or with the management API: + +```sh +curl -X PUT \ + -H "Authorization: Bearer $CODER_SESSION_TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"dynamic_client_registration_enabled": true}' \ + "$CODER_URL/api/v2/oauth2-provider/settings" +``` + +```sh +curl -H "Authorization: Bearer $CODER_SESSION_TOKEN" \ + "$CODER_URL/api/v2/oauth2-provider/settings" +``` + +Disabling only blocks *new* self-registrations. Applications that already +registered while it was enabled keep authorizing and exchanging tokens +normally; disabling does not revoke or otherwise affect them. + ## Integration Patterns ### Client Authentication Methods diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 127eefd19a..b5cb32ba16 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -35,6 +35,7 @@ We track the following resources: | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
client_id_issued_atfalse
client_secret_expires_attrue
client_typetrue
client_uritrue
contactstrue
created_atfalse
dynamically_registeredtrue
grant_typestrue
icontrue
idfalse
jwkstrue
jwks_uritrue
logo_uritrue
nametrue
policy_uritrue
redirect_uristrue
registration_access_tokentrue
registration_client_uritrue
response_typestrue
scopetrue
software_idtrue
software_versiontrue
token_endpoint_auth_methodtrue
tos_uritrue
updated_atfalse
| | OAuth2ProviderAppSecret
| |
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| +| OAuth2ProviderSettings
| |
FieldTracked
dynamic_client_registration_enabledtrue
idfalse
| | Organization
| |
FieldTracked
created_atfalse
default_org_member_rolestrue
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
shareable_workspace_ownerstrue
updated_attrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | PrebuildsSettings
| |
FieldTracked
idfalse
reconciliation_pausedtrue
| diff --git a/docs/manifest.json b/docs/manifest.json index 9faabdc777..07c91463ff 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1878,6 +1878,26 @@ "description": "Send a test notification", "path": "reference/cli/notifications_test.md" }, + { + "title": "oauth2-provider", + "description": "Manage Coder OAuth2 provider settings", + "path": "reference/cli/oauth2-provider.md" + }, + { + "title": "oauth2-provider dcr", + "description": "Manage OAuth2 dynamic client registration (RFC 7591)", + "path": "reference/cli/oauth2-provider_dcr.md" + }, + { + "title": "oauth2-provider dcr enable", + "description": "Enable OAuth2 dynamic client registration", + "path": "reference/cli/oauth2-provider_dcr_enable.md" + }, + { + "title": "oauth2-provider dcr disable", + "description": "Disable OAuth2 dynamic client registration", + "path": "reference/cli/oauth2-provider_dcr_disable.md" + }, { "title": "open", "description": "Open a workspace", diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index 88026e8599..c36264a33d 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -1758,6 +1758,83 @@ curl -X DELETE http://coder-server:8080/api/v2/oauth2-provider/apps/{app}/secret To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get OAuth2 provider settings + +### Code samples + +```sh +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/oauth2-provider/settings \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /api/v2/oauth2-provider/settings` + +### Example responses + +> 200 Response + +```json +{ + "dynamic_client_registration_enabled": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|------------------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OAuth2ProviderSettings](schemas.md#codersdkoauth2providersettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Update OAuth2 provider settings + +### Code samples + +```sh +# Example request using curl +curl -X PUT http://coder-server:8080/api/v2/oauth2-provider/settings \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PUT /api/v2/oauth2-provider/settings` + +> Body parameter + +```json +{ + "dynamic_client_registration_enabled": true +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +|--------|------|------------------------------------------------------------------------------|----------|----------------------------------| +| `body` | body | [codersdk.OAuth2ProviderSettings](schemas.md#codersdkoauth2providersettings) | true | OAuth2 provider settings request | + +### Example responses + +> 200 Response + +```json +{ + "dynamic_client_registration_enabled": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|------------------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OAuth2ProviderSettings](schemas.md#codersdkoauth2providersettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Export organization AI spend as CSV ### Code samples diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a4054b18ad..b463560699 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -9059,6 +9059,20 @@ Only certain features set these fields: - FeatureManagedAgentLimit| |-----------------| | `code`, `token` | +## codersdk.OAuth2ProviderSettings + +```json +{ + "dynamic_client_registration_enabled": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|---------------------------------------|---------|----------|--------------|-------------| +| `dynamic_client_registration_enabled` | boolean | false | | | + ## codersdk.OAuth2TokenEndpointAuthMethod ```json @@ -11036,9 +11050,9 @@ Only certain features set these fields: - FeatureManagedAgentLimit| #### Enumerated Values -| Value(s) | -|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `ai_gateway_key`, `ai_provider`, `ai_provider_key`, `ai_seat`, `api_key`, `chat`, `convert_login`, `custom_role`, `git_ssh_key`, `group`, `group_ai_budget`, `health_settings`, `idp_sync_settings_group`, `idp_sync_settings_organization`, `idp_sync_settings_role`, `license`, `notification_template`, `notifications_settings`, `oauth2_provider_app`, `oauth2_provider_app_secret`, `organization`, `organization_member`, `prebuilds_settings`, `task`, `template`, `template_version`, `user`, `user_ai_budget_override`, `user_secret`, `user_skill`, `workspace`, `workspace_agent`, `workspace_app`, `workspace_build`, `workspace_proxy` | +| Value(s) | +|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `ai_gateway_key`, `ai_provider`, `ai_provider_key`, `ai_seat`, `api_key`, `chat`, `convert_login`, `custom_role`, `git_ssh_key`, `group`, `group_ai_budget`, `health_settings`, `idp_sync_settings_group`, `idp_sync_settings_organization`, `idp_sync_settings_role`, `license`, `notification_template`, `notifications_settings`, `oauth2_provider_app`, `oauth2_provider_app_secret`, `oauth2_provider_settings`, `organization`, `organization_member`, `prebuilds_settings`, `task`, `template`, `template_version`, `user`, `user_ai_budget_override`, `user_secret`, `user_skill`, `workspace`, `workspace_agent`, `workspace_app`, `workspace_build`, `workspace_proxy` | ## codersdk.Response diff --git a/docs/reference/cli/index.md b/docs/reference/cli/index.md index 218cffa7db..219673d771 100644 --- a/docs/reference/cli/index.md +++ b/docs/reference/cli/index.md @@ -31,6 +31,7 @@ Coder — A tool for provisioning self-hosted development environments with Terr | [logout](./logout.md) | Unauthenticate your local session | | [netcheck](./netcheck.md) | Print network debug information for DERP and STUN | | [notifications](./notifications.md) | Manage Coder notifications | +| [oauth2-provider](./oauth2-provider.md) | Manage Coder OAuth2 provider settings | | [organizations](./organizations.md) | Organization related commands | | [port-forward](./port-forward.md) | Forward ports from a workspace to the local machine. For reverse port forwarding, use "coder ssh -R". | | [publickey](./publickey.md) | Output your Coder public key used for Git operations | diff --git a/docs/reference/cli/oauth2-provider.md b/docs/reference/cli/oauth2-provider.md new file mode 100644 index 0000000000..e3ea5b7638 --- /dev/null +++ b/docs/reference/cli/oauth2-provider.md @@ -0,0 +1,31 @@ + +# oauth2-provider + +Manage Coder OAuth2 provider settings + +## Usage + +```console +coder oauth2-provider +``` + +## Description + +```console +Administrators can use these commands to change OAuth2 provider settings. + - Enable dynamic client registration (RFC 7591), allowing OAuth2/MCP clients to +self-register without an admin creating an app first: + + $ coder oauth2-provider dcr enable + + - Disable dynamic client registration. Clients that already registered are +unaffected; only new self-registration attempts are rejected: + + $ coder oauth2-provider dcr disable +``` + +## Subcommands + +| Name | Purpose | +|----------------------------------------------|------------------------------------------------------| +| [dcr](./oauth2-provider_dcr.md) | Manage OAuth2 dynamic client registration (RFC 7591) | diff --git a/docs/reference/cli/oauth2-provider_dcr.md b/docs/reference/cli/oauth2-provider_dcr.md new file mode 100644 index 0000000000..24904d0334 --- /dev/null +++ b/docs/reference/cli/oauth2-provider_dcr.md @@ -0,0 +1,17 @@ + +# oauth2-provider dcr + +Manage OAuth2 dynamic client registration (RFC 7591) + +## Usage + +```console +coder oauth2-provider dcr +``` + +## Subcommands + +| Name | Purpose | +|----------------------------------------------------------|--------------------------------------------| +| [enable](./oauth2-provider_dcr_enable.md) | Enable OAuth2 dynamic client registration | +| [disable](./oauth2-provider_dcr_disable.md) | Disable OAuth2 dynamic client registration | diff --git a/docs/reference/cli/oauth2-provider_dcr_disable.md b/docs/reference/cli/oauth2-provider_dcr_disable.md new file mode 100644 index 0000000000..689dacdfa4 --- /dev/null +++ b/docs/reference/cli/oauth2-provider_dcr_disable.md @@ -0,0 +1,10 @@ + +# oauth2-provider dcr disable + +Disable OAuth2 dynamic client registration + +## Usage + +```console +coder oauth2-provider dcr disable +``` diff --git a/docs/reference/cli/oauth2-provider_dcr_enable.md b/docs/reference/cli/oauth2-provider_dcr_enable.md new file mode 100644 index 0000000000..282b94c6c9 --- /dev/null +++ b/docs/reference/cli/oauth2-provider_dcr_enable.md @@ -0,0 +1,10 @@ + +# oauth2-provider dcr enable + +Enable OAuth2 dynamic client registration + +## Usage + +```console +coder oauth2-provider dcr enable +``` diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index ea66b57933..24cbdcf1dc 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -279,6 +279,10 @@ var auditableResourcesTypes = map[any]map[string]Action{ "id": ActionIgnore, "reconciliation_paused": ActionTrack, }, + &database.OAuth2ProviderSettings{}: { + "id": ActionIgnore, + "dynamic_client_registration_enabled": ActionTrack, + }, // TODO: track an ID here when the below ticket is completed: // https://github.com/coder/coder/pull/6012 &database.License{}: { diff --git a/enterprise/coderd/oauth2providersettings_audit_test.go b/enterprise/coderd/oauth2providersettings_audit_test.go new file mode 100644 index 0000000000..bcaa34cebe --- /dev/null +++ b/enterprise/coderd/oauth2providersettings_audit_test.go @@ -0,0 +1,101 @@ +package coderd_test + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/codersdk" + entaudit "github.com/coder/coder/v2/enterprise/audit" + "github.com/coder/coder/v2/enterprise/audit/backends" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/testutil" +) + +// TestOAuth2ProviderSettingsAuditDiff guards against a regression where +// disabling dynamic client registration produced an empty audit diff. The +// handler only ever set aReq.New, leaving aReq.Old at its zero value +// (DynamicClientRegistrationEnabled: false). Enabling (false -> true) +// happened to diff correctly since the zero value matched the real prior +// state, masking that disabling (true -> false) diffed the zero value +// against itself and showed no change at all. The mock auditor used in +// coderd's own oauth2_provider_settings_test.go always returns an empty +// diff, so only the real enterprise auditor used here can catch this. +func TestOAuth2ProviderSettingsAuditDiff(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + auditor := entaudit.NewAuditor( + db, + entaudit.DefaultFilter, + backends.NewPostgres(db, true), + ) + + ownerClient, _ := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, + Options: &coderdtest.Options{ + Database: db, + Pubsub: ps, + Auditor: auditor, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAuditLog: 1, + }, + }, + }) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) + defer cancel() + + //nolint:gocritic // Updating OAuth2 provider settings is owner-only. + _, err := ownerClient.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{ + DynamicClientRegistrationEnabled: ptr.Ref(true), + }) + require.NoError(t, err) + + //nolint:gocritic // Updating OAuth2 provider settings is owner-only. + _, err = ownerClient.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{ + DynamicClientRegistrationEnabled: ptr.Ref(false), + }) + require.NoError(t, err) + + // Read straight from the database. AsSystemRestricted is necessary + // because the test does not authenticate as an admin when querying the + // store directly. + rows, err := db.GetAuditLogsOffset( + dbauthz.AsSystemRestricted(ctx), + database.GetAuditLogsOffsetParams{ + ResourceType: string(database.ResourceTypeOauth2ProviderSettings), + LimitOpt: 10, + }, + ) + require.NoError(t, err) + require.Equal(t, 2, len(rows), "expected exactly two rows") + // GetAuditLogsOffset returns entries sorted by time in descending order. + enableLog := rows[1].AuditLog + disableLog := rows[0].AuditLog + + var enableDiff audit.Map + require.NoError(t, json.Unmarshal(enableLog.Diff, &enableDiff)) + if assert.Contains(t, enableDiff, "dynamic_client_registration_enabled", "tracked field missing from enableDiff") { + assert.Equal(t, false, enableDiff["dynamic_client_registration_enabled"].Old) + assert.Equal(t, true, enableDiff["dynamic_client_registration_enabled"].New) + } + + var disableDiff audit.Map + require.NoError(t, json.Unmarshal(disableLog.Diff, &disableDiff)) + if assert.Contains(t, disableDiff, "dynamic_client_registration_enabled", "tracked field missing from disableDiff") { + assert.Equal(t, true, disableDiff["dynamic_client_registration_enabled"].Old) + assert.Equal(t, false, disableDiff["dynamic_client_registration_enabled"].New) + } +} diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c619c53682..4fb23dceea 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -6658,6 +6658,22 @@ export const OAuth2ProviderResponseTypes: OAuth2ProviderResponseType[] = [ "token", ]; +// From codersdk/oauth2.go +/** + * OAuth2ProviderSettings controls deployment-wide OAuth2 provider behavior. + * + * DynamicClientRegistrationEnabled is a pointer so a PUT can omit it to leave + * the current value unchanged, rather than a decoded zero value silently + * resetting it to false. This matters once a second field lands in this + * struct (e.g. a future initial-access-token requirement): a client built + * against an older, single-field version of this struct would otherwise + * always encode the newer field's zero value, silently clearing it on every + * unrelated update. GET always returns a non-nil value. + */ +export interface OAuth2ProviderSettings { + readonly dynamic_client_registration_enabled?: boolean; +} + // From codersdk/client.go /** * OAuth2RedirectCookie is the name of the cookie that stores the oauth2 redirect. @@ -7868,6 +7884,7 @@ export type ResourceType = | "notifications_settings" | "oauth2_provider_app" | "oauth2_provider_app_secret" + | "oauth2_provider_settings" | "organization" | "organization_member" | "prebuilds_settings" @@ -7905,6 +7922,7 @@ export const ResourceTypes: ResourceType[] = [ "notifications_settings", "oauth2_provider_app", "oauth2_provider_app_secret", + "oauth2_provider_settings", "organization", "organization_member", "prebuilds_settings",