mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
`POST /oauth2/register` (RFC 7591 Dynamic Client Registration) has exactly one gate today: `ExperimentOAuth2`, a static, process-lifetime flag that wraps the entire `/oauth2/*` route tree as an all-or-nothing switch. That flag is scheduled for removal at GA, which would leave DCR with zero admin control at all once it is gone. Add a persistent, DCR-specific `oauth2_dcr_enabled` deployment setting, independent of the experiment system, so admin control over DCR survives GA. `POST /oauth2/register` checks the flag and rejects new registrations with an RFC 7591-shaped `403` when disabled; discovery metadata (`GET /.well-known/oauth-authorization-server`) conditionally omits `registration_endpoint`. A new audited `GET`/`PUT /api/v2/oauth2-provider/settings` endpoint lets an owner toggle it live, no restart required. The setting defaults to disabled, matching the canonical design proposal; disabling only stops new self-registrations, clients that already registered continue to authorize and exchange tokens normally. Address issue described in [ENG-3056](https://linear.app/codercom/issue/ENG-3056/oauth2-dcr-admin-configurable-enabledisable). ## Where this sits in the request path ```mermaid sequenceDiagram autonumber participant A as Admin participant S as coderd participant DB as site_configs<br/>(oauth2_dcr_enabled) participant C as OAuth2/MCP Client Note over A,S: Admin toggles DCR (new) A->>S: PUT /api/v2/oauth2-provider/settings<br/>{dynamic_client_registration_enabled: false} S->>S: authorizeContext(ActionUpdate, ResourceDeploymentConfig) S->>DB: UPSERT oauth2_dcr_enabled = false S-->>A: 200 OK (audited) Note over C,S: Client discovery + registration afterward C->>S: GET /.well-known/oauth-authorization-server S->>DB: GetOAuth2DCREnabled (system ctx, every request, no cache) DB-->>S: false S-->>C: 200 metadata, registration_endpoint omitted C->>S: POST /oauth2/register S->>DB: GetOAuth2DCREnabled (system ctx, every request, no cache) DB-->>S: false S-->>C: 403 invalid_request,<br/>"Dynamic client registration is disabled" Note over C,S: A client that registered before the change is unaffected C->>S: GET /oauth2/authorize?client_id=... Note over S: no DCR-enabled check on this path S-->>C: 200 (proceeds normally) C->>S: PUT/DELETE /oauth2/clients/{client_id} (RFC 7592 self-management) Note over S: no DCR-enabled check on this path either S-->>C: 200 (proceeds normally) ``` ## Files changed: manual vs. generated Reviewers should focus on the **manual** files. The **generated** ones are `make gen` output that follows mechanically from the manual changes and don't need direct review. <details> <summary><b>Manual files (26)</b> — click to expand, grouped the same way as "Suggested review order" below</summary> **1. Database** | File | What changed | |---|---| | `coderd/database/queries/siteconfig.sql` | New `GetOAuth2DCREnabled`/`UpsertOAuth2DCREnabled` query pair on the existing generic `site_configs` table. No schema change. | | `coderd/database/dbauthz/dbauthz.go` | RBAC check (`rbac.ResourceDeploymentConfig`) on the two new query methods; extends the `subjectSystemOAuth2` system-actor role with read-only `ResourceDeploymentConfig` access, needed so the public discovery/registration endpoints can read the flag via `dbauthz.AsSystemOAuth2`. | | `coderd/database/dbauthz/dbauthz_test.go` | RBAC assertion coverage for `GetOAuth2DCREnabled`/`UpsertOAuth2DCREnabled` in the method-coverage test suite. | **2. Request gating (the actual feature)** | File | What changed | |---|---| | `coderd/oauth2provider/registration.go` | The actual gate: `CreateDynamicClientRegistration` reads the flag first and returns an RFC 7591-shaped `403` when disabled (defaults disabled if never configured). | | `coderd/oauth2provider/registration_test.go` | New unit test, `TestCreateDynamicClientRegistration_DCREnabled`: calls the handler directly (no HTTP server), covering enabled / explicitly disabled / never-configured. | | `coderd/oauth2provider/metadata.go` | `GetAuthorizationServerMetadata` conditionally omits `registration_endpoint` from discovery metadata when DCR is disabled. | | `coderd/oauth2provider/metadata_test.go` | New unit test, `TestGetAuthorizationServerMetadata_DCREnabled`: same three states, for the discovery handler. | **3. Admin settings endpoint** | File | What changed | |---|---| | `codersdk/oauth2.go` | New `OAuth2ProviderSettings` SDK type plus `Client.OAuth2ProviderSettings`/`PutOAuth2ProviderSettings` methods. | | `coderd/oauth2.go` | New `oauth2ProviderSettings`/`putOAuth2ProviderSettings` admin handlers (audited via `audit.InitRequest`); updates the `GetAuthorizationServerMetadata` call site to pass `api.Database`. | | `coderd/coderd.go` | Registers `GET`/`PUT /api/v2/oauth2-provider/settings`. | | `coderd/oauth2_provider_settings_test.go` | New test file: admin `GET`/`PUT` round-trip, default-disabled-before-any-`PUT`, and `403` for a non-owner on both `GET` and `PUT`. | **4. Audit wiring** | File | What changed | |---|---| | `coderd/database/types.go` | New `database.OAuth2ProviderSettings` audit-only struct (mirrors `NotificationsSettings`). | | `coderd/audit/diff.go` | Adds the new struct to the `Auditable` type union. | | `coderd/audit/request.go` | Adds the new struct to all four dispatch switches (`ResourceTarget`, `ResourceID`, `ResourceType`, `ResourceRequiresOrgID`). | | `codersdk/audit.go` | New API-facing `ResourceTypeOAuth2ProviderSettings` constant and its `FriendlyString` case. | | `enterprise/audit/table.go` | Field-level audit action map (`ActionTrack`/`ActionIgnore`) for the new struct. | | `coderd/database/migrations/000546_audit_oauth2_provider_settings.up.sql` | Adds `oauth2_provider_settings` to the `resource_type` Postgres enum, required for the audit wiring above (`resource_type` is a real enum, not a Go-only value). | | `coderd/database/migrations/000546_audit_oauth2_provider_settings.down.sql` | No-op (`ALTER TYPE ... ADD VALUE` can't be reverted). | **5. Test-suite ripple from the disabled-by-default flip** | File | What changed | |---|---| | `coderd/oauth2provider/oauth2providertest/helpers.go` | New shared test helper, `EnableDCR`, since DCR now defaults to disabled and many pre-existing tests need it turned on to register a client. | | `coderd/oauth2_test.go` | Adds `TestOAuth2DynamicClientRegistrationDisabled` (registers a client, disables DCR, verifies new registration is rejected while the existing client's self-management, authorize, and token exchange all keep working); calls `EnableDCR` in every pre-existing test that registers a client. | | `coderd/oauth2_error_compliance_test.go` | Calls `EnableDCR` in every test that registers a client, so RFC-error-format assertions aren't masked by the new disabled-by-default gate. | | `coderd/oauth2_metadata_validation_test.go` | Same: `EnableDCR` added to every registration-dependent test. | | `coderd/oauth2_security_test.go` | Same. | | `coderd/oauth2provider/validation_test.go` | Same (near-duplicate of `oauth2_metadata_validation_test.go` in a different package). | | `coderd/oauth2provider/provider_test.go` | Same. | | `coderd/mcp/mcp_e2e_test.go` | Same, for the MCP end-to-end dynamic-registration flow test. | </details> <details> <summary><b>Generated files (12)</b> — from <code>make gen</code>, no need to review directly</summary> `coderd/apidoc/docs.go`, `coderd/apidoc/swagger.json`, `coderd/database/dbmetrics/querymetrics.go`, `coderd/database/dbmock/dbmock.go`, `coderd/database/dump.sql`, `coderd/database/models.go`, `coderd/database/querier.go`, `coderd/database/queries.sql.go`, `docs/admin/security/audit-logs.md`, `docs/reference/api/enterprise.md`, `docs/reference/api/schemas.md`, `site/src/api/typesGenerated.ts`. </details> ## Suggested review order ### 1. Database Establishes the persisted setting and its RBAC rule; everything else builds on `GetOAuth2DCREnabled`/`UpsertOAuth2DCREnabled`. 1. `coderd/database/queries/siteconfig.sql` — the two new queries. Same boolean-encoding pattern as the existing `oauth2_github_default_eligible` key right above them in the same file. 2. `coderd/database/dbauthz/dbauthz.go` — the RBAC wrapper for those two queries, plus the `subjectSystemOAuth2` role extension (search this file for `ResourceDeploymentConfig`, it appears in both spots). 3. `coderd/database/dbauthz/dbauthz_test.go` — asserts the RBAC checks from (2) actually fire. ### 2. Request gating (the actual feature) Where `POST /oauth2/register` and discovery metadata change behavior. 1. `coderd/oauth2provider/registration.go` — the primary gate. Read this first; it's the feature. 2. `coderd/oauth2provider/registration_test.go` — its new unit test, exercising the gate's three states directly against the handler. 3. `coderd/oauth2provider/metadata.go` — the same gating pattern applied to the discovery `GET` endpoint. 4. `coderd/oauth2provider/metadata_test.go` — its new unit test. ### 3. Admin settings endpoint How an owner flips the setting live. 1. `codersdk/oauth2.go` — the `OAuth2ProviderSettings` SDK type and `Client` methods first; this is the public contract everything below implements against. 2. `coderd/oauth2.go` — the `GET`/`PUT` handlers themselves. 3. `coderd/coderd.go` — route registration, to see where those handlers get wired in. 4. `coderd/oauth2_provider_settings_test.go` — round-trip and permission tests. ### 4. Audit wiring Plumbing required so step 3's `PUT` is auditable; mechanical except for (3). 1. `coderd/database/types.go` — the audit-only struct; everything else in this layer exists to plumb it through. 2. `coderd/audit/diff.go` — adds it to the `Auditable` type union (the compiler enforces this one). 3. `coderd/audit/request.go` — the four dispatch switches; the one part of this layer worth reading closely. 4. `codersdk/audit.go` — the API-facing resource type constant. 5. `enterprise/audit/table.go` — the field-action map. 6. `coderd/database/migrations/000546_audit_oauth2_provider_settings.{up,down}.sql` — read last; a consequence of needing a new `resource_type` enum value for (1)-(5), not a design decision of its own. ### 5. Test-suite ripple from the disabled-by-default flip 1. `coderd/oauth2provider/oauth2providertest/helpers.go` — the new `EnableDCR` helper. Read first to understand the fix pattern before seeing it applied repeatedly. 2. `coderd/oauth2_test.go` — next, since it also contains the new `TestOAuth2DynamicClientRegistrationDisabled`, not just `EnableDCR` call sites. 3. The rest, in any order, they're mechanical repeats of the same one-line addition: `coderd/oauth2_error_compliance_test.go`, `coderd/oauth2_metadata_validation_test.go`, `coderd/oauth2_security_test.go`, `coderd/oauth2provider/validation_test.go`, `coderd/oauth2provider/provider_test.go`, `coderd/mcp/mcp_e2e_test.go`. ## Explicitly out of scope Per the design proposal: rate limiting on `POST /oauth2/register` (tracked separately), retroactively affecting already-registered clients when DCR is disabled (this only gates new self-registration), and an Initial Access Token requirement (a separate, follow-up ticket).
773 lines
20 KiB
Go
773 lines
20 KiB
Go
package coderd_test
|
|
|
|
import (
|
|
"fmt"
|
|
"net/url"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"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"
|
|
)
|
|
|
|
// TestOAuth2ClientMetadataValidation tests enhanced metadata validation per RFC 7591
|
|
func TestOAuth2ClientMetadataValidation(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// 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()
|
|
|
|
tests := []struct {
|
|
name string
|
|
redirectURIs []string
|
|
expectError bool
|
|
errorContains string
|
|
}{
|
|
{
|
|
name: "ValidHTTPS",
|
|
redirectURIs: []string{"https://example.com/callback"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidLocalhost",
|
|
redirectURIs: []string{"http://localhost:8080/callback"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidLocalhostIP",
|
|
redirectURIs: []string{"http://127.0.0.1:8080/callback"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidCustomScheme",
|
|
redirectURIs: []string{"com.example.myapp://auth/callback"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidHTTPNonLocalhost",
|
|
redirectURIs: []string{"http://example.com/callback"},
|
|
expectError: true,
|
|
errorContains: "redirect_uri",
|
|
},
|
|
{
|
|
name: "InvalidWithFragment",
|
|
redirectURIs: []string{"https://example.com/callback#fragment"},
|
|
expectError: true,
|
|
errorContains: "fragment",
|
|
},
|
|
{
|
|
name: "InvalidJavaScriptScheme",
|
|
redirectURIs: []string{"javascript:alert('xss')"},
|
|
expectError: true,
|
|
errorContains: "dangerous scheme",
|
|
},
|
|
{
|
|
name: "InvalidDataScheme",
|
|
redirectURIs: []string{"data:text/html,<script>alert('xss')</script>"},
|
|
expectError: true,
|
|
errorContains: "dangerous scheme",
|
|
},
|
|
{
|
|
name: "InvalidFileScheme",
|
|
redirectURIs: []string{"file:///etc/passwd"},
|
|
expectError: true,
|
|
errorContains: "dangerous scheme",
|
|
},
|
|
{
|
|
name: "EmptyString",
|
|
redirectURIs: []string{""},
|
|
expectError: true,
|
|
errorContains: "redirect_uri",
|
|
},
|
|
{
|
|
name: "RelativeURL",
|
|
redirectURIs: []string{"/callback"},
|
|
expectError: true,
|
|
errorContains: "redirect_uri",
|
|
},
|
|
{
|
|
name: "MultipleValid",
|
|
redirectURIs: []string{"https://example.com/callback", "com.example.app://auth"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "MixedValidInvalid",
|
|
redirectURIs: []string{"https://example.com/callback", "http://example.com/callback"},
|
|
expectError: true,
|
|
errorContains: "redirect_uri",
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: test.redirectURIs,
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
if test.errorContains != "" {
|
|
require.Contains(t, strings.ToLower(err.Error()), strings.ToLower(test.errorContains))
|
|
}
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
})
|
|
|
|
t.Run("ClientURIValidation", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
clientURI string
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "ValidHTTPS",
|
|
clientURI: "https://example.com",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidHTTPLocalhost",
|
|
clientURI: "http://localhost:8080",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidWithPath",
|
|
clientURI: "https://example.com/app",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidWithQuery",
|
|
clientURI: "https://example.com/app?param=value",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidNotURL",
|
|
clientURI: "not-a-url",
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "ValidWithFragment",
|
|
clientURI: "https://example.com#fragment",
|
|
expectError: false, // Fragments are allowed in client_uri, unlike redirect_uri
|
|
},
|
|
{
|
|
name: "InvalidJavaScript",
|
|
clientURI: "javascript:alert('xss')",
|
|
expectError: true, // Only http/https allowed for client_uri
|
|
},
|
|
{
|
|
name: "InvalidFTP",
|
|
clientURI: "ftp://example.com",
|
|
expectError: true, // Only http/https allowed for client_uri
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
ClientURI: test.clientURI,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
})
|
|
|
|
t.Run("LogoURIValidation", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
logoURI string
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "ValidHTTPS",
|
|
logoURI: "https://example.com/logo.png",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidHTTPLocalhost",
|
|
logoURI: "http://localhost:8080/logo.png",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidWithQuery",
|
|
logoURI: "https://example.com/logo.png?size=large",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidNotURL",
|
|
logoURI: "not-a-url",
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "ValidWithFragment",
|
|
logoURI: "https://example.com/logo.png#fragment",
|
|
expectError: false, // Fragments are allowed in logo_uri
|
|
},
|
|
{
|
|
name: "InvalidJavaScript",
|
|
logoURI: "javascript:alert('xss')",
|
|
expectError: true, // Only http/https allowed for logo_uri
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
LogoURI: test.logoURI,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
})
|
|
|
|
t.Run("GrantTypeValidation", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
grantTypes []codersdk.OAuth2ProviderGrantType
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "DefaultEmpty",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidAuthorizationCode",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"authorization_code"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidRefreshTokenAlone",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"refresh_token"},
|
|
expectError: true, // refresh_token requires authorization_code to be present
|
|
},
|
|
{
|
|
name: "ValidMultiple",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"authorization_code", "refresh_token"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidUnsupported",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"client_credentials"},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "InvalidPassword",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"password"},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "InvalidImplicit",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"implicit"},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "MixedValidInvalid",
|
|
grantTypes: []codersdk.OAuth2ProviderGrantType{"authorization_code", "client_credentials"},
|
|
expectError: true,
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
GrantTypes: test.grantTypes,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
})
|
|
|
|
t.Run("ResponseTypeValidation", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
responseTypes []codersdk.OAuth2ProviderResponseType
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "DefaultEmpty",
|
|
responseTypes: []codersdk.OAuth2ProviderResponseType{},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidCode",
|
|
responseTypes: []codersdk.OAuth2ProviderResponseType{"code"},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidToken",
|
|
responseTypes: []codersdk.OAuth2ProviderResponseType{"token"},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "InvalidImplicit",
|
|
responseTypes: []codersdk.OAuth2ProviderResponseType{"id_token"},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "InvalidMultiple",
|
|
responseTypes: []codersdk.OAuth2ProviderResponseType{"code", "token"},
|
|
expectError: true,
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
ResponseTypes: test.responseTypes,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
})
|
|
|
|
t.Run("TokenEndpointAuthMethodValidation", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
authMethod codersdk.OAuth2TokenEndpointAuthMethod
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "DefaultEmpty",
|
|
authMethod: "",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidClientSecretBasic",
|
|
authMethod: "client_secret_basic",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidClientSecretPost",
|
|
authMethod: "client_secret_post",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidNone",
|
|
authMethod: "none",
|
|
expectError: false, // "none" is valid for public clients per RFC 7591
|
|
},
|
|
{
|
|
name: "InvalidPrivateKeyJWT",
|
|
authMethod: "private_key_jwt",
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "InvalidClientSecretJWT",
|
|
authMethod: "client_secret_jwt",
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "InvalidCustom",
|
|
authMethod: "custom_method",
|
|
expectError: true,
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
TokenEndpointAuthMethod: test.authMethod,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
})
|
|
}
|
|
|
|
// TestOAuth2ClientNameValidation tests client name validation requirements
|
|
func TestOAuth2ClientNameValidation(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// 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
|
|
clientName string
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "ValidBasic",
|
|
clientName: "My App",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidWithNumbers",
|
|
clientName: "My App 2.0",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidWithSpecialChars",
|
|
clientName: "My-App_v1.0",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidUnicode",
|
|
clientName: "My App 🚀",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidLong",
|
|
clientName: strings.Repeat("A", 100),
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidEmpty",
|
|
clientName: "",
|
|
expectError: false, // Empty names are allowed, defaults are applied
|
|
},
|
|
{
|
|
name: "ValidWhitespaceOnly",
|
|
clientName: " ",
|
|
expectError: false, // Whitespace-only names are allowed
|
|
},
|
|
{
|
|
name: "ValidTooLong",
|
|
clientName: strings.Repeat("A", 1000),
|
|
expectError: false, // Very long names are allowed
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: test.clientName,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestOAuth2ClientScopeValidation tests scope parameter validation
|
|
func TestOAuth2ClientScopeValidation(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// 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
|
|
scope string
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "DefaultEmpty",
|
|
scope: "",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidRead",
|
|
scope: "read",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidWrite",
|
|
scope: "write",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidMultiple",
|
|
scope: "read write",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidOpenID",
|
|
scope: "openid",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidProfile",
|
|
scope: "profile",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidEmail",
|
|
scope: "email",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "ValidCombined",
|
|
scope: "openid profile email read write",
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "InvalidAdmin",
|
|
scope: "admin",
|
|
expectError: false, // Admin scope should be allowed but validated during authorization
|
|
},
|
|
{
|
|
name: "ValidCustom",
|
|
scope: "custom:scope",
|
|
expectError: false,
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
Scope: test.scope,
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
|
|
if test.expectError {
|
|
require.Error(t, err)
|
|
} else {
|
|
require.NoError(t, err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestOAuth2ClientMetadataDefaults tests that default values are properly applied
|
|
func TestOAuth2ClientMetadataDefaults(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
oauth2providertest.EnableDCR(t, client)
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
// Register a minimal client to test defaults
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
resp, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
require.NoError(t, err)
|
|
|
|
// Get the configuration to check defaults
|
|
config, err := client.GetOAuth2ClientConfiguration(ctx, resp.ClientID, resp.RegistrationAccessToken)
|
|
require.NoError(t, err)
|
|
|
|
// Should default to authorization_code
|
|
require.Contains(t, config.GrantTypes, codersdk.OAuth2ProviderGrantTypeAuthorizationCode)
|
|
|
|
// Should default to code
|
|
require.Contains(t, config.ResponseTypes, codersdk.OAuth2ProviderResponseTypeCode)
|
|
|
|
// Should default to client_secret_basic or client_secret_post
|
|
require.True(t, config.TokenEndpointAuthMethod == codersdk.OAuth2TokenEndpointAuthMethodClientSecretBasic ||
|
|
config.TokenEndpointAuthMethod == codersdk.OAuth2TokenEndpointAuthMethodClientSecretPost ||
|
|
config.TokenEndpointAuthMethod == "")
|
|
|
|
// Client secret should be generated
|
|
require.NotEmpty(t, resp.ClientSecret)
|
|
require.Greater(t, len(resp.ClientSecret), 20)
|
|
|
|
// Registration access token should be generated
|
|
require.NotEmpty(t, resp.RegistrationAccessToken)
|
|
require.Greater(t, len(resp.RegistrationAccessToken), 20)
|
|
}
|
|
|
|
// TestOAuth2ClientMetadataEdgeCases tests edge cases and boundary conditions
|
|
func TestOAuth2ClientMetadataEdgeCases(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// 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()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
// Create a very long but valid HTTPS URI
|
|
longPath := strings.Repeat("a", 2000)
|
|
longURI := "https://example.com/" + longPath
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{longURI},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
// This might be accepted or rejected depending on URI length limits
|
|
// The test verifies the behavior is consistent
|
|
if err != nil {
|
|
require.Contains(t, strings.ToLower(err.Error()), "uri")
|
|
}
|
|
})
|
|
|
|
t.Run("ManyRedirectURIs", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
// Test with many redirect URIs
|
|
redirectURIs := make([]string, 20)
|
|
for i := 0; i < 20; i++ {
|
|
redirectURIs[i] = fmt.Sprintf("https://example%d.com/callback", i)
|
|
}
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: redirectURIs,
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
// Should handle multiple redirect URIs gracefully
|
|
require.NoError(t, err)
|
|
})
|
|
|
|
t.Run("URIWithUnusualPort", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com:8443/callback"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
require.NoError(t, err)
|
|
})
|
|
|
|
t.Run("URIWithComplexPath", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{"https://example.com/path/to/callback?param=value&other=123"},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
require.NoError(t, err)
|
|
})
|
|
|
|
t.Run("URIWithEncodedCharacters", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := testutil.Context(t, testutil.WaitLong)
|
|
|
|
// Test with URL-encoded characters
|
|
encodedURI := "https://example.com/callback?param=" + url.QueryEscape("value with spaces")
|
|
|
|
req := codersdk.OAuth2ClientRegistrationRequest{
|
|
RedirectURIs: []string{encodedURI},
|
|
ClientName: fmt.Sprintf("test-client-%d", time.Now().UnixNano()),
|
|
}
|
|
|
|
_, err := client.PostOAuth2ClientRegistration(ctx, req)
|
|
require.NoError(t, err)
|
|
})
|
|
}
|