zorn-v / nextcloud-social-login

GNU Affero General Public License v3.0
199 stars 138 forks source link

[OpenID Connect] Not able to login from ERPNext #275

Closed rtdany10 closed 3 years ago

rtdany10 commented 3 years ago

This is my OpenID Connect configuration on Nextcloud. image

The first one is a modified ERPNext v13, second one a ERPNext v12 and third one an untouched ERPNext v13.

I can successfully login using the v12 one. Other two throws an error. Screenshot of the error is attached. Nothing on the console log or log files. image

ERPNext returns standard response which can be found at https://frappeframework.com/docs/user/en/guides/integration/rest_api/oauth2#openid-user-info-endpoint

zorn-v commented 3 years ago

Try to dump provider responses https://github.com/zorn-v/nextcloud-social-login/issues/268#issuecomment-847263180 https://github.com/zorn-v/nextcloud-social-login/issues/268#issuecomment-847489998

rtdany10 commented 3 years ago

Sorry for the delayed reply. Providers response:

Hybridauth\Data\Collection Object ( [collection:protected] => stdClass Object ( [sub] => removed [name] => Dany Robert [given_name] => Dany [family_name] => Robert [email] => danyrt@removed.com [picture] => [roles] => Array ( [0] => Customer [1] => All [2] => Guest ) [iss] => https://removed.com ) )

zorn-v commented 3 years ago

Is there are slashes in [sub] => removed ? Can you show "removed" with alphanumeric masked ?

I bet, problem in this. I was do it for OpenID (not connect) - it uses urls for identifiers sometimes. https://github.com/zorn-v/nextcloud-social-login/blob/c420d85242e153c16933a7a5d9ba453ab90af5c7/lib/Service/ProviderService.php#L323

rtdany10 commented 3 years ago

Is there are slashes in [sub] => removed ? Can you show "removed" with alphanumeric masked ?

I bet, problem in this. I was do it for OpenID (not connect) - it uses urls for identifiers sometimes.

https://github.com/zorn-v/nextcloud-social-login/blob/c420d85242e153c16933a7a5d9ba453ab90af5c7/lib/Service/ProviderService.php#L323

No slashes actually. 69c33708f03ae75bd7e54a8fb9dbcac11d54cff nc

zorn-v commented 3 years ago

Then, you doing something wrong and logs not from "problem" requests.

rtdany10 commented 3 years ago

Then, you doing something wrong and logs not from "problem" requests.

Haven't been able to login even once. Throws the same error always.

Is your "test suite" isolated from logins ?

I didn't understand 🤔

zorn-v commented 3 years ago

"Can not get identifier" - if "sub" not provided. In your case it is provided. And there is no slashes etc.

zorn-v commented 3 years ago

So try to find problem by yourself

rtdany10 commented 3 years ago

Not a pro with php, is there anyway I can log $profile to browser console so that the flow isn't interrupted and I can see the values in console as well.

rtdany10 commented 3 years ago

Printed out profile from lib/Service/ProviderService.php after $profile = $adapter->getUserProfile();

Hybridauth\User\Profile Object ( [identifier] => [webSiteURL] => [profileURL] => [photoURL] => [displayName] => Guest [description] => [firstName] => [lastName] => [gender] => [language] => [age] => [birthDay] => [birthMonth] => [birthYear] => [email] => guest@example.com [emailVerified] => [phone] => [address] => [country] => [region] => [city] => [zip] => [data] => Array ( ) )

Data doesn't seem to have reached here from getUserProfile() function.

rtdany10 commented 3 years ago

I was able to solve the issue. The issue was in getUserProfile() function.

The issue was with $data = new Data\Collection($user);. This variable $data had values of Guest user. I tried printing it out using print_r($data) and I noticed it was getting executed just after I was redirected to login page of ERPNext(I didn't enter my credentials nor was I logged in).

The thing is, ERPNext assigns every non-logged in user as a "Guest" user and hence cookies are set accordingly. Social Login misunderstood this(not really misunderstanding, but for the sake of readers) for my actual users and populated $userProfile with data of Guest user, which apparently doesn't have a sub due to obvious reasons.

To overcome this situation, I had to remove the checks and set sub from $profile. Here is my updated code:

$profile = new Data\Collection( $this->apiRequest($userInfoUrl) );
$userProfile->identifier = $profile->get('sub');
$userProfile->displayName = $profile->get('name');
$userProfile->photoURL = $profile->get('picture') ?: $profile->get('avatar');
if (preg_match('#<img.+src=["\'](.+?)["\']#', $userProfile->photoURL, $m)) {
    $userProfile->photoURL = $m[1];
}
$userProfile->email = $profile->get('email');
if (null !== $groups = $this->getGroups($profile)) {
    $userProfile->data['groups'] = $groups;
}

Thank you for your time, @zorn-v And thank you for this wonderful app. Much love!