feat: Allow specifying first and last name during registration/signin#3427
feat: Allow specifying first and last name during registration/signin#3427AshCorr wants to merge 2 commits into
Conversation
1507f10 to
7678c0d
Compare
7879f9b to
b283bb3
Compare
b283bb3 to
9e94dd2
Compare
9e94dd2 to
724ad2b
Compare
724ad2b to
01a459b
Compare
| confirmationPagePath?: StartIdxFlowParams['authorizationCodeFlowOptions']['confirmationPagePath']; | ||
| }): Promise<void> => { | ||
| const { email = '' } = req.body; | ||
| const { email = '', firstName, secondName } = req.body; |
There was a problem hiding this comment.
if they're missing, they'll be null I guess?
|
Clever solution! I guess we would need to be careful with this if it was something more security sensitive because (I think...) it would allow someone to send someone else a link that prefilled this data with query params and then when they authenticated it would get submitted without them necessarily being aware, but honestly in this case with it just being their name I don't think we need to worry about that. |
| if (authState.data?.firstName || authState.data?.lastName) { | ||
| await updateUser(sub, { | ||
| profile: { | ||
| firstName: authState.data.firstName, | ||
| lastName: authState.data.lastName, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
We probably need to do some error handling and checking if 1 field is null but the other isnt here, but we can do that in a follow up PR when we actually begin implementing the multiple account flow.
There was a problem hiding this comment.
Oh. Copilot did it for me... Hello?
We use POST requests rather than GET requests for our forms, so information like the passcode, first name, and last name aren't transmitted as query parameters, but instead as part of the request body. In theory meaning that you couldn't make a URL that prefills any of these fields. The only field we do prefill sometimes is the email, but we have a specific query parameter for that 1 field. If we were still using magic links it could be a risk as you could trigger a magic link to get sent to someone, but even then its not particularly feasible as you can only open a magic link in the same browser that triggered it to be sent in the first place. |
| try { | ||
| await updateUser(sub, { | ||
| profile: { | ||
| firstName: authState.data.firstName, |
There was a problem hiding this comment.
yeah so I guess authState.data is definitely not null, because at least one of the values tested at line 430 has to be true, and it's ok for either the firstName or the lastName in the updateUser() to be null because they are optional fields.
What does this change?
Adds 2 fields to the sign-in forms which accept a string value for the users
firstNameandlastNameproperties. At the end of the authorization flow these are then used to update the users Okta profile to set theirfirstNameandlastNameproperties.Note that this is a backend change at the moment, none of our login forms actually include fields for entering these details yet. Eventually we'd like to update the IFramed Sign-in flow to let guest users submit their name when finishing their Guardian account.
Flow
firstNameandlastNamefields from the sign-in/registration request.firstNameandlastNamein the Authorization state cookie.firstNameandlastNamecookies from the Authorization state cookie and update the users Okta profile.Alternatively we could simply set the users first and last name in their Okta profile the moment they submit the login form, however that could potentially allow malicious actors to modify a users first and last name without verifying that they own the account. By pushing it to the end of the authorization flow we can ensure that the user legitimately owns the account.
How has this change been tested?
Ran locally and modified the HTML of the sign in form to have 2 extra fields for firstName and lastName