Skip to content

Commit

Permalink
fix: add issuer, audience and azp checks in bearer token validator, m…
Browse files Browse the repository at this point in the history
…ake it possible to disable audience check via config

Signed-off-by: Julien Veyssier <[email protected]>
  • Loading branch information
julien-nc committed May 10, 2024
1 parent a17af82 commit 99ea021
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
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
41 changes: 40 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,36 @@ 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) {
if (!($payload->aud === $provider->getClientId() || in_array($provider->getClientId(), $payload->aud, true))) {
$this->logger->debug('This token is not for us, the audience does not match the client ID');
return null;
}

// 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($payload->aud) && count($payload->aud) > 1) {
if (isset($payload->azp)) {
if ($payload->azp !== $provider->getClientId()) {
$this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID');
return null;
}
} else {
$this->logger->debug('Multiple audiences but no authorized party (azp) in the id token');
return null;
}
}
}

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

0 comments on commit 99ea021

Please sign in to comment.