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 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
Changes from 4 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
27 changes: 22 additions & 5 deletions src/OpenIDConnectClient.php
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,23 +442,28 @@ 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);
}

Expand Down