-
Notifications
You must be signed in to change notification settings - Fork 5
[ALS-9325] Update Terms of Service endpoint to take in request body content #261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release
Are you sure you want to change the base?
Conversation
.authorizeHttpRequests( | ||
(authorizeRequests) -> authorizeRequests.requestMatchers( | ||
"/actuator/health", "/actuator/info", "/authentication", "/authentication/**", "/swagger.yaml", "/swagger.json", | ||
"/user/me/queryTemplate", "/user/me/queryTemplate/**", "/tos/latest", "/open/validate", "/logout", "/cache/**" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The critical part here is this added /tos/latest
public endpoint.
if (!tosService.hasUserAcceptedLatest(authenticatedUser.getUser().getSubject())) { | ||
|
||
if ( | ||
!request.getRequestURI().endsWith("/tos/accept") && !tosService.hasUserAcceptedLatest(authenticatedUser.getUser().getSubject()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added /tos/accept
exclusion so we can ensure that if they are on the accept route it doesn't matter if they have previously accepted terms or not.
logger.info("Getting latest Terms of Service"); | ||
return PICSUREResponse.success(tosService.getLatest()); | ||
} | ||
|
||
@Operation(description = "Update the Terms of Service html body") | ||
@RolesAllowed({AuthNaming.AuthRoleNaming.ADMIN, SUPER_ADMIN}) | ||
@PostMapping(path = "/update", consumes = "text/html", produces = "application/json") | ||
public ResponseEntity<?> updateTermsOfService( | ||
@Parameter(required = true, description = "A html page for updating") String html){ | ||
public ResponseEntity<?> updateTermsOfService(@RequestBody String html) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added RequestBody annotation to save incoming html in the database.
User user = tosService.acceptTermsOfService(userSubject); | ||
userService.updateUser(List.of(user)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User updating the TOS automatically accepts it so they're not logged out of the UI on next api call and forced to log back in.
SecurityContext context = SecurityContextHolder.getContext(); | ||
String userSubject = context.getAuthentication().getName(); | ||
CustomUserDetails customUserDetails = (CustomUserDetails) context.getAuthentication().getPrincipal(); | ||
String userSubject = customUserDetails.getUser().getSubject(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensuring that the user details we're passing are actually the subject - during testing, I found that getName
returns an email.
} | ||
|
||
return false; | ||
Optional<TermsOfService> latestTOS = this.termsOfServiceRepo.findTopByOrderByDateUpdatedDesc(); | ||
return latestTOS.filter(termsOfService -> acceptedTOS.compareTo(termsOfService.getDateUpdated()) >= 0).isPresent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When admin updates TOS, and their user automatically accepts their updated tos, the time and date may be the same. So accept an equal timestamp as accepted.
Note: The pre-commit hook formatted the file and there were a lot of things to fix. Review with whitespace off and a critical eye to stuff NOT related to formatting. I added comments of critical functionality changes to make it easier.