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

Extending OAuth2Client - isStateless is private #232

Closed
leevigraham opened this issue Mar 18, 2020 · 6 comments · Fixed by #234
Closed

Extending OAuth2Client - isStateless is private #232

leevigraham opened this issue Mar 18, 2020 · 6 comments · Fixed by #234

Comments

@leevigraham
Copy link

I was trying to extend OAuth2Client in my custom Client in order to override the getAccessToken method. However I can't access the isStateless property becuase it's private, in fact there's a bunch of private properties and methods.

How can I create a new provider and just override the one method?

From the docs:

image

I was also planning to override the to do something similar to #230 and override getAccessToken.

Looking at the OAuth2ClientInterface should I be creating a class that implements this interface rather than extends OAuth2Client?

Cheers

@weaverryan
Copy link
Member

Yo @leevigraham!

Hmm. I'm definitely not against making this protected. What's your reason for wanting to override getAccessToken()? In #230, it was to allow passing more options to $this->provider->getAccessToken(), which makes sense - and seemed to me like the one obvious "missing" piece of flexibility in that method. But it seems you have another use-case. Let me know what it is :).

Looking at the OAuth2ClientInterface should I be creating a class that implements this interface rather than extends OAuth2Client?

You, of course, can do this, but I think you are hoping to not have to reinvent-the-wheel in your custom class by reimplementing a lot of logic (which is fair). The main reason we have this client_class is really meant so that you can create a sub-class and override the methods just to hint what the real "user" class is that a few of the methods will return - e.g.

* @return FacebookUser|\League\OAuth2\Client\Provider\ResourceOwnerInterface
- actually changing some behavior is a different thing. That's why I want to know exactly what you'd like to change :)

Cheers!

@weaverryan
Copy link
Member

Ping @leevigraham! I'm holding off on a new major release, just in case your use-case might require additional changes. Can you describe what you want to accomplish?

Thanks!

@leevigraham
Copy link
Author

Thanks for chasing @weaverryan,

I ended up implementing League\OAuth2 directly and creating my own provider that extended the Generic provider.

In my use case I need to pass extra data to the getAccessToken method as described in #230 .

I can't quite remember what else I was trying to do (given all the other madness in the world right now).

@weaverryan
Copy link
Member

Cheers! Thanks for the update :)

@benjaminmal
Copy link

Yo @weaverryan !

First, thanks for your work !

Just to add some use-case to the change you'll make, I need to rewrite the redirect() method by extending OAuth2Client.

In fact, my app is embedded in an iframe which call an authorization server. But it serves X-Frame-Options: DENY header (I guess most authorization servers do the same) which disallow redirections within an iframe.

So I need to redirect via javascript using :

<script type='text/javascript'>
  window.top.location.href = '{AUTHORIZATION_URL}';
</script>

Here is the redirect method I overwrited :

    public function redirect(array $scopes = [], array $options = [])
    {
        if (!empty($scopes)) {
            $options['scope'] = $scopes;
        }

        $url = $this->getOAuth2Provider()->getAuthorizationUrl($options);

        // set the state (unless we're stateless)
        if (!$this->isStateless) {
            $this->getSession()->set(
                self::OAUTH2_SESSION_STATE_KEY,
                $this->getOAuth2Provider()->getState()
            );
        }

        $jsRedirect =   "<script type='text/javascript'>
                            window.top.location.href = '$url';
                        </script>";

        return new Response($jsRedirect);
    }

But as seen above, $isStateless is private. I need to copy the entire OAuth2Client class in order to rewrite only this method and return a Response object, not a RedirectResponse object.

Also, you can consider making $provider protected even if there is getOAuth2Provider().

Thanks a lot !

@weaverryan
Copy link
Member

Thanks for the extra info @benjaminmal :). It seems reasonable - check out #234.

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 a pull request may close this issue.

3 participants