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

Basic authentication header is set regardless of use of basic auth? #120

Open
jberculo opened this issue Mar 13, 2018 · 3 comments
Open

Comments

@jberculo
Copy link

jberculo commented Mar 13, 2018

In the requestTokens function, the following code was added two years ago:

# Consider Basic authentication if provider config is set this way
if (in_array('client_secret_basic', $token_endpoint_auth_methods_supported)) {
    $headers = ['Authorization: Basic ' . base64_encode($this->clientID . ':' . $this->clientSecret)];
    unset($token_params['client_secret']);
}

So if client_secret_basic is one of the possible token endpoint authentication methods, that header will be added, regardless if the method is actually used. For example, for Azure I was using client_secret_post, and that failed, because the client_secret token param is also unset.

Or am I missing something here?

@EAnushan
Copy link

You can disable it as follows:

$oidc->providerConfigParam([
    'token_endpoint_auth_methods_supported' => []
]);

@devicenull
Copy link

I think this ends up breaking logging in with Okta, which throws an error. This was mentioned in #75, but I didn't see a great fix.

Cannot supply multiple client credentials. Use one of the following: credentials in the Authorization header, credentials in the post body, or a client_assertion in the post body.

Okta's well-known config reports:

  "token_endpoint_auth_methods_supported": Array[5][
    "client_secret_basic",
    "client_secret_post",
    "client_secret_jwt",
    "private_key_jwt",
    "none"
  ],

@abulhol
Copy link

abulhol commented Dec 16, 2020

This "auto discovery" of the authentication method (Basic vs. POST) is likely not to work properly, I also got stuck on it using OneLogin. Maybe it would be better to remove it altogether, and add a class property authorizationMethod for it and a public setter method. Default should be POST I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants