feat(idp-migration): redirection for account confirmation link #29003
Conversation
When a user supplies unrecognized SSO credentials associated with an email address for an existing user profile, offer them a chance to link their identity to that profile instead of automatically provisioning a new profile. Introduce a feature flag. The feature is incomplete and will hit unimplemented code if the flag is set.
…d auth:one-time-key prefix from email
… & added identity[id] to verification value
| from sentry.web.helpers import render_to_response | ||
|
|
||
|
|
||
| def _respond( |
There was a problem hiding this comment.
is this function here for typing purposes? it doesn't add much IMO, setting a 302 is fine to do in two places.
There was a problem hiding this comment.
i made a private function for this since auth helper used it for the exact same purpose, looking at it, it doesn't add much and could be replaced with a direct render_to_response() call.
| Something went wrong and your email could not be confirmed. Click the button below to go back to the login page. | ||
| </p> | ||
|
|
||
| <a href="{{ url }}" class="btn">Go back</a> |
There was a problem hiding this comment.
A back button in an email is unusual.
There was a problem hiding this comment.
this one isn't in an email -- perhaps the filename is a tad ambiguous.
|
|
||
| <form class="form-stacked" action="" method="post"> {% csrf_token %} | ||
| <a href="{{ url }}" class="btn" type="submit">Continue</a> | ||
| </form> |
There was a problem hiding this comment.
Why is there a link inside a form? I don't think you'll have much luck with a form inside an email either.
| return redirect | ||
| # TODO add view that shows key not found | ||
| return HttpResponseNotFound() | ||
| class IDPView(BaseView): |
There was a problem hiding this comment.
| class IDPView(BaseView): | |
| class AccountConfirmationView(BaseView): |
There was a problem hiding this comment.
would prefer to have things named account_verification around this feature, as email_verification is a seperate feature we don't want to conflate.
There was a problem hiding this comment.
sounds good, i've changed the name of this class
| @@ -0,0 +1,15 @@ | |||
| {% extends "sentry/emails/base.html" %} | |||
There was a problem hiding this comment.
if we have extra time at the end of this project, we should see if it would be an easy swap to react views for these
There was a problem hiding this comment.
should i make tickets for these and work on it if i have time at the end/during spare cycles?
| # TODO add view that shows key not found | ||
| return HttpResponseNotFound() | ||
| class AccountConfirmationView(BaseView): | ||
| auth_required = False |
There was a problem hiding this comment.
would add a quick comment explaining that the user using this endpoint is currently locked out of their account, so auth isn't required on this endpoint.
pages to redirect to after user clicks confirm account link in email