From 4a6416d33e44a353dad3543176002ace58271238 Mon Sep 17 00:00:00 2001 From: yasima-csiro Date: Wed, 14 Dec 2022 09:48:14 +1100 Subject: [PATCH] Refactor web services #141 --- userdetails-gorm/build.gradle | 2 +- .../userdetails/gorm/GormUserService.groovy | 73 +++++++++++++++++++ .../ala/userdetails/PropertyController.groovy | 12 ++- .../userdetails/RoleBasedInterceptor.groovy | 16 ++-- .../userdetails/UserDetailsController.groovy | 12 +-- .../UserDetailsWebServicesInterceptor.groovy | 16 ++-- .../AuthorisedSystemService.groovy | 12 +-- 7 files changed, 111 insertions(+), 32 deletions(-) diff --git a/userdetails-gorm/build.gradle b/userdetails-gorm/build.gradle index 20477ca3..d3dc6eaf 100644 --- a/userdetails-gorm/build.gradle +++ b/userdetails-gorm/build.gradle @@ -107,7 +107,7 @@ dependencies { implementation "org.grails.plugins:ala-bootstrap3:4.1.0" implementation "org.grails.plugins:ala-ws-plugin:3.1.1" - implementation "org.grails.plugins:ala-ws-security-plugin:4.3.3-SNAPSHOT" + implementation "org.grails.plugins:ala-ws-security-plugin:4.3.5-SNAPSHOT" implementation "org.grails.plugins:ala-auth:5.2.0-CognitoLogoutFix-SNAPSHOT" implementation "org.grails.plugins:ala-admin-plugin:2.3.0" diff --git a/userdetails-gorm/src/main/groovy/au/org/ala/userdetails/gorm/GormUserService.groovy b/userdetails-gorm/src/main/groovy/au/org/ala/userdetails/gorm/GormUserService.groovy index 5c0fbc9e..70b09b2c 100644 --- a/userdetails-gorm/src/main/groovy/au/org/ala/userdetails/gorm/GormUserService.groovy +++ b/userdetails-gorm/src/main/groovy/au/org/ala/userdetails/gorm/GormUserService.groovy @@ -61,6 +61,7 @@ class GormUserService implements IUserService { LocationService locationService MessageSource messageSource WebService webService + ProfileService profileService @Value('${password.encoder}') String passwordEncoderType = 'bcrypt' @@ -730,4 +731,76 @@ class GormUserService implements IUserService { @Override void enableMfa(String userId, boolean enable){} + boolean removeUserRole(User user, Role role) { + return false + } + + @Override + User findByUserNameOrEmail(String userName) { + return User.findByUserNameOrEmail(userName, userName) + } + + @Override + def findUsersByRole(String roleName, List numberIds, List userIds, String pageOrToken) { + ScrollableResults results = null + // stream the results just in case someone requests ROLE_USER or something + User.withStatelessSession { session -> + Role role = Role.findByRole(roleName) + if (!role) { + return [error: "Role not found"] + } + + def c = User.createCriteria() + results = c.scroll { + or { + if (numberIds) { + inList('id', numberIds*.toLong()) + } + if (userIds) { + inList('userName', userIds) + inList('email', userIds) + } + } + userRoles { + eq("role", role) + } + } as ScrollableResults + } + return [results: results] + } + + def getUserDetailsFromIdList(List idList){ + def c = User.createCriteria() + def results = c.list() { + 'in'("id", idList.collect { userId -> userId as long } ) + } + return results + } + + def searchByUsernameOrEmail(String q, int max){ + + ScrollableResults results = null + + User.withStatelessSession { session -> + def c = User.createCriteria() + results = c.scroll { + or { + ilike('userName', "%$q%") + ilike('email', "%$q%") + ilike('displayName', "%$q%") + } + maxResults(max) + } as ScrollableResults + } + return [results: results] + } + + def saveCustomUserProperty(User user, String name, String value){ + UserProperty property = profileService.saveUserProperty(user, name, value) + return property.hasErrors() ? null: property + } + + def getCustomUserProperty(User user, String name){ + return profileService.getUserProperty(user, name); + } } diff --git a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/PropertyController.groovy b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/PropertyController.groovy index c9305b50..a1a18df2 100644 --- a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/PropertyController.groovy +++ b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/PropertyController.groovy @@ -64,7 +64,6 @@ class PropertyController extends BaseController { name = "alaId", in = QUERY, description = "The user's ALA ID", - schema = @Schema(implementation = Long), required = true ), @Parameter( @@ -100,14 +99,14 @@ class PropertyController extends BaseController { @PreAuthorise(requiredScope = 'users/read') def getProperty() { String name = params.name - Long alaId = params.long('alaId') + String alaId = params.alaId if (!name || !alaId) { badRequest "name and alaId must be provided"; } else { UserRecord user = userService.getUserById(alaId); List props if (user) { - props = profileService.getUserProperty(user, name); + props = userService.getCustomUserProperty(user, name) render text: props as JSON, contentType: 'application/json' } else { notFound "Could not find user for id: ${alaId}"; @@ -130,7 +129,6 @@ class PropertyController extends BaseController { name = "alaId", in = QUERY, description = "The user's ALA ID", - schema = @Schema(implementation = Long), required = true ), @Parameter( @@ -178,15 +176,15 @@ class PropertyController extends BaseController { def saveProperty(){ String name = params.name; String value = params.value; - Long alaId = params.long('alaId'); + String alaId = params.alaId if (!name || !alaId) { badRequest "name and alaId must be provided"; } else { UserRecord user = userService.getUserById(alaId); UserPropertyRecord property if (user) { - property = profileService.saveUserProperty(user, name, value); - if (property.hasErrors()) { + property = userService.saveCustomUserProperty(user, name, value); + if (!property) { saveFailed() } else { render text: property as JSON, contentType: 'application/json' diff --git a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/RoleBasedInterceptor.groovy b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/RoleBasedInterceptor.groovy index 57dde8bc..20252afe 100644 --- a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/RoleBasedInterceptor.groovy +++ b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/RoleBasedInterceptor.groovy @@ -42,12 +42,18 @@ class RoleBasedInterceptor { PreAuthorise pa = method.getAnnotation(PreAuthorise) ?: controllerClass.getAnnotation(PreAuthorise) response.withFormat { json { - if (!authorisedSystemService.isAuthorisedRequest(request, response, pa.requiredRole(), pa.requiredScope())) { - log.warn("Denying access to $actionName from remote addr: ${request.remoteAddr}, remote host: ${request.remoteHost}") - response.status = HttpStatus.SC_UNAUTHORIZED - render(['error': "Unauthorized"] as JSON) + try{ + if (!authorisedSystemService.isAuthorisedRequest(request, response, pa.requiredRole(), pa.requiredScope())) { + log.warn("Denying access to $actionName from remote addr: ${request.remoteAddr}, remote host: ${request.remoteHost}") + response.status = HttpStatus.SC_UNAUTHORIZED + render(['error': "Unauthorized"] as JSON) - result = false + result = false + } + } + catch (Exception e){ + response.sendError(HttpStatus.SC_UNAUTHORIZED, e.getMessage()) + return false } } '*' { diff --git a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsController.groovy b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsController.groovy index c0850ca8..5ac27446 100644 --- a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsController.groovy +++ b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsController.groovy @@ -272,7 +272,6 @@ class UserDetailsController { user = userService.getUserById(userName) } else { user = userService.findByUserNameOrEmail(userName) -// user = UserRecord.findByUserNameOrEmail(userName, userName) } } else { render status:400, text: "Missing parameter: userName" @@ -422,12 +421,9 @@ class UserDetailsController { if (req && req.userIds) { try { - List idList = req.userIds.collect { userId -> userId as long } + List idList = req.userIds - def c = UserRecord.createCriteria() - def results = c.list() { - 'in'("id", idList) - } + def results = userService.getUserDetailsFromIdList(idList) String jsonConfig = includeProps ? UserMarshaller.WITH_PROPERTIES_CONFIG : null try { @@ -435,11 +431,11 @@ class UserDetailsController { def resultsMap = [users:[:], invalidIds:[], success: true] results.each { user -> - resultsMap.users[user.id] = user + resultsMap.users[user.userId] = user } idList.each { - if (!resultsMap.users[it]) { + if (!resultsMap.users[it.toString()]) { resultsMap.invalidIds << it } } diff --git a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsWebServicesInterceptor.groovy b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsWebServicesInterceptor.groovy index 25d3753a..fb880003 100644 --- a/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsWebServicesInterceptor.groovy +++ b/userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/UserDetailsWebServicesInterceptor.groovy @@ -30,13 +30,19 @@ class UserDetailsWebServicesInterceptor { } boolean before() { - if (!authorisedSystemService.isAuthorisedRequest(request, response, null, 'users/read')) { - log.warn("Denying access to $actionName from remote addr: ${request.remoteAddr}, remote host: ${request.remoteHost}") - response.sendError(HttpStatus.SC_UNAUTHORIZED) - + try { + if (!authorisedSystemService.isAuthorisedRequest(request, response, null, 'users/read')) { + log.warn("Denying access to $actionName from remote addr: ${request.remoteAddr}, remote host: ${request.remoteHost}") + response.sendError(HttpStatus.SC_UNAUTHORIZED) + + return false + } + return true + } + catch (Exception e){ + response.sendError(HttpStatus.SC_UNAUTHORIZED, e.getMessage()) return false } - return true } boolean after() { true } diff --git a/userdetails-plugin/grails-app/services/au/org/ala/userdetails/AuthorisedSystemService.groovy b/userdetails-plugin/grails-app/services/au/org/ala/userdetails/AuthorisedSystemService.groovy index 1c60e6cf..ef5a6d1a 100644 --- a/userdetails-plugin/grails-app/services/au/org/ala/userdetails/AuthorisedSystemService.groovy +++ b/userdetails-plugin/grails-app/services/au/org/ala/userdetails/AuthorisedSystemService.groovy @@ -21,7 +21,7 @@ import org.pac4j.core.context.WebContext import org.pac4j.core.profile.ProfileManager import org.pac4j.core.profile.UserProfile import org.pac4j.core.util.FindBest -import org.pac4j.http.client.direct.DirectBearerAuthClient +import au.org.ala.ws.security.client.AlaAuthClient import org.pac4j.jee.context.JEEContextFactory import org.springframework.beans.factory.annotation.Autowired @@ -35,7 +35,7 @@ class AuthorisedSystemService { @Autowired(required = false) Config config @Autowired(required = false) - DirectBearerAuthClient directBearerAuthClient + AlaAuthClient alaAuthClient @Autowired IAuthorisedSystemRepository authorisedSystemRepository @@ -64,15 +64,15 @@ class AuthorisedSystemService { ProfileManager profileManager = new ProfileManager(context, config.sessionStore) profileManager.setConfig(config) - def credentials = directBearerAuthClient.getCredentials(context, config.sessionStore) + def credentials = alaAuthClient.getCredentials(context, config.sessionStore) if (credentials.isPresent()) { - def profile = directBearerAuthClient.getUserProfile(credentials.get(), context, config.sessionStore) + def profile = alaAuthClient.getUserProfile(credentials.get(), context, config.sessionStore) if (profile.isPresent()) { def userProfile = profile.get() profileManager.save( - directBearerAuthClient.getSaveProfileInSession(context, userProfile), + alaAuthClient.getSaveProfileInSession(context, userProfile), userProfile, - directBearerAuthClient.isMultiProfile(context, userProfile) + alaAuthClient.isMultiProfile(context, userProfile) ) result = true