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

fix issue where social user saving fails after oauth callback #134

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

zackAJ
Copy link
Contributor

@zackAJ zackAJ commented Oct 2, 2024

Greetings dojo community, thank you for this awesome project

Issue

  • Assuming this project supports laravel 11 ? I'm having an error when using social auth
  • Oauth callback throws an error when saving a SocialProviderUser

image

Reproduce

1- use the Laravel installer and install a fresh latest version application (I used inertia vue stack)
2- install and configure the auth package
3- configure your social auth service (I tried both google and github)
4- try to register with provider, it will fail on the callback when creating a user

Cause

the model SocialProviderUser has a custom (composite?) primaryKey attribute that is an array, see :051748d#diff-79bf4b0babdccd06a4491c7c0c134e34a898e01eac9be350b7430c4184ad44c1R14

this will cause this function in the laravel framework to throw a type error : laravel/framework@92beae3#diff-59d24f1a8f0dd1e51ee7dffc8778133711634e928ecef4a91b63fc3851cb1579R435

array_key_exists() take a $key that must be a primitive, in this case an array is passed ['user_id', 'provider_slug'] which causes a type error:

array_key_exists(): Argument https://github.com/thedevdojo/auth/pull/1 ($key) must be a valid array offset type

The fix

  • Change protected $primaryKey = ['user_id', 'provider_slug']; to protected $primaryKey = 'user_id'; and it will work
  • I don't know tho why it's an array in the first place, I left a comment to the line owner and I didn't get a response

@tnylea tnylea changed the base branch from main to zackAJmain October 25, 2024 18:52
@tnylea
Copy link
Contributor

tnylea commented Oct 25, 2024

Hey @zackAJ!

Thanks for the PR. I'm merging this into a branch on the main repo so we can confirm the tests run and then get it merged into main. Thanks!

@tnylea tnylea merged commit 9ef1248 into thedevdojo:zackAJmain Oct 25, 2024
@tnylea
Copy link
Contributor

tnylea commented Oct 25, 2024

Here's the updated PR ;) #136

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.

2 participants