From d574c53b8166aec4ab26c02096f621ee5908dfe6 Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Sat, 19 Sep 2026 18:44:04 +0200 Subject: [PATCH] fix(oauth): a dynamic client without a declared scope holds the realm's opted-in scopes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from the field (amZettel, priority high), the follow-up to the loopback fix one check deeper: Claude Code's CIMD document — like every static MCP-client document, one file for every server in the world — declares no `scope`, and Modgud granted a dynamic client exactly the scopes its document or DCR registration declared. Such a client held no scope permission at all, and OpenIddict refused everything but openid/offline_access with invalid_request (ID2051) before the per-scope opt-in was ever consulted. The opt-in built for exactly this case was unreachable. One rule, DynamicClientScopePolicy, behind three seams: a dynamic client may hold every enabled scope with AllowDynamicRegistrationClients — app-scoped or global; the flag is the boundary, the App link never was — plus the standard scopes other than modgud.management (immutable, so judged by a built-in verdict; the management selector is the "tenant.admin.*" case the flag exists for, and the seeder already strips the flag from it). A declared scope is an upper bound intersected with that set; none declared means the whole set (RFC 7591 §2, "a default set of scopes"). - CimdClientResolver: permissions are computed per resolve from the live set (memoized per request), the document stays cached — ticking the flag takes effect at once, like Cimd.Enabled. - OAuthAdminService: a DCR registration stores the resolved set; the /connect/register response echoes what was registered (§3.2.1). - DynamicClientScopeHandler, ordered just before OpenIddict's ValidateScopePermissions: a refused scope is answered with Modgud's own invalid_scope naming the scope and the flag, not the opaque ID2051. The authorize endpoint's own check (still reached from the consent continuation) uses the same policy and wording. Contract change (pre-1.0): a custom global scope needs the flag for dynamic clients; existing DCR clients keep their stored permissions. A refused scope is now an error page from request validation (as every OpenIddict validation failure is) instead of a redirect with error=invalid_scope. Tests: CIMD document without scope (the exact Claude Code document) reaches consent for an opted-in scope and the OIDC scopes, is refused for one without the flag and for modgud.management, completes the flow, and a declaring document cannot exceed the set; DCR registration without scope echoes and stores the set, a declared list is intersected. Red without the fix, green with it; 1708 unit + 839 integration green. Co-Authored-By: Claude Fable 5.1 --- docs/admin/client-id-metadata-documents.md | 10 +- docs/admin/dynamic-client-registration.md | 16 ++- .../Authorization/CimdFullFlowTests.cs | 76 ++++++++++++- .../Authorization/DcrFullFlowTests.cs | 100 ++++++++++++++--- .../Auth/OAuth/AuthorizationEndpoints.cs | 29 +++-- .../Auth/OAuth/DcrRegistrationEndpoints.cs | 14 ++- .../Dcr/DynamicClientScopePolicy.cs | 88 +++++++++++++++ .../Services/OAuthAdminService.cs | 11 +- .../OpenIddict/Cimd/CimdClientResolver.cs | 48 ++++++-- .../OpenIddict/DynamicClientScopeHandler.cs | 100 +++++++++++++++++ .../OpenIddict/OpenIddictExtensions.cs | 1 + .../Dcr/DynamicClientScopePolicyTests.cs | 104 ++++++++++++++++++ 12 files changed, 549 insertions(+), 48 deletions(-) create mode 100644 src/dotnet/Modgud.Application/Dcr/DynamicClientScopePolicy.cs create mode 100644 src/dotnet/Modgud.Infrastructure/OpenIddict/DynamicClientScopeHandler.cs create mode 100644 src/dotnet/Modgud.Tests.Unit/OAuth/Dcr/DynamicClientScopePolicyTests.cs diff --git a/docs/admin/client-id-metadata-documents.md b/docs/admin/client-id-metadata-documents.md index ca9e4699..c08d8456 100644 --- a/docs/admin/client-id-metadata-documents.md +++ b/docs/admin/client-id-metadata-documents.md @@ -39,7 +39,7 @@ When a client presents an HTTPS-URL `client_id` that isn't a known stored client 1. Validates the URL shape — `https` scheme, has a path, no fragment, no userinfo, no dot-segments. 2. Fetches it over an **SSRF-hardened** transport (see below), capped at **5 KB**, `Accept: application/json`, ~5 s timeout, **no redirects**. 3. Validates the document (table below). -4. Synthesizes an in-memory public PKCE client with a deterministic id derived from the URL, JWT access tokens, and the redirect URIs / scopes the document declares. +4. Synthesizes an in-memory public PKCE client with a deterministic id derived from the URL, JWT access tokens, the redirect URIs the document declares, and the realm's **dynamic-client scopes** (see below) — narrowed to the document's `scope` when it has one. 5. Caches the validated document per URL, respecting `Cache-Control` (clamped to between 5 minutes and 24 hours; failures are never cached). No database row is created. A refresh after the cache expires re-fetches and re-validates the live document — if the URL becomes unreachable, refresh fails and the client must re-authenticate. @@ -58,7 +58,9 @@ CIMD shares DCR's resource/scope opt-in surface — flipping the master toggle d | Per-API allow-list | [OAuth APIs](./oauth-apis) → **Allow DCR** checkbox per row | Off | | Per-Scope allow-list | [OAuth Scopes](./oauth-scopes) → **Allow DCR Clients** checkbox per row | Off | -A CIMD client must send a `resource=` parameter, and the target resource server must have **Allow DCR** enabled — otherwise the token endpoint rejects with `invalid_target`. App-scoped scopes are reachable only when **Allow DCR Clients** is ticked, exactly as for DCR clients (a CIMD client is realm-wide and has no App link of its own). Global scopes (`openid`, `email`, `profile`, …) are always reachable. +A CIMD client must send a `resource=` parameter, and the target resource server must have **Allow DCR** enabled — otherwise the token endpoint rejects with `invalid_target`. The scopes it can hold and request are the realm's **dynamic-client scopes**: every scope with **Allow DCR Clients** ticked (app-scoped or not — a CIMD client is realm-wide and has no App link of its own) plus the standard scopes other than `modgud.management` (`openid`, `profile`, `email`, `phone`, `address`, `offline_access`, `roles`, `permissions`). Same rule as for [DCR clients](./dynamic-client-registration#how-the-per-scope-flag-interacts-with-app-scoped-scopes). + +A document **without** `scope` — the normal case: Claude Code's document is one static file for every MCP server in the world and cannot know one server's scopes, the client reads them from the server's protected-resource metadata at run time — holds that whole set. A document **with** `scope` holds the intersection; it can narrow what its client may ask for, never widen it. The set is read live on every resolve, so ticking **Allow DCR Clients** on a scope takes effect at once, even for an already-cached document. Requesting a scope outside the set fails at `/connect/authorize` with `invalid_scope` and a description naming the scope and the flag. ## Enabling CIMD for a realm @@ -67,7 +69,7 @@ A CIMD client must send a `resource=` parameter, and the target resource server - **Access-token lifetime** (default 15 min) — shorter than admin-created clients on purpose; a leaked token from an unverified, domain-bound client has a smaller blast radius. - **Refresh-token lifetime** (default 7 d). 3. **OAuth APIs → your MCP-server API** → tick **Allow DCR**. -4. **OAuth Scopes → the scope(s) the MCP server gates** → tick **Allow DCR Clients** (for app-scoped scopes). +4. **OAuth Scopes → the scope(s) the MCP server gates** → tick **Allow DCR Clients**. ## What's accepted in the metadata document @@ -79,7 +81,7 @@ A CIMD client must send a `resource=` parameter, and the target resource server | `token_endpoint_auth_method` | `none` or omitted. **v1 is public-only** — a `client_secret*` method or any `client_secret` field is rejected. | | `grant_types` | Subset of `{authorization_code, refresh_token}`; must include `authorization_code`. | | `response_types` | Subset of `{code}`. | -| `scope` | Optional, space-delimited. The scopes the client may request (still subject to the opt-in gates above). | +| `scope` | Optional, space-delimited. An **upper bound**: the client holds these scopes intersected with the realm's dynamic-client scopes. Omitted (as every static MCP-client document does), the client holds the whole set. | | `client_name` | Optional. Used as the display name; the consent screen also shows the URL hostname regardless. | A document that fails any rule is rejected and never cached; the authorize request fails as "unknown client". diff --git a/docs/admin/dynamic-client-registration.md b/docs/admin/dynamic-client-registration.md index 9f814230..7f666603 100644 --- a/docs/admin/dynamic-client-registration.md +++ b/docs/admin/dynamic-client-registration.md @@ -48,13 +48,18 @@ The master toggle just turns the registration endpoint on. The per-API flag cont ### How the per-Scope flag interacts with app-scoped scopes -Most scopes you create in Modgud are **app-scoped** — they belong to one [Application](./applications) (`Scope.AppId` is set). Non-DCR clients are restricted to scopes whose `AppId` matches one of their own linked Apps. **DCR clients have no `AppId`** by design (they're realm-wide public PKCE clients), so the per-Scope `Allow DCR Clients` flag replaces the app-link check for them: +Most scopes you create in Modgud are **app-scoped** — they belong to one [Application](./applications) (`Scope.AppId` is set). Non-DCR clients are restricted to scopes whose `AppId` matches one of their own linked Apps. **DCR clients have no `AppId`** by design (they're realm-wide public PKCE clients), so the per-Scope `Allow DCR Clients` flag replaces the app-link check for them. The scopes a DCR client can hold and request — its **dynamic-client scopes** — are: -- **Global scopes** (`AppId = null` — the OIDC standards `openid`, `email`, `profile`, … plus any cross-app scope you create): always reachable by DCR clients. -- **App-scoped scope with `Allow DCR Clients = true`**: reachable by DCR clients. The realm-admin has explicitly opted this scope in for anonymous-registrant access. -- **App-scoped scope with `Allow DCR Clients = false`** (default): `/connect/authorize` rejects the request with `invalid_scope` before the user ever sees the consent screen. The agent gets a clear error description. +- **Any scope with `Allow DCR Clients = true`**, app-scoped or global. The realm-admin has explicitly opted this scope in for anonymous-registrant access. +- **The standard scopes other than `modgud.management`**: `openid`, `profile`, `email`, `phone`, `address`, `offline_access`, `roles`, `permissions`. They cannot carry the flag (standard scopes are immutable) and name no resource privilege of their own. The management-API selector is never available to a dynamic client. -The combined effect: enabling DCR safely requires you to walk through your existing scopes once and decide which ones agents are allowed to ask for. Until you tick `Allow DCR Clients` on at least one app-scoped scope (or create a fresh global scope), DCR clients can only request the OIDC standard scopes. +A custom scope **without** the flag (default) — app-scoped or global — is not: `/connect/authorize` rejects the request with `invalid_scope` before the user ever sees the consent screen, with a description naming the scope and the flag to enable. + +The combined effect: enabling DCR safely requires you to walk through your existing scopes once and decide which ones agents are allowed to ask for. Until you tick `Allow DCR Clients` on at least one scope, DCR clients can only request the standard scopes. + +### What a registration gets when it omits `scope` + +RFC 7591 leaves it to the server ("a default set of scopes"). Modgud's default is the realm's dynamic-client scopes as they are at registration time; a registration **with** `scope` gets the intersection of what it asked for and that set — its own list is an upper bound, never a grant. The `201` response echoes the registered set in `scope`, so an agent can see what it holds. A client keeps the set it was registered with; a scope opted in later needs a re-registration (or a CIMD document, which is resolved live). ## Enabling DCR for a realm @@ -80,6 +85,7 @@ After these four steps, an agent that POSTs to `/connect/register` with a valid | `token_endpoint_auth_method` | Must be `none` (or omitted). Public PKCE only — no secret-storage. | | `grant_types` | Subset of `{authorization_code, refresh_token}`. | | `response_types` | Subset of `{code}`. No implicit / hybrid flows. | +| `scope` | Optional, space-delimited. An upper bound intersected with the realm's dynamic-client scopes; omitted, the client gets the whole set. The response echoes what was registered. | On success the endpoint returns `201 Created` with the assigned `client_id` per RFC 7591 §3.2.1. On rejection it returns `400 Bad Request` with `{ error, error_description }` per §3.2.2. Hitting the rate-limit returns `429`. diff --git a/src/dotnet/Modgud.Api.Tests/Authorization/CimdFullFlowTests.cs b/src/dotnet/Modgud.Api.Tests/Authorization/CimdFullFlowTests.cs index 85d1f08e..ef90b38c 100644 --- a/src/dotnet/Modgud.Api.Tests/Authorization/CimdFullFlowTests.cs +++ b/src/dotnet/Modgud.Api.Tests/Authorization/CimdFullFlowTests.cs @@ -112,6 +112,66 @@ public async Task Loopback_redirect_with_an_ephemeral_port_is_accepted_for_a_cim Assert.Contains(AllowedAudience, new JwtSecurityTokenHandler().ReadJwtToken(accessToken).Audiences); } + /// + /// A static CIMD document has no scope — Claude Code's is one document + /// for every MCP server in the world and cannot know one server's scopes; the + /// client learns them from the server's protected-resource metadata at run + /// time. Such a client must hold the realm's dynamic-client scope set (the + /// opted-in scopes plus the open standard scopes), so the per-scope opt-in is + /// reachable at all. It used to hold nothing, and OpenIddict refused every + /// scope but openid/offline_access with ID2051 before the opt-in + /// was ever consulted (reported from the field). A scope the admin did not + /// opt in stays refused — with Modgud's own message, not the opaque one — and + /// a document that does declare scope cannot name its way past that. + /// + [Fact] + public async Task A_document_without_scope_holds_the_realms_dynamic_client_scopes() + { + await SeedAsync(); + const string notOptedIn = "cimd-not-opted-in-scope"; + await CreateScopeAsync(notOptedIn, allowDynamicClients: false); + Factory.CimdDocuments[_clientIdUrl] = JsonSerializer.Serialize(new Dictionary + { + ["client_id"] = _clientIdUrl, + ["client_name"] = "Claude Code", + ["redirect_uris"] = new[] { "http://localhost/callback", "http://127.0.0.1/callback" }, + ["grant_types"] = new[] { "authorization_code", "refresh_token" }, + ["response_types"] = new[] { "code" }, + ["token_endpoint_auth_method"] = "none", + }); + const string callback = "http://localhost:40489/callback"; + + // The opted-in scope and the OIDC identity scopes reach consent. + Assert.StartsWith("/consent?ticket=", await DriveAuthorizeAsync(_clientIdUrl, $"openid {ScopeName}", callback)); + Assert.StartsWith("/consent?ticket=", await DriveAuthorizeAsync(_clientIdUrl, "openid profile email offline_access", callback)); + + // A scope without the opt-in is refused — by Modgud, naming the flag. Request + // validation errors are rendered by OpenIddict as a page, not redirected. + await AssertRefusedScopeAsync(_clientIdUrl, $"openid {notOptedIn}", callback, "AllowDynamicRegistrationClients"); + + // The management selector is never a dynamic client's to ask for. + await AssertRefusedScopeAsync(_clientIdUrl, "openid modgud.management", callback, "not available to dynamically registered clients"); + + // And the whole flow completes on the default set. + var (accessToken, _) = await DriveCimdAuthCodeFlowAsync(_clientIdUrl, $"openid {ScopeName}", AllowedAudience, redirectUri: callback); + Assert.Contains(AllowedAudience, new JwtSecurityTokenHandler().ReadJwtToken(accessToken).Audiences); + + // A document that declares scope gets the intersection, never more. + var declaring = $"https://cimd-app.test/oauth/{Guid.NewGuid():N}/declaring.json"; + Factory.CimdDocuments[declaring] = BuildDocument(declaring, $"openid {ScopeName} {notOptedIn}"); + Assert.StartsWith("/consent?ticket=", await DriveAuthorizeAsync(declaring, $"openid {ScopeName}")); + await AssertRefusedScopeAsync(declaring, $"openid {notOptedIn}", RedirectUri, "AllowDynamicRegistrationClients"); + } + + private async Task AssertRefusedScopeAsync(string clientId, string scope, string redirectUri, string expectedDescriptionPart) + { + var resp = await DriveAuthorizeResponseAsync(clientId, scope, redirectUri); + var body = await resp.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(resp.StatusCode == HttpStatusCode.BadRequest, $"expected 400, got {(int)resp.StatusCode} {resp.Headers.Location}\n{body}"); + Assert.Contains("invalid_scope", body); + Assert.Contains(expectedDescriptionPart, body); + } + [Fact] public async Task Consent_surfaces_hostname_and_unverified_marker() { @@ -237,16 +297,18 @@ private async Task CreateAllowedApiAsync() Assert.False(result.IsError, DescribeErrors(result.Errors)); } - private async Task CreateAllowedScopeAsync() + private Task CreateAllowedScopeAsync() => CreateScopeAsync(ScopeName, allowDynamicClients: true); + + private async Task CreateScopeAsync(string name, bool allowDynamicClients) { using var scope = NewSystemTenantScope(); var oauthAdmin = scope.ServiceProvider.GetRequiredService(); var result = await oauthAdmin.CreateScopeAsync(new CreateOAuthScopeDto { - Name = ScopeName, - DisplayName = ScopeName, + Name = name, + DisplayName = name, Resources = new List { AllowedAudience }, - AllowDynamicRegistrationClients = true, + AllowDynamicRegistrationClients = allowDynamicClients, }, TestContext.Current.CancellationToken); Assert.False(result.IsError, DescribeErrors(result.Errors)); } @@ -372,13 +434,15 @@ private async Task DriveCimdFlowThroughToTokenAsync( /// Drives just the authorize GET; returns the redirect Location /// (or null when the response isn't a redirect). private async Task DriveAuthorizeAsync(string clientId, string scope, string? redirectUri = null) + => (await DriveAuthorizeResponseAsync(clientId, scope, redirectUri)).Headers.Location?.ToString(); + + private async Task DriveAuthorizeResponseAsync(string clientId, string scope, string? redirectUri = null) { var challenge = GeneratePkceS256Challenge(GeneratePkceVerifier()); var cookieClient = await CreateAuthenticatedClientAsync("tu", "TestPass1234"); - var resp = await cookieClient.GetAsync( + return await cookieClient.GetAsync( BuildAuthorizeUri(clientId, scope, challenge, AllowedAudience, redirectUri ?? RedirectUri), TestContext.Current.CancellationToken); - return resp.Headers.Location?.ToString(); } private async Task DiscoveryHasCimdFlagAsync() diff --git a/src/dotnet/Modgud.Api.Tests/Authorization/DcrFullFlowTests.cs b/src/dotnet/Modgud.Api.Tests/Authorization/DcrFullFlowTests.cs index 02197c8d..36fa168f 100644 --- a/src/dotnet/Modgud.Api.Tests/Authorization/DcrFullFlowTests.cs +++ b/src/dotnet/Modgud.Api.Tests/Authorization/DcrFullFlowTests.cs @@ -130,7 +130,73 @@ public async Task Loopback_redirect_with_an_ephemeral_port_is_accepted_for_a_dcr Assert.False(doc.RootElement.TryGetProperty("application_type", out _)); } - private async Task AuthorizeLocationAsync(string clientId, string redirectUri) + /// + /// RFC 7591 §2: a registration without scope gets "a default set of + /// scopes" — Modgud's is the realm's dynamic-client set (opted-in scopes plus + /// the open standard scopes), the same one a scope-less CIMD document holds. + /// It used to be the empty set, which left such a client unable to request + /// anything but openid/offline_access. A declared scope + /// is an upper bound intersected with that set, and the response echoes what + /// was actually registered (§3.2.1). + /// + [Fact] + public async Task Registration_without_scope_gets_the_realms_dynamic_client_scopes() + { + await SeedAsync(); + var ct = TestContext.Current.CancellationToken; + var http = Factory.CreateClient(); + var notOptedIn = $"dcr-not-opted-in-{Guid.NewGuid():N}"; + await CreateScopeAsync(notOptedIn, allowDynamicClients: false); + + var reg = await http.PostAsync("/connect/register", JsonContent.Create(new + { + client_name = "Scope-less MCP Client", + redirect_uris = new[] { "http://localhost/callback" }, + grant_types = new[] { "authorization_code", "refresh_token" }, + }), ct); + var regBody = await reg.Content.ReadAsStringAsync(ct); + Assert.True(reg.StatusCode == HttpStatusCode.Created, regBody); + string clientId; + string[] registered; + using (var doc = JsonDocument.Parse(regBody)) + { + clientId = doc.RootElement.GetProperty("client_id").GetString()!; + registered = doc.RootElement.GetProperty("scope").GetString()!.Split(' '); + } + Assert.Contains(ScopeName, registered); + Assert.Contains("openid", registered); + Assert.Contains("profile", registered); + Assert.Contains("email", registered); + Assert.Contains("offline_access", registered); + Assert.DoesNotContain(notOptedIn, registered); + Assert.DoesNotContain("modgud.management", registered); + Assert.Equal(registered, (await LoadStoredAsync(clientId)).Permissions + .Where(p => p.StartsWith("scp:")).Select(p => p["scp:".Length..]).ToArray()); + + Assert.StartsWith("/consent?ticket=", await AuthorizeLocationAsync(clientId, "http://localhost:43210/callback")); + var refused = await AuthorizeResponseAsync(clientId, "http://localhost:43210/callback", $"openid {notOptedIn}"); + var refusedBody = await refused.Content.ReadAsStringAsync(ct); + Assert.Equal(HttpStatusCode.BadRequest, refused.StatusCode); + Assert.Contains("invalid_scope", refusedBody); + Assert.Contains("AllowDynamicRegistrationClients", refusedBody); + + // Declared scope: the intersection is registered and echoed, nothing more. + var declaring = await http.PostAsync("/connect/register", JsonContent.Create(new + { + client_name = "Declaring MCP Client", + redirect_uris = new[] { RedirectUri }, + scope = $"openid {ScopeName} {notOptedIn} no-such-scope", + }), ct); + var declaringBody = await declaring.Content.ReadAsStringAsync(ct); + Assert.True(declaring.StatusCode == HttpStatusCode.Created, declaringBody); + using (var doc = JsonDocument.Parse(declaringBody)) + Assert.Equal(new[] { "openid", ScopeName }, doc.RootElement.GetProperty("scope").GetString()!.Split(' ')); + } + + private async Task AuthorizeLocationAsync(string clientId, string redirectUri, string? scope = null) + => (await AuthorizeResponseAsync(clientId, redirectUri, scope)).Headers.Location?.ToString(); + + private async Task AuthorizeResponseAsync(string clientId, string redirectUri, string? scope = null) { var challenge = GeneratePkceS256Challenge(GeneratePkceVerifier()); var cookieClient = await CreateAuthenticatedClientAsync("tu", "TestPass1234"); @@ -139,14 +205,13 @@ public async Task Loopback_redirect_with_an_ephemeral_port_is_accepted_for_a_dcr "response_type=code", $"client_id={Uri.EscapeDataString(clientId)}", $"redirect_uri={Uri.EscapeDataString(redirectUri)}", - $"scope={Uri.EscapeDataString($"openid {ScopeName}")}", + $"scope={Uri.EscapeDataString(scope ?? $"openid {ScopeName}")}", $"state={Guid.NewGuid():N}", $"code_challenge={challenge}", "code_challenge_method=S256", $"resource={Uri.EscapeDataString(AllowedAudience)}", }); - var resp = await cookieClient.GetAsync(uri, TestContext.Current.CancellationToken); - return resp.Headers.Location?.ToString(); + return await cookieClient.GetAsync(uri, TestContext.Current.CancellationToken); } [Fact] @@ -312,7 +377,12 @@ public async Task App_scoped_scope_without_AllowDcrClients_rejects_authorize_for { // Negative twin: the same setup but AllowDynamicRegistrationClients=false // on the scope. /connect/authorize must short-circuit with - // invalid_scope before the user ever lands on the consent screen. + // invalid_scope before the user ever lands on the consent screen. The + // scope is not even registered on the client any more (declared scopes + // are intersected with the opted-in set), so the refusal comes from + // DynamicClientScopeHandler inside OpenIddict's request validation — + // rendered as an error page like every validation failure, naming the + // scope and the flag, instead of OpenIddict's generic ID2051. await SeedAsync(); var app = await CreateAppAsync($"dcr-app-{Guid.NewGuid():N}", "DCR Test App (negative)"); var apiName = "https://dcr-appscoped-negative.example/"; @@ -339,11 +409,11 @@ public async Task App_scoped_scope_without_AllowDcrClients_rejects_authorize_for }); var resp = await cookieClient.GetAsync(authorizeUri, TestContext.Current.CancellationToken); - // The endpoint redirects back to redirect_uri with error=invalid_scope. - AssertRedirect(resp); - var loc = resp.Headers.Location!.ToString(); - Assert.Contains("error=invalid_scope", loc); - Assert.Contains("error_description=", loc); + var body = await resp.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(resp.StatusCode == HttpStatusCode.BadRequest, $"expected 400, got {(int)resp.StatusCode} {resp.Headers.Location}\n{body}"); + Assert.Contains("invalid_scope", body); + Assert.Contains(appScopedScope, body); + Assert.Contains("AllowDynamicRegistrationClients", body); } [Fact] @@ -466,17 +536,19 @@ private async Task CreateDisallowedApiAsync() /// Creates an OAuthScope whose Resources include the allowed /// API name (so principals granted this scope receive that audience) /// AND whose Properties dict carries AllowDynamicRegistrationClients=true. - private async Task CreateAllowedScopeAsync() + private Task CreateAllowedScopeAsync() => CreateScopeAsync(ScopeName, allowDynamicClients: true); + + private async Task CreateScopeAsync(string name, bool allowDynamicClients) { using var scope = NewSystemTenantScope(); var oauthAdmin = scope.ServiceProvider.GetRequiredService(); var createResult = await oauthAdmin.CreateScopeAsync(new CreateOAuthScopeDto { - Name = ScopeName, - DisplayName = ScopeName, + Name = name, + DisplayName = name, Resources = new List { AllowedAudience }, - AllowDynamicRegistrationClients = true, + AllowDynamicRegistrationClients = allowDynamicClients, }, TestContext.Current.CancellationToken); Assert.False(createResult.IsError, diff --git a/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs b/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs index b74217b2..17bf6110 100644 --- a/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs @@ -5,6 +5,7 @@ using BuildingBlocks.Helper; using Modgud.Api.Features.Auth.PositionTerminals; using Modgud.Api.Features.Auth.Staffing; +using Modgud.Application.Dcr; using Modgud.Authentication.Applications; using Modgud.Authentication.Sessions; using Modgud.Authentication.Domain; @@ -2770,6 +2771,9 @@ private static async Task IsApplicationEnabledAsync( /// (standard OIDC scopes, cross-app utility scopes). /// Scope registered with a non-null AppId → only allowed /// when the calling client's AppId matches. + /// Dynamic client (DCR / CIMD) → every registered scope must pass + /// instead: the per-scope + /// opt-in, or an open standard scope. /// /// private static async Task ValidateScopeRestrictionAsync( @@ -2784,9 +2788,12 @@ private static async Task IsApplicationEnabledAsync( .Where(s => scopeNames.Contains(s.Name) && !s.IsDeleted) .ToListAsync(cancellationToken); - // If no requested scope is app-scoped, there's nothing to restrict. + // Nothing to restrict when every requested scope is one any client may + // hold — an open standard scope — so the common `openid profile email` + // request never pays for a client lookup. App-scoped and custom global + // scopes need the client to decide. var appScoped = scopes.Where(s => s.AppId.HasValue).ToList(); - if (appScoped.Count == 0) return null; + if (scopes.All(s => DynamicClientScopePolicy.IsOpenStandardScope(s.Name))) return null; // Resolve the calling client. We need it twice: once for the // app-link set, once for the IsDynamicallyRegistered flag. @@ -2816,18 +2823,19 @@ private static async Task IsApplicationEnabledAsync( if (isDcrClient) { - var notOptedIn = appScoped.FirstOrDefault(s => - !ReadDcrFlag(s.Properties, ScopePropertyKeys.AllowDynamicRegistrationClients)); - if (notOptedIn is not null) + // Same verdict as DynamicClientScopeHandler in the OpenIddict pipeline; + // this copy also covers the consent continuation, where that pipeline + // does not run. Global custom scopes need the opt-in as much as + // app-scoped ones — the flag is the boundary, not the App link. + var refused = scopes.FirstOrDefault(s => !DynamicClientScopePolicy.IsRequestable(s)); + if (refused is not null) { return Results.Forbid( new AuthenticationProperties(new Dictionary { [OpenIddictServerAspNetCoreConstants.Properties.Error] = Errors.InvalidScope, [OpenIddictServerAspNetCoreConstants.Properties.ErrorDescription] = - $"Scope '{notOptedIn.Name}' is not opted in for Dynamic Client Registration clients. " + - "Ask the realm admin to enable AllowDynamicRegistrationClients on the scope, " + - "or use a global (cross-app) scope.", + DynamicClientScopeHandler.DescribeRefusal(refused), }), new[] { OpenIddictServerAspNetCoreDefaults.AuthenticationScheme }); } @@ -2835,8 +2843,9 @@ private static async Task IsApplicationEnabledAsync( } // Non-DCR client: app-scoped scope must intersect the client's - // own App set. (Global scopes — AppId == null — were filtered - // out above.) + // own App set. (Global scopes — AppId == null — are not restricted + // for registered clients.) + if (appScoped.Count == 0) return null; var clientAppIds = client?.AppIds.ToHashSet() ?? new HashSet(); var bad = appScoped.FirstOrDefault(s => !clientAppIds.Contains(s.AppId!.Value)); if (bad is not null) diff --git a/src/dotnet/Modgud.Api/Features/Auth/OAuth/DcrRegistrationEndpoints.cs b/src/dotnet/Modgud.Api/Features/Auth/OAuth/DcrRegistrationEndpoints.cs index 116dcf69..2cc16150 100644 --- a/src/dotnet/Modgud.Api/Features/Auth/OAuth/DcrRegistrationEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Auth/OAuth/DcrRegistrationEndpoints.cs @@ -191,7 +191,10 @@ private static async Task RegisterAsync( ClientName = created.DisplayName, ClientUri = request.ClientUri, LogoUri = request.LogoUri, - Scope = normalized.Scopes.Count == 0 ? null : string.Join(' ', normalized.Scopes), + // The REGISTERED set (RFC 7591 §3.2.1): the request's `scope` was an upper + // bound intersected with the realm's dynamic-client scopes, and a request + // without one got that whole set — the response tells the client which. + Scope = RegisteredScope(created.Permissions), Contacts = request.Contacts, TosUri = request.TosUri, PolicyUri = request.PolicyUri, @@ -200,6 +203,15 @@ private static async Task RegisterAsync( }); } + private static string? RegisteredScope(IEnumerable permissions) + { + var scopes = permissions + .Where(p => p.StartsWith(OAuthPermissions.Prefixes.Scope, StringComparison.Ordinal)) + .Select(p => p[OAuthPermissions.Prefixes.Scope.Length..]) + .ToList(); + return scopes.Count == 0 ? null : string.Join(' ', scopes); + } + private static string ResolveSourceIp(HttpContext ctx) { // CDN / reverse-proxy headers are honoured upstream by diff --git a/src/dotnet/Modgud.Application/Dcr/DynamicClientScopePolicy.cs b/src/dotnet/Modgud.Application/Dcr/DynamicClientScopePolicy.cs new file mode 100644 index 00000000..70c60266 --- /dev/null +++ b/src/dotnet/Modgud.Application/Dcr/DynamicClientScopePolicy.cs @@ -0,0 +1,88 @@ +using System.Text.Json; +using Marten; +using Modgud.Domain.OAuth.Scopes; + +namespace Modgud.Application.Dcr; + +/// +/// Which scopes a dynamic client — one minted via Dynamic Client +/// Registration or synthesized from a CIMD document — may hold and request. +/// The one rule behind three seams: the CIMD resolver (permissions on the +/// synthesized client), the DCR registration (permissions on the stored +/// client) and the authorize-time check. +/// +/// A scope is requestable by dynamic clients when it is enabled and +/// either carries +/// — the admin's per-scope opt-in, the flag's whole meaning — or is one of the +/// standard scopes other than modgud.management. The standard scopes +/// cannot carry the flag (they are immutable), so they need a built-in +/// verdict: the identity and claim-gate scopes name no resource privilege of +/// their own, an unverified client may ask for them; the management selector +/// names the IdP's own management API and is exactly the "tenant.admin.*" +/// case the flag exists to keep away from a habit-click consent (the realm +/// seeder strips the flag from it for the same reason). +/// +/// The client's own scope declaration is an upper bound, never +/// a grant: declared scopes are intersected with this set, and a client that +/// declares none gets the whole set (RFC 7591 §2 — "a default set of +/// scopes"). A static CIMD document published for every MCP server in the +/// world cannot know one server's scopes; the set makes such a client work +/// without letting it invent permissions. +/// +public static class DynamicClientScopePolicy +{ + /// The scope may be held and requested by dynamic clients. A + /// standard scope is judged by the built-in verdict alone — a flag that + /// found its way onto one (the seeder strips it) changes nothing. + public static bool IsRequestable(OAuthScopeState scope) + => scope is { IsDeleted: false, Enabled: true } + && (StandardScopes.IsStandard(scope.Name) ? IsOpenStandardScope(scope.Name) : HasOptIn(scope.Properties)); + + /// A standard scope that dynamic clients may always request — + /// every standard scope except the management-API selector. + public static bool IsOpenStandardScope(string? name) + => StandardScopes.IsStandard(name) && name != StandardScopes.Management; + + /// Reads the per-scope opt-in flag (plain bool or JsonElement, + /// depending on which serializer wrote the document). + public static bool HasOptIn(IDictionary properties) + { + if (!properties.TryGetValue(ScopePropertyKeys.AllowDynamicRegistrationClients, out var raw) || raw is null) + return false; + return raw switch + { + bool b => b, + JsonElement e when e.ValueKind is JsonValueKind.True => true, + _ => false, + }; + } + + /// + /// The scope names a dynamic client ends up holding: the declared names + /// intersected with the requestable set, or the whole requestable set when + /// nothing was declared. Order is preserved from + /// so the result is stable across resolves. + /// + public static List Resolve(IReadOnlyCollection declared, IReadOnlyList requestable) + { + if (declared.Count == 0) return requestable.ToList(); + var wanted = declared.ToHashSet(StringComparer.Ordinal); + return requestable.Where(wanted.Contains).ToList(); + } + + /// + /// Loads the requestable scope names of the current realm: standard scopes + /// first in their canonical order, then the opted-in ones by name. + /// + public static async Task> LoadRequestableNamesAsync(IQuerySession session, CancellationToken ct) + { + var scopes = await session.Query() + .Where(s => !s.IsDeleted && s.Enabled) + .ToListAsync(ct); + var requestable = scopes.Where(IsRequestable).Select(s => s.Name).ToHashSet(StringComparer.Ordinal); + + var ordered = StandardScopes.All.Where(requestable.Contains).ToList(); + ordered.AddRange(requestable.Except(StandardScopes.All, StringComparer.Ordinal).Order(StringComparer.Ordinal)); + return ordered; + } +} diff --git a/src/dotnet/Modgud.Application/Services/OAuthAdminService.cs b/src/dotnet/Modgud.Application/Services/OAuthAdminService.cs index 4b3afe40..4105184d 100644 --- a/src/dotnet/Modgud.Application/Services/OAuthAdminService.cs +++ b/src/dotnet/Modgud.Application/Services/OAuthAdminService.cs @@ -237,8 +237,17 @@ public async Task> CreateClientAsync( clientSecret = dto.ClientSecret ?? (createJwks is null ? GenerateSecret() : null); } + // A dynamically registered client holds what the realm lets dynamic clients + // hold: its own `scope` is an upper bound intersected with the opted-in set, + // and a registration without one gets the whole set (RFC 7591 §2 — "a + // default set of scopes"). Registering the declared list verbatim used to + // leave a scope-less registration with no scope permission at all. + var scopes = dcrMetadata is null + ? dto.Scopes + : DynamicClientScopePolicy.Resolve(dto.Scopes, await DynamicClientScopePolicy.LoadRequestableNamesAsync(_session, ct)); + // Build permissions list (endpoints + grant types + scopes). - var permissions = BuildClientPermissions(dto.AllowedGrantTypes, dto.Scopes, dto.ClientType, dto.Capabilities); + var permissions = BuildClientPermissions(dto.AllowedGrantTypes, scopes, dto.ClientType, dto.Capabilities); var pinnedClientId = await PinnedEntityId.ResolveAsync( _session, dto.Id, "OAuthClient", s => s.IsDeleted, ct); diff --git a/src/dotnet/Modgud.Infrastructure/OpenIddict/Cimd/CimdClientResolver.cs b/src/dotnet/Modgud.Infrastructure/OpenIddict/Cimd/CimdClientResolver.cs index af6da8ab..b98ab442 100644 --- a/src/dotnet/Modgud.Infrastructure/OpenIddict/Cimd/CimdClientResolver.cs +++ b/src/dotnet/Modgud.Infrastructure/OpenIddict/Cimd/CimdClientResolver.cs @@ -3,6 +3,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.Json; +using Modgud.Application.Dcr; using Modgud.Domain.Applications; using Modgud.Domain.OAuth.Applications; using Modgud.Domain.OAuth.Common; @@ -98,6 +99,7 @@ public CimdClientResolver( // session (NOT the resolver) to preserve this resolver's no-eager-tenant- // session design (it must not throw on a realm without a physical DB). CimdSettings? settings; + IReadOnlyList requestableScopes; await using (var session = _sessionFactory.OpenQuerySession()) { var realmCimd = (await session.LoadAsync(RealmSettingsDoc.SingletonId, cancellationToken))?.Cimd; @@ -117,12 +119,19 @@ public CimdClientResolver( { settings = realmCimd; } + if (settings is null || !settings.Enabled) return null; + + // The scope permissions are NOT part of the cached document: they are + // the realm's live opt-in set (DynamicClientScopePolicy) intersected with + // whatever the document declares, recomputed on every resolve so a scope + // opted in or out by an admin takes effect at once — same liveness rule + // as the Enabled check above. The document itself stays memoized. + requestableScopes = await LoadRequestableScopesAsync(session, cancellationToken); } - if (settings is null || !settings.Enabled) return null; var cacheKey = $"cimd:doc:{TenantContext.Current}:{clientId}"; if (_cache.TryGetValue(cacheKey, out var cached) && cached is not null) - return Synthesize(cached); + return Synthesize(cached, requestableScopes); if (!CimdClientId.TryValidateUrl(clientId, out var uri, out var urlError) || uri is null) { @@ -137,7 +146,26 @@ public CimdClientResolver( if (ttl > TimeSpan.Zero) _cache.Set(cacheKey, entry, ttl); - return Synthesize(entry); + return Synthesize(entry, requestableScopes); + } + + private const string RequestableScopesItemKey = "Modgud.Cimd.RequestableScopes"; + + /// + /// One realm-wide scope query per request, not per resolve: the store, the + /// audience-containment handler and the authorize endpoint each resolve the + /// same client once per request, so the set is memoized on HttpContext.Items + /// (a background resolve without a request context simply queries). + /// + private async Task> LoadRequestableScopesAsync(IQuerySession session, CancellationToken cancellationToken) + { + var items = _httpContextAccessor.HttpContext?.Items; + if (items is not null && items.TryGetValue(RequestableScopesItemKey, out var memo) && memo is IReadOnlyList known) + return known; + + var loaded = await DynamicClientScopePolicy.LoadRequestableNamesAsync(session, cancellationToken); + if (items is not null) items[RequestableScopesItemKey] = loaded; + return loaded; } private async Task<(CimdMetadata? Metadata, TimeSpan Ttl)> FetchAndValidateAsync( @@ -223,7 +251,7 @@ private static TimeSpan ResolveTtl(CacheControlHeaderValue? cacheControl) return ttl; } - private static OAuthApplicationState Synthesize(CachedCimd entry) + private static OAuthApplicationState Synthesize(CachedCimd entry, IReadOnlyList requestableScopes) { var meta = entry.Metadata; return new OAuthApplicationState @@ -245,7 +273,7 @@ private static OAuthApplicationState Synthesize(CachedCimd entry) ?? OAuthApplicationTypes.Web, RedirectUris = meta.RedirectUris.ToList(), PostLogoutRedirectUris = new List(), - Permissions = BuildPermissions(meta), + Permissions = BuildPermissions(meta, requestableScopes), Requirements = new List(), // global RequireProofKeyForCodeExchange enforces PKCE Settings = new Dictionary { @@ -273,7 +301,7 @@ private static OAuthApplicationState Synthesize(CachedCimd entry) }; } - private static List BuildPermissions(CimdMetadata meta) + private static List BuildPermissions(CimdMetadata meta, IReadOnlyList requestableScopes) { var permissions = new List { @@ -298,7 +326,13 @@ private static List BuildPermissions(CimdMetadata meta) // authorization_code is guaranteed present (the parser requires it). permissions.Add(OAuthPermissions.ResponseTypes.Code); - foreach (var scope in meta.Scopes) + // The document's `scope` is an upper bound, not a grant; a document without + // one (every static MCP-client document — it cannot know one server's + // scopes) holds the realm's whole dynamic-client set. Granting only the + // declared scopes used to leave such a client with no scope permission at + // all, so OpenIddict refused everything but openid/offline_access (ID2051) + // before the per-scope opt-in was ever consulted. + foreach (var scope in DynamicClientScopePolicy.Resolve(meta.Scopes, requestableScopes)) permissions.Add(OAuthPermissions.Prefixes.Scope + scope); return permissions; diff --git a/src/dotnet/Modgud.Infrastructure/OpenIddict/DynamicClientScopeHandler.cs b/src/dotnet/Modgud.Infrastructure/OpenIddict/DynamicClientScopeHandler.cs new file mode 100644 index 00000000..b363066e --- /dev/null +++ b/src/dotnet/Modgud.Infrastructure/OpenIddict/DynamicClientScopeHandler.cs @@ -0,0 +1,100 @@ +using System.Text.Json; +using Marten; +using Modgud.Application.Dcr; +using Modgud.Domain.OAuth.Applications; +using Modgud.Domain.OAuth.Scopes; +using OpenIddict.Abstractions; +using OpenIddict.Server; +using static OpenIddict.Abstractions.OpenIddictConstants; +using static OpenIddict.Server.OpenIddictServerEvents; + +namespace Modgud.Infrastructure.OpenIddict; + +/// +/// Authorize-time half of : a dynamic +/// client (DCR-minted or CIMD-synthesized) that requests a scope the realm has +/// not opted in for dynamic clients is refused with invalid_scope and a +/// description that names the scope and the flag to flip. +/// +/// Ordered just BEFORE OpenIddict's ValidateScopePermissions on +/// purpose. A dynamic client's permissions are derived from the same policy, +/// so a scope that fails here also has no scp: permission — and +/// OpenIddict's check would reject it first with the generic ID2051 ("not +/// allowed to use the specified scope"), which tells an admin nothing about +/// AllowDynamicRegistrationClients. Pre-compute, not override: the +/// reject stops the pipeline, so Order - 1 is the right side. +/// +/// Runs after OpenIddict's ValidateScopes, so an unregistered +/// scope name still gets OpenIddict's own error; this handler only judges +/// scopes the realm knows. +/// +public sealed class DynamicClientScopeHandler : IOpenIddictServerHandler +{ + public static OpenIddictServerHandlerDescriptor Descriptor { get; } + = OpenIddictServerHandlerDescriptor.CreateBuilder() + .UseScopedHandler() + .SetOrder(OpenIddictServerHandlers.Authentication.ValidateScopePermissions.Descriptor.Order - 1) + .SetType(OpenIddictServerHandlerType.Custom) + .Build(); + + private readonly IDocumentSession _session; + private readonly IOpenIddictApplicationManager _applications; + + public DynamicClientScopeHandler(IDocumentSession session, IOpenIddictApplicationManager applications) + { + _session = session; + _applications = applications; + } + + public async ValueTask HandleAsync(ValidateAuthorizationRequestContext context) + { + var clientId = context.ClientId; + if (string.IsNullOrEmpty(clientId)) return; + + var requested = context.Request.GetScopes(); + if (requested.Length == 0) return; + + // Through the manager, not a direct query: ValidateClientId just loaded this + // client, so this is a cache hit for every registered client, and the store + // already falls back to the CIMD resolver for a synthesized one. + if (await _applications.FindByClientIdAsync(clientId, context.CancellationToken) is not OAuthApplicationState application + || !IsDynamicClient(application.Properties)) + return; + + var names = requested.ToArray(); + var scopes = await _session.Query() + .Where(s => names.Contains(s.Name) && !s.IsDeleted) + .ToListAsync(context.CancellationToken); + + var refused = scopes.FirstOrDefault(s => !DynamicClientScopePolicy.IsRequestable(s)); + if (refused is null) return; + + context.Reject( + error: Errors.InvalidScope, + description: DescribeRefusal(refused)); + } + + /// The same wording the authorize endpoint's own check uses, so an + /// agent sees one message whichever layer catches it. + public static string DescribeRefusal(OAuthScopeState scope) + { + if (!scope.Enabled) + return $"Scope '{scope.Name}' is disabled on this realm."; + if (StandardScopes.IsStandard(scope.Name)) + return $"Scope '{scope.Name}' is not available to dynamically registered clients."; + return $"Scope '{scope.Name}' is not opted in for Dynamic Client Registration clients. " + + "Ask the realm admin to enable AllowDynamicRegistrationClients on the scope."; + } + + private static bool IsDynamicClient(IDictionary props) + { + if (!props.TryGetValue(OAuthApplicationPropertyKeys.DcrIsDynamicallyRegistered, out var raw) || raw is null) + return false; + return raw switch + { + bool b => b, + JsonElement e when e.ValueKind is JsonValueKind.True => true, + _ => false, + }; + } +} diff --git a/src/dotnet/Modgud.Infrastructure/OpenIddict/OpenIddictExtensions.cs b/src/dotnet/Modgud.Infrastructure/OpenIddict/OpenIddictExtensions.cs index 959de56c..61af4a10 100644 --- a/src/dotnet/Modgud.Infrastructure/OpenIddict/OpenIddictExtensions.cs +++ b/src/dotnet/Modgud.Infrastructure/OpenIddict/OpenIddictExtensions.cs @@ -356,6 +356,7 @@ public static IServiceCollection AddOpenIddictWithMarten( options.AddEventHandler(AccessTokenTypeHandler.Descriptor); options.AddEventHandler(TokenMintMetricHandler.Descriptor); options.AddEventHandler(ResourceIndicatorHandler.Descriptor); + options.AddEventHandler(DynamicClientScopeHandler.Descriptor); options.AddEventHandler(DcrAudienceContainmentHandler.Descriptor); options.AddEventHandler(DcrLastUsedTrackerHandler.Descriptor); options.AddEventHandler(RealmSigningKeyHandler.Descriptor); diff --git a/src/dotnet/Modgud.Tests.Unit/OAuth/Dcr/DynamicClientScopePolicyTests.cs b/src/dotnet/Modgud.Tests.Unit/OAuth/Dcr/DynamicClientScopePolicyTests.cs new file mode 100644 index 00000000..0eff6b21 --- /dev/null +++ b/src/dotnet/Modgud.Tests.Unit/OAuth/Dcr/DynamicClientScopePolicyTests.cs @@ -0,0 +1,104 @@ +using System.Text.Json; +using Modgud.Application.Dcr; +using Modgud.Domain.OAuth.Scopes; + +namespace Modgud.Tests.Unit.OAuth.Dcr; + +/// +/// Pins the one rule behind the CIMD permissions, the DCR registration and the +/// authorize-time check: which scopes a dynamic client may hold, and how a +/// declared scope is reconciled with that set. +/// +public class DynamicClientScopePolicyTests +{ + [Theory] + [InlineData("openid")] + [InlineData("profile")] + [InlineData("email")] + [InlineData("phone")] + [InlineData("address")] + [InlineData("offline_access")] + [InlineData("roles")] + [InlineData("permissions")] + public void Every_standard_scope_but_the_management_selector_is_open(string name) + { + Assert.True(DynamicClientScopePolicy.IsOpenStandardScope(name)); + Assert.True(DynamicClientScopePolicy.IsRequestable(Scope(name))); + } + + [Fact] + public void The_management_selector_is_never_requestable_by_dynamic_clients() + { + Assert.False(DynamicClientScopePolicy.IsOpenStandardScope(StandardScopes.Management)); + Assert.False(DynamicClientScopePolicy.IsRequestable(Scope(StandardScopes.Management))); + // Not even with the flag — the seeder strips it, the policy ignores it. + Assert.False(DynamicClientScopePolicy.IsRequestable(Scope(StandardScopes.Management, optIn: true))); + } + + [Fact] + public void A_custom_scope_needs_the_opt_in_whether_app_scoped_or_global() + { + Assert.False(DynamicClientScopePolicy.IsRequestable(Scope("lists.read"))); + Assert.True(DynamicClientScopePolicy.IsRequestable(Scope("lists.read", optIn: true))); + Assert.False(DynamicClientScopePolicy.IsRequestable(Scope("lists.read", appId: Guid.NewGuid()))); + Assert.True(DynamicClientScopePolicy.IsRequestable(Scope("lists.read", optIn: true, appId: Guid.NewGuid()))); + } + + [Fact] + public void The_opt_in_reads_as_plain_bool_or_json_element() + { + var plain = Scope("a"); + plain.Properties[ScopePropertyKeys.AllowDynamicRegistrationClients] = true; + Assert.True(DynamicClientScopePolicy.HasOptIn(plain.Properties)); + + var element = Scope("b"); + element.Properties[ScopePropertyKeys.AllowDynamicRegistrationClients] = JsonSerializer.SerializeToElement(true); + Assert.True(DynamicClientScopePolicy.HasOptIn(element.Properties)); + + var off = Scope("c"); + off.Properties[ScopePropertyKeys.AllowDynamicRegistrationClients] = JsonSerializer.SerializeToElement(false); + Assert.False(DynamicClientScopePolicy.HasOptIn(off.Properties)); + Assert.False(DynamicClientScopePolicy.HasOptIn(new Dictionary())); + } + + [Fact] + public void A_disabled_or_deleted_scope_is_not_requestable_even_with_the_opt_in() + { + var disabled = Scope("x", optIn: true); + disabled.Enabled = false; + Assert.False(DynamicClientScopePolicy.IsRequestable(disabled)); + + var deleted = Scope("y", optIn: true); + deleted.IsDeleted = true; + Assert.False(DynamicClientScopePolicy.IsRequestable(deleted)); + } + + [Fact] + public void Nothing_declared_means_the_whole_requestable_set() + { + var requestable = new[] { "openid", "profile", "lists.read" }; + Assert.Equal(requestable, DynamicClientScopePolicy.Resolve(Array.Empty(), requestable)); + } + + [Fact] + public void A_declaration_is_an_upper_bound_intersected_in_requestable_order() + { + var requestable = new[] { "openid", "profile", "lists.read", "lists.write" }; + var declared = new[] { "lists.write", "admin", "openid", "no-such-scope" }; + Assert.Equal(new[] { "openid", "lists.write" }, DynamicClientScopePolicy.Resolve(declared, requestable)); + } + + [Fact] + public void A_declaration_with_nothing_requestable_in_it_yields_the_empty_set() + { + Assert.Empty(DynamicClientScopePolicy.Resolve(new[] { "admin" }, new[] { "openid", "lists.read" })); + } + + private static OAuthScopeState Scope(string name, bool optIn = false, Guid? appId = null) + { + var scope = new OAuthScopeState { Id = Guid.NewGuid(), Name = name, AppId = appId }; + if (optIn) + scope.Properties[ScopePropertyKeys.AllowDynamicRegistrationClients] = JsonSerializer.SerializeToElement(true); + return scope; + } +}