Skip to content

Commit

Permalink
Add PKCE support
Browse files Browse the repository at this point in the history
For the providers that support it. Add extra security.
See also https://datatracker.ietf.org/doc/html/rfc7636

Signed-off-by: Roeland Jago Douma <[email protected]>
  • Loading branch information
rullzer authored and nc-fkl committed Nov 24, 2023
1 parent 6da79e3 commit 02d199f
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions lib/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class LoginController extends BaseOidcController {
public const PROVIDERID = 'oidc.providerid';
private const REDIRECT_AFTER_LOGIN = 'oidc.redirect';
private const ID_TOKEN = 'oidc.id_token';
private const CODE_VERIFIER = 'oidc.code_verifier';

/** @var ISecureRandom */
private $random;
Expand Down Expand Up @@ -229,6 +230,10 @@ public function login(int $providerId, string $redirectUrl = null) {
$nonce = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER);
$this->session->set(self::NONCE, $nonce);

// PKCE code_challenge see https://datatracker.ietf.org/doc/html/rfc7636
$code_verifier = $this->random->generage(128, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER);
$this->session->set(self::CODE_VERIFIER, $code_verifier);

$this->session->set(self::PROVIDERID, $providerId);
$this->session->close();

Expand Down Expand Up @@ -280,6 +285,8 @@ public function login(int $providerId, string $redirectUrl = null) {
'claims' => json_encode($claims),
'state' => $state,
'nonce' => $nonce,
'code_challenge' => $this->toCodeChallenge($code_verifier),
'code_challenge_method' => 'S256',
];
// pass discovery query parameters also on to the authentication
$discoveryUrl = parse_url($provider->getDiscoveryEndpoint());
Expand Down Expand Up @@ -368,6 +375,8 @@ public function code(string $state = '', string $code = '', string $scope = '',
return $this->buildErrorTemplateResponse($message, Http::STATUS_BAD_REQUEST, [], false);
}

$code_verifier = $this->session->get(self::CODE_VERIFIER);

$discovery = $this->discoveryService->obtainDiscovery($provider);

$this->logger->debug('Obtainting data from: ' . $discovery['token_endpoint']);
Expand All @@ -383,6 +392,7 @@ public function code(string $state = '', string $code = '', string $scope = '',
'client_secret' => $providerClientSecret,
'redirect_uri' => $this->urlGenerator->linkToRouteAbsolute(Application::APP_ID . '.login.code'),
'grant_type' => 'authorization_code',
'code_verifier' => $code_verifier, // Set for the PKCE flow
],
]
);
Expand Down Expand Up @@ -730,4 +740,14 @@ private function getBackchannelLogoutErrorResponse(string $error, string $descri
}
return $response;
}

private function toCodeChallenge(string $data): string {
// Basically one big work around for the base64url decode being weird
$h = pack('H*', hash('sha256', $data));
$s = base64_encode($h); // Regular base64 encoder
$s = explode('=', $s)[0]; // Remove any trailing '='s
$s = str_replace('+', '-', $s); // 62nd char of encoding
$s = str_replace('/', '_', $s); // 63rd char of encoding
return $s;
}
}

0 comments on commit 02d199f

Please sign in to comment.