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

Rename MetadataEndpoint._grant_types -> _grant_types_supported #808

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bennr01
Copy link

@bennr01 bennr01 commented Mar 17, 2022

Hello,

this PR renames the _grant_types attribute of the MetadataEndpoint to _grant_types_supported.
Initially introduced in 6bd865b, the _grant_types attribute (type list) collides with the _grant_types attribute (type dict) of the TokenEndpoint when inheriting from both classes.

This caused a problem: If I want to create an Server-like combined endpoint inheriting from both the Server and MetadataEndpoint classes and pass a reference to an instance of this class to the constructor of the MetadataEndpoint, initializing the MetadataEndpoint will fail due to it setting self._grant_types to a list but expecting each of the endpoint working on (which, in my case, includes the metadata endpoint itself) to be of type dict.

For example, the following code would fail:

from oauthlib import oauth2


class ServerWithMetadata(oauth2.Server, oauth2.MetadataEndpoint):
    """
    An extension of L{oauth2.Server} featuring the L{oauth2.MetaDataEndpoint}.
    """
    def __init__(
        self,
        request_validator,
        token_expires_in=None,
        token_generator=None,
        refresh_token_generator=None,
        claims={},
        *args,
        **kwargs,
    ):
        oauth2.Server.__init__(
            self,
            request_validator,
            token_expires_in=token_expires_in,
            token_generator=token_generator,
            refresh_token_generator=refresh_token_generator,
            *args,
            **kwargs,
        )
        oauth2.MetadataEndpoint.__init__(
            self,
            [self],
            claims=claims,
            raise_errors=True,
        )

A possible solution is renaming one of the _grant_types attributes. This PR does this.

Copy link
Collaborator

@thedrow thedrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write a test to ensure the bug doesn't regress?

@bennr01
Copy link
Author

bennr01 commented Mar 17, 2022

@thedrow I've added a regression test for the issue. The new test passes with the fix but fails without it. I wasn't exactly sure where to implement it, but as the changes were solely within the code of the MetadataEndpoint I've decided to put the test in the corresponding file. Is this ok?

@bennr01
Copy link
Author

bennr01 commented Apr 28, 2022

Hi, are there any problems with the changes in this pull request? While one of the test build fails, this is only caused by bandit complaining over the user of possible hardcoded tokens (e,g, Bearer and access_token), which are failures unrelated to this PR and mostly false positives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants