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.

Sign up for GitHub

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

Jump to bottom

add login_hint as method to identify end-user in signOut #277

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

add login_hint as method to identify end-user in signOut #277

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
79beba8
add login_hint as method to identify end-user in signOut
piuma Nov 4, 2021
1bc3dc7
fix: getState() return empty string
piuma Nov 24, 2021
7cb2f69
fix: getState() return empty string
piuma Nov 24, 2021
d2c116e
Fix: setNonce when it's empty
Aladdin-97 Jun 15, 2023
7070331
Merge pull request #1 from Aladdin-97/login_hint
piuma Jun 15, 2023
15c2510
resolv conflict
piuma Nov 8, 2023
b493b44
fix: http_build_query(): Passing null to parameter #2 ($numeric_prefi…
piuma Nov 8, 2023
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
43 changes: 30 additions & 13 deletions src/OpenIDConnectClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,11 @@ public function authenticate() {
throw new OpenIDConnectClientException('Got response: ' . $token_json->error);
}

// Sometime getState() return an empty string
// and the authentication process fail
if ($this->getState() == "")
$this->setState($_REQUEST['state']);
Comment on lines +312 to +313
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens when the session does not contain the state.

Does this introduce security issues?
It looks to me that it is now possible to reload the page with the ?code=<somecode>&state=<some state>. Instead that it was normally not possible because the state was unset.

Maybe the specs says something about this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should for sure not do this - if the session does not hold the necessary values there is something broken.

Same for the nonce below


// Do an OpenID Connect session check
if ($_REQUEST['state'] !== $this->getState()) {
throw new OpenIDConnectClientException('Unable to determine state');
Expand Down Expand Up @@ -339,6 +344,13 @@ public function authenticate() {
// Save the access token
$this->accessToken = $token_json->access_token;

// During verifyJWTclaims sometime return an empty string (probably caused by the session timeout between KC and Client)
// Which cause issue in the *_auth.php, this should "patch" the randomic emptiness.
if ($this->getNonce() == "") {
$this->setNonce($claims->nonce);
user_error('Warning: Function getNonce return empty, setting in the session!');
};

// If this is a valid claim
if ($this->verifyJWTclaims($claims, $token_json->access_token)) {

Expand Down Expand Up @@ -430,27 +442,32 @@ public function authenticate() {
* Connect provider that the end-user has logged out of the relying party site
* (the client application).
*
* @param string $accessToken ID token (obtained at login)
* @param string $enduserIdentification ID token (obtained at login) or username for $method like as "login_hint"
* @param string|null $redirect URL to which the RP is requesting that the End-User's User Agent
* be redirected after a logout has been performed. The value MUST have been previously
* registered with the OP. Value can be null.
* @param string $method The way of identifying the end-user for whom authentication is being requested.
* The default setting is "id_token_hint". There are two modes supported: "login_hint" and "id_token_hint"
*
* @throws OpenIDConnectClientException
*/
public function signOut($accessToken, $redirect) {
$signout_endpoint = $this->getProviderConfigValue('end_session_endpoint');
public function signOut($enduserIdentification, $redirect, $method = "id_token_hint") {
if (!in_array($method, array("id_token_hint", "login_hint")))
throw new OpenIDConnectClientException('method must be "id_token_hint" or "login_hint"');

$signout_endpoint = $this->getProviderConfigValue("end_session_endpoint");

$signout_params = null;
if($redirect === null){
$signout_params = array('id_token_hint' => $accessToken);
$signout_params = array($method => $enduserIdentification);
}
else {
$signout_params = array(
'id_token_hint' => $accessToken,
$method => $enduserIdentification,
'post_logout_redirect_uri' => $redirect);
}

$signout_endpoint .= (strpos($signout_endpoint, '?') === false ? '?' : '&') . http_build_query( $signout_params, null, '&', $this->enc_type);
$signout_endpoint .= (strpos($signout_endpoint, '?') === false ? '?' : '&') . http_build_query( $signout_params, '', '&', $this->enc_type);
$this->redirect($signout_endpoint);
}

Expand Down Expand Up @@ -675,7 +692,7 @@ private function requestAuthorization() {
));
}

$auth_endpoint .= (strpos($auth_endpoint, '?') === false ? '?' : '&') . http_build_query($auth_params, null, '&', $this->enc_type);
$auth_endpoint .= (strpos($auth_endpoint, '?') === false ? '?' : '&') . http_build_query($auth_params, '', '&', $this->enc_type);

$this->commitSession();
$this->redirect($auth_endpoint);
Expand All @@ -701,7 +718,7 @@ public function requestClientCredentialsToken() {
);

// Convert token params to string format
$post_params = http_build_query($post_data, null, '&', $this->enc_type);
$post_params = http_build_query($post_data, '', '&', $this->enc_type);

return json_decode($this->fetchURL($token_endpoint, $post_params, $headers));
}
Expand Down Expand Up @@ -736,7 +753,7 @@ public function requestResourceOwnerToken($bClientAuth = FALSE) {
}

// Convert token params to string format
$post_params = http_build_query($post_data, null, '&', $this->enc_type);
$post_params = http_build_query($post_data, '', '&', $this->enc_type);

return json_decode($this->fetchURL($token_endpoint, $post_params, $headers));
}
Expand Down Expand Up @@ -782,7 +799,7 @@ protected function requestTokens($code) {
}

// Convert token params to string format
$token_params = http_build_query($token_params, null, '&', $this->enc_type);
$token_params = http_build_query($token_params, '', '&', $this->enc_type);

$this->tokenResponse = json_decode($this->fetchURL($token_endpoint, $token_params, $headers));

Expand All @@ -809,7 +826,7 @@ public function refreshToken($refresh_token) {
);

// Convert token params to string format
$token_params = http_build_query($token_params, null, '&', $this->enc_type);
$token_params = http_build_query($token_params, '', '&', $this->enc_type);

$json = json_decode($this->fetchURL($token_endpoint, $token_params));

Expand Down Expand Up @@ -1424,7 +1441,7 @@ public function introspectToken($token, $token_type_hint = '', $clientId = null,
$clientSecret = $clientSecret !== null ? $clientSecret : $this->clientSecret;

// Convert token params to string format
$post_params = http_build_query($post_data, null, '&');
$post_params = http_build_query($post_data, '', '&');
$headers = ['Authorization: Basic ' . base64_encode(urlencode($clientId) . ':' . urlencode($clientSecret)),
'Accept: application/json'];

Expand Down Expand Up @@ -1455,7 +1472,7 @@ public function revokeToken($token, $token_type_hint = '', $clientId = null, $cl
$clientSecret = $clientSecret !== null ? $clientSecret : $this->clientSecret;

// Convert token params to string format
$post_params = http_build_query($post_data, null, '&');
$post_params = http_build_query($post_data, '', '&');
$headers = ['Authorization: Basic ' . base64_encode(urlencode($clientId) . ':' . urlencode($clientSecret)),
'Accept: application/json'];

Expand Down