Skip to content
This repository was archived by the owner on May 9, 2022. It is now read-only.

Facebook Registration Flow Cleanup #867

Open
jsharkey13 opened this issue Jul 17, 2017 · 1 comment
Open

Facebook Registration Flow Cleanup #867

jsharkey13 opened this issue Jul 17, 2017 · 1 comment
Assignees
@jsharkey13
Labels
enhancement

Comments

@jsharkey13
Copy link
Member

I've seen this registration failure maybe a dozen times recently; I don't know how the callback endpoint gets hit with a user object that ends up being all null mind you! It happened three times today, but that may have been the same user. Could it be one of the providers not sending back all the information we're expecting?

[2017-07-17 13:08:19] [ERROR] AuthenticationFacade:261 - Internal Database error during authentication
uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException: Postgres exception
    at uk.ac.cam.cl.dtg.segue.dao.users.PgUsers.createUser(PgUsers.java:515)
    at uk.ac.cam.cl.dtg.segue.dao.users.PgUsers.createOrUpdateUser(PgUsers.java:392)
    at uk.ac.cam.cl.dtg.segue.dao.users.PgUsers.registerNewUserWithProvider(PgUsers.java:63)
    at uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager.registerUserWithFederatedProvider(UserAccountManager.java:1311)
    at uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager.authenticateCallback(UserAccountManager.java:290)
    at uk.ac.cam.cl.dtg.segue.api.AuthenticationFacade.authenticationCallback(AuthenticationFacade.java:236)
    ...
Caused by: org.postgresql.util.PSQLException: ERROR: null value in column "email" violates not-null constraint
  Detail: Failing row contains (63702, null, null, null, null, STUDENT, null, null, 2017-07-17 13:08:19.482, null, null, null, NOT_VERIFIED, null, null, null, null, null, null, null).
    ...
    at uk.ac.cam.cl.dtg.segue.dao.users.PgUsers.createUser(PgUsers.java:500) ~[classes/:?]
    ... 47 more

The only non-null rows are those either generated by the database, or default values. Maybe we're not validating the callback carefully enough?
@jsharkey13 jsharkey13 added the bug label Jul 17, 2017
jsharkey13 added a commit to isaacphysics/isaac-api that referenced this issue Jul 27, 2017
@jsharkey13
Fix Facebook sign-up flow errors in simplest way
5670926 
# We're currently seeing errors with null user object fields when users try to
# register with Facebook. This seems to be because their API changed recently
# to stop sending all info when requesting the user info endpoint. We now have
# to be explicit about what we want from that endpoint. May want refactoring later,
# but this works and it's an urgent issue!
# Requires the line:
# FACEBOOK_USER_FIELDS=id,first_name,last_name,email,verified
# to be added to the segue-config.properties file to work.
# Relevant to isaacphysics/isaac-app#867
@jsharkey13
Copy link
Member Author

So this turned out to be Facebook, and it happens because the only thing they now return when you request the user info endpoint is the ID of the user. You need to manually and explicitly request the fields you want. Meurig found this: https://developers.facebook.com/docs/graph-api/using-graph-api/#reading

The way I've fixed it isn't too neat, so it may want looking at by @sacummins at some point!

jsharkey13 added a commit to isaacphysics/isaac-api that referenced this issue Jul 27, 2017
@jsharkey13
Fix broken FacebookAuthenticator test
2f5ffdd 
# This should have been in the last commit, and also needs review!
# Relevant to isaacphysics/isaac-app#867
@jsharkey13 jsharkey13 changed the title Errors During User Registration from 3rd Party Providers Facebook Registration Flow Cleanup Jul 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jsharkey13 jsharkey13

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sacummins @jsharkey13