Support basic u13 SSO flow via RPF for Ada - #823
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #823 +/- ##
==========================================
+ Coverage 41.00% 41.67% +0.66%
==========================================
Files 550 551 +1
Lines 23602 23693 +91
Branches 2848 2869 +21
==========================================
+ Hits 9678 9874 +196
+ Misses 13052 12923 -129
- Partials 872 896 +24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Also prevents users changing email while in an age-restricted account
| } else { | ||
| if (existingUser.getEmailVerificationStatus() == EmailVerificationStatus.AGE_RESTRICTED | ||
| && updatedUser.getDateOfBirth().before(DateUtils.addYears(new Date(), -13))) { | ||
| userToSave.setEmailVerificationStatus(EmailVerificationStatus.DELIVERY_FAILED); |
There was a problem hiding this comment.
This isn't a big deal if the Quartz job you mention is reliable and carried out right at the start/end of each month, but as-is I'm finding it quite confusing when this transition happens after saving on the Account page.
The save occurs just fine, but in the same instance a new field appears with a quite a glaring error beneath it - which makes it feel like the save did not occur.
If we think there's a reasonable chance that a user could see this behaviour, it ought to be better explained. Maybe we should detect in the front-end when a switch from AGE_RESTRICTED to DELIVERY_FAILED has happened and explain it in a modal (or something)?
There was a problem hiding this comment.
Also on this topic: you talk about needing a Quarts job in the PR description, but there isn't one actually included in the PR. Is there a plan about doing that later?
There was a problem hiding this comment.
I discussed this with James who believes the model of being able to update an AGE_RESTRICTED account, ever, isn't what we should be going for. That is, u13 accounts are to be entirely separate from non-u13 accounts, and a user will need to make a new account to update it. We might change this in the future, but it "isn't a problem we should solve now".
I've undone the changes here but will copy your comments into a new card.
sjd210
left a comment
sjd210
left a comment
There was a problem hiding this comment.
I'm happy with this. I definitely think it's easier and safer to introduce the accounts with no explicit way of upgrading them to o13 - as long as we do discuss a mechanism for doing this upgrade eventually (once we're a little less pressed for time) ((I've seen that there's a card)) 👍
2 participants
Allows the RPF SSO flow to correctly manage u13 users. These users do not have a (valid, string-typed)
nameornicknamefield; instead, they use theusernamefield. For these users, we create an account with a blank given name and the last name set to theirusername. It should be noted that these accounts can be detected via their email address, which is alwaysdummy@example.com.Since u13 users do not have a valid email address we can tie to their account, this PR creates a new
EmailVerificationStatus,EmailVerificationStatus.AGE_RESTRICTED. This is intended to circumvent issues with reusing any of the other verification statuses, which would imply issues (or non-issues) with the account that are not genuine. This also acts as an indicator in the user object for whether the account is under 13 or not, which we make use of in the front-end. A downside to this approach is that we will need a Quartz job to run once a month to update allAGE_RESTRICTEDusers who have had their 13th birthday in that month (we do not store the day of the DoB for u13) toDELIVERY_FAILED; while not perfect, this highlights that they cannot receive important communications from us without an email and they should add one to their account.