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

Bug fix: Not getting correct user when using a custom oauth authenticator #932

Closed
wants to merge 1 commit into from

Conversation

arctouch-jonathansouza
Copy link

Hi everyone,

we are facing a issue here that the logged user cannot be found when using a custom OAuth authenticator. This patch resolves this issue, but I really don't know if this change is valid or not. Please, let me know if this solution is correct or there is another way to resolve this issue!

@codecov-io
Copy link

Current coverage is 90.53%

Merging #932 into master will decrease coverage by -0.77% as of 8850d77

@@            master    #932   diff @@
======================================
  Files           72      72       
  Stmts         4301    4301       
  Branches       862     862       
  Methods          0       0       
======================================
- Hit           3927    3894    -33
  Partial         10      10       
- Missed         364     397    +33

Review entire Coverage Diff as of 8850d77

Powered by Codecov. Updated on successful CI builds.

@flovilmart
Copy link
Contributor

Can you add tests that shows how the fix is supposedly behaving? Thanks!

@flovilmart
Copy link
Contributor

@arctouch-jonathansouza I can't reproduce the problem you're describing with unit tests on master. That doesn't seems safe to use this key instead of user.

How is your custom oAuth provider configured?

@gfosco
Copy link
Contributor

gfosco commented Mar 9, 2016

If this did fix something, the right fix would be elsewhere, as the 'rest' format shouldn't know about an implementation detail like _p_.

@flovilmart
Copy link
Contributor

@gfosco I've added unit tests checking of id conformance and I can't really reproduce... The _p_user don't make any sense to me

@flovilmart
Copy link
Contributor

I belive the problem is that at one point, the client is sending a null in the {authData: {facebook: null}} that unlinks the user....

@tanmays
Copy link

tanmays commented Mar 9, 2016

I'm not sure if its related but there was a similar problem using oAuth wherein incorrect user was linked to the sessionToken which was fixed by #857

@flovilmart
Copy link
Contributor

@tanmays good catch oauth and Facebook rely on the same implementation! That's why it's impossible to reproduce on master :)

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.

6 participants