Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

oauth2 logins do not work #587

Closed
zyclonite opened this issue Nov 15, 2018 · 11 comments
Closed

oauth2 logins do not work #587

zyclonite opened this issue Nov 15, 2018 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@zyclonite
Copy link

Bug Report

Steps to Reproduce

configure google as oauth2 provider and login

Expected Behavior

working login

Actual Behavior

when the auth code is received on the api (/auth/sso/google/callback) it throws an internal server error with the following log statement
Error: Call to undefined method League\OAuth2\Client\Provider\Exception\IdentityProviderException::getErrorCode() in /var/www/html/src/endpoints/Auth.php:233

Other Context & Screenshots

Technical Details

  • Device: *
  • OS: Linux
  • Database: mariadb 10.3.10
  • Install Method: docker run ... directus/api:2.0.7
@rijkvanzanten rijkvanzanten added the bug Something isn't working label Nov 15, 2018
@rijkvanzanten
Copy link
Member

Is this same error popping up for every oAuth provider? Or just google?

@zyclonite
Copy link
Author

twitter does not work at all but i think this is using oauth1...
github does run into the same error, so i guess its all providers for oauth2

@srjrol
Copy link

srjrol commented Nov 16, 2018

I get the same error on 7.07. I had opened this issue but I guess you were able to connect somehow so it was closed. https://github.com/directus/app/issues/1113

{"error":{"code":null,"message":"Call to undefined method League\OAuth2\Client\Provider\Exception\IdentityProviderException::getErrorCode()","class":"Error","file":"/home/drivve5/public_html/directus/src/endpoints/Auth.php","line":233}}

@rijkvanzanten
Copy link
Member

@srjrol that was another error message though. The good news is that you both have the same error. The bad news is is that it's actually working as expected for me 🧐

@zyclonite
Copy link
Author

i checked the code... so far i can tell, there is a bug in the catch block...
the exception thrown is an IdentityProviderException which extends \Exception and there is not method getErrorCode (only getCode)
the method getErrorCode relates to directus/Exception and not the plain php \Exception...

so before calling getErrorCode https://github.com/directus/api/blob/94269268bec54fbfb8f994010d3114800bf91c2b/src/endpoints/Auth.php#L233
there should be a block like

if (!$e instanceof IdentityProviderException) {
    $urlParams['code'] = $e->getErrorCode();
}

@rijkvanzanten
Copy link
Member

@wellingguzman 🔔

@benhaynes benhaynes added this to Needs triage in Bug Triage via automation Nov 21, 2018
@benhaynes benhaynes added this to To do in v2.0.9 via automation Nov 21, 2018
@wellingguzman wellingguzman moved this from Needs triage to Low priority in Bug Triage Nov 21, 2018
@wellingguzman wellingguzman moved this from To do to In progress in v2.0.9 Nov 21, 2018
@wellingguzman
Copy link
Contributor

I updated the code to make sure to call getErrorCode only on Directus Exceptions, otherwise it will fallback to 0. Added by f6f3a7a

@zyclonite you can now check the master branch to see if this change at least give you the correct error.

@wellingguzman wellingguzman moved this from In progress to To do in v2.0.9 Nov 21, 2018
@zyclonite
Copy link
Author

@wellingguzman yes that helps

logging this errors (if not thrown) would be really important... in my case i had google+ api not enabled, so auth was failing but i could not figure out why...

as google+ will be closed down in the near future, i guess the api and scope will be gone as well...
having the scope set to email only looks good -> https://github.com/directus/api/blob/167bda705a677da60854878f4498158ba32fbd1d/public/extensions/core/auth/google/Provider.php#L21
but it looks like the vendor dependency does require the google plus api
https://github.com/thephpleague/oauth2-google/blob/1001ecf1f0/src/Provider/Google.php#L58
which can be prevented if you would upgrade to 2.2.x version and use openid
https://github.com/thephpleague/oauth2-google/blob/master/src/Provider/Google.php#L66

@wellingguzman wellingguzman moved this from To do to In progress in v2.0.9 Nov 23, 2018
@wellingguzman
Copy link
Contributor

@zyclonite interesting, I will check this out, I will improve the auth configuration to allow user to set options that can be passed to the dependency, so useOidcMode can be used.

In the meantime, If someone want to jump in a fix/implement this in a PR that will be great (and I can help them).

At the moment we only want to use useOidcMode as we don't want to know the user details at the moment beside their email.

@wellingguzman wellingguzman moved this from In progress to To do in v2.0.9 Nov 23, 2018
@wellingguzman wellingguzman added this to To do in v2.0.10 Nov 26, 2018
@wellingguzman wellingguzman moved this from To do to In progress in v2.0.10 Dec 3, 2018
@rijkvanzanten rijkvanzanten added this to To do in v2.0.11 Dec 3, 2018
Bug Triage automation moved this from Low priority to Closed Dec 3, 2018
v2.0.11 automation moved this from To do to Done Dec 3, 2018
@wellingguzman
Copy link
Contributor

wellingguzman commented Dec 3, 2018

As it was brought to our attention by @zyclonite, by default the Google OAuth provider library uses the Google+ API. You should either enable the Google+ API or set use_oidc_mode => true in your configuration. I didn't want to set this was the default to prevent issues with current users that already uses the Google+ API. I don't know if there actually any difference, but I wanted to go safe due to my lack of knowledge here.

In Summary:

  • In your google auth configuration add 'use_oidc_mode' => true
  • Or Enable the Google+ API

@zyclonite
Copy link
Author

@wellingguzman tested oidc mode with version 2.0.11, works great, thank you

samvasko pushed a commit to samvasko/api that referenced this issue Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
Bug Triage
  
Closed
v2.0.10
  
In progress
v2.0.11
  
Done
v2.0.9
  
To do
Development

No branches or pull requests

4 participants