Skip to content

Commit

Permalink
Merge pull request #715 from nextcloud/fix/noid/id4me-token-validation
Browse files Browse the repository at this point in the history
Improve ID4ME token validation
  • Loading branch information
julien-nc authored Nov 22, 2023
2 parents 8e6a8d7 + 1689dab commit 9f591c6
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 3 deletions.
13 changes: 13 additions & 0 deletions css/id4me-login.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#id4me-submit {
position: relative;
right: -6px;
top: 5px;
border: 2px solid var(--color-border-maxcontrast);
border-bottom-right-radius: var(--border-radius-large);
border-top-right-radius: var(--border-radius-large);
}

#domain {
border-bottom-left-radius: var(--border-radius-large);
border-top-left-radius: var(--border-radius-large);
}
46 changes: 43 additions & 3 deletions lib/Controller/Id4meController.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IL10N;
Expand All @@ -53,6 +54,7 @@
use Id4me\RP\Service;
use Id4me\RP\Exception\InvalidOpenIdDomainException;
use Id4me\RP\Model\OpenIdConfig;
use OCP\Util;
use Psr\Log\LoggerInterface;

class Id4meController extends BaseOidcController {
Expand Down Expand Up @@ -84,13 +86,16 @@ class Id4meController extends BaseOidcController {
private $logger;
/** @var ICrypto */
private $crypto;
/** @var ITimeFactory */
private $timeFactory;

public function __construct(
IRequest $request,
ISecureRandom $random,
ISession $session,
IConfig $config,
IL10N $l10n,
ITimeFactory $timeFactory,
IClientService $clientService,
IURLGenerator $urlGenerator,
UserMapper $userMapper,
Expand All @@ -115,6 +120,7 @@ public function __construct(
$this->l10n = $l10n;
$this->logger = $logger;
$this->crypto = $crypto;
$this->timeFactory = $timeFactory;
}

/**
Expand All @@ -123,6 +129,7 @@ public function __construct(
* @UseSession
*/
public function showLogin() {
Util::addStyle(Application::APP_ID, 'id4me-login');
$response = new Http\TemplateResponse('user_oidc', 'id4me/login', [], 'guest');

$csp = new Http\ContentSecurityPolicy();
Expand Down Expand Up @@ -270,22 +277,55 @@ public function code(string $state = '', string $code = '', string $scope = '')

$data = json_decode($result->getBody(), true);

// documentation about token validation:
// https://gitlab.com/ID4me/documentation/blob/master/id4ME%20Relying%20Party%20Implementation%20Guide.pdf
// section 4.5.3. ID Token Validation

// Decode header and token
[$header, $payload, $signature] = explode('.', $data['id_token']);
$plainHeaders = json_decode(base64_decode($header), true);
$plainPayload = json_decode(base64_decode($payload), true);

/** TODO: VALIATE SIGNATURE! */

// TODO: validate expiration
// Check expiration
if ($plainPayload['exp'] < $this->timeFactory->getTime()) {
$this->logger->debug('Token expired');
$message = $this->l10n->t('The received token is expired.');
return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'token expired']);
}

// Verify audience
if ($plainPayload['aud'] !== $id4Me->getClientId()) {
if (!(
$plainPayload['aud'] === $id4Me->getClientId()
|| (is_array($plainPayload['aud']) && in_array($id4Me->getClientId(), $plainPayload['aud'], 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' => $plainPayload['aud']]);
}

// TODO: VALIDATE NONCE (if set)
// 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($plainPayload['aud']) && count($plainPayload['aud']) > 1) {
if (isset($plainPayload['azp'])) {
if ($plainPayload['azp'] !== $id4Me->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' => $plainPayload['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']);
}
}

// Check nonce
if (isset($plainPayload['nonce']) && $plainPayload['nonce'] !== $this->session->get(self::NONCE)) {
$message = $this->l10n->t('The nonce does not match');
return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_nonce' => $plainPayload['nonce']]);
}

// Insert or update user
$backendUser = $this->userMapper->getOrCreate($id4Me->getId(), $plainPayload['sub'], true);
Expand Down

0 comments on commit 9f591c6

Please sign in to comment.