Skip to content

Commit 63c41eb

Browse files
cicnavicicnavi
committed
WIP
1 parent a5d9c9b commit 63c41eb

File tree

17 files changed

+145
-165
lines changed

17 files changed

+145
-165
lines changed

‎config/module_oidc.php.dist‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ $config = [
4747
*/
4848
// (optional) The private key passphrase.
4949
/** @deprecated */
50-
// ModuleConfig::OPTION_PKI_PRIVATE_KEY_PASSPHRASE => 'secret',
5150
// The certificate and private key filenames, with given defaults.
5251
/** @deprecated */
5352
ModuleConfig::OPTION_PKI_PRIVATE_KEY_FILENAME => ModuleConfig::DEFAULT_PKI_PRIVATE_KEY_FILENAME,

‎docs/6-oidc-upgrade.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ apply those relevant to your deployment.
77

88
New features:
99

10+
- Instance can now be configured to support multiple protocol (Connect) and
11+
Federation signing algorithms and key pairs. This was introduced in order to
12+
support signature algorithm negotiation with the clients.
1013
- Clients can now be configured with new properties:
1114
- ID Token Signing Algorithm (id_token_signed_response_alg)
1215
- Initial support for OpenID for Verifiable Credential Issuance
@@ -15,11 +18,16 @@ it in production.
1518

1619
New configuration options:
1720

18-
- Several new options are available in module config file regarding support for
19-
OpenID4VCI.
21+
- ModuleConfig::OPTION_PROTOCOL_SIGNATURE_KEY_PAIRS - enables defining multiple
22+
protocol (Connect) related signing algorithms and key pairs.
23+
- ModuleConfig::OPTION_FEDERATION_SIGNATURE_KEY_PAIRS - enables defining
24+
multiple Federation related signing algorithms and key pairs.
25+
- Several new options regarding experimental support for OpenID4VCI.
2026

2127
Major impact changes:
2228

29+
- The following configuration options are removed:
30+
-
2331
- In v6 of the module, when defining custom scopes, there was a possibility to
2432
use standard claims with the 'are_multiple_claim_values_allowed' option.
2533
This would allow multiple values (array of values) for standard claims which

‎src/Controllers/EndSessionController.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use SimpleSAML\Module\oidc\Services\LoggerService;
1616
use SimpleSAML\Module\oidc\Services\SessionService;
1717
use SimpleSAML\Module\oidc\Stores\Session\LogoutTicketStoreBuilder;
18+
use SimpleSAML\OpenID\Codebooks\ClaimsEnum;
1819
use SimpleSAML\Session;
1920
use Symfony\Component\HttpFoundation\RedirectResponse;
2021
use Symfony\Component\HttpFoundation\Request;
@@ -60,9 +61,9 @@ public function __invoke(ServerRequestInterface $request): Response
6061
// If id_token_hint was provided, resolve session ID
6162
$idTokenHint = $logoutRequest->getIdTokenHint();
6263
if ($idTokenHint !== null) {
63-
$sidClaim = empty($idTokenHint->claims()->get('sid')) ?
64-
null :
65-
(string)$idTokenHint->claims()->get('sid');
64+
/** @psalm-suppress MixedAssignment */
65+
$sidClaim = $idTokenHint->getPayloadClaim(ClaimsEnum::Sid->value);
66+
$sidClaim = is_string($sidClaim) && $sidClaim !== '' ? $sidClaim : null;
6667
}
6768

6869
$this->loggerService->debug(

‎src/Factories/RequestRulesManagerFactory.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@
4444
use SimpleSAML\Module\oidc\Utils\JwksResolver;
4545
use SimpleSAML\Module\oidc\Utils\ProtocolCache;
4646
use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
47+
use SimpleSAML\OpenID\Core;
4748
use SimpleSAML\OpenID\Federation;
49+
use SimpleSAML\OpenID\Jwks;
4850

4951
class RequestRulesManagerFactory
5052
{
@@ -57,14 +59,15 @@ public function __construct(
5759
private readonly ScopeRepository $scopeRepository,
5860
private readonly CodeChallengeVerifiersRepository $codeChallengeVerifiersRepository,
5961
private readonly ClaimTranslatorExtractor $claimTranslatorExtractor,
60-
private readonly CryptKeyFactory $cryptKeyFactory,
6162
private readonly RequestParamsResolver $requestParamsResolver,
6263
private readonly ClientEntityFactory $clientEntityFactory,
6364
private readonly Federation $federation,
6465
private readonly Helpers $helpers,
6566
private readonly JwksResolver $jwksResolver,
6667
private readonly FederationParticipationValidator $federationParticipationValidator,
6768
private readonly SspBridge $sspBridge,
69+
private readonly Jwks $jwks,
70+
private readonly Core $core,
6871
private readonly ?FederationCache $federationCache = null,
6972
private readonly ?ProtocolCache $protocolCache = null,
7073
) {
@@ -132,7 +135,8 @@ private function getDefaultRules(): array
132135
$this->requestParamsResolver,
133136
$this->helpers,
134137
$this->moduleConfig,
135-
$this->cryptKeyFactory,
138+
$this->jwks,
139+
$this->core,
136140
),
137141
new PostLogoutRedirectUriRule($this->requestParamsResolver, $this->helpers, $this->clientRepository),
138142
new UiLocalesRule($this->requestParamsResolver, $this->helpers),

‎src/Server/AuthorizationServer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public function validateLogoutRequest(ServerRequestInterface $request): LogoutRe
185185
throw new BadRequest($reason);
186186
}
187187

188-
/** @var \Lcobucci\JWT\UnencryptedToken|null $idTokenHint */
188+
/** @var \SimpleSAML\OpenID\Core\IdToken|null $idTokenHint */
189189
$idTokenHint = $resultBag->getOrFail(IdTokenHintRule::class)->getValue();
190190
/** @var string|null $postLogoutRedirectUri */
191191
$postLogoutRedirectUri = $resultBag->getOrFail(PostLogoutRedirectUriRule::class)->getValue();

‎src/Server/RequestRules/Rules/IdTokenHintRule.php‎

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,7 @@
44

55
namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules;
66

7-
use Lcobucci\JWT\Configuration;
8-
use Lcobucci\JWT\Signer\Key\InMemory;
9-
use Lcobucci\JWT\Validation\Constraint\IssuedBy;
10-
use Lcobucci\JWT\Validation\Constraint\SignedWith;
117
use Psr\Http\Message\ServerRequestInterface;
12-
use SimpleSAML\Module\oidc\Factories\CryptKeyFactory;
138
use SimpleSAML\Module\oidc\Helpers;
149
use SimpleSAML\Module\oidc\ModuleConfig;
1510
use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
@@ -20,15 +15,17 @@
2015
use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
2116
use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;
2217
use SimpleSAML\OpenID\Codebooks\ParamsEnum;
23-
use Throwable;
18+
use SimpleSAML\OpenID\Core;
19+
use SimpleSAML\OpenID\Jwks;
2420

2521
class IdTokenHintRule extends AbstractRule
2622
{
2723
public function __construct(
2824
RequestParamsResolver $requestParamsResolver,
2925
Helpers $helpers,
30-
protected ModuleConfig $moduleConfig,
31-
protected CryptKeyFactory $cryptKeyFactory,
26+
protected readonly ModuleConfig $moduleConfig,
27+
protected readonly Jwks $jwks,
28+
protected readonly Core $core,
3229
) {
3330
parent::__construct($requestParamsResolver, $helpers);
3431
}
@@ -58,16 +55,6 @@ public function checkRule(
5855
return new Result($this->getKey(), $idTokenHintParam);
5956
}
6057

61-
// TODO v7 mivanci Fix: unmockable services... inject instead.
62-
$privateKey = $this->cryptKeyFactory->buildPrivateKey();
63-
$publicKey = $this->cryptKeyFactory->buildPublicKey();
64-
/** @psalm-suppress ArgumentTypeCoercion */
65-
$jwtConfig = Configuration::forAsymmetricSigner(
66-
$this->moduleConfig->getProtocolSigner(),
67-
InMemory::plainText($privateKey->getKeyContents(), $privateKey->getPassPhrase() ?? ''),
68-
InMemory::plainText($publicKey->getKeyContents()),
69-
);
70-
7158
if (empty($idTokenHintParam)) {
7259
throw OidcServerException::invalidRequest(
7360
ParamsEnum::IdTokenHint->value,
@@ -78,23 +65,25 @@ public function checkRule(
7865
);
7966
}
8067

81-
try {
82-
/** @var \Lcobucci\JWT\UnencryptedToken $idTokenHint */
83-
$idTokenHint = $jwtConfig->parser()->parse($idTokenHintParam);
68+
$jwks = $this->jwks->jwksDecoratorFactory()->fromJwkDecorators(
69+
...$this->moduleConfig->getProtocolSignatureKeyPairBag()->getAllPublicKeys(),
70+
)->jsonSerialize();
71+
72+
$idTokenHint = $this->core->idTokenFactory()->fromToken($idTokenHintParam);
8473

85-
/** @psalm-suppress ArgumentTypeCoercion */
86-
$jwtConfig->validator()->assert(
87-
$idTokenHint,
88-
new IssuedBy($this->moduleConfig->getIssuer()),
89-
// Note: although logout spec does not mention it, validating signature seems like an important check
90-
// to make. However, checking the signature in a key roll-over scenario will fail for ID tokens
91-
// signed with previous key...
92-
new SignedWith(
93-
$this->moduleConfig->getProtocolSigner(),
94-
InMemory::plainText($publicKey->getKeyContents()),
95-
),
74+
if ($idTokenHint->getIssuer() !== $this->moduleConfig->getIssuer()) {
75+
throw OidcServerException::invalidRequest(
76+
ParamsEnum::IdTokenHint->value,
77+
'Invalid ID Token Hint Issuer',
78+
null,
79+
null,
80+
$state,
9681
);
97-
} catch (Throwable $exception) {
82+
}
83+
84+
try {
85+
$idTokenHint->verifyWithKeySet($jwks);
86+
} catch (\Throwable $exception) {
9887
throw OidcServerException::invalidRequest(
9988
ParamsEnum::IdTokenHint->value,
10089
$exception->getMessage(),
@@ -104,6 +93,7 @@ public function checkRule(
10493
);
10594
}
10695

96+
10797
return new Result($this->getKey(), $idTokenHint);
10898
}
10999
}

‎src/Server/RequestRules/Rules/PostLogoutRedirectUriRule.php‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function checkRule(
4141
/** @var string|null $state */
4242
$state = $currentResultBag->getOrFail(StateRule::class)->getValue();
4343

44-
/** @var \Lcobucci\JWT\UnencryptedToken|null $idTokenHint */
44+
/** @var \SimpleSAML\OpenID\Core\IdToken|null $idTokenHint */
4545
$idTokenHint = $currentResultBag->getOrFail(IdTokenHintRule::class)->getValue();
4646

4747
$postLogoutRedirectUri = $this->requestParamsResolver->getAsStringBasedOnAllowedMethods(
@@ -61,19 +61,7 @@ public function checkRule(
6161
throw OidcServerException::invalidRequest('id_token_hint', $hint);
6262
}
6363

64-
$claims = $idTokenHint->claims()->all();
65-
66-
if (empty($claims['aud'])) {
67-
throw OidcServerException::invalidRequest(
68-
ParamsEnum::IdTokenHint->value,
69-
'aud claim not present',
70-
null,
71-
null,
72-
$state,
73-
);
74-
}
75-
/** @var string[] $auds */
76-
$auds = is_array($claims['aud']) ? $claims['aud'] : [$claims['aud']];
64+
$auds = $idTokenHint->getAudience();
7765

7866
$isPostLogoutRedirectUriRegistered = false;
7967
foreach ($auds as $aud) {

‎src/Server/RequestTypes/LogoutRequest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace SimpleSAML\Module\oidc\Server\RequestTypes;
66

7-
use Lcobucci\JWT\UnencryptedToken;
7+
use SimpleSAML\OpenID\Core\IdToken;
88

99
class LogoutRequest
1010
{
@@ -14,7 +14,7 @@ public function __construct(
1414
* current authenticated session with the Client. This is used as an indication of the identity of the
1515
* End-User that the RP is requesting be logged out by the OP.
1616
*/
17-
protected ?UnencryptedToken $idTokenHint = null,
17+
protected ?IdToken $idTokenHint = null,
1818
/**
1919
* URL to which the RP is requesting that the End-User's User Agent be redirected after a logout has been
2020
* performed. The value MUST have been previously registered with the OP.An id_token_hint is also
@@ -35,12 +35,12 @@ public function __construct(
3535
) {
3636
}
3737

38-
public function getIdTokenHint(): ?UnencryptedToken
38+
public function getIdTokenHint(): ?IdToken
3939
{
4040
return $this->idTokenHint;
4141
}
4242

43-
public function setIdTokenHint(?UnencryptedToken $idTokenHint): LogoutRequest
43+
public function setIdTokenHint(?IdToken $idTokenHint): LogoutRequest
4444
{
4545
$this->idTokenHint = $idTokenHint;
4646
return $this;

‎src/Services/Container.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,13 @@ public function __construct()
406406
new AddClaimsToIdTokenRule($requestParamsResolver, $helpers),
407407
new RequiredNonceRule($requestParamsResolver, $helpers),
408408
new ResponseTypeRule($requestParamsResolver, $helpers),
409-
new IdTokenHintRule($requestParamsResolver, $helpers, $moduleConfig, $cryptKeyFactory),
409+
new IdTokenHintRule(
410+
$requestParamsResolver,
411+
$helpers,
412+
$moduleConfig,
413+
$jwks,
414+
$core,
415+
),
410416
new PostLogoutRedirectUriRule($requestParamsResolver, $helpers, $clientRepository),
411417
new UiLocalesRule($requestParamsResolver, $helpers),
412418
new AcrValuesRule($requestParamsResolver, $helpers),

‎src/Services/LogoutTokenBuilder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function forRelyingPartyAssociation(RelyingPartyAssociationInterface $rel
5252
ClaimsEnum::Iss->value => $this->moduleConfig->getIssuer(),
5353
ClaimsEnum::Iat->value => $currentTimestamp,
5454
ClaimsEnum::Exp->value => $this->core->helpers()->dateTime()->getUtc()->add(
55-
$this->moduleConfig->getAccessTokenDuration(),
55+
$this->moduleConfig->getAuthCodeDuration(),
5656
)->getTimestamp(),
5757
ClaimsEnum::Jti->value => $this->core->helpers()->random()->string(),
5858
ClaimsEnum::Aud->value => $relyingPartyAssociation->getClientId(),

0 commit comments

Comments
 (0)