OpenIdConnect refreshes should probably validate the ID Token

[davidhalter]

Expected behaviour

OpenIdConnectAuth should reprocess the ID Token if refresh_token is called.

Actual behaviour

The ID Token is nly validated whenever OpenIdConnect.request_access_token is called. It is not validated in the refresh_token case.

While the ID Token is not mandatory for refresh response, it may be provided (see https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse). The OIDC specification unfortunately does not make it clear what clients should do with the ID Token. In my opinion it is better in that case to validate it, since it is going to be used to update user_details in social_core/pipeline/user.py. While this might not seem relevant to some people, because everything works well if the server works correctly, I feel like security is at its best if both sides verify everything, even if it is "not that important".

My suggestion is to add this method to the OpenIdConnect class:

def process_refresh_token_response(self, response, *args, **kwargs):
    json = super().process_refresh_token_response(response, *args, **kwargs)
    id_token = json.get('id_token')
    if id_token is not None:
        self.validate_and_return_id_token(id_token, json['access_token'], check_nonce=False)
    return json

As a small note: Checking the nonce does not work anymore since it was deleted from the database earlier, so we would need to write a few lines of code to make sure it's not checked.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[davidhalter]

I think this is still relevant if I get feedback I'm happy to shoot over a pull request.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[davidhalter]

Still relevant.

[nijel]

It makes sense to validate the token as you describe, but my knowledge of OpenIdConnect is zero, that's why I was not commenting on this issue...

[davidhalter]

No worries.

Since I maintain another big Python repository (Jedi), would it make sense to add me as a collaborator to work on only the OpenID Connect part of this software? I won't promise to work on the rest of the issues, but I'm happy to work on OpenID Connect. We use it in our company and I'm pretty sure we are working as close to the specification as possible. There's also #591 (by me), which is definitely a bug in this software, because it's not how OpenID Connect works.

[nijel]

See https://github.com/python-social-auth/social-core/issues/539 for the process (@omab is taking care of that).