openid_connect provider

[TheSteveBurgess]

Hi,

Are you open to pull requests for modifications to the socialaccount providers?

I’d love to be able to use the openid_connect provider but pass a boolean in the configuration that made it put the id_token into the extra data field (after validating the token of course)

Thanks.

[pennersr]

Can you elaborate a bit? The token, in decoded form, is available in SocialAccount.extra_data. What is the use case of storing the encoded token? And, would SocialAccount be the proper place even, given that there is also SocialToken ?

[pennersr]

IMO it's a bit arbitrary what happens there in that code. Ideally, I would like to have clarity at all times at what SocialToken.token means, as it stands now it's a bit fuzzy. Options:

1. Keep SocialToken as it is (=representing an access token), and store the ID token in SocialAccount.extra_data in decoded form. Rationale: the ID token is not an access token, typically has a short life time.
2. Introduce SocialToken.id_token and store it there, in encoded form. Rationale: it's a token after all.
3. Clean up SocialToken, introduce a proper type = ACCESS_TOKEN | REFRESH_TOKEN | ID_TOKEN.

Only if there is a clear use case for having the ID token in its original form, option 2 and 3 would make sense.

As for option 1, a few thoughts:

• Given that we have a verified ID token, actually fetching the profile URL is strictly speaking not needed for authentication purposes. So, the OIDC provider could skip fetching. Note that the Google provider is doing this (skipping) as well.
• So, there are actually 2 "modes" in which the OIDC provider could operate: fetch profile, or just use the ID token payload. That could become a configuration parameter.

But, must admit, I am drifting off a bit here. Storing the id_token in the extra data, in decoded form, does make sense. So I would be happy to merge a PR that adds support for something like this:

    SOCIALACCOUNT_PROVIDERS = {
        "openid_connect": {
            "STORE_ID_TOKEN": True,  # <-- NEW
            "APPS": [
                {
                    "provider_id": "my-server",
                    "name": "My Login Server",
                    "client_id": "your.service.id",
                    "secret": "your.service.secret",
                    "settings": {
                        "server_url": "https://my.server.example.com",
                        "store_id_token": True,  # <-- NEW
                    },
                },
            ]
        }
    }

[TheSteveBurgess]

Awesome 😊

Just to confirm your thinking. Would STORE_ID_TOKEN at the openid_connect level be used as the default for all APPS that don’t have a value set for store_id_token at the settings level?

Would you also be open to allowing a configuration option for the value that the UID maps to?

At the moment its mapping to “sub”. However, for something like Azure the sub is unique per application in the tenant. For a large enterprise with lots of applications (which is essentially the environment I’m in) This means it’s hard to 100% join users up between applications. The id token contains the oid which is unique per tenant, which would be a better fit for UID in this instance.

Maybe a setting could be introduce called something like “UID_FIELD_TO_MAP_TO” that lets you specify which field you want the UID to map to. It looks like the UID is set via “extract_uid” which is called from sociallogin_from_response, which is in turned passed extra_data. So it would be possible to add some logic in the extac_uid methoid to check the value of UID_FIELD_TO_MAP_TO and if it exists in the data array use that value. You could either then default to sub it the field doesn’t exist or throw an error (which I think is probably the better option)

[pennersr]

Just to confirm your thinking. Would STORE_ID_TOKEN at the openid_connect level be used as the default for all APPS that don’t have a value set for store_id_token at the settings level?

Yes. Historically, there where only app-global settings. The app specific settings where introduced later on.

Maybe a setting could be introduce called something like “UID_FIELD_TO_MAP_TO”...

Suppose you store the ID token -- I assume that will be in extra_data['id_token']. Meaning, UID_FIELD_TO_MAP_TO needs to be able to handle a dotted notation, id_token.oid. I would also name it just uid_field.

[TheSteveBurgess]

That's a good point, i'll make sure it can handle dotted notation.

I'll get some time assigned and put a PR in.

Am i correct in thinking i should be doing that here: https://codeberg.org/allauth/django-allauth/ ?

[pennersr]

Thank you -- yes.