Provide ability to update email with confirmation #188 #192
Conversation
userdetails-plugin/grails-app/views/registration/createAccount.gsp
Outdated
Show resolved
Hide resolved
|
Everything looks fine. |
…re/update_email # Conflicts: # userdetails-plugin/grails-app/views/registration/createAccount.gsp
| application.a13=For any questions or technical assistance, please contact our dedicated support team at <a href="mailto:[email protected]">[email protected]</a>. | ||
| update.email=Update your email | ||
| update.email.desc.1=Please enter your new email to update the email. The new email will receive a code to which needs to be submitted to verify the new email address. |
There was a problem hiding this comment.
This should be moved into the userdetails-gorm/userdetails-cognito messages.properties with the same key, then remove the if branch in the createAccount.gsp below.
| <div id="newEmailDiv"> | ||
| <p id="newEmailMessage" hidden></p> | ||
| <input id="newEmail" name="newEmail" type="text" class="form-control" data-validation-engine="validate[required]"/> | ||
| <g:if test="${grailsApplication.config.getProperty('userdetails.cognito.auth', boolean, false)}"> |
There was a problem hiding this comment.
I would prefer this to be a feature based check rather than a cognito check - ie userdetails.activation.enabled, userdetails.activation.manual: true or similar, then remove userdetails.cognito.auth from properties (applicationForm will need to change too, use userdetails.applications.insecure-urls-allowed or similar)
| <g:if test="${edit}"> | ||
| <h2><g:message code="update.email" /></h2> | ||
| <p> | ||
| <g:if test="${grailsApplication.config.getProperty('userdetails.cognito.auth', boolean, false)}"> |
There was a problem hiding this comment.
Remove this as per above comment
| } | ||
| else { | ||
| $.ajax({ | ||
| url: "${createLink(uri:'/registration/update')}?email=" + newEmail, |
There was a problem hiding this comment.
newEmail probably needs to be encoded?
There was a problem hiding this comment.
Or use jquerys $.ajax({data: ...})
| $("#requestCode").click(function(e) { | ||
| var newEmail = $("#newEmail").val(); | ||
| if(newEmail == null || newEmail === "") { | ||
| document.getElementById("newEmailMessage").innerHTML = "Invalid email" |
There was a problem hiding this comment.
All these inner HTML messages should be i18n
| window.location = "${createLink(uri:'/logout')}?url=/" | ||
| } | ||
| else{ | ||
| document.getElementById("newEmailMessage").innerHTML = result.error |
There was a problem hiding this comment.
Potential XSS issue? If the userservice exception reflects the user input code or something then this is a problem.
There was a problem hiding this comment.
This is why I prefer to never touch innerHTML from jquery.
| } | ||
| else { | ||
| $.ajax({ | ||
| url: "${createLink(uri:'/registration/update')}?email=" + newEmail, |
| let error = "A user is already registered with the email address" | ||
| if(result.success) { | ||
| $.ajax({ | ||
| url: "${createLink(uri:'/registration/deactivateAccountAndSendActivationEmail')}?email=" + newEmail, |
There was a problem hiding this comment.
Also, sending 2 requests for this seems wrong - the user could update their email then block the deactivateAccountAndSendActivationEmail call? Surely the deactivateAccountAndSendUserEmail should happen atomically on the server side with the update if required?
| @@ -370,7 +394,7 @@ | |||
|
|
|||
| $("#setupMFA").click(function(e) { | |||
| $.ajax({ | |||
| url: "${createLink(action:'getSecretForMfa', controller: 'Registration')}", | |||
| url: "${createLink(uri:'/registration/getSecretForMfa')}", | |||
There was a problem hiding this comment.
Why change from action: controller: which should be stable under refactoring or URL Mappings changes to uri?
| url: "${createLink(uri:'/registration/deactivateAccountAndSendActivationEmail')}?email=" + newEmail, | ||
| type: "GET", | ||
| success: function(result){ | ||
| if(result.success) { |
There was a problem hiding this comment.
Seems to be some overlap with the validation of #requestCode
…re/update_email # Conflicts: # userdetails-plugin/grails-app/i18n/messages.properties
|
Fixed all the comments. |
No description provided.