jumbojett / OpenID-Connect-PHP

Minimalist OpenID Connect client
https://github.com/jumbojett/OpenID-Connect-PHP
Apache License 2.0
618 stars 368 forks source link

Authentication with Azure AD fails #190

Open Shachar opened 4 years ago

Shachar commented 4 years ago

I set up a wiki to connect to Azure Active Directory using OpenId connect. Installation went seemingly fine, but when I try to actually log-in, I get directed to the Azure installation, log in successfully there, and then redirected back to the wiki, where I get "Fatal error authenticating user. "

Turning on logging, I see the following error in the logs:

[OpenID Connect] Jumbojett\OpenIDConnectClientException: Unable to find a key for (algorithm, kid):RS256, R23ciW2cfZ50VtxOnk_xvJ68bcs) in /srv/www/wiki.driveu.auto/internal/vendor/jumbojett/openid-connect-php/src/OpenIDConnectClient.php:738
Stack trace:
#0 /srv/www/wiki.driveu.auto/internal/vendor/jumbojett/openid-connect-php/src/OpenIDConnectClient.php(825): Jumbojett\OpenIDConnectClient->get_key_for_header(Array, Object(stdClass))
#1 /srv/www/wiki.driveu.auto/internal/vendor/jumbojett/openid-connect-php/src/OpenIDConnectClient.php(279): Jumbojett\OpenIDConnectClient->verifyJWTsignature('eyJ0eXAiOiJKV1Q...')
#2 /srv/www/wiki.driveu.auto/internal/extensions/OpenIDConnect/src/OpenIDConnect.php(161): Jumbojett\OpenIDConnectClient->authenticate()
#3 /srv/www/wiki.driveu.auto/internal/extensions/PluggableAuth/includes/PluggableAuthLogin.php(31): OpenIDConnect->authenticate(NULL, NULL, NULL, NULL, NULL)
#4 /srv/www/wiki.driveu.auto/internal/includes/specialpage/SpecialPage.php(569): PluggableAuthLogin->execute(NULL)
#5 /srv/www/wiki.driveu.auto/internal/includes/specialpage/SpecialPageFactory.php(558): SpecialPage->run(NULL)
#6 /srv/www/wiki.driveu.auto/internal/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Object(Title), Object(RequestContext))
#7 /srv/www/wiki.driveu.auto/internal/includes/MediaWiki.php(865): MediaWiki->performRequest()
#8 /srv/www/wiki.driveu.auto/internal/includes/MediaWiki.php(515): MediaWiki->main()
#9 /srv/www/wiki.driveu.auto/internal/index.php(42): MediaWiki->run()
#10 {main}

Adding a log line before that line, it seems that it is looking for the key "RS256" in the json it tries to get from https://login.microsoftonline.com/e55be731-abdc-4bff-a054-abd200632cc4/discovery/v2.0/keys, but no such json key exists there.

I submitted this problem on https://phabricator.wikimedia.org/T239794, and was told this might be a more correct forum.

SimonDatapas commented 4 years ago

Ran into the same issue here for one of our clients.

The problem lies in the claims-mapping feature of Azure where it derives slightly from the OAuth2.0 flow. Stated in the Microsoft documentation: (https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-protocols-oidc)

If your app has custom signing keys as a result of using the claims-mapping feature, you must append an appid query parameter containing the app ID in order to get a jwks_uri pointing to your app's signing key information. For example: https://login.microsoftonline.com/{tenant}/v2.0/.well-known/openid-configuration?appid=6731de76-14a6-49ae-97bc-6eba6914391e contains a jwks_uri of https://login.microsoftonline.com/{tenant}/discovery/v2.0/keys?appid=6731de76-14a6-49ae-97bc-6eba6914391e.

AND

When your web app needs to authenticate the user, it can direct the user to the /authorize endpoint. This request is similar to the first leg of the OAuth 2.0 authorization code flow, with these important distinctions:

  • The request must include the openid scope in the scope parameter.
  • The response_type parameter must include id_token.
  • The request must include the nonce parameter.

A way to support this is to add a way of adding the clientID to the url when fetching the wellKnown, see example below:

private function getWellKnownConfigValue($param, $default = null) {

// If the configuration value is not available, attempt to fetch it from a well known config endpoint
// This is also known as auto "discovery"
if(!$this->wellKnown) {
    $well_known_config_url = rtrim($this->getProviderURL(),'/') . '/.well-known/openid-configuration'
    . '?appid=' . $this->clientID; // <-- EXAMPLE MODIFICATION
    $this->wellKnown = json_decode($this->fetchURL($well_known_config_url));
}

Perhaps a setter for the well-known could be added or some other abstract way to provide this key-value pair to this request.

Shachar commented 4 years ago

I ended up a different approach. For my own use, I did not need that mapping. Sadly, MS support sounded guarded when it became apparent we need to deconfigure it, and we ended up deleting and re-defining the app. I made it clear to them that I consider it them violating the standard, and therefor a bug.

Longer term, we're just switching to a different SSO provider.

nicolus commented 2 years ago

Over 2 years later I found this issue while researching exactly the same problem with an Azure AD Identity provider : Unable to find a key for (algorithm, kid):RS256

Good news is there's now a clean solution. @SimonDatapas 's explanation above is absolutely correct : Azure AD requires you to pass a ?appid=<clientid> param to the well known configuration endpoint, so that it will add the param to the jwks_uri, which in turn will provide you with the correct certificate for your app. However hardcoding it in the lib is not really a viable solution.

There's now a setWellKnownConfigParameters() method that will add params to the well known config, so basically all you need to do is add the appid param when you instantiate the client :

// Assuming $identity_provider_url, $client_id and $client_secret were previously set : 

$oidcClient = new OpenIDConnectClient(
    $identity_provider_url,
    $client_id,
    $client_secret
);

$oidcClient->setWellKnownConfigParameters(['appid' => $client_id]);

$oidcClient->authenticate()

Hope this helps someone !

ashleyw-gh commented 3 months ago

so Ive just tried to dial this into MediaWiki using the https://www.mediawiki.org/wiki/Extension:OpenID_Connect plugin, and when I dialed the config into AzureAd, It didn't work. I ended up having to change line 654 in '/OpenIDConnect/vendor/jumbojett/openid-connect-php/src/OpenIDConnectClient.php' from; $well_known_config_url = rtrim($this->getProviderURL(), '/') . '/.well-known/openid-configuration'; to; $well_known_config_url = rtrim($this->getProviderURL(), '/') . '/.well-known/openid-configuration' . '?appid=' . $this->clientID;

Is this perhaps something that the author of OpenID-Connect-PHP can work with the MediaWiki plugin owner to fix as AzureAD is becoming common for OpenID these days as it would have saved me a lot of time? any thoughts @cicalese In terms of the OpenID mediawiki plugin the only change that is required is to OpenIDConnect/includes/OpenIDConnect.php and to add this code segment at line 223;

   $this->openIDConnectClient->setWellKnownConfigParameters(['appid' =>
            $this->getData()->get( 'clientID' ) ]
   );

If the second approach is taken then the upstream /OpenIDConnect/vendor/jumbojett/openid-connect-php/src/OpenIDConnectClient.php file does not need to be modified, but given that AzureAD/Entra and OpenID is a common enough solution it would be great if the best place for code changes to be made could be discussed, in-case there are other libraries other than the MediaWiki OpenID plugin that is impacted.