Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add issuer, audience and azp checks in bearer token validator #864

Merged
merged 3 commits into from
Jun 7, 2024
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
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,16 @@ it is possible to disable the classic "self-encoded" validation:
],
```

### Disable audience check in bearer token validation

The `audience` and `azp` token claims will be checked when validating a bearer token for authenticated API requests.
You can disable this check with this config value:
``` php
'user_oidc' => [
'selfencoded_bearer_validation_audience_check' => false,
],
```

## Building the app

Requirements for building:
Expand Down
27 changes: 12 additions & 15 deletions lib/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -455,26 +455,23 @@ public function code(string $state = '', string $code = '', string $scope = '',
}

// Verify audience
if (!($idTokenPayload->aud === $provider->getClientId() || in_array($provider->getClientId(), $idTokenPayload->aud, true))) {
$tokenAudience = $idTokenPayload->aud;
$providerClientId = $provider->getClientId();
if (
(is_string($tokenAudience) && $tokenAudience !== $providerClientId)
|| (is_array($tokenAudience) && !in_array($providerClientId, $tokenAudience, true))
) {
$this->logger->debug('This token is not for us');
$message = $this->l10n->t('The audience does not match ours');
return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_audience' => $idTokenPayload->aud]);
}

// If the ID Token contains multiple audiences, the Client SHOULD verify that an azp Claim is present.
// If an azp (authorized party) Claim is present, the Client SHOULD verify that its client_id is the Claim Value.
if (is_array($idTokenPayload->aud) && count($idTokenPayload->aud) > 1) {
if (isset($idTokenPayload->azp)) {
if ($idTokenPayload->azp !== $provider->getClientId()) {
$this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID');
$message = $this->l10n->t('The authorized party does not match ours');
return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_azp' => $idTokenPayload->azp]);
}
} else {
$this->logger->debug('Multiple audiences but no authorized party (azp) in the id token');
$message = $this->l10n->t('No authorized party');
return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['missing_azp']);
}
// ref https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
// If the azp claim is present, it should be the client ID
if (isset($idTokenPayload->azp) && $idTokenPayload->azp !== $provider->getClientId()) {
$this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID');
$message = $this->l10n->t('The authorized party does not match ours');
return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_azp' => $idTokenPayload->azp]);
}

if (isset($idTokenPayload->nonce) && $idTokenPayload->nonce !== $this->session->get(self::NONCE)) {
Expand Down
39 changes: 38 additions & 1 deletion lib/User/Validator/SelfEncodedValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use OCA\UserOIDC\User\Provisioning\SelfEncodedTokenProvisioning;
use OCA\UserOIDC\Vendor\Firebase\JWT\JWT;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use Psr\Log\LoggerInterface;
use Throwable;

Expand All @@ -42,11 +43,19 @@ class SelfEncodedValidator implements IBearerTokenValidator {
private $logger;
/** @var ITimeFactory */
private $timeFactory;
/** @var IConfig */
private $config;

public function __construct(DiscoveryService $discoveryService, LoggerInterface $logger, ITimeFactory $timeFactory) {
public function __construct(
DiscoveryService $discoveryService,
LoggerInterface $logger,
ITimeFactory $timeFactory,
IConfig $config
) {
$this->discoveryService = $discoveryService;
$this->logger = $logger;
$this->timeFactory = $timeFactory;
$this->config = $config;
}

public function isValidBearerToken(Provider $provider, string $bearerToken): ?string {
Expand All @@ -70,6 +79,34 @@ public function isValidBearerToken(Provider $provider, string $bearerToken): ?st
return null;
}

$discovery = $this->discoveryService->obtainDiscovery($provider);
if ($payload->iss !== $discovery['issuer']) {
$this->logger->debug('This token is issued by the wrong issuer, it does not match the one from the discovery endpoint');
return null;
}

$oidcSystemConfig = $this->config->getSystemValue('user_oidc', []);
$checkAudience = !isset($oidcSystemConfig['selfencoded_bearer_validation_audience_check'])
|| !in_array($oidcSystemConfig['selfencoded_bearer_validation_audience_check'], [false, 'false', 0, '0'], true);
if ($checkAudience) {
$tokenAudience = $payload->aud;
$providerClientId = $provider->getClientId();
if (
(is_string($tokenAudience) && $tokenAudience !== $providerClientId)
|| (is_array($tokenAudience) && !in_array($providerClientId, $tokenAudience, true))
) {
$this->logger->debug('This token is not for us, the audience does not match the client ID');
return null;
}

// ref https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
// If the azp claim is present, it should be the client ID
if (isset($payload->azp) && $payload->azp !== $providerClientId) {
$this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID');
return null;
}
}

// find the user ID
if (!isset($payload->{$uidAttribute})) {
return null;
Expand Down
Loading