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 PKCE support #56

Merged
merged 2 commits into from
Feb 9, 2021
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
31 changes: 29 additions & 2 deletions Classes/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,16 @@ protected function performRedirectToLogin()
$service = GeneralUtility::makeInstance(\Causal\Oidc\Service\OAuthService::class);
$service->setSettings($this->settings);

$authorizationUrl = $service->getAuthorizationUrl();

if (session_id() === '') {
session_start();
}
if ($this->settings['enableCodeVerifier']) {
$codeVerifier = $this->generateCodeVerifier();
$codeChallenge = $this->convertVerifierToChallenge($codeVerifier);
$options = $this->addCodeChallengeToOptions($codeChallenge);
$_SESSION['oidc_code_verifier'] = $codeVerifier;
}
$authorizationUrl = $service->getAuthorizationUrl($options?: []);

$state = $service->getState();
$_SESSION['oidc_state'] = $state;
Expand Down Expand Up @@ -118,4 +123,26 @@ protected function determineRedirectUrl()

return '/';
}

protected function generateCodeVerifier(): string
{
return bin2hex(random_bytes(64));
}

protected function convertVerifierToChallenge($codeVerifier)
{
return rtrim(strtr(base64_encode(hash('sha256', $codeVerifier, true)), '+/', '-_'), '=');
}

protected function addCodeChallengeToOptions($codeChallenge, array $options = []): array
{
return array_merge(
$options,
[
'code_challenge' => $codeChallenge,
'code_challenge_method' => 'S256',
]
);
}

}
18 changes: 15 additions & 3 deletions Classes/Service/AuthenticationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ public function __construct()
}
}

protected function getCodeVerifierFromSession()
{
if (session_id() === '') {
session_start();
}
return @$_SESSION['oidc_code_verifier'];
}

/**
* Finds a user.
*
Expand All @@ -97,7 +105,11 @@ public function getUser()
}

if ($code !== null) {
$user = $this->authenticateWithAuhorizationCode($code);
$codeVerifier = null;
if ($this->config['enableCodeVerifier']) {
$codeVerifier = $this->getCodeVerifierFromSession();
}
$user = $this->authenticateWithAuhorizationCode($code, $codeVerifier);
} elseif (!(empty($username) || empty($password))) {
$user = $this->authenticateWithResourceOwnerPasswordCredentials($username, $password);
}
Expand All @@ -120,7 +132,7 @@ public function getUser()
* @param string $code
* @return array|bool
*/
protected function authenticateWithAuhorizationCode($code)
protected function authenticateWithAuhorizationCode($code, $codeVerifier = null)
{
static::getLogger()->debug('Initializing OpenID Connect service');

Expand All @@ -131,7 +143,7 @@ protected function authenticateWithAuhorizationCode($code)
// Try to get an access token using the authorization code grant
try {
static::getLogger()->debug('Retrieving an access token');
$accessToken = $service->getAccessToken($code);
$accessToken = $service->getAccessToken($code, null, $codeVerifier);
static::getLogger()->debug('Access token retrieved', $accessToken->jsonSerialize());
} catch (\League\OAuth2\Client\Provider\Exception\IdentityProviderException $e) {
// Probably a "server_error", meaning the code is not valid anymore
Expand Down
18 changes: 11 additions & 7 deletions Classes/Service/OAuthService.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,19 @@ public function getState()
* credentials grant.
*
* @param string $codeOrUsername Either a code or the username (if password is provided)
* @param string $password Optional parameter if authenticating with authorization code grant
* @param null $password Optional parameter if authenticating with authorization code grant
* @param null $codeVerifier Code verifier for PKCE
* @return AccessToken
* @throws \League\OAuth2\Client\Provider\Exception\IdentityProviderException
*/
public function getAccessToken($codeOrUsername, $password = null)
public function getAccessToken($codeOrUsername, $password = null, $codeVerifier = null)
{
if ($password === null) {
$accessToken = $this->getProvider()->getAccessToken('authorization_code', [
'code' => $codeOrUsername,
]);
$options = ['code' => $codeOrUsername];
if ($codeVerifier !== null) {
$options['code_verifier'] = $codeVerifier;
}
$accessToken = $this->getProvider()->getAccessToken('authorization_code', $options);
} else {
$accessToken = $this->getProvider()->getAccessToken('password', [
'username' => $codeOrUsername,
Expand All @@ -118,7 +122,7 @@ public function getAccessToken($codeOrUsername, $password = null)
*/
public function getAccessTokenWithRequestPathAuthentication($username, $password)
{
$redirectUri = GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST') . '/typo3conf/ext/oidc/Resources/Public/callback.php';
$redirectUri = $this->settings['oidcRedirectUri'] ?: GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST') . '/typo3conf/ext/oidc/Resources/Public/callback.php';
$url = $this->settings['oidcEndpointAuthorize'] . '?'. http_build_query([
'response_type' => 'code',
'client_id' => $this->settings['oidcClientKey'],
Expand Down Expand Up @@ -206,7 +210,7 @@ public function revokeToken(AccessToken $token)
protected function getProvider()
{
if ($this->provider === null) {
$redirectUri = GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST') . '/typo3conf/ext/oidc/Resources/Public/callback.php';
$redirectUri = $this->settings['oidcRedirectUri'] ?: GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST') . '/typo3conf/ext/oidc/Resources/Public/callback.php';

$this->provider = new \League\OAuth2\Client\Provider\GenericProvider([
'clientId' => $this->settings['oidcClientKey'],
Expand Down
16 changes: 11 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ the authorization endpoint of the authorization server.

## OIDC Login

If openid_connect is your only means of frontend login, you can use the included "OIDC Login" plugin. Add it to your
login page, where you would normally add the felogin box. After adding the OIDC Login plugin, requests to the login
If openid_connect is your only means of frontend login, you can use the included "OIDC Login" plugin. Add it to your
login page, where you would normally add the felogin box. After adding the OIDC Login plugin, requests to the login
page will immediately be redirected to the authorization server.

After the login process, the user will be redirected:
Expand All @@ -26,7 +26,13 @@ After the login process, the user will be redirected:
- If no parameter is set, OIDC Login will redirect the user to the page configured at
`plugin.tx_oidc_login.defaultRedirectPid`.
- If that configuration is not set either, the user will be redirected to '/'.


## PKCE (Proof of Key for Code Exchange)

If your OIDC Login supports _Proof of Key for Code Exchange_ you can enable it by
checking `enableCodeVerifier` in the extension configuration. A shared secret will
be sent along preventing _Authorization Code Interception Attacks_. See
https://tools.ietf.org/html/rfc7636 for details.

## Configuring

Expand Down Expand Up @@ -69,8 +75,8 @@ After the login process, the user will be redirected:

### OIDC Login

- `plugin.tx_oidc_login.defaultRedirectPid` UID of the page that users will be redirected to, if no `redirect_url`
parameter is set.
- `plugin.tx_oidc_login.defaultRedirectPid` UID of the page that users will be redirected to, if no `redirect_url`
parameter is set.

## Logging

Expand Down
6 changes: 6 additions & 0 deletions Resources/Private/Language/locallang_db.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,15 @@
<trans-unit id="settings.oidcDisableCSRFProtection">
<source>Disable CSRF attack mitigation: CAUTION! This is a security protection which checks the return state with the expected value. Disable this protection at your own risk.</source>
</trans-unit>
<trans-unit id="settings.enableCodeVerifier">
<source>Enable PKCE: Enable PKCE flow. Code challenge and code verifier will be sent along.</source>
</trans-unit>
<trans-unit id="fe_groups.tx_oidc_pattern">
<source>Case-insensitive pattern to match OpenID Connect role names ("*" matches every character, "|" to separate expressions)</source>
</trans-unit>
<trans-unit id="settings.oidcRedirectUri">
<source>Redirect URI: The authentication server callback will point to this URI.</source>
</trans-unit>
<trans-unit id="fe_users.tx_oidc">
<source>OpenID Connect Identifier</source>
</trans-unit>
Expand Down
6 changes: 6 additions & 0 deletions ext_conf_template.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ undeleteFrontendUsers = 0
# cat=basic/enable/4; type=boolean; label=LLL:EXT:oidc/Resources/Private/Language/locallang_db.xlf:settings.frontendUserMustExistLocally
frontendUserMustExistLocally = 0

# cat=basic/enable/5; type=boolean; label=LLL:EXT:oidc/Resources/Private/Language/locallang_db.xlf:settings.enableCodeVerifier
enableCodeVerifier = 0

# cat=basic//1; type=int; label=Storage Pid: The Storage Pid of the Page, where the fe_users should be stored
usersStoragePid =

# cat=basic//2; type=string; label=Default user group(s) (comma-separated list of UIDs)
usersDefaultGroup =

# cat=basic//2a; type=string; label=LLL:EXT:oidc/Resources/Private/Language/locallang_db.xlf:settings.oidcRedirectUri
oidcRedirectUri =

# cat=basic//3; type=string; label=LLL:EXT:oidc/Resources/Private/Language/locallang_db.xlf:settings.oidcClientKey
oidcClientKey =

Expand Down