Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ https://openid.net/specs/openid-connect-core-1_0.html#RequestObject
https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

## New configuration options

- (from v6.1) Show `claims_supported` claim in OP Discovery endpoint - you can now choose to show supported claims,
as is recommended by OpenID Connect Discovery specification https://openid.net/specs/openid-connect-discovery-1_0.html.
- (optional) Issuer - you can now override the issuer (OP identifier). If not set, it falls back to current scheme, host
and optionally a port (as in all previous module versions).
- (optional) Protocol caching adapter and its arguments
Expand Down
8 changes: 7 additions & 1 deletion config/module_oidc.php.dist
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ $config = [
// to use this attribute as the 'sub' claim since it designates unique identifier for the user).
ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE => 'uid',

// The default translate table from SAML attributes to OIDC claims.
// The default translate table from SAML attributes to OIDC claims. If you don't want to support specific default
// claim, set it to an empty array.
ModuleConfig::OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE => [
/*
* The basic format is
Expand Down Expand Up @@ -243,6 +244,11 @@ $config = [
// ModuleConfig::OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION => '0',
ModuleConfig::OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION => null,

// Choose if OP discovery document will include 'claims_supported' claim, which is recommended per OpenID Connect
// Discovery specification https://openid.net/specs/openid-connect-discovery-1_0.html. The list will include all
// claims for which "SAML attribute to OIDC claim translation" has been defined above.
ModuleConfig::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED => false,

// Settings regarding Authentication Processing Filters.
// Note: OIDC authN state array will not contain all the keys which are available during SAML authN,
// like Service Provider metadata, etc.
Expand Down
9 changes: 9 additions & 0 deletions src/ModuleConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class ModuleConfig
final public const OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments';
final public const OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION = 'protocol_user_entity_cache_duration';
final public const OPTION_PROTOCOL_CLIENT_ENTITY_CACHE_DURATION = 'protocol_client_entity_cache_duration';
final public const OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED = 'protocol_discover_show_claims_supported';
final public const OPTION_FEDERATION_PARTICIPATION_LIMIT_BY_TRUST_MARKS =
'federation_participation_limit_by_trust_marks';

Expand Down Expand Up @@ -504,6 +505,14 @@ public function getProtocolClientEntityCacheDuration(): DateInterval
);
}

public function getProtocolDiscoveryShowClaimsSupported(): bool
{
return $this->config()->getOptionalBoolean(
self::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED,
false,
);
}


/*****************************************************************************************************************
* OpenID Federation related config.
Expand Down
12 changes: 6 additions & 6 deletions src/Services/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,6 @@ public function __construct()
);
$this->services[TemplateFactory::class] = $templateFactory;

$opMetadataService = new OpMetadataService($moduleConfig);
$this->services[OpMetadataService::class] = $opMetadataService;

$metadataStorageHandler = MetaDataStorageHandler::getMetadataHandler();
$this->services[MetaDataStorageHandler::class] = $metadataStorageHandler;

$claimSetEntityFactory = new ClaimSetEntityFactory();
$this->services[ClaimSetEntityFactory::class] = $claimSetEntityFactory;

Expand All @@ -204,6 +198,12 @@ public function __construct()
))->build();
$this->services[ClaimTranslatorExtractor::class] = $claimTranslatorExtractor;

$opMetadataService = new OpMetadataService($moduleConfig, $claimTranslatorExtractor);
$this->services[OpMetadataService::class] = $opMetadataService;

$metadataStorageHandler = MetaDataStorageHandler::getMetadataHandler();
$this->services[MetaDataStorageHandler::class] = $metadataStorageHandler;

$processingChainFactory = new ProcessingChainFactory($moduleConfig);
$this->services[ProcessingChainFactory::class] = $processingChainFactory;

Expand Down
7 changes: 7 additions & 0 deletions src/Services/OpMetadataService.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use SimpleSAML\Module\oidc\Codebooks\RoutesEnum;
use SimpleSAML\Module\oidc\ModuleConfig;
use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor;
use SimpleSAML\OpenID\Codebooks\ClaimsEnum;
use SimpleSAML\OpenID\Codebooks\TokenEndpointAuthMethodsEnum;

Expand All @@ -24,6 +25,7 @@ class OpMetadataService
*/
public function __construct(
private readonly ModuleConfig $moduleConfig,
private readonly ClaimTranslatorExtractor $claimTranslatorExtractor,
) {
$this->initMetadata();
}
Expand Down Expand Up @@ -75,6 +77,11 @@ private function initMetadata(): void
}
$this->metadata[ClaimsEnum::BackChannelLogoutSupported->value] = true;
$this->metadata[ClaimsEnum::BackChannelLogoutSessionSupported->value] = true;

if ($this->moduleConfig->getProtocolDiscoveryShowClaimsSupported()) {
$claimsSupported = $this->claimTranslatorExtractor->getSupportedClaims();
$this->metadata[ClaimsEnum::ClaimsSupported->value] = $claimsSupported;
}
}

/**
Expand Down
19 changes: 14 additions & 5 deletions src/Utils/ClaimTranslatorExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ClaimTranslatorExtractor
protected array $claimSets = [];

/** @var string[] */
protected array $protectedClaims = ['openid', 'profile', 'email', 'address', 'phone'];
protected array $protectedScopes = ['openid', 'profile', 'email', 'address', 'phone'];

protected array $translationTable = [
'sub' => [
Expand Down Expand Up @@ -85,13 +85,13 @@ class ClaimTranslatorExtractor
'preferredLanguage',
],
'updated_at' => [
'type' => 'int',
// 'type' => 'int',
],
'email' => [
'mail',
],
'email_verified' => [
'type' => 'bool',
// 'type' => 'bool',
],
'address' => [
'type' => 'json',
Expand All @@ -105,7 +105,7 @@ class ClaimTranslatorExtractor
'homePhone',
],
'phone_number_verified' => [
'type' => 'bool',
// 'type' => 'bool',
// Empty
],
];
Expand Down Expand Up @@ -198,7 +198,7 @@ public function addClaimSet(ClaimSetEntityInterface $claimSet): self
{
$scope = $claimSet->getScope();

if (in_array($scope, $this->protectedClaims, true) && isset($this->claimSets[$scope])) {
if (in_array($scope, $this->protectedScopes, true) && isset($this->claimSets[$scope])) {
throw OidcServerException::serverError(
sprintf("%s is a protected scope and is pre-defined by the OpenID Connect specification.", $scope),
);
Expand Down Expand Up @@ -352,4 +352,13 @@ private function extractAdditionalClaims(array $requestedClaims, array $claims):
ARRAY_FILTER_USE_KEY,
);
}

/**
* Get supported claims for this OP. This will return all the claims for which the "SAML attribute to OIDC claim
* translation" has been defined in module config, meaning it is expected for OP to release those claims.
*/
public function getSupportedClaims(): array
{
return array_keys(array_filter($this->translationTable));
}
}
11 changes: 11 additions & 0 deletions tests/unit/src/ModuleConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,15 @@ public function testCanGetProtocolCacheConfiguration(): void
$this->assertInstanceOf(DateInterval::class, $this->sut()->getProtocolUserEntityCacheDuration());
$this->assertInstanceOf(DateInterval::class, $this->sut()->getProtocolClientEntityCacheDuration());
}

public function testCanGetProtocolDiscoveryShowClaimsSupported(): void
{
$this->assertFalse($this->sut()->getProtocolDiscoveryShowClaimsSupported());
$this->assertTrue(
$this->sut(
null,
[ModuleConfig::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED => true],
)->getProtocolDiscoveryShowClaimsSupported(),
);
}
}
39 changes: 34 additions & 5 deletions tests/unit/src/Services/OpMetadataServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
use SimpleSAML\Module\oidc\Codebooks\RoutesEnum;
use SimpleSAML\Module\oidc\ModuleConfig;
use SimpleSAML\Module\oidc\Services\OpMetadataService;
use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor;
use SimpleSAML\OpenID\Codebooks\ClaimsEnum;

/**
* @covers \SimpleSAML\Module\oidc\Services\OpMetadataService
*/
class OpMetadataServiceTest extends TestCase
{
protected MockObject $moduleConfigMock;
protected MockObject $claimTranslatorExtractorMock;

/**
* @throws \Exception
Expand Down Expand Up @@ -46,14 +49,24 @@ public function setUp(): void
$signer = $this->createMock(Rsa::class);
$signer->method('algorithmId')->willReturn('RS256');
$this->moduleConfigMock->method('getProtocolSigner')->willReturn($signer);

$this->claimTranslatorExtractorMock = $this->createMock(ClaimTranslatorExtractor::class);
}

/**
* @throws \Exception
*/
protected function prepareMockedInstance(): OpMetadataService
{
return new OpMetadataService($this->moduleConfigMock);
protected function sut(
?ModuleConfig $moduleConfig = null,
?ClaimTranslatorExtractor $claimTranslatorExtractor = null,
): OpMetadataService {
$moduleConfig = $moduleConfig ?? $this->moduleConfigMock;
$claimTranslatorExtractor = $claimTranslatorExtractor ?? $this->claimTranslatorExtractorMock;

return new OpMetadataService(
$moduleConfig,
$claimTranslatorExtractor,
);
}

/**
Expand All @@ -63,7 +76,7 @@ public function testItIsInitializable(): void
{
$this->assertInstanceOf(
OpMetadataService::class,
$this->prepareMockedInstance(),
$this->sut(),
);
}

Expand Down Expand Up @@ -102,7 +115,23 @@ public function testItReturnsExpectedMetadata(): void
'backchannel_logout_supported' => true,
'backchannel_logout_session_supported' => true,
],
$this->prepareMockedInstance()->getMetadata(),
$this->sut()->getMetadata(),
);
}

public function testCanShowClaimsSupportedClaim(): void
{
$this->moduleConfigMock->method('getProtocolDiscoveryShowClaimsSupported')->willReturn(true);
$this->claimTranslatorExtractorMock->method('getSupportedClaims')->willReturn(['sample']);

$sut = $this->sut();
$this->assertArrayHasKey(
ClaimsEnum::ClaimsSupported->value,
$sut->getMetadata(),
);

$this->assertTrue(
in_array('sample', $sut->getMetadata()[ClaimsEnum::ClaimsSupported->value], true),
);
}
}
20 changes: 20 additions & 0 deletions tests/unit/src/Utils/ClaimTranslatorExtractorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,24 @@ public function testExtractRequestClaimsIdToken(): void
$claims = $claimTranslator->extractAdditionalIdTokenClaims($requestClaims, ['displayName' => ['bob']]);
$this->assertEquals(['name' => 'bob'], $claims);
}

public function testCanGetSupportedClaims(): void
{
$translate = [
'custom' => [
'type' => 'int',
'custom_attr',
],
];

$this->assertTrue(in_array('custom', $this->mock([], $translate)->getSupportedClaims(), true));
}

public function testCanUnsetClaimWhichIsSupportedByDefault(): void
{
$this->assertTrue(in_array('nickname', $this->mock()->getSupportedClaims(), true));

$translate = ['nickname' => []];
$this->assertFalse(in_array('nickname', $this->mock([], $translate)->getSupportedClaims(), true));
}
}