loginpass icon indicating copy to clipboard operation
loginpass copied to clipboard

Loginpass + Azure - claim validation error for claim "iss"

Open blackdwarf opened this issue 5 years ago • 7 comments

What I did?

Installed loginpass using pipenv. Used the example to pull in Google and Azure provider. Tried to log in using Azure provider.

The authorization is below, it is incredibly simple. The rest is boilerplate loginpass code.

def handle_authorization(remote, token, user_info):
    printer.pprint(user_info)
    return user_info

What should happen?

The flow should complete and I should get the user_info dumped to the screen.

What actually happened?

An error authlib.jose.errors.InvalidClaimError: invalid_claim: Invalid claim "iss" is thrown and login does not proceed.

What I think is the root cause

I think that loginpass is actually using OpenID Connect to get the metadata from the server for Azure, which returns the default iss claim as a template with the {tenant} placeholder that needs to be replaced with a GUID, in my case a static one (as per https://docs.microsoft.com/en-us/azure/active-directory/develop/id-tokens). Looking at loginpass code, specifically azure.py at line 30, this should work, but I'm still getting the error.

blackdwarf avatar Jul 01 '20 23:07 blackdwarf

Can you leave the discovery endpoint URL? In our code: https://github.com/authlib/loginpass/blob/master/loginpass/azure.py#L34

issuer = issuer.replace('{tenantid}', tenant)

It is replacing {tenantid} instead of {tenant}, I think this is the problem.

lepture avatar Jul 16 '20 04:07 lepture

@lepture I think I figured out what is wrong, and managed to unblock myself, however, there could be a more sinister issue in there.

The docs @ https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-protocols-oidc give all the info needed. Basically, what value is used for the tenant in the loginpass code:

issuer = issuer.replace('{tenantid}', tenant)

is dependent on what tenant was used for the discovery URL. If you put in common, loginpass will try to validate the iss claim in the id_token returned as https://login.microsoftonline.com/common/v2.0, however, that is not what will be returned for personal Microsoft accounts. What will be returned is https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0. This will fail validation.

However, if you use the consumers OpenID Connect URL (https://login.microsoftonline.com/consumers/v2.0/.well-known/openid-configuration), you can see that the issuer is set to https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0 which is correct. If you're wondering why this works (after all consumers != [guid]), it is because the response URL in issuer doesn't even contain {tenantid} string, nothing gets replaced. Thus, to unblock myself, I've simply put AZURE_SERVER_METADATA_URL in my config, set it to the consumers tenant discovery endpoint, and that worked.

But, the issue still persists, since for common, which is configured by default (and I imagine a lot of people will use), you can still get two different issuer responses in the main metadata response when you hit the endpoint, and you can still run into this invalid claim situation. I can't think what a proper fix would be, unfortunately, since the validation code in Authlib is pretty generic and the "fixer" in loginpass, at the time of replacing {tenantid} doesn't really know what will be the returning issuer.

blackdwarf avatar Jul 18 '20 07:07 blackdwarf

It seems like this is a known issue that's not going to be corrected. See MicrosoftDocs/azure-docs#38427.

As an alternative, I made a provider that assumes no OpenID Connect support. E.g.

class Microsoft:
    NAME = 'microsoft'
    OAUTH_CONFIG = {
        'api_base_url':
            'https://graph.microsoft.com/',
        'authorize_url':
            'https://login.microsoftonline.com/common/oauth2/v2.0/authorize',
        'access_token_url':
            'https://login.microsoftonline.com/common/oauth2/v2.0/token',
        'jwks_uri':
            'https://login.microsoftonline.com/common/discovery/v2.0/keys',
        'userinfo_endpoint':
            'https://graph.microsoft.com/oidc/userinfo',
        'client_kwargs': {
            'scope': 'openid email profile'
        },
    }

Microsoft recommends not hard-coding the UserInfo endpoint, though.

vmsp avatar Aug 29 '20 16:08 vmsp

@blackdwarf I've improved create_azure_backend function, I think this will solve all the problem. You can always pass a compliance_fix function to fix the returned value of server metadata.

lepture avatar Oct 31 '20 00:10 lepture

@blackdwarf I've improved create_azure_backend function, I think this will solve all the problem. You can always pass a compliance_fix function to fix the returned value of server metadata.

This isn't released yet, right?

Edit: Nope

I also think it should be a good idea to add some testing to Azure, there isn't nothing related in tests/. Or maybe providing a use example.

mavaras avatar Nov 06 '20 15:11 mavaras

@lepture thanks for the fix!

blackdwarf avatar Nov 25 '20 02:11 blackdwarf