Skip to content

Commit eedc82b

Browse files
committed
WIP
1 parent 3df80fa commit eedc82b

6 files changed

Lines changed: 82 additions & 16 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ contract for what was honored.
5959
| `token_endpoint_auth_signing_alg` | Ignored | TBD | Relevant to `private_key_jwt` / `client_secret_jwt`. |
6060
| `default_max_age` | **Honored** (validate + store + echo + enforce) | Honored | Admin-editable. Default applied when max_age omitted (MaxAgeRule). |
6161
| `require_auth_time` | **Honored** (validate + store + echo + enforce) | Honored | Admin-editable. Forces auth_time into the ID Token (MaxAgeRule -> AuthCodeGrant). |
62-
| `default_acr_values` | **Honored** (validate + store + echo + enforce) | Honored | Admin-editable. Default applied when acr_values omitted (AcrValuesRule). |
62+
| `default_acr_values` | **Honored** (validate + store + echo + enforce) | Honored | Constrained to `acr_values_supported`: DCR rejects unsupported values; the admin field is a multi-select of the supported ACRs. Default applied when acr_values omitted (AcrValuesRule). |
6363
| `initiate_login_uri` | **Validated + echoed** | Validated + echoed | Admin-editable; https URI. Informational. |
6464
| `backchannel_logout_session_required` | Ignored | TBD | |
6565
| `frontchannel_logout_uri` / `..._session_required` | **Reject** if requested | Reject | Front-channel logout not supported. |

src/Forms/ClientForm.php

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -418,9 +418,10 @@ public function getValues(string|object|bool|null $returnType = null, ?array $co
418418
$values[ClaimsEnum::RequireAuthTime->value] = (bool)($values[ClaimsEnum::RequireAuthTime->value] ?? false);
419419

420420
/** @var mixed $defaultAcrValues */
421-
$defaultAcrValues = $values[ClaimsEnum::DefaultAcrValues->value] ?? '';
422-
$values[ClaimsEnum::DefaultAcrValues->value] = $this->helpers->str()->convertTextToArray(
423-
is_string($defaultAcrValues) ? $defaultAcrValues : '',
421+
$defaultAcrValues = $values[ClaimsEnum::DefaultAcrValues->value] ?? null;
422+
$defaultAcrValues = is_array($defaultAcrValues) ? $defaultAcrValues : [];
423+
$values[ClaimsEnum::DefaultAcrValues->value] = array_values(
424+
array_intersect($defaultAcrValues, array_keys($this->getSupportedAcrValues())),
424425
);
425426

426427
foreach (
@@ -569,14 +570,11 @@ public function setDefaults(object|array $values, bool $erase = false): static
569570
/** @var mixed $defaultAcrValues */
570571
$defaultAcrValues = $values[ClaimsEnum::DefaultAcrValues->value] ?? null;
571572
$defaultAcrValues = is_array($defaultAcrValues) ? $defaultAcrValues : [];
572-
$defaultAcrStrings = [];
573-
/** @var mixed $acr */
574-
foreach ($defaultAcrValues as $acr) {
575-
if (is_string($acr)) {
576-
$defaultAcrStrings[] = $acr;
577-
}
578-
}
579-
$values[ClaimsEnum::DefaultAcrValues->value] = implode("\n", $defaultAcrStrings);
573+
// The field is a multi-select bound to the OP's supported ACRs; keep only currently-supported values so the
574+
// control can render them (values no longer supported are dropped rather than shown as invalid options).
575+
$values[ClaimsEnum::DefaultAcrValues->value] = array_values(
576+
array_intersect($defaultAcrValues, array_keys($this->getSupportedAcrValues())),
577+
);
580578

581579
/** @var mixed $contacts */
582580
$contacts = $values[ClaimsEnum::Contacts->value] ?? null;
@@ -729,10 +727,12 @@ protected function buildForm(): void
729727

730728
$this->addCheckbox(ClaimsEnum::RequireAuthTime->value, Translate::noop('Require auth_time in ID Token'));
731729

732-
$this->addTextArea(
730+
// Bound to the OP's supported ACRs (acr_values_supported). When the OP advertises no ACRs, this has no
731+
// items and the field is hidden in the template (a per-client default ACR cannot do anything in that case).
732+
$this->addMultiSelect(
733733
ClaimsEnum::DefaultAcrValues->value,
734-
Translate::noop('Default ACR Values (one per line)'),
735-
null,
734+
Translate::noop('Default ACR Values'),
735+
$this->getSupportedAcrValues(),
736736
3,
737737
)->setHtmlAttribute('class', 'full-width');
738738

@@ -865,6 +865,28 @@ protected function getSupportedTokenEndpointAuthMethods(): array
865865
return array_combine($supported, $supported);
866866
}
867867

868+
/**
869+
* The OP's supported ACR values (value => label), as configured via OPTION_AUTH_ACR_VALUES_SUPPORTED and
870+
* advertised in discovery as acr_values_supported. Empty when the OP advertises no ACRs.
871+
*
872+
* @return array<string,string>
873+
*/
874+
protected function getSupportedAcrValues(): array
875+
{
876+
$supported = array_values(array_filter($this->moduleConfig->getAcrValuesSupported(), 'is_string'));
877+
878+
return array_combine($supported, $supported);
879+
}
880+
881+
/**
882+
* Whether the OP has any supported ACR values configured. Used by the template to hide the per-client
883+
* default_acr_values field when there is nothing to select.
884+
*/
885+
public function hasConfiguredAcrValues(): bool
886+
{
887+
return $this->getSupportedAcrValues() !== [];
888+
}
889+
868890
/**
869891
* Application types the client may register (value => label).
870892
*

src/Server/Registration/ClientMetadataValidator.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,13 +294,21 @@ private function validateAdditionalMetadata(array $metadata): void
294294
if (!is_array($defaultAcrValues)) {
295295
throw OidcServerException::invalidClientMetadata('default_acr_values must be an array.');
296296
}
297+
$supportedAcrValues = $this->moduleConfig->getAcrValuesSupported();
297298
/** @var mixed $acr */
298299
foreach ($defaultAcrValues as $acr) {
299300
if (!is_string($acr) || $acr === '') {
300301
throw OidcServerException::invalidClientMetadata(
301302
'default_acr_values must be an array of non-empty strings.',
302303
);
303304
}
305+
// Reject ACRs the OP does not support (advertised in discovery as acr_values_supported); requesting
306+
// an unsupported ACR could never be satisfied at the authorization endpoint.
307+
if (!in_array($acr, $supportedAcrValues, true)) {
308+
throw OidcServerException::invalidClientMetadata(
309+
sprintf('default_acr_values contains an unsupported ACR value: "%s".', $acr),
310+
);
311+
}
304312
}
305313
}
306314

templates/clients/includes/form.twig

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,14 +210,16 @@
210210
{% trans %}When enabled, the auth_time claim is always included in ID Tokens issued to this Client.{% endtrans %}
211211
</span>
212212

213+
{% if form.hasConfiguredAcrValues() %}
213214
<label for="frm-default_acr_values">{{ 'Default ACR Values'|trans }}</label>
214215
{{ form.default_acr_values.control | raw }}
215216
<span class="pure-form-message">
216-
{% trans %}Default requested Authentication Context Class Reference values, applied when the authorization request omits acr_values. One per line.{% endtrans %}
217+
{% trans %}Default requested Authentication Context Class Reference values, applied when the authorization request omits acr_values. Selected from the ACRs supported by this OP.{% endtrans %}
217218
</span>
218219
{% if form.default_acr_values.hasErrors %}
219220
<span class="pure-form-message red-text">{{ form.default_acr_values.getError }}</span>
220221
{% endif %}
222+
{% endif %}
221223

222224
<label for="frm-initiate_login_uri">{{ 'Initiate Login URI'|trans }}</label>
223225
{{ form.initiate_login_uri.control | raw }}

tests/unit/src/Forms/ClientFormTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public function setUp(): void
4747
$this->csrfProtectionMock = $this->createMock(CsrfProtection::class);
4848
$this->moduleConfigMock = $this->createMock(ModuleConfig::class);
4949
$this->moduleConfigMock->method('getSupportedResponseModes')->willReturn(['query', 'fragment', 'form_post']);
50+
$this->moduleConfigMock->method('getAcrValuesSupported')
51+
->willReturn(['urn:mace:incommon:iap:silver', 'urn:mace:incommon:iap:bronze']);
5052

5153
$signatureKeyPairBagMock = $this->createMock(SignatureKeyPairBag::class);
5254
$signatureKeyPairBagMock->method('getAllAlgorithmNamesUnique')->willReturn(['RS256', 'ES256']);
@@ -249,6 +251,24 @@ public function testEmptyTokenEndpointAuthMethodNormalizesToNull(): void
249251
$this->assertSame([], $values[ClaimsEnum::ResponseTypes->value]);
250252
}
251253

254+
public function testDefaultAcrValuesAreConstrainedToSupported(): void
255+
{
256+
// The field is a multi-select bound to the OP's supported ACRs. setDefaults (the edit path) drops values
257+
// that are no longer supported, so the control never receives an out-of-range value.
258+
$data = array_merge($this->clientDataSample, [
259+
ClaimsEnum::DefaultAcrValues->value => [
260+
'urn:mace:incommon:iap:silver',
261+
'urn:not:supported',
262+
],
263+
]);
264+
$sut = $this->sut()->setDefaults($data);
265+
266+
$values = $sut->getValues();
267+
268+
$this->assertSame(['urn:mace:incommon:iap:silver'], $values[ClaimsEnum::DefaultAcrValues->value]);
269+
$this->assertTrue($sut->hasConfiguredAcrValues());
270+
}
271+
252272
public function testInformationalMetadataRoundTrip(): void
253273
{
254274
$sut = $this->sut();

tests/unit/src/Server/Registration/ClientMetadataValidatorTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ protected function setUp(): void
2121
$this->moduleConfigMock = $this->createMock(ModuleConfig::class);
2222
// Default: impersonation protection on.
2323
$this->moduleConfigMock->method('getDcrImpersonationProtectionEnabled')->willReturn(true);
24+
// Default: the OP advertises a single supported ACR.
25+
$this->moduleConfigMock->method('getAcrValuesSupported')->willReturn(['urn:mace:incommon:iap:silver']);
2426
}
2527

2628
protected function sut(): ClientMetadataValidator
@@ -223,6 +225,18 @@ public function testNonArrayDefaultAcrValuesIsRejected(): void
223225
);
224226
}
225227

228+
public function testUnsupportedDefaultAcrValueIsRejected(): void
229+
{
230+
$this->assertRejected(
231+
[
232+
'redirect_uris' => ['https://client.example.org/cb'],
233+
'default_acr_values' => ['urn:mace:incommon:iap:silver', 'urn:not:supported'],
234+
],
235+
'invalid_client_metadata',
236+
'default_acr_values',
237+
);
238+
}
239+
226240
public function testNonHttpsInitiateLoginUriIsRejected(): void
227241
{
228242
$this->assertRejected(

0 commit comments

Comments
 (0)