Add password reset request and verify console actions#2775
Conversation
546980e to
ca3111e
Compare
ptkach
left a comment
There was a problem hiding this comment.
Reviewed 3 of 11 files at r1, 3 of 4 files at r2, all commit messages.
Reviewable status: 6 of 11 files reviewed, 2 unresolved discussions
core/src/main/java/google/registry/module/frontend/FrontendRequestComponent.java line 89 at r2 (raw file):
FlowComponent.Builder flowComponentBuilder(); PasswordResetRequestAction passwordResetRequestAction();
We no longer deploy to GAE, this change is not needed
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java line 45 at r2 (raw file):
static final String PATH = "/console-api/password-reset-request"; static final String VERIFICATION_EMAIL_TEMPLATE =
Have you tested it? How does it look? My main concern is sending the link as URL string. Is possible to generate html email?
gbrodman
left a comment
There was a problem hiding this comment.
Reviewable status: 6 of 11 files reviewed, 2 unresolved discussions (waiting on @ptkach)
core/src/main/java/google/registry/module/frontend/FrontendRequestComponent.java line 89 at r2 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
We no longer deploy to GAE, this change is not needed
fair point -- is there any reason why we should not (in a separate PR) remove this and related files?
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java line 45 at r2 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
Have you tested it? How does it look? My main concern is sending the link as URL string. Is possible to generate html email?
I think we could, but with the registry lock email we wanted to just go with the simplest concept.
As far as I remember, it shows up in the email as you'd expect. The registry lock testing was long enough ago that I no longer have those emails, though.
ptkach
left a comment
There was a problem hiding this comment.
Reviewed 2 of 11 files at r1, 1 of 4 files at r2, 1 of 2 files at r3.
Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @gbrodman)
core/src/main/java/google/registry/module/frontend/FrontendRequestComponent.java line 89 at r2 (raw file):
Previously, gbrodman wrote…
fair point -- is there any reason why we should not (in a separate PR) remove this and related files?
No reason, in fact we should do a clean up at some point - I had disabled GAE and deployment not so long ago so I wanted to give it time to make sure we're good
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java line 45 at r2 (raw file):
Previously, gbrodman wrote…
I think we could, but with the registry lock email we wanted to just go with the simplest concept.
As far as I remember, it shows up in the email as you'd expect. The registry lock testing was long enough ago that I no longer have those emails, though.
We'd need to test this. I had previously seen gmail sending emails to spam with URLs in high up in content. I'm not sure that's still the case and whether it depends on URL string position in email.
gbrodman
left a comment
There was a problem hiding this comment.
Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @ptkach)
core/src/main/java/google/registry/module/frontend/FrontendRequestComponent.java line 89 at r2 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
No reason, in fact we should do a clean up at some point - I had disabled GAE and deployment not so long ago so I wanted to give it time to make sure we're good
acknowledged, sounds good to me
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java line 45 at r2 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
We'd need to test this. I had previously seen gmail sending emails to spam with URLs in high up in content. I'm not sure that's still the case and whether it depends on URL string position in email.
I don't think that we've had issues with registry lock emails ever going to spam, and I think / hope we'd be OK given that we're not really sending out much email and we haven't been marked as spam
I don't think it's possible to fully test this without deploying it to sandbox / prod, right? I don't think the other environments allow for email sending
ptkach
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman)
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java line 45 at r2 (raw file):
Previously, gbrodman wrote…
I don't think that we've had issues with registry lock emails ever going to spam, and I think / hope we'd be OK given that we're not really sending out much email and we haven't been marked as spam
I don't think it's possible to fully test this without deploying it to sandbox / prod, right? I don't think the other environments allow for email sending
I recall sending emails in qa. It's controlled via config flag
gbrodman
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ptkach)
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java line 45 at r2 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
I recall sending emails in qa. It's controlled via config flag
synced up offline -- the testing environments aren't working for email due to various reasons (theoretically it's sending in crash, but not arriving) so we're going to gate this behind an admin flag and test in sandbox later
ac17f2b to
8b7406f
Compare
ptkach
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @gbrodman)
gbrodman
left a comment
There was a problem hiding this comment.
fwiw i'm adding the classes back to FrontendRequestComponent because removing them causes the routing tests to fail -- that will require a more substantial change that should be in a separate PR
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @gbrodman)
This works fairly similarly to the registry lock request and verification mechanism. The request action generates a UUI which is emailed (in link form) to the user in question. The frontend will send a request to the verify action with the UUID and hopefully the action should be finalized. EPP password requests can be sent by anyone with edit-registrar permissions and must be approved by an admin POC email. Registry lock password resets can only be sent by primary contacts, and are verified/performed by the user in question.
8b7406f to
09a6a60
Compare
This works fairly similarly to the registry lock request and verification mechanism. The request action generates a UUI which is emailed (in link form) to the user in question. The frontend will send a request to the verify action with the UUID and hopefully the action should be finalized.
EPP password requests can be sent by anyone with edit-registrar permissions and must be approved by an admin POC email.
Registry lock password resets can only be sent by primary contacts, and are verified/performed by the user in question.
This change is