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

ask to transform inline function "load_key" to method of OpenIDMixin #610

Open
danilovmy opened this issue Dec 21, 2023 · 0 comments
Open
Assignees
Labels

Comments

@danilovmy
Copy link

Hello, to make existing code more usable, I propose reimagining the OpenIDMixin.parse_id_token method. This method contains an inline function definition def load_key(header, _), and I don't see any reason why this function is not a method of the OpenIDMixin. Moreover, this function uses other self.methods, which marks they as a method of OpenIDMixin rather than a standalone function. Lastly, if it were a method, it could be easily tested and used for other purposes. For example, at present, there's a minor bug in functionality as it does not raise an error if new keys are loaded but still do not contain the desired 'kid'.

Before:

#  authlib\integrations\base_client\sync_openid.py
class OpenIDMixin(object):
    ...

    def parse_id_token(self, token, nonce, claims_options=None, leeway=120):
        """Return an instance of UserInfo from token's ``id_token``."""
        if 'id_token' not in token:
            return None

        def load_key(header, _):
            jwk_set = JsonWebKey.import_key_set(self.fetch_jwk_set())
            try:
                return jwk_set.find_by_kid(header.get('kid'))
            except ValueError:
                # re-try with new jwk set
                jwk_set = JsonWebKey.import_key_set(self.fetch_jwk_set(force=True))
                return jwk_set.find_by_kid(header.get('kid'))
      ...

After suggested refactoring:

#  authlib\integrations\base_client\sync_openid.py
class OpenIDMixin(object):
    ...
    def load_key(self, header, force=False):
        jwk_set = JsonWebKey.import_key_set(self.fetch_jwk_set())
        try:
            return jwk_set.find_by_kid(header.get('kid'))
        except ValueError:
            if not force:  # re-try with new jwk set
                return self.load_key(header, force=True)
            raise RuntimeError('Missing "kid" in "jwk_set"')

    def parse_id_token(self, token, nonce, claims_options=None, leeway=120):
        """Return an instance of UserInfo from token's ``id_token``."""
        if 'id_token' not in token:
            return None
      ...
      claims = _jwt.decode(
            token['id_token'], key=self.load_key,
            ...
        )
      ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants