Skip to content

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.

@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.

1 participant