Userdetails web services refactoring (#143)

* Refactor web services #141

Co-authored-by: Simon Bear <simon.bear@csiro.au>

Author: yasima-csiro

userdetails-cognito/src/main/groovy/au/org/ala/userdetails/CognitoUserService.groovy

@@ -16,7 +16,6 @@
 package au.org.ala.userdetails.gorm
 
 import au.org.ala.auth.PreAuthorise
-import au.org.ala.users.AuthorisedSystem
 import grails.converters.JSON
 import org.springframework.dao.DataIntegrityViolationException
 

userdetails-gorm/grails-app/controllers/au/org/ala/userdetails/gorm/AuthorisedSystemController.groovy

@@ -116,12 +116,7 @@ class User extends UserRecord<Long> implements Serializable {
     def propsAsMap(){
         def map = [:]
         this.getUserProperties().each {
-            if(it.name == "enableMFA"){
-                map.put(it.name, it.value.toBoolean())
-            }
-            else {
-                map.put(it.name.startsWith('custom:') ? it.name.substring(7) : it.name, it.value)
-            }
+            map.put(it.name.startsWith('custom:') ? it.name.substring(7) : it.name, it.value)
         }
         map
     }

userdetails-gorm/grails-app/domain/au/org/ala/userdetails/gorm/User.groovy

@@ -21,6 +21,7 @@ import au.org.ala.userdetails.IPasswordOperations
 import au.org.ala.userdetails.IUserService
 import au.org.ala.userdetails.LocationService
 import au.org.ala.userdetails.PasswordService
+import au.org.ala.userdetails.ProfileService
 import au.org.ala.web.AuthService
 import au.org.ala.ws.service.WebService
 import grails.boot.GrailsApp
@@ -52,7 +53,9 @@ class Application extends GrailsAutoConfiguration {
                              AuthService authService,
                              LocationService locationService,
                              MessageSource messageSource,
-                             WebService webService) {
+                             WebService webService,
+                             ProfileService profileService
+                             ) {
 
 //        grailsApplication.addArtefact(DomainClassArtefactHandler.TYPE, UserRecord)
 //        grailsApplication.addArtefact(DomainClassArtefactHandler.TYPE, UserPropertyRecord)
@@ -67,6 +70,7 @@ class Application extends GrailsAutoConfiguration {
 
         userService.grailsApplication = grailsApplication
         userService.messageSource = messageSource
+        userService.profileService = profileService
 
         userService.affiliationsEnabled = grailsApplication.config.getProperty('attributes.affiliations.enabled', Boolean, false)
 

userdetails-gorm/grails-app/init/au/org/ala/userdetails/gorm/Application.groovy

@@ -22,6 +22,7 @@ import au.org.ala.userdetails.IUserService
 import au.org.ala.userdetails.LocationService
 import au.org.ala.userdetails.PagedResult
 import au.org.ala.userdetails.PasswordService
+import au.org.ala.userdetails.ProfileService
 import au.org.ala.userdetails.ResultStreamer
 import au.org.ala.users.RoleRecord
 import au.org.ala.users.UserPropertyRecord
@@ -54,6 +55,7 @@ class GormUserService implements IUserService {
     LocationService locationService
     MessageSource messageSource
     WebService webService
+    ProfileService profileService
 
     @Value('${attributes.affiliations.enabled:false}')
     boolean affiliationsEnabled = false
@@ -113,11 +115,6 @@ class GormUserService implements IUserService {
         return user?.locked ?: false
     }
 
-    @Transactional(readOnly = true)
-    boolean isEmailRegistered(String email) {
-        return User.findByEmailOrUserName(email?.toLowerCase(), email?.toLowerCase()) != null
-    }
-
     @Transactional(readOnly = true)
     boolean isEmailInUse(String newEmail) {
         return User.findByEmailOrUserName(newEmail?.toLowerCase(), newEmail?.toLowerCase())
@@ -155,7 +152,7 @@ class GormUserService implements IUserService {
             return User.findAllByEmailLikeOrLastNameLikeOrFirstNameLike(q, q, q, [offset: paginationToken as int, max: maxResults ])
         }
 
-        return User.list([offset: paginationToken as int, max: maxResults ])
+        return User.list([offset: (paginationToken ?: 0) as int, max: maxResults ])
     }
 
     @Override
@@ -253,7 +250,7 @@ class GormUserService implements IUserService {
 
                     // Now send a temporary password to the user...
                     try {
-                        resetAndSendTemporaryPassword(userInstance, emailSubject, emailTitle, emailBody, password)
+                        passwordService.resetAndSendTemporaryPassword(userInstance, emailSubject, emailTitle, emailBody, password)
                     } catch (PasswordResetFailedException ex) {
                         // Catching the checked exception should prevent the transaction from failing
                         log.error("Failed to send temporary password via email!", ex)
@@ -274,7 +271,7 @@ class GormUserService implements IUserService {
 
         properties.keySet().each { String propName ->
             def propValue = properties[propName] ?: ''
-            setUserProperty(user, propName, propValue)
+            setUserProperty(user, propName, propValue as String)
         }
     }
 
@@ -530,39 +527,44 @@ class GormUserService implements IUserService {
     @Override
     void addRoles(Collection<RoleRecord> roleRecords) {
         Role.saveAll(roleRecords.collect { new Role(role: it.role, description:  it.description) })
-
     }
 
-    @Override
-    List<UserPropertyRecord> findAllAttributesByName(String s) {
-        UserProperty.findAllByName("flickrId")
-    }
+//        *********** Property related services *************
 
     @Override
-    void addOrUpdateProperty(UserRecord userRecord, String name, String value) {
+    UserPropertyRecord addOrUpdateProperty(UserRecord userRecord, String name, String value) {
         assert userRecord instanceof User
-        UserProperty.addOrUpdateProperty(userRecord, name, value)
+        return UserProperty.addOrUpdateProperty(userRecord, name, value)
     }
 
     @Override
-    void removeUserAttributes(UserRecord user, ArrayList<String> attrs) {
-        def props = UserProperty.findAllByUserAndNameInList(user, attrs)
+    void removeUserProperty(UserRecord user, ArrayList<String> attrs) {
+        def props = UserProperty.findAllByUserAndNameInList(user as User, attrs)
         if (props) UserProperty.deleteAll(props)
     }
 
     @Override
-    void getUserAttribute(UserRecord userRecord, String attribute) {
-        UserProperty.findAllByUserAndName(userRecord, attribute)
-    }
+    List<UserPropertyRecord> searchProperty(UserRecord userRecord, String attribute) {
+        List<UserPropertyRecord> propList = []
 
-    @Override
-    List<UserProperty> getAllAvailableProperties() {
-        UserProperty.withCriteria {
-            projections {
-                distinct("name")
-            }
-            order("name")
+        if(userRecord && attribute){
+            List properties = UserProperty.findAllByUserAndName(userRecord as User, attribute)
+            propList =  properties.collect {new UserPropertyRecord(user: userRecord, name: it.name, value: it.value) }
         }
+        else if(attribute){
+            propList = UserProperty.findAllByName(attribute)
+        }
+        else{
+            List properties = UserProperty.withCriteria {
+                projections {
+                    distinct("name")
+                }
+                order("name")
+            } as List
+
+            propList = properties.collect { new UserPropertyRecord(user: it.user, name: it.name, value: it.value) }
+        }
+        return propList
     }
 
     @Override
@@ -652,6 +654,8 @@ class GormUserService implements IUserService {
         emailService.sendAccountActivation(user, user.tempAuthKey)
     }
 
+    //    *********** MFA services *************
+
     @Override
     String getSecretForMfa() {}
 
@@ -660,4 +664,12 @@ class GormUserService implements IUserService {
 
     @Override
     void enableMfa(String userId, boolean enable){}
+
+    def getUserDetailsFromIdList(List idList){
+        def c = User.createCriteria()
+        def results = c.list() {
+            'in'("id", idList.collect { userId -> userId as long } )
+        }
+        return results
+    }
 }

userdetails-gorm/src/main/groovy/au/org/ala/userdetails/gorm/GormUserService.groovy

@@ -112,7 +112,7 @@ dependencies {
 
     api "org.grails.plugins:ala-bootstrap3:4.1.0"
     api "org.grails.plugins:ala-ws-plugin:3.1.1"
-    api "org.grails.plugins:ala-ws-security-plugin:4.3.3-SNAPSHOT"
+    api "org.grails.plugins:ala-ws-security-plugin:4.3.5-SNAPSHOT"
     api "org.grails.plugins:ala-auth:5.2.0-SNAPSHOT"
     api "org.grails.plugins:ala-admin-plugin:2.3.0"
 

userdetails-plugin/build.gradle

@@ -60,7 +60,7 @@ class ExternalSiteController {
     @Produces("application/json")
     def flickr() {
 
-        def flickrIds = userService.findAllAttributesByName("flickrId") // UserPropertyRecord.findAllByName("flickrId")
+        def flickrIds = userService.searchProperty(null, "flickrId")
         render(contentType: "application/json") {
             flickrUsers(flickrIds) { UserPropertyRecord flickrId ->
                 id flickrId.user.id.toString()

userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/ExternalSiteController.groovy

@@ -177,7 +177,7 @@ class ProfileController {
         }
 
         if (user) {
-            userService.removeUserAttributes(user, attrs)
+            userService.removeUserProperty(user, attrs)
         } else {
             flash.message = 'Failed to retrieve user details!'
         }

userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/ProfileController.groovy

@@ -43,7 +43,6 @@ class PropertyController extends BaseController {
     IUserService userService
 
     def profileService
-    def authorisedSystemService
 
     static allowedMethods = [getProperty: "GET", saveProperty: "POST"]
 
@@ -64,7 +63,6 @@ class PropertyController extends BaseController {
                             name = "alaId",
                             in = QUERY,
                             description = "The user's ALA ID",
-                            schema = @Schema(implementation = Long),
                             required = true
                     ),
                     @Parameter(
@@ -100,14 +98,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
+            List<UserPropertyRecord> props
             if (user) {
-                props = profileService.getUserProperty(user, name);
+                props = profileService.getUserProperty(user, name)
                 render text: props as JSON, contentType: 'application/json'
             } else {
                 notFound "Could not find user for id: ${alaId}";
@@ -130,7 +128,6 @@ class PropertyController extends BaseController {
                             name = "alaId",
                             in = QUERY,
                             description = "The user's ALA ID",
-                            schema = @Schema(implementation = Long),
                             required = true
                     ),
                     @Parameter(
@@ -178,15 +175,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()) {
+                if (!property) {
                     saveFailed()
                 } else {
                     render text: property as JSON, contentType: 'application/json'

userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/PropertyController.groovy

@@ -293,7 +293,7 @@ class RegistrationController {
             }
 
             //create user account...
-            if (!paramsEmail || userService.isEmailRegistered(paramsEmail)) {
+            if (!paramsEmail || userService.isEmailInUse(paramsEmail)) {
                 def inactiveUser = !userService.isActive(paramsEmail)
                 def lockedUser = userService.isLocked(paramsEmail)
                 render(view: 'createAccount', model: [edit: false, user: params, props: params, alreadyRegistered: true, inactiveUser: inactiveUser, lockedUser: lockedUser, passwordPolicy: passwordService.buildPasswordPolicy()])
@@ -371,12 +371,13 @@ class RegistrationController {
     }
 
     def getSecretForMfa() {
+        def mfaResponse
         try {
-            def response = [success: true, code: userService.getSecretForMfa()]
+            mfaResponse = [success: true, code: userService.getSecretForMfa()]
         } catch (e) {
-            def response = [success: false, error: e.message]
+            mfaResponse = [success: false, error: e.message]
         }
-        render(response as JSON)
+        render(mfaResponse as JSON)
     }
 
     def verifyAndActivateMfa() {

userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/RegistrationController.groovy

@@ -42,12 +42,19 @@ 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){
+                        log.error(e.getMessage(), e)
+                        response.sendError(HttpStatus.SC_UNAUTHORIZED)
+                        return false
                     }
                 }
                 '*' {

userdetails-plugin/grails-app/controllers/au/org/ala/userdetails/RoleBasedInterceptor.groovy

@@ -42,21 +42,21 @@ class RoleController {
     }
 
     def create() {
-        [roleInstance: new RoleRecord(params) ]
+        [roleInstance: new RoleRecord() ]
     }
 
     def save() {
 
         def pattern = ~/[A-Z_]{1,}/
 
         if(pattern.matcher(params.role).matches()){
-            def roleInstance = new RoleRecord(params)
+            def roleInstance = new RoleRecord(role: params.role, description: params.description)
             def saved = userService.addRole(roleInstance) // roleInstance.save(flush: true)
             if (!saved) {
                 render(view: "create", model: [roleInstance: roleInstance])
                 return
             }
-            flash.message = message(code: 'default.created.message', args: [message(code: 'role.label', default: 'Role'), roleInstance.id])
+            flash.message = message(code: 'default.created.message', args: [message(code: 'role.label', default: 'Role'), roleInstance.role])
             redirect(action: "list")
         } else {
             flash.message = 'RoleRecord must consist of uppercase characters and underscores only'