WIP

Author: cicnavi

docs/6-oidc-upgrade.md

@@ -149,12 +149,20 @@ per-field metadata policy (honored / validated / rejected) is documented in
   `response_types`, and `token_endpoint_auth_method` is presence-based, so existing
   clients (which do not have these properties set) are **not** restricted and
   require no action — their behavior is unchanged. The OIDC DCR defaults are
-  applied only to newly, dynamically registered clients, not retroactively. If you
-  want to start constraining an existing client to specific grant types / response
-  types / authentication method, set those properties on the client in the admin
-  UI; leaving them empty preserves the previous unconstrained behavior. DCR is also
-  opt-in (disabled by default), so unless you enable it, nothing changes for your
-  deployment.
+  applied only to newly, dynamically registered clients, not retroactively. In the
+  admin UI, a pre-upgrade client shows these new fields as **unset** (no grant
+  types / response types selected, no authentication method chosen) — that is the
+  honest representation of "not registered", and saving such a client does **not**
+  silently impose the spec defaults or otherwise constrain it. To start constraining
+  an existing client to specific grant types / response types / authentication
+  method, select them explicitly and save; leaving them empty preserves the
+  unconstrained behavior. (Implementation note: in v7 the client getters for these
+  fields return the raw registered value — empty / null when unset — rather than the
+  OIDC DCR spec default, so the stored value is the single source of truth; the
+  defaults are applied where it matters, i.e. when a client registers dynamically. A
+  future major version may switch the getters to fall back to the spec defaults.)
+  DCR is also opt-in (disabled by default), so unless you enable it, nothing changes
+  for your deployment.
 
 New configuration options:
 

docs/9-oidc-dcr-client-metadata.md

@@ -32,9 +32,9 @@ contract for what was honored.
 | `redirect_uris` | Honored | Honored | Required; scheme required, fragment rejected. |
 | `client_name` | Honored | Honored | Defaults to client_id. |
 | `scope` | Honored | Honored | DCR default = `OPTION_DCR_DEFAULT_SCOPES`. |
-| `grant_types` | **Honored** (persist + echo + enforce) | Honored | Default `["authorization_code"]`. Enforced for the code grant; refresh grant exempt (see note). |
-| `response_types` | **Honored** (persist + echo + enforce) | Honored | Default `["code"]`. Enforced at the authorization endpoint. |
-| `token_endpoint_auth_method` | **Honored** (persist + echo + enforce) | Honored | Default `client_secret_basic` (or `none` for public). Enforced at the token endpoint. |
+| `grant_types` | **Honored** (persist + echo + enforce) | Honored | DCR default `["authorization_code"]` stored at registration. Enforced for the code grant (presence + non-empty); refresh grant exempt (see note). |
+| `response_types` | **Honored** (persist + echo + enforce) | Honored | DCR default `["code"]` stored at registration. Enforced at the authorization endpoint (presence + non-empty). |
+| `token_endpoint_auth_method` | **Honored** (persist + echo + enforce) | Honored | DCR default `client_secret_basic` (or `none` for public) stored at registration. Enforced at the token endpoint (presence). |
 | `jwks` | Honored | Honored | Stored (column). |
 | `jwks_uri` | Honored | Honored | Stored (column); fetched for client auth / request objects. |
 | `signed_jwks_uri` | Honored | Honored | Stored (column). |
@@ -70,12 +70,34 @@ contract for what was honored.
 
 Per-client enforcement of `grant_types` / `response_types` /
 `token_endpoint_auth_method` is **presence-based**: a field is enforced for a
-client only when that client has it explicitly registered. Dynamically registered
-clients always do (the OIDC DCR defaults are applied at registration); clients
-that do not have it configured are not constrained. This avoids regressing
-manually-managed clients while still honoring the registered metadata. All client
-metadata is stored in the existing `extra_metadata` JSON column (no DB migration),
-and is exposed as editable fields in the admin UI.
+client only when that client has it explicitly registered. For the array-valued
+fields (`grant_types`, `response_types`) a present-but-**empty** list also counts as
+"not configured" and is not enforced (it never means "allow nothing"; this matches
+the admin form's "if none are selected, the client is not restricted"). Dynamically
+registered clients always have these stored (the OIDC DCR defaults are applied at
+registration); clients that do not have them configured are not constrained. This
+avoids regressing manually-managed and pre-DCR clients while still honoring the
+registered metadata. All client metadata is stored in the existing `extra_metadata`
+JSON column (no DB migration), and is exposed as editable fields in the admin UI.
+
+### Single source of truth (v7 transition)
+
+The entity getters for these fields (`getGrantTypes()`, `getResponseTypes()`,
+`getTokenEndpointAuthMethod()`) return the **raw registered value** — an empty array
+/ `null` when the client has nothing registered — rather than synthesizing the OIDC
+DCR spec default. This keeps the stored value the single source of truth, so:
+
+- the admin UI shows exactly what is registered (a pre-DCR client shows these fields
+  as unset, not as phantom defaults), and saving such a client does not silently
+  impose constraints;
+- the registration response still echoes the spec defaults, because for dynamic
+  registrations those defaults are persisted at registration time (in
+  `ClientEntityFactory`), not invented at read time.
+
+A future major version may move the spec defaults into the getters themselves (so an
+unset value resolves to the spec default everywhere). v7 deliberately does not, to
+give deployments a transition window in which to set explicit values on clients that
+predate these properties.
 
 ## Implementation order
 

src/Entities/ClientEntity.php

@@ -23,10 +23,7 @@
 use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface;
 use SimpleSAML\OpenID\Codebooks\ClaimsEnum;
 use SimpleSAML\OpenID\Codebooks\ClientRegistrationTypesEnum;
-use SimpleSAML\OpenID\Codebooks\GrantTypesEnum;
 use SimpleSAML\OpenID\Codebooks\ResponseModesEnum;
-use SimpleSAML\OpenID\Codebooks\ResponseTypesEnum;
-use SimpleSAML\OpenID\Codebooks\TokenEndpointAuthMethodsEnum;
 
 class ClientEntity implements ClientEntityInterface
 {
@@ -539,8 +536,14 @@ public function getRequestUris(): array
     }
 
     /**
-     * The OAuth 2.0 grant types the client is registered to use. Defaults to ["authorization_code"]
-     * (OpenID Connect Dynamic Client Registration 1.0 default) when not explicitly registered.
+     * The OAuth 2.0 grant types the client is registered to use, or an empty array when none are registered.
+     *
+     * v7 transition: this returns the raw registered value (empty when unset) rather than synthesizing the
+     * OpenID Connect Dynamic Client Registration 1.0 default (["authorization_code"]). This keeps the stored
+     * value the single source of truth, so the admin UI and the registration echo reflect exactly what is
+     * registered, and clients created before this property existed are not retroactively constrained. The DCR
+     * default is still applied at registration time for dynamic clients (see ClientEntityFactory). A future
+     * major version may switch this getter to return the spec default when unset.
      *
      * @return string[]
      */
@@ -551,15 +554,17 @@ public function getGrantTypes(): array
         ($this->extraMetadata[ClaimsEnum::GrantTypes->value] ?? null) : null;
 
         if (!is_array($grantTypes)) {
-            return [GrantTypesEnum::AuthorizationCode->value];
+            return [];
         }
 
         return array_values(array_filter($grantTypes, 'is_string'));
     }
 
     /**
-     * The OAuth 2.0 response types the client is registered to use. Defaults to ["code"]
-     * (OpenID Connect Dynamic Client Registration 1.0 default) when not explicitly registered.
+     * The OAuth 2.0 response types the client is registered to use, or an empty array when none are registered.
+     *
+     * v7 transition: returns the raw registered value (empty when unset) rather than synthesizing the OpenID
+     * Connect Dynamic Client Registration 1.0 default (["code"]). See getGrantTypes() for the rationale.
      *
      * @return string[]
      */
@@ -570,18 +575,21 @@ public function getResponseTypes(): array
         ($this->extraMetadata[ClaimsEnum::ResponseTypes->value] ?? null) : null;
 
         if (!is_array($responseTypes)) {
-            return [ResponseTypesEnum::Code->value];
+            return [];
         }
 
         return array_values(array_filter($responseTypes, 'is_string'));
     }
 
     /**
-     * The client authentication method the client is registered to use at the token endpoint. Defaults to
-     * 'client_secret_basic' for confidential clients and 'none' for public clients when not explicitly registered
-     * (OpenID Connect Dynamic Client Registration 1.0).
+     * The client authentication method the client is registered to use at the token endpoint, or null when none
+     * is registered.
+     *
+     * v7 transition: returns the raw registered value (null when unset) rather than synthesizing the OpenID
+     * Connect Dynamic Client Registration 1.0 default ('client_secret_basic' / 'none'). See getGrantTypes() for
+     * the rationale.
      */
-    public function getTokenEndpointAuthMethod(): string
+    public function getTokenEndpointAuthMethod(): ?string
     {
         /** @var mixed $method */
         $method = is_array($this->extraMetadata) ?
@@ -591,9 +599,7 @@ public function getTokenEndpointAuthMethod(): string
             return $method;
         }
 
-        return $this->isConfidential() ?
-        TokenEndpointAuthMethodsEnum::ClientSecretBasic->value :
-        TokenEndpointAuthMethodsEnum::None->value;
+        return null;
     }
 
     /**
@@ -604,7 +610,7 @@ public function getDefaultMaxAge(): ?int
     {
         /** @var mixed $value */
         $value = is_array($this->extraMetadata) ?
-            ($this->extraMetadata[ClaimsEnum::DefaultMaxAge->value] ?? null) : null;
+        ($this->extraMetadata[ClaimsEnum::DefaultMaxAge->value] ?? null) : null;
 
         if (is_int($value) || is_string($value)) {
             $filtered = filter_var($value, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]);
@@ -621,7 +627,7 @@ public function getRequireAuthTime(): bool
     {
         /** @var mixed $value */
         $value = is_array($this->extraMetadata) ?
-            ($this->extraMetadata[ClaimsEnum::RequireAuthTime->value] ?? null) : null;
+        ($this->extraMetadata[ClaimsEnum::RequireAuthTime->value] ?? null) : null;
 
         return filter_var($value, FILTER_VALIDATE_BOOLEAN);
     }

src/Entities/Interfaces/ClientEntityInterface.php

@@ -102,7 +102,7 @@ public function getGrantTypes(): array;
      */
     public function getResponseTypes(): array;
 
-    public function getTokenEndpointAuthMethod(): string;
+    public function getTokenEndpointAuthMethod(): ?string;
 
     public function getDefaultMaxAge(): ?int;
 

src/Server/Grants/AuthCodeGrant.php

@@ -528,15 +528,17 @@ public function respondToAccessTokenRequest(
         // it is predefined as the ClientRule result and authenticated against by ClientAuthenticationRule above.
         $client = $authorizationClientEntity;
 
-        // Per-client grant_types enforcement: if the client explicitly registered grant_types, it must include
-        // 'authorization_code' to exchange a code here. Enforced only when explicitly registered, preserving
-        // behavior for manually-managed clients that do not have it configured. The refresh_token grant is
-        // intentionally NOT gated on grant_types (see RefreshTokenGrant): a refresh token is only issued when
-        // offline_access was granted and consented, which is itself the authorization to refresh.
+        // Per-client grant_types enforcement: if the client explicitly registered a non-empty grant_types list, it
+        // must include 'authorization_code' to exchange a code here. Enforced only when explicitly registered
+        // (present and non-empty), preserving behavior for manually-managed and pre-DCR clients that do not have it
+        // configured - or have it as an empty list. The refresh_token grant is intentionally NOT gated on
+        // grant_types (see RefreshTokenGrant): a refresh token is only issued when offline_access was granted and
+        // consented, which is itself the authorization to refresh.
         /** @var mixed $registeredGrantTypes */
         $registeredGrantTypes = $client->getExtraMetadata()[ClaimsEnum::GrantTypes->value] ?? null;
         if (
             is_array($registeredGrantTypes) &&
+            $registeredGrantTypes !== [] &&
             !in_array(GrantTypesEnum::AuthorizationCode->value, $registeredGrantTypes, true)
         ) {
             throw OidcServerException::unauthorizedClient(

src/Server/RequestRules/Rules/ResponseTypeRule.php

@@ -57,17 +57,21 @@ public function checkRule(
 
         $responseType = (string)$requestParams[ParamsEnum::ResponseType->value];
 
-        // Per-client enforcement: if the client has explicitly registered response_types, the requested
-        // response_type must be one of them. We enforce only when the value was explicitly registered (present in
-        // the client's metadata); clients that do not have it configured are not constrained, preserving behavior
-        // for manually-managed clients. Dynamically registered clients always have it (the OIDC DCR default is
-        // applied at registration).
+        // Per-client enforcement: if the client has explicitly registered a non-empty response_types list, the
+        // requested response_type must be one of them. We enforce only when the value was explicitly registered
+        // (present and non-empty in the client's metadata); clients that do not have it configured - or have it as
+        // an empty list - are not constrained, preserving behavior for manually-managed and pre-DCR clients.
+        // Dynamically registered clients always have it (the OIDC DCR default is applied at registration).
         $client = $currentResultBag->getOrFail(ClientRule::class)->getValue();
         /** @var mixed $registeredResponseTypes */
         $registeredResponseTypes = ($client instanceof ClientEntityInterface) ?
         ($client->getExtraMetadata()[ClaimsEnum::ResponseTypes->value] ?? null) : null;
 
-        if (is_array($registeredResponseTypes) && !in_array($responseType, $registeredResponseTypes, true)) {
+        if (
+            is_array($registeredResponseTypes) &&
+            $registeredResponseTypes !== [] &&
+            !in_array($responseType, $registeredResponseTypes, true)
+        ) {
             $loggerService->error(
                 'ResponseTypeRule: response_type not registered for client.',
                 ['response_type' => $responseType, 'registered' => $registeredResponseTypes],

tests/unit/src/Entities/ClientEntityTest.php

@@ -230,13 +230,9 @@ public function testCanExportAsArray(): void
                 'require_pushed_authorization_requests' => false,
                 'require_signed_request_object' => false,
                 'request_uris' => [],
-                'grant_types' => [
-                    'authorization_code',
-                ],
-                'response_types' => [
-                    'code',
-                ],
-                'token_endpoint_auth_method' => 'none',
+                'grant_types' => [],
+                'response_types' => [],
+                'token_endpoint_auth_method' => null,
                 'default_max_age' => null,
                 'require_auth_time' => false,
                 'default_acr_values' => [],
@@ -298,4 +294,50 @@ public function testCanGetAuthProcFilters(): void
         $this->assertSame($authProcFilters, $clientEntity->getAuthProcFilters());
         $this->assertSame($authProcFilters, $clientEntity->toArray()[ClientEntity::KEY_AUTH_PROC_FILTERS]);
     }
+
+    public function testEnforcementGettersReturnRawRegisteredValues(): void
+    {
+        // v7 transition: when not registered, these getters return the raw "unset" value (empty / null) rather
+        // than synthesizing the OIDC DCR spec defaults, so the stored value stays the single source of truth and
+        // pre-DCR clients are not retroactively constrained.
+        $unset = $this->mock();
+        $this->assertSame([], $unset->getGrantTypes());
+        $this->assertSame([], $unset->getResponseTypes());
+        $this->assertNull($unset->getTokenEndpointAuthMethod());
+
+        // When registered, the stored values are returned.
+        $registered = new ClientEntity(
+            $this->id,
+            $this->secret,
+            $this->name,
+            $this->description,
+            $this->redirectUri,
+            $this->scopes,
+            $this->isEnabled,
+            $this->isConfidential,
+            $this->authSource,
+            $this->owner,
+            $this->postLogoutRedirectUri,
+            $this->backChannelLogoutUri,
+            $this->entityIdentifier,
+            $this->clientRegistrationTypes,
+            $this->federationJwks,
+            $this->jwks,
+            $this->jwksUri,
+            $this->signedJwksUri,
+            $this->registrationType,
+            $this->updatedAt,
+            $this->createdAt,
+            $this->expiresAt,
+            $this->isGeneric,
+            [
+                'grant_types' => ['authorization_code', 'refresh_token'],
+                'response_types' => ['code'],
+                'token_endpoint_auth_method' => 'private_key_jwt',
+            ],
+        );
+        $this->assertSame(['authorization_code', 'refresh_token'], $registered->getGrantTypes());
+        $this->assertSame(['code'], $registered->getResponseTypes());
+        $this->assertSame('private_key_jwt', $registered->getTokenEndpointAuthMethod());
+    }
 }

tests/unit/src/Server/RequestRules/Rules/ResponseTypeRuleTest.php

@@ -138,6 +138,30 @@ public function testRejectsResponseTypeNotRegisteredForClient(): void
         );
     }
 
+    public function testEmptyRegisteredResponseTypesIsNotEnforced(): void
+    {
+        // A present-but-empty response_types list means "not configured / unconstrained", not "allow nothing".
+        // This preserves behavior for pre-DCR clients that get an empty list persisted on an admin save.
+        $client = $this->createStub(ClientEntityInterface::class);
+        $client->method('getExtraMetadata')->willReturn(['response_types' => []]);
+
+        $bag = new ResultBag();
+        $bag->add(new Result(ClientRule::class, $client));
+
+        $this->requestParams['response_type'] = 'id_token';
+        $this->requestParamsResolverStub->method('getAllBasedOnAllowedMethods')->willReturn($this->requestParams);
+
+        $result = $this->sut()->checkRule(
+            $this->requestStub,
+            $bag,
+            $this->loggerServiceStub,
+            [],
+            $this->responseModeStub,
+        );
+
+        $this->assertSame('id_token', $result?->getValue());
+    }
+
     public function testResponseTypeRuleThrowsWithNoResponseTypeParamTest()
     {
         $params = $this->requestParams;