AUTHLIB-178 LDAP Domain Validaiton - #98
Conversation
Add front-end validation in authlib.js and back-end validation in UserController.java, to ensure that user emails and validated against the LDAP domain if their authentication method is set to LDAP.
There was a problem hiding this comment.
I couldn't quickly make sense of how to implement a new validator, but the ConstraintViolations were ultimately converted to FieldError's, so inserting a FieldError in this context seemed pretty safe.
Simplify logic that controls the LDAP search button, and removed a block of commented out code.
Some changes intended for the previous commit were omitted.
Finish a half completed section of comments.
| document.getElementById('last_name').value = jsonData.lastName; | ||
| document.getElementById('email').value = jsonData.email; | ||
| document.getElementById('institution').value = jsonData.institution; | ||
| document.getElementById('email').dispatchEvent(new Event('change')); |
There was a problem hiding this comment.
Ensures that the validation is run after the LDAP search button is used, which effectively clears the previous feedback message.
| const feedbackElement = findErrorDivForElement(inputElement); | ||
| if (feedbackElement) { | ||
| feedbackElement.textContent = serverError.dataset.message; | ||
| } |
There was a problem hiding this comment.
Updates the feedback message with server error messages.
|
|
||
| usernameInput.addEventListener('input', searchHandler); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Most of the code was moved inside the onLoad handler, largely to reduce the number of redundant variable declarations across the two contexts.
heathharrelson
left a comment
heathharrelson
left a comment
There was a problem hiding this comment.
This was a bit hard to review because you combined feature development and refactoring, although it seems to work. To make things easier for your reviewers, please make separate PRs for features and refactoring, or make the changes two separate commits and call that out in the PR description.
2 participants
Overview
Add front-end validation in authlib.js and back-end validation in UserController.java, to ensure that user emails and validated against the LDAP domain if their authentication method is set to LDAP.
Issues
AUTHLIB-178
[x] Added to CHANGELOG.md
Discussion
The diff for
authlib.jslooks pretty attrocious, but a lot of the changes are just shuffling blocks around to reduce redundant variables and to ensure that everything executes in the proper order. I'll leave some comments pointing out biggest changes.Testing Responsiveness
I've tried to ensure that the front-end validation is responsive to a few different actions:
Testing Server Validation
I tested the server validation by temporarily setting
domainMatches = .. || truein authlib.js, so that I could submit bad emails. The server correctly generated errors if the email didn't match, and I've updated authlib.js to populate the feedback content with the server's error message.Screenshots
Error message