From ec01768ab2d83fd1e016bae5b965e3ebc9bb382d Mon Sep 17 00:00:00 2001
From: Sushant G <105408469+sughics@users.noreply.github.com>
Date: Thu, 15 Dec 2022 11:53:31 +1100
Subject: [PATCH 01/10] Bumping SNAPSHOT version

---
 build.gradle | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build.gradle b/build.gradle
index 9feaddce..3e3d0e7c 100644
--- a/build.gradle
+++ b/build.gradle
@@ -17,7 +17,7 @@ plugins {
     id "com.gorylenko.gradle-git-properties" version "2.4.0-rc2"
 }
 
-version "3.1.1-SNAPSHOT"
+version "3.1.2-SNAPSHOT"
 
 
 group "au.org.ala"

From a19f9df39feb22bff6e0fb40e16b6648da60d32e Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Fri, 13 Jan 2023 17:20:02 +1100
Subject: [PATCH 02/10] issue #175 adding AlaSecured annotation to relevant
 controllers and  methods, updating legacy use of ROLE_COLLECTION_ADMIN and
 ROLE_COLLECTION_EDITOR

---
 grails-app/conf/application.yml               |  1 +
 .../org/ala/collectory/AdminController.groovy | 11 ++----
 .../ala/collectory/ContactController.groovy   | 16 +++------
 .../ala/collectory/LicenceController.groovy   | 36 +++++++++++--------
 .../collectory/ProviderGroupController.groovy | 19 +++++-----
 .../collectory/ProviderMapController.groovy   | 21 +++++------
 .../ala/collectory/CollectoryTagLib.groovy    |  8 ++---
 grails-app/views/manage/list.gsp              | 24 ++++++-------
 8 files changed, 63 insertions(+), 73 deletions(-)

diff --git a/grails-app/conf/application.yml b/grails-app/conf/application.yml
index 15ee4de1..cfaadf17 100644
--- a/grails-app/conf/application.yml
+++ b/grails-app/conf/application.yml
@@ -209,6 +209,7 @@ biocacheUiURL: https://biocache.ala.org.au
 isPipelinesCompatible: true
 
 ROLE_ADMIN: 'ROLE_ADMIN'
+ROLE_EDITOR: 'ROLE_EDITOR'
 
 rifcs:
     excludeBounds: true
diff --git a/grails-app/controllers/au/org/ala/collectory/AdminController.groovy b/grails-app/controllers/au/org/ala/collectory/AdminController.groovy
index cdd68480..49fa1c46 100644
--- a/grails-app/controllers/au/org/ala/collectory/AdminController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/AdminController.groovy
@@ -1,21 +1,16 @@
 package au.org.ala.collectory
 
+import au.org.ala.web.AlaSecured
 import com.opencsv.CSVReader
 import grails.converters.JSON
 import grails.web.JSONBuilder
 import org.springframework.core.io.support.PathMatchingResourcePatternResolver
+
+@AlaSecured(value = ["ROLE_ADMIN"], message =  "You are not authorised to access this page. You do not have 'Admin' rights.")
 class AdminController {
 
     def dataLoaderService, idGeneratorService, collectoryAuthService, metadataService, activityLogService, providerGroupService
 
-    def auth() {
-        if (!collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN) && grailsApplication.config.security.oidc.enabled.toBoolean()) {
-            render "You are not authorised to access this page."
-            return false
-        }
-        return true
-    }
-
     def index = {
         redirect(controller: 'manage')
     }
diff --git a/grails-app/controllers/au/org/ala/collectory/ContactController.groovy b/grails-app/controllers/au/org/ala/collectory/ContactController.groovy
index eaea0145..37266c1d 100644
--- a/grails-app/controllers/au/org/ala/collectory/ContactController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ContactController.groovy
@@ -1,5 +1,8 @@
 package au.org.ala.collectory
 
+import au.org.ala.web.AlaSecured
+
+@AlaSecured(value = ["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true,  message =  "You are not authorised to access this page. You do not have 'editor' rights.")
 class ContactController {
 
     static allowedMethods = [save: "POST", update: "POST", delete: "POST"]
@@ -14,14 +17,6 @@ class ContactController {
     def activityLogService
     def providerGroupService
 
-    def auth() {
-        if (!collectoryAuthService?.userInRole(grailsApplication.config.ROLE_EDITOR) && grailsApplication.config.security.oidc.enabled.toBoolean()) {
-            render "You are not authorised to access this page."
-            return false
-        }
-        return true
-    }
-
     /**
      End access control
      */
@@ -136,10 +131,10 @@ class ContactController {
     /**
      * MEW - modified to cascade delete all ContactFor links for the contact
      */
+    @AlaSecured(value = ["ROLE_ADMIN"], message =  "You are not authorised to access this page.")
     def delete() {
         def contactInstance = Contact.get(params.id)
         if (contactInstance) {
-            if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
                 try {
                     activityLogService.log collectoryAuthService?.username(), collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN), Action.DELETE, "contact ${contactInstance.buildName()}"
                     // need to delete any ContactFor links first
@@ -157,9 +152,6 @@ class ContactController {
                     flash.message = "${message(code: 'default.not.deleted.message', args: [message(code: 'contact.label', default: 'Contact'), params.id])}"
                     redirect(action: "show", id: params.id)
                 }
-            } else {
-                render "You are not authorised to access this page."
-            }
         }
         else {
             flash.message = "${message(code: 'default.not.found.message', args: [message(code: 'contact.label', default: 'Contact'), params.id])}"
diff --git a/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy b/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
index 7c16c9b5..741bc89e 100644
--- a/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
@@ -1,6 +1,7 @@
 package au.org.ala.collectory
 
 import au.org.ala.plugins.openapi.Path
+import au.org.ala.web.AlaSecured
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties
 import grails.converters.JSON
 import io.swagger.v3.oas.annotations.Operation
@@ -17,8 +18,11 @@ import javax.ws.rs.Produces
 /**
  * Simple webservice providing support licences in the system.
  */
+@AlaSecured(value = ["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true,   message =  "You are not authorised to access this page. You do not have 'Collection editor' rights.")
 class LicenceController {
 
+    def collectoryAuthService
+
     @Operation(
             method = "GET",
             tags = "licence",
@@ -143,23 +147,25 @@ class LicenceController {
     }
 
     def delete(Long id) {
-        def licenceInstance = Licence.get(id)
-        if (!licenceInstance) {
-            flash.message = message(code: 'default.not.found.message', args: [message(code: 'licence.label', default: 'Licence'), id])
-            redirect(action: "list")
-            return
-        }
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
+            def licenceInstance = Licence.get(id)
+            if (!licenceInstance) {
+                flash.message = message(code: 'default.not.found.message', args: [message(code: 'licence.label', default: 'Licence'), id])
+                redirect(action: "list")
+                return
+            }
 
-        try {
-            Licence.withTransaction {
-                licenceInstance.delete(flush: true)
+            try {
+                Licence.withTransaction {
+                    licenceInstance.delete(flush: true)
+                }
+                flash.message = message(code: 'default.deleted.message', args: [message(code: 'licence.label', default: 'Licence'), id])
+                redirect(action: "list")
+            }
+            catch (DataIntegrityViolationException e) {
+                flash.message = message(code: 'default.not.deleted.message', args: [message(code: 'licence.label', default: 'Licence'), id])
+                redirect(action: "show", id: id)
             }
-            flash.message = message(code: 'default.deleted.message', args: [message(code: 'licence.label', default: 'Licence'), id])
-            redirect(action: "list")
-        }
-        catch (DataIntegrityViolationException e) {
-            flash.message = message(code: 'default.not.deleted.message', args: [message(code: 'licence.label', default: 'Licence'), id])
-            redirect(action: "show", id: id)
         }
     }
 }
diff --git a/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy b/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
index cde9ea76..685d1ee9 100644
--- a/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
@@ -1,5 +1,6 @@
 package au.org.ala.collectory
 
+import au.org.ala.web.AlaSecured
 import grails.converters.JSON
 import org.springframework.dao.DataIntegrityViolationException
 import org.springframework.web.context.request.RequestContextHolder
@@ -10,6 +11,7 @@ import org.springframework.web.multipart.MultipartFile
  *
  * It provides common code for shared attributes like contacts.
  */
+@AlaSecured(value = ["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true, message =  "You are not authorised to access this page. You do not have 'editor' rights.")
 abstract class ProviderGroupController {
 
     String entityName = "ProviderGroup"
@@ -18,20 +20,19 @@ abstract class ProviderGroupController {
 
     def idGeneratorService, collectoryAuthService, metadataService, gbifService, dataImportService, providerGroupService, activityLogService
 
-    def auth() {
-        if (!collectoryAuthService?.userInRole(grailsApplication.config.ROLE_EDITOR) && grailsApplication.config.security.oidc.enabled.toBoolean()) {
-            response.setHeader("Content-type", "text/plain; charset=UTF-8")
-            render message(code: "provider.group.controller.01", default: "You are not authorised to access this page. You do not have 'Collection editor' rights.")
-            return false
-        }
-    }
+    /*
+     * Access control
+     *
+     * All methods require EDITOR role.
+     * Edit methods require ADMIN or the user to be an administrator for the entity.
+     */
 
     // helpers for subclasses
     protected username = {
         collectoryAuthService?.username() ?: 'unavailable'
     }
 
-    protected isAdmin = {
+    protected isAdmin () {
         collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN) ?: false
     }
     /*
@@ -853,7 +854,7 @@ abstract class ProviderGroupController {
     }
 
     protected boolean isAuthorisedToEdit(uid) {
-        if (!grailsApplication.config.security.oidc.enabled.toBoolean() || isAdmin()) {
+        if (isAdmin()) {
             return true
         } else {
             def email = RequestContextHolder.currentRequestAttributes()?.getUserPrincipal()?.name
diff --git a/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy b/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy
index 46d82660..6e63d4ff 100644
--- a/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy
@@ -1,26 +1,21 @@
 package au.org.ala.collectory
 
+import au.org.ala.web.AlaSecured
 import grails.gorm.transactions.Transactional
 
+@AlaSecured(value =["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true,  message =  "You are not authorised to access this page. You do not have 'editor' rights.")
 class ProviderMapController {
 
     static allowedMethods = [save: "POST", update: "POST", delete: "POST"]
 
     def collectoryAuthService
-/*
- * Access control
- *
- * All methods require EDITOR role.
- * Edit methods require ADMIN or the user to be an administrator for the entity.
- */
-    def beforeInterceptor = [action:this.&auth]
 
-    def auth() {
-        if (!collectoryAuthService?.userInRole(grailsApplication.config.ROLE_EDITOR) && grailsApplication.config.security.oidc.enabled.toBoolean()) {
-            render "You are not authorised to access this page."
-            return false
-        }
-    }
+    /*
+     * Access control
+     *
+     * All methods require EDITOR role.
+     * Edit methods require ADMIN or the user to be an administrator for the entity.
+     */
 
     def index = {
         redirect(action: "list", params: params)
diff --git a/grails-app/taglib/au/org/ala/collectory/CollectoryTagLib.groovy b/grails-app/taglib/au/org/ala/collectory/CollectoryTagLib.groovy
index c09ed8b8..58103bfd 100644
--- a/grails-app/taglib/au/org/ala/collectory/CollectoryTagLib.groovy
+++ b/grails-app/taglib/au/org/ala/collectory/CollectoryTagLib.groovy
@@ -80,7 +80,7 @@ class CollectoryTagLib {
     }
 
     /**
-     * <g:ifAllGranted role="ROLE_COLLECTION_EDITOR,ROLE_COLLECTION_ADMIN">
+     * <g:ifAllGranted role="ROLE_EDITOR,ROLE_ADMIN">
      *  All the listed roles must be granted for the tag to output its body.
      * </g:ifAllGranted>
      */
@@ -102,7 +102,7 @@ class CollectoryTagLib {
     }
 
     /**
-     * <cl:ifGranted role="ROLE_COLLECTION_ADMIN">
+     * <cl:ifGranted role="ROLE_ADMIN">
      *  The specified role must be granted for the tag to output its body.
      * </g:ifGranted>
      * @attr role the role to check
@@ -114,7 +114,7 @@ class CollectoryTagLib {
     }
 
     /**
-     * <cl:ifNotGranted role="ROLE_COLLECTION_ADMIN">
+     * <cl:ifNotGranted role="ROLE_ADMIN">
      *  The specified role must be missing for the tag to output its body.
      * </g:ifNotGranted>
      * @attr role the role to check
@@ -130,7 +130,7 @@ class CollectoryTagLib {
      */
     def roles = {
         def roles = []
-        ['ROLE_ADMIN','ROLE_COLLECTION_EDITOR','ROLE_COLLECTION_ADMIN'].each {
+        ['ROLE_ADMIN','ROLE_EDITOR'].each {
             if (collectoryAuthService.userInRole(it)) {
                 roles << it
             }
diff --git a/grails-app/views/manage/list.gsp b/grails-app/views/manage/list.gsp
index 3dbfb3f6..b6e84399 100644
--- a/grails-app/views/manage/list.gsp
+++ b/grails-app/views/manage/list.gsp
@@ -41,8 +41,8 @@
                             <g:set var="username" value="${request.userPrincipal?.name}"/>
                             <g:if test="${username}">
                                 <p><g:message code="manage.list.username.des01" /> ${username}.</p>
-                                <p>User ${request.isUserInRole('ROLE_COLLECTION_ADMIN') ? 'has' : 'does not have'} ROLE_COLLECTION_ADMIN.</p>
-                                <p>User ${request.isUserInRole('ROLE_COLLECTION_EDITOR') ? 'has' : 'does not have'} ROLE_COLLECTION_EDITOR.</p>
+                                <p>User ${request.isUserInRole('ROLE_ADMIN') ? 'has' : 'does not have'} ROLE_ADMIN.</p>
+                                <p>User ${request.isUserInRole('ROLE_EDITOR') ? 'has' : 'does not have'} ROLE_EDITOR.</p>
                             </g:if>
                             <g:else>
                                 <p><g:message code="manage.list.des03" />.</p>
@@ -76,17 +76,17 @@
                         </table>
                     </g:if>
                     <g:else>
-                        <cl:ifGranted role="ROLE_COLLECTION_ADMIN">
+                        <cl:ifGranted role="ROLE_ADMIN">
                             <p><strong><em><g:message code="manage.list.des05" />.</em></strong></p>
                         </cl:ifGranted>
-                        <cl:ifNotGranted role="ROLE_COLLECTION_ADMIN">
+                        <cl:ifNotGranted role="ROLE_ADMIN">
                             <p style="font-style: italic;margin: 10px;color: black;"><g:message code="manage.list.des06" />.</p>
                         </cl:ifNotGranted>
 
-                        <cl:ifGranted role="ROLE_COLLECTION_EDITOR">
+                        <cl:ifGranted role="ROLE_EDITOR">
                             <p><g:message code="manage.list.des07" />.</p>
                         </cl:ifGranted>
-                        <cl:ifNotGranted role="ROLE_COLLECTION_EDITOR">
+                        <cl:ifNotGranted role="ROLE_EDITOR">
                             <p><g:message code="manage.list.des08" />
                             <span class="link" onclick="return sendEmail('support(SPAM_MAIL@ALA.ORG.AU)ala.org.au')"><g:message code="manage.list.des09" /></span>
                             <g:message code="manage.list.des10" />.</p>
@@ -109,16 +109,16 @@
                             <h4 class="ok"><g:message code="manage.list.title06" /></h4>
                             <p><g:message code="manage.list.des14" /> <em><cl:loggedInUsername/></em>.</p>
 
-                            <cl:ifGranted role="ROLE_COLLECTION_EDITOR">
+                            <cl:ifGranted role="ROLE_EDITOR">
                                 <h4 class="ok"><g:message code="manage.list.title07" /></h4>
                             </cl:ifGranted>
-                            <cl:ifNotGranted role="ROLE_COLLECTION_EDITOR">
+                            <cl:ifNotGranted role="ROLE_EDITOR">
                                 <h4 class="missing"><g:message code="manage.list.title08" />!</h4>
                                 <p><g:message code="manage.list.des15" /> <span class="link" onclick="return sendEmail('support(SPAM_MAIL@ALA.ORG.AU)ala.org.au')"><g:message code="manage.list.des16" /></span>
-                                <g:message code="manage.list.des17" /> ROLE_COLLECTION_EDITOR.</p>
+                                <g:message code="manage.list.des17" /> ROLE_EDITOR.</p>
                             </cl:ifNotGranted>
 
-                            <cl:ifGranted role="ROLE_COLLECTION_EDITOR">
+                            <cl:ifGranted role="ROLE_EDITOR">
                                 <g:if test="${!entities}">
                                     <h4 class="missing"><g:message code="manage.list.title09" />!</h4>
                                 </g:if>
@@ -126,7 +126,7 @@
                                     <h4 class="ok"><g:message code="manage.list.title10" args="[entities.size()]" />.</h4>
                                 </g:else>
                             </cl:ifGranted>
-                            <cl:ifNotGranted role="ROLE_COLLECTION_EDITOR">
+                            <cl:ifNotGranted role="ROLE_EDITOR">
                                 <h4><g:message code="manage.list.title11" />.</h4>
                             </cl:ifNotGranted>
                             <p><g:message code="manage.list.des18" />.</p>
@@ -143,7 +143,7 @@
                 </div>
 
                 <div id="addCollection" class="hidden infoSection">
-                    <cl:ifGranted role="ROLE_COLLECTION_EDITOR">
+                    <cl:ifGranted role="ROLE_EDITOR">
 
                         <h2><g:message code="manage.list.addcollection.title01" /></h2>
                         <p><g:message code="manage.list.addcollection.des01" />:</p>

From 42385b11e6d9cc97515172d9dea3c630f6aaddfc Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Tue, 17 Jan 2023 18:00:55 +1100
Subject: [PATCH 03/10] reverting to usage to interceptors instead of
 AlaSecured annotation as it is unable to enfore role from config i.e.
 non-static value, replacing beforeIntercetor with standard grails
 intercepttors, adding ROLE_ADMIN enforcement for provider map deletion

---
 .../org/ala/collectory/AdminController.groovy |  5 +-
 .../collectory/AdminRoleInterceptor.groovy    | 45 +++++++++++++
 .../ala/collectory/ContactController.groovy   | 21 +++---
 .../ala/collectory/EditRoleInterceptor.groovy | 54 +++++++++++++++
 .../ala/collectory/LicenceController.groovy   | 62 +++++++++--------
 .../collectory/ProviderGroupController.groovy |  3 +-
 .../collectory/ProviderMapController.groovy   | 51 +++++++-------
 .../ala/collectory/TempDataInterceptor.groovy | 66 +++++++++++++++++++
 .../TempDataResourceController.groovy         | 32 ---------
 .../collectory/EditRoleInterceptorSpec.groovy | 22 +++++++
 10 files changed, 262 insertions(+), 99 deletions(-)
 create mode 100644 grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy
 create mode 100644 grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
 create mode 100644 grails-app/controllers/au/org/ala/collectory/TempDataInterceptor.groovy
 create mode 100644 src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy

diff --git a/grails-app/controllers/au/org/ala/collectory/AdminController.groovy b/grails-app/controllers/au/org/ala/collectory/AdminController.groovy
index 49fa1c46..ac0a99f8 100644
--- a/grails-app/controllers/au/org/ala/collectory/AdminController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/AdminController.groovy
@@ -1,15 +1,14 @@
 package au.org.ala.collectory
 
-import au.org.ala.web.AlaSecured
+
 import com.opencsv.CSVReader
 import grails.converters.JSON
 import grails.web.JSONBuilder
 import org.springframework.core.io.support.PathMatchingResourcePatternResolver
 
-@AlaSecured(value = ["ROLE_ADMIN"], message =  "You are not authorised to access this page. You do not have 'Admin' rights.")
 class AdminController {
 
-    def dataLoaderService, idGeneratorService, collectoryAuthService, metadataService, activityLogService, providerGroupService
+    def dataLoaderService, idGeneratorService, metadataService
 
     def index = {
         redirect(controller: 'manage')
diff --git a/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy
new file mode 100644
index 00000000..600cc532
--- /dev/null
+++ b/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2023 Atlas of Living Australia
+ * All Rights Reserved.
+ *
+ * The contents of this file are subject to the Mozilla Public
+ * License Version 1.1 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ */
+
+package au.org.ala.collectory
+
+import org.apache.http.HttpStatus
+
+
+class AdminRoleInterceptor {
+
+    CollectoryAuthService collectoryAuthService
+
+    AdminRoleInterceptor(){
+        match(controller: 'admin')
+    }
+
+    boolean before() {
+        // check roles using userInRoles.userInRole methods also returns true if user has the application configured ROLE_ADMIN.
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
+            return true
+        }
+        response.sendError(HttpStatus.SC_UNAUTHORIZED)
+        return false
+    }
+
+
+
+    boolean after() { true }
+
+    void afterView() {
+        // no-op
+    }
+}
diff --git a/grails-app/controllers/au/org/ala/collectory/ContactController.groovy b/grails-app/controllers/au/org/ala/collectory/ContactController.groovy
index 37266c1d..f4b10d70 100644
--- a/grails-app/controllers/au/org/ala/collectory/ContactController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ContactController.groovy
@@ -1,8 +1,7 @@
 package au.org.ala.collectory
 
-import au.org.ala.web.AlaSecured
 
-@AlaSecured(value = ["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true,  message =  "You are not authorised to access this page. You do not have 'editor' rights.")
+
 class ContactController {
 
     static allowedMethods = [save: "POST", update: "POST", delete: "POST"]
@@ -16,7 +15,6 @@ class ContactController {
     def collectoryAuthService
     def activityLogService
     def providerGroupService
-
     /**
      End access control
      */
@@ -131,10 +129,10 @@ class ContactController {
     /**
      * MEW - modified to cascade delete all ContactFor links for the contact
      */
-    @AlaSecured(value = ["ROLE_ADMIN"], message =  "You are not authorised to access this page.")
     def delete() {
-        def contactInstance = Contact.get(params.id)
-        if (contactInstance) {
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
+            def contactInstance = Contact.get(params.id)
+            if (contactInstance) {
                 try {
                     activityLogService.log collectoryAuthService?.username(), collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN), Action.DELETE, "contact ${contactInstance.buildName()}"
                     // need to delete any ContactFor links first
@@ -152,10 +150,13 @@ class ContactController {
                     flash.message = "${message(code: 'default.not.deleted.message', args: [message(code: 'contact.label', default: 'Contact'), params.id])}"
                     redirect(action: "show", id: params.id)
                 }
-        }
-        else {
-            flash.message = "${message(code: 'default.not.found.message', args: [message(code: 'contact.label', default: 'Contact'), params.id])}"
-            redirect(action: "list")
+            } else {
+                flash.message = "${message(code: 'default.not.found.message', args: [message(code: 'contact.label', default: 'Contact'), params.id])}"
+                redirect(action: "list")
+            }
+        } else{
+            response.setHeader("Content-type", "text/plain; charset=UTF-8")
+            render(message(code: "provider.group.controller.04", default: "You are not authorised to access this page."))
         }
     }
 
diff --git a/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
new file mode 100644
index 00000000..bb67d41f
--- /dev/null
+++ b/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2023 Atlas of Living Australia
+ * All Rights Reserved.
+ *
+ * The contents of this file are subject to the Mozilla Public
+ * License Version 1.1 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ */
+
+package au.org.ala.collectory
+
+import org.apache.http.HttpStatus
+
+
+class EditRoleInterceptor {
+
+    CollectoryAuthService collectoryAuthService
+
+    EditRoleInterceptor(){
+        match(controller: 'collection')
+        match(controller: 'institution')
+        match(controller: 'contact')
+        match(controller: 'licence')
+        match('controller':'providerGroup')
+        match('controller':'providerMap')
+        match('controller':'providerCode')
+        match('controller':'dataProvider')
+        match('controller':'dataHub')
+        match('controller':'reports')
+    }
+
+    boolean before() {
+        // check roles using userInRoles.userInRole methods also returns true if user has the application configured ROLE_ADMIN.
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_EDITOR)) {
+            return true
+        }
+        response.sendError(HttpStatus.SC_UNAUTHORIZED)
+        return false
+    }
+
+
+
+    boolean after() { true }
+
+    void afterView() {
+        // no-op
+    }
+}
diff --git a/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy b/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
index 741bc89e..3975e52e 100644
--- a/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
@@ -1,7 +1,7 @@
 package au.org.ala.collectory
 
 import au.org.ala.plugins.openapi.Path
-import au.org.ala.web.AlaSecured
+
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties
 import grails.converters.JSON
 import io.swagger.v3.oas.annotations.Operation
@@ -18,11 +18,9 @@ import javax.ws.rs.Produces
 /**
  * Simple webservice providing support licences in the system.
  */
-@AlaSecured(value = ["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true,   message =  "You are not authorised to access this page. You do not have 'Collection editor' rights.")
 class LicenceController {
 
     def collectoryAuthService
-
     @Operation(
             method = "GET",
             tags = "licence",
@@ -113,37 +111,42 @@ class LicenceController {
     }
 
     def update(Long id, Long version) {
-        def licenceInstance = Licence.get(id)
-        if (!licenceInstance) {
-            flash.message = message(code: 'default.not.found.message', args: [message(code: 'licence.label', default: 'Licence'), id])
-            redirect(action: "list")
-            return
-        }
-
-        if (version != null) {
-            if (licenceInstance.version > version) {
-                licenceInstance.errors.rejectValue("version", "default.optimistic.locking.failure",
-                        [message(code: 'licence.label', default: 'Licence')] as Object[],
-                        "Another user has updated this Licence while you were editing")
-                render(view: "edit", model: [licenceInstance: licenceInstance])
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
+            def licenceInstance = Licence.get(id)
+            if (!licenceInstance) {
+                flash.message = message(code: 'default.not.found.message', args: [message(code: 'licence.label', default: 'Licence'), id])
+                redirect(action: "list")
                 return
             }
-        }
 
-        licenceInstance.properties = params
+            if (version != null) {
+                if (licenceInstance.version > version) {
+                    licenceInstance.errors.rejectValue("version", "default.optimistic.locking.failure",
+                            [message(code: 'licence.label', default: 'Licence')] as Object[],
+                            "Another user has updated this Licence while you were editing")
+                    render(view: "edit", model: [licenceInstance: licenceInstance])
+                    return
+                }
+            }
 
-        def savedInstance = null
-        Licence.withTransaction {
-            savedInstance = licenceInstance.save(flush: true)
-        }
+            licenceInstance.properties = params
 
-        if (!savedInstance) {
-            render(view: "edit", model: [licenceInstance: licenceInstance])
-            return
-        }
+            def savedInstance = null
+            Licence.withTransaction {
+                savedInstance = licenceInstance.save(flush: true)
+            }
 
-        flash.message = message(code: 'default.updated.message', args: [message(code: 'licence.label', default: 'Licence'), licenceInstance.id])
-        redirect(action: "show", id: licenceInstance.id)
+            if (!savedInstance) {
+                render(view: "edit", model: [licenceInstance: licenceInstance])
+                return
+            }
+
+            flash.message = message(code: 'default.updated.message', args: [message(code: 'licence.label', default: 'Licence'), licenceInstance.id])
+            redirect(action: "show", id: licenceInstance.id)
+        } else{
+            response.setHeader("Content-type", "text/plain; charset=UTF-8")
+            render(message(code: "provider.group.controller.04", default: "You are not authorised to access this page."))
+        }
     }
 
     def delete(Long id) {
@@ -166,6 +169,9 @@ class LicenceController {
                 flash.message = message(code: 'default.not.deleted.message', args: [message(code: 'licence.label', default: 'Licence'), id])
                 redirect(action: "show", id: id)
             }
+        } else{
+            response.setHeader("Content-type", "text/plain; charset=UTF-8")
+            render(message(code: "provider.group.controller.04", default: "You are not authorised to access this page."))
         }
     }
 }
diff --git a/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy b/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
index 685d1ee9..4bb9fb11 100644
--- a/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
@@ -1,6 +1,6 @@
 package au.org.ala.collectory
 
-import au.org.ala.web.AlaSecured
+
 import grails.converters.JSON
 import org.springframework.dao.DataIntegrityViolationException
 import org.springframework.web.context.request.RequestContextHolder
@@ -11,7 +11,6 @@ import org.springframework.web.multipart.MultipartFile
  *
  * It provides common code for shared attributes like contacts.
  */
-@AlaSecured(value = ["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true, message =  "You are not authorised to access this page. You do not have 'editor' rights.")
 abstract class ProviderGroupController {
 
     String entityName = "ProviderGroup"
diff --git a/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy b/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy
index 6e63d4ff..80440c8d 100644
--- a/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ProviderMapController.groovy
@@ -1,9 +1,8 @@
 package au.org.ala.collectory
 
-import au.org.ala.web.AlaSecured
+
 import grails.gorm.transactions.Transactional
 
-@AlaSecured(value =["ROLE_ADMIN", "ROLE_EDITOR"], anyRole = true,  message =  "You are not authorised to access this page. You do not have 'editor' rights.")
 class ProviderMapController {
 
     static allowedMethods = [save: "POST", update: "POST", delete: "POST"]
@@ -114,31 +113,35 @@ class ProviderMapController {
 
     @Transactional
     def delete () {
-        def providerMapInstance = ProviderMap.get(params.id)
-        if (providerMapInstance) {
-            if (providerMapInstance.collection.uid) {
-                try {
-                    // remove collection link
-                    providerMapInstance.collection?.providerMap = null
-                    // remove code links
-                    providerMapInstance.collectionCodes.removeAll(providerMapInstance.collectionCodes)
-                    providerMapInstance.institutionCodes.removeAll(providerMapInstance.institutionCodes)
-                    // remove map
-                    providerMapInstance.delete(flush: true)
-                    flash.message = "${message(code: 'default.deleted.message', args: [message(code: 'providerMap.label', default: 'ProviderMap'), params.id])}"
-                    redirect(action: "list", params:[returnTo: params.returnTo])
-                }
-                catch (org.springframework.dao.DataIntegrityViolationException e) {
-                    flash.message = "${message(code: 'default.not.deleted.message', args: [message(code: 'providerMap.label', default: 'ProviderMap'), params.id])}"
-                    redirect(action: "show", id: params.id, params:[returnTo: params.returnTo])
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
+            def providerMapInstance = ProviderMap.get(params.id)
+            if (providerMapInstance) {
+                if (providerMapInstance.collection.uid) {
+                    try {
+                        // remove collection link
+                        providerMapInstance.collection?.providerMap = null
+                        // remove code links
+                        providerMapInstance.collectionCodes.removeAll(providerMapInstance.collectionCodes)
+                        providerMapInstance.institutionCodes.removeAll(providerMapInstance.institutionCodes)
+                        // remove map
+                        providerMapInstance.delete(flush: true)
+                        flash.message = "${message(code: 'default.deleted.message', args: [message(code: 'providerMap.label', default: 'ProviderMap'), params.id])}"
+                        redirect(action: "list", params: [returnTo: params.returnTo])
+                    }
+                    catch (org.springframework.dao.DataIntegrityViolationException e) {
+                        flash.message = "${message(code: 'default.not.deleted.message', args: [message(code: 'providerMap.label', default: 'ProviderMap'), params.id])}"
+                        redirect(action: "show", id: params.id, params: [returnTo: params.returnTo])
+                    }
+                } else {
+                    render "You are not authorised to access this page."
                 }
             } else {
-                render "You are not authorised to access this page."
+                flash.message = "${message(code: 'default.not.found.message', args: [message(code: 'providerMap.label', default: 'ProviderMap'), params.id])}"
+                redirect(action: "list", params: [returnTo: params.returnTo])
             }
-        }
-        else {
-            flash.message = "${message(code: 'default.not.found.message', args: [message(code: 'providerMap.label', default: 'ProviderMap'), params.id])}"
-            redirect(action: "list", params:[returnTo: params.returnTo])
+        } else{
+            response.setHeader("Content-type", "text/plain; charset=UTF-8")
+            render(message(code: "provider.group.controller.04", default: "You are not authorised to access this page."))
         }
     }
 }
diff --git a/grails-app/controllers/au/org/ala/collectory/TempDataInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/TempDataInterceptor.groovy
new file mode 100644
index 00000000..6d8be60a
--- /dev/null
+++ b/grails-app/controllers/au/org/ala/collectory/TempDataInterceptor.groovy
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2023 Atlas of Living Australia
+ * All Rights Reserved.
+ *
+ * The contents of this file are subject to the Mozilla Public
+ * License Version 1.1 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ */
+
+package au.org.ala.collectory
+
+class TempDataInterceptor {
+
+    TempDataInterceptor(){
+        match(controller: 'tempDataResource')
+    }
+
+    boolean before() {
+        /** make sure that uid params point to an existing entity and json is parsable **/
+        def uid = params.uid
+        if (uid) {
+            // it must exist
+            def drt = TempDataResource.findByUid(uid)
+            if (drt) {
+                params.drt = drt
+                return true
+            } else {
+                // doesn't exist
+                notFound "no entity with uid = ${uid}"
+                return false
+            }
+        }
+
+        // check payload
+        if (request.getContentLength() == 0) {
+            // no payload so return OK as entity exists (if specified)
+            success "no post body"
+            return false
+        }
+        try {
+            params.json = request.JSON
+            return true
+        } catch (Exception e) {
+            println "exception caught ${e}"
+            if (request.getContentLength() > 0) {
+                badRequest 'cannot parse request body as JSON'
+                return false
+            }
+        }
+        false
+    }
+
+
+
+    boolean after() { true }
+
+    void afterView() {
+        // no-op
+    }
+}
diff --git a/grails-app/controllers/au/org/ala/collectory/TempDataResourceController.groovy b/grails-app/controllers/au/org/ala/collectory/TempDataResourceController.groovy
index 9eb13996..ea4a1c5a 100644
--- a/grails-app/controllers/au/org/ala/collectory/TempDataResourceController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/TempDataResourceController.groovy
@@ -25,38 +25,6 @@ class TempDataResourceController {
 
     static allowedMethods = [save: "POST", update: "POST", delete: "POST"]
 
-    /** make sure that uid params point to an existing entity and json is parsable **/
-    def beforeInterceptor = this.&check
-
-    def check() {
-        def uid = params.uid
-        if (uid) {
-            // it must exist
-            def drt = TempDataResource.findByUid(uid)
-            if (drt) {
-                params.drt = drt
-            } else {
-                // doesn't exist
-                notFound "no entity with uid = ${uid}"
-                return false
-            }
-        }
-        // check payload
-        if (request.getContentLength() == 0) {
-            // no payload so return OK as entity exists (if specified)
-            success "no post body"
-            return false
-        }
-        try {
-            params.json = request.JSON
-        } catch (Exception e) {
-            println "exception caught ${e}"
-            if (request.getContentLength() > 0) {
-                badRequest 'cannot parse request body as JSON'
-                return false
-            }
-        }
-    }
 
     def unauthorised = {
         // using the 'forbidden' response code here as 401 causes the client to ask for a log in
diff --git a/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy b/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy
new file mode 100644
index 00000000..c29c1e0a
--- /dev/null
+++ b/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy
@@ -0,0 +1,22 @@
+package au.org.ala.collectory
+
+import grails.testing.web.interceptor.InterceptorUnitTest
+import spock.lang.Specification
+
+class EditRoleInterceptorSpec extends Specification implements InterceptorUnitTest<EditRoleInterceptor> {
+
+    def setup() {
+    }
+
+    def cleanup() {
+
+    }
+
+    void "Test role interceptor matching"() {
+        when:"A request matches the interceptor"
+            withRequest(controller:"role")
+
+        then:"The interceptor does match"
+            interceptor.doesMatch()
+    }
+}

From e11b5cafcd951ca015c2e79c5d47fdaa4aad0dd1 Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Wed, 18 Jan 2023 14:19:50 +1100
Subject: [PATCH 04/10] adding issue-175  branch to travis build config

---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 5433c2fe..65508237 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,7 +6,7 @@ branches:
   only:
     - master
     - develop
-    - feature/oidc
+    - feature/issue-175
 
 before_cache:
   - rm -f  $HOME/.gradle/caches/modules-2/modules-2.lock

From 19c4aa4d797342694caef7ed403b5f075e724a4c Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Wed, 18 Jan 2023 15:13:58 +1100
Subject: [PATCH 05/10] issue #175 adding edit role permission to licence
 actions to allow open access to licence index method via webservice

---
 .../ala/collectory/EditRoleInterceptor.groovy |  5 +-
 .../ala/collectory/LicenceController.groovy   | 55 +++++++++----------
 2 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
index bb67d41f..84eff8a5 100644
--- a/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
@@ -26,7 +26,10 @@ class EditRoleInterceptor {
         match(controller: 'collection')
         match(controller: 'institution')
         match(controller: 'contact')
-        match(controller: 'licence')
+        match(controller: 'licence', action: 'create')
+        match(controller: 'licence', action: 'edit')
+        match(controller: 'licence', action: 'show')
+        match(controller: 'licence', action: 'save')
         match('controller':'providerGroup')
         match('controller':'providerMap')
         match('controller':'providerCode')
diff --git a/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy b/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
index 3975e52e..7519fc09 100644
--- a/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/LicenceController.groovy
@@ -111,42 +111,37 @@ class LicenceController {
     }
 
     def update(Long id, Long version) {
-        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
-            def licenceInstance = Licence.get(id)
-            if (!licenceInstance) {
-                flash.message = message(code: 'default.not.found.message', args: [message(code: 'licence.label', default: 'Licence'), id])
-                redirect(action: "list")
-                return
-            }
-
-            if (version != null) {
-                if (licenceInstance.version > version) {
-                    licenceInstance.errors.rejectValue("version", "default.optimistic.locking.failure",
-                            [message(code: 'licence.label', default: 'Licence')] as Object[],
-                            "Another user has updated this Licence while you were editing")
-                    render(view: "edit", model: [licenceInstance: licenceInstance])
-                    return
-                }
-            }
-
-            licenceInstance.properties = params
-
-            def savedInstance = null
-            Licence.withTransaction {
-                savedInstance = licenceInstance.save(flush: true)
-            }
+        def licenceInstance = Licence.get(id)
+        if (!licenceInstance) {
+            flash.message = message(code: 'default.not.found.message', args: [message(code: 'licence.label', default: 'Licence'), id])
+            redirect(action: "list")
+            return
+        }
 
-            if (!savedInstance) {
+        if (version != null) {
+            if (licenceInstance.version > version) {
+                licenceInstance.errors.rejectValue("version", "default.optimistic.locking.failure",
+                        [message(code: 'licence.label', default: 'Licence')] as Object[],
+                        "Another user has updated this Licence while you were editing")
                 render(view: "edit", model: [licenceInstance: licenceInstance])
                 return
             }
+        }
 
-            flash.message = message(code: 'default.updated.message', args: [message(code: 'licence.label', default: 'Licence'), licenceInstance.id])
-            redirect(action: "show", id: licenceInstance.id)
-        } else{
-            response.setHeader("Content-type", "text/plain; charset=UTF-8")
-            render(message(code: "provider.group.controller.04", default: "You are not authorised to access this page."))
+        licenceInstance.properties = params
+
+        def savedInstance = null
+        Licence.withTransaction {
+            savedInstance = licenceInstance.save(flush: true)
+        }
+
+        if (!savedInstance) {
+            render(view: "edit", model: [licenceInstance: licenceInstance])
+            return
         }
+
+        flash.message = message(code: 'default.updated.message', args: [message(code: 'licence.label', default: 'Licence'), licenceInstance.id])
+        redirect(action: "show", id: licenceInstance.id)
     }
 
     def delete(Long id) {

From 18c90e8c9b06d036f1d5f14a64349eda641a89e8 Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Wed, 18 Jan 2023 15:33:22 +1100
Subject: [PATCH 06/10] issue #175 updaing unit test for edit and admin
 intercetptors

---
 .../AdminRoleInterceptorSpec.groovy           | 32 +++++++++++++++++++
 .../collectory/EditRoleInterceptorSpec.groovy | 31 ++++++++++++++++--
 2 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 src/test/groovy/au/org/ala/collectory/AdminRoleInterceptorSpec.groovy

diff --git a/src/test/groovy/au/org/ala/collectory/AdminRoleInterceptorSpec.groovy b/src/test/groovy/au/org/ala/collectory/AdminRoleInterceptorSpec.groovy
new file mode 100644
index 00000000..a7f479a2
--- /dev/null
+++ b/src/test/groovy/au/org/ala/collectory/AdminRoleInterceptorSpec.groovy
@@ -0,0 +1,32 @@
+package au.org.ala.collectory
+
+import grails.testing.web.interceptor.InterceptorUnitTest
+import spock.lang.Specification
+
+class AdminRoleInterceptorSpec extends Specification implements InterceptorUnitTest<AdminRoleInterceptor> {
+
+    def setup() {
+    }
+
+    def cleanup() {
+
+    }
+
+    void "Test AdminRole interceptor matching"() {
+        // admin role interceptor should also match everything from editrole interceptor.
+        when:"A request matches the interceptor"
+        withRequest(controller: 'admin', action: 'reloadConfig')
+
+
+        then:"The interceptor does match"
+        interceptor.doesMatch()
+    }
+
+
+    void "Test AdminRole interceptor not matching"() {
+        when:"A request matches the interceptor"
+        withRequest(controller: 'admins', action: 'test')
+        then:"The interceptor does not match"
+        !interceptor.doesMatch()
+    }
+}
diff --git a/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy b/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy
index c29c1e0a..eff6e647 100644
--- a/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy
+++ b/src/test/groovy/au/org/ala/collectory/EditRoleInterceptorSpec.groovy
@@ -12,11 +12,36 @@ class EditRoleInterceptorSpec extends Specification implements InterceptorUnitTe
 
     }
 
-    void "Test role interceptor matching"() {
+    void "Test EditRole interceptor matching"() {
         when:"A request matches the interceptor"
-            withRequest(controller:"role")
+        withRequest(controller: 'collection', action: "list")
+        withRequest('controller': 'collection')
+        withRequest('controller': 'institution', action: 'list')
+        withRequest('controller': 'contact')
+        withRequest('controller': 'licence', action: 'create')
+        withRequest('controller': 'licence', action: 'edit')
+        withRequest('controller': 'licence', action: 'show')
+        withRequest('controller': 'licence', action: 'save')
+        withRequest('controller':'providerGroup')
+        withRequest('controller':'providerMap')
+        withRequest('controller':'providerCode')
+        withRequest('controller':'dataProvider')
+        withRequest('controller':'dataHub')
+        withRequest('controller':'reports')
 
         then:"The interceptor does match"
-            interceptor.doesMatch()
+        interceptor.doesMatch()
+    }
+
+
+    void "Test EditRole interceptor not matching"() {
+        when:"A request matches the interceptor"
+        withRequest(controller: 'data', action: "getEntity")
+        withRequest(controller: 'data', action: "mapToCsv")
+        withRequest(controller: 'admin')
+        withRequest(controller: 'randomController', action: "randomAction")
+
+        then:"The interceptor does not match"
+        !interceptor.doesMatch()
     }
 }

From 17903af665d11beab66dadcd0684d0d993557828 Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Thu, 19 Jan 2023 12:31:07 +1100
Subject: [PATCH 07/10] issur #175 adding requiredRoles check to collectory
 custom JWT authorizer, removing check for user/read scope, settig minumum JWT
 role for protected APIs to ROLE_EDITOR and (ROLE_ADMIN or
 gbifRegistrationRole) for GBIF and IPT actions and contollers

---
 .../CollectoryWebServicesInterceptor.groovy   | 22 ++++++++---
 .../collectory/CollectoryAuthService.groovy   | 39 ++++++++-----------
 2 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/grails-app/controllers/au/org/ala/collectory/CollectoryWebServicesInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/CollectoryWebServicesInterceptor.groovy
index a3da6a84..9dd94bbd 100644
--- a/grails-app/controllers/au/org/ala/collectory/CollectoryWebServicesInterceptor.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/CollectoryWebServicesInterceptor.groovy
@@ -33,13 +33,25 @@ class CollectoryWebServicesInterceptor {
 
     }
     boolean before() {
-        if (!collectoryAuthService.isAuthorisedWsRequest(params, request, response)) {
-            log.warn("Denying access to $actionName from remote addr: ${request.remoteAddr}, remote host: ${request.remoteHost}")
-            response.sendError(HttpStatus.SC_UNAUTHORIZED)
+        // set default role requirement for protected ROLE_EDITOR as the same info is only available to ROLE_EDITOR via the UI.
+        String[] requiredRoles = [grailsApplication.config.ROLE_EDITOR]
 
-            return false
+        // set gbifRegistrationRole role requirement for GBIF  operations
+        if(controllerName == 'gbif' || actionName == 'syncGBIF'){
+            requiredRoles = [grailsApplication.config.gbifRegistrationRole]
         }
-        return true
+
+        // set ROLE_ADMIN role requirement for certain controllers and actions as per admin UI
+        if( controllerName == 'ipt'){
+            requiredRoles = [grailsApplication.config.ROLE_ADMIN]
+        }
+
+        if (collectoryAuthService.isAuthorisedWsRequest(params, request, response, requiredRoles, null)) {
+            return true
+        }
+        log.warn("Denying access to $actionName from remote addr: ${request.remoteAddr}, remote host: ${request.remoteHost}")
+        response.sendError(HttpStatus.SC_UNAUTHORIZED)
+        return false
     }
 
     boolean after() { true }
diff --git a/grails-app/services/au/org/ala/collectory/CollectoryAuthService.groovy b/grails-app/services/au/org/ala/collectory/CollectoryAuthService.groovy
index 2dbbb95c..5b2faecf 100644
--- a/grails-app/services/au/org/ala/collectory/CollectoryAuthService.groovy
+++ b/grails-app/services/au/org/ala/collectory/CollectoryAuthService.groovy
@@ -150,7 +150,7 @@ class CollectoryAuthService{
 
 
 
-    def checkJWT(HttpServletRequest request, HttpServletResponse response, String role, String scope) {
+    def checkJWT(HttpServletRequest request, HttpServletResponse response, String[] requiredRoles, String[] requiredScopes) {
         def result = false
             def context = context(request, response)
             ProfileManager profileManager = new ProfileManager(context, config.sessionStore)
@@ -168,30 +168,25 @@ class CollectoryAuthService{
                     )
 
                     result = true
-                    if (role) {
-                        result = userProfile.roles.contains(role)
+                    if (result && requiredRoles) {
+                        def roles = userProfile.roles
+                        // check if the user profile has ROLE_ADMIN otherwise check their roles against required roles
+                        def hasAdminRole = roles.contains(grailsApplication.config.ROLE_ADMIN)
+                        result = hasAdminRole ?: requiredRoles.every() {
+                            // set true if the user has the required role or the configured ROLE_ADMIN
+                            roles.contains(it)
+                        }
                     }
 
-                    if (result && scope) {
-                        result = userProfile.permissions.contains(scope) || profileHasScope(userProfile, scope)
+                    if (result && requiredScopes) {
+                        def scope = userProfile.permissions //attributes['scope'] as List<String>
+                        result = requiredScopes.every {
+                            scope.contains(it)
+                        }
                     }
-                }
-            }
-        return result
-    }
 
-    private static boolean profileHasScope(UserProfile userProfile, String scope) {
-        def scopes = userProfile.attributes['scope']
-        def result = false
-        if (scopes != null) {
-            if (scopes instanceof String) {
-                result = scopes.tokenize(',').contains(scope)
-            } else if (scopes.class.isArray()) {
-                result =scopes.any { it?.toString() == scope }
-            } else if (scopes instanceof Collection) {
-                result =scopes.any { it?.toString() == scope }
+                }
             }
-        }
         return result
     }
 
@@ -200,10 +195,10 @@ class CollectoryAuthService{
         return context
     }
 
-    def isAuthorisedWsRequest(GrailsParameterMap params, HttpServletRequest request, HttpServletResponse response){
+    def isAuthorisedWsRequest(GrailsParameterMap params, HttpServletRequest request, HttpServletResponse response, String[] requiredRoles, String[] requiredScopes){
         Boolean authorised
         // check for JWT first
-        authorised = checkJWT(request, response, null, 'users/read')
+        authorised = checkJWT(request, response, requiredRoles, requiredScopes)
         // if still unauthorised, check for and attempt  to validate API key
         if(!authorised && grailsApplication.config.security.apikey.checkEnabled.toBoolean()){
             def apiKey = getApiKey(params, request)

From d824abf834c9753b8407ae8bc2202a0b492b7980 Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Thu, 19 Jan 2023 13:03:44 +1100
Subject: [PATCH 08/10] issue #175 adding role enforcement for JWT usage on
 optional authorization accepted for getEntity

---
 .../controllers/au/org/ala/collectory/DataController.groovy    | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/grails-app/controllers/au/org/ala/collectory/DataController.groovy b/grails-app/controllers/au/org/ala/collectory/DataController.groovy
index 581418f4..bf8e3d6b 100644
--- a/grails-app/controllers/au/org/ala/collectory/DataController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/DataController.groovy
@@ -449,7 +449,8 @@ class DataController {
                 def entityInJson
                 if (clazz == 'DataResource') {
                     // this auth check (JWT or API key) is a special case handling to support backwards compatibility(which used to check for API key).
-                    def authCheck = collectoryAuthService.isAuthorisedWsRequest(getParams(), request, response)
+                    String [] requiredRoles = [grailsApplication.config.ROLE_ADMIN]
+                    def authCheck = collectoryAuthService.isAuthorisedWsRequest(getParams(), request, response, requiredRoles,null)
                     entityInJson = crudService."read${clazz}"(params.pg, authCheck)
                 } else {
                     entityInJson = crudService."read${clazz}"(params.pg)

From de9a5f50a775abda872fb7c5f9d3576752ce073d Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Thu, 19 Jan 2023 17:11:23 +1100
Subject: [PATCH 09/10] issue #158 updating isAuthorisedToEdit methods,
 removing duplicate declarations of activityLogServic and 
 providerGroupService on controllers which extend ProviderGroupController to
 prevent providerGroupService from being set to null

---
 .../au/org/ala/collectory/CollectionController.groovy      | 2 --
 .../au/org/ala/collectory/DataHubController.groovy         | 2 --
 .../au/org/ala/collectory/DataProviderController.groovy    | 2 --
 .../au/org/ala/collectory/DataResourceController.groovy    | 2 +-
 .../au/org/ala/collectory/InstitutionController.groovy     | 2 +-
 .../au/org/ala/collectory/ProviderGroupController.groovy   | 7 +++++--
 6 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/grails-app/controllers/au/org/ala/collectory/CollectionController.groovy b/grails-app/controllers/au/org/ala/collectory/CollectionController.groovy
index 347463ad..0f0f7d69 100644
--- a/grails-app/controllers/au/org/ala/collectory/CollectionController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/CollectionController.groovy
@@ -9,8 +9,6 @@ import java.text.ParseException
 
 class CollectionController extends ProviderGroupController {
 
-    def activityLogService, providerGroupService
-
     CollectionController() {
         entityName = "Collection"
         entityNameLower = "collection"
diff --git a/grails-app/controllers/au/org/ala/collectory/DataHubController.groovy b/grails-app/controllers/au/org/ala/collectory/DataHubController.groovy
index 7e5c4708..34bc3611 100644
--- a/grails-app/controllers/au/org/ala/collectory/DataHubController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/DataHubController.groovy
@@ -2,8 +2,6 @@ package au.org.ala.collectory
 
 class DataHubController extends ProviderGroupController {
 
-    def activityLogService, providerGroupService
-
     DataHubController() {
         entityName = "DataHub"
         entityNameLower = "dataHub"
diff --git a/grails-app/controllers/au/org/ala/collectory/DataProviderController.groovy b/grails-app/controllers/au/org/ala/collectory/DataProviderController.groovy
index 76475db4..480a78d5 100644
--- a/grails-app/controllers/au/org/ala/collectory/DataProviderController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/DataProviderController.groovy
@@ -4,8 +4,6 @@ class DataProviderController extends ProviderGroupController {
 
     def gbifRegistryService
     def authService
-    def activityLogService
-    def providerGroupService
 
     DataProviderController() {
         entityName = "DataProvider"
diff --git a/grails-app/controllers/au/org/ala/collectory/DataResourceController.groovy b/grails-app/controllers/au/org/ala/collectory/DataResourceController.groovy
index 63e66086..74a5ff8a 100644
--- a/grails-app/controllers/au/org/ala/collectory/DataResourceController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/DataResourceController.groovy
@@ -8,7 +8,7 @@ import au.org.ala.collectory.resources.DarwinCoreFields
 
 class DataResourceController extends ProviderGroupController {
 
-    def metadataService, dataImportService, gbifRegistryService, authService, activityLogService, providerGroupService
+    def metadataService, dataImportService, gbifRegistryService, authService
 
     DataResourceController() {
         entityName = "DataResource"
diff --git a/grails-app/controllers/au/org/ala/collectory/InstitutionController.groovy b/grails-app/controllers/au/org/ala/collectory/InstitutionController.groovy
index 21b6071b..433df1e4 100644
--- a/grails-app/controllers/au/org/ala/collectory/InstitutionController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/InstitutionController.groovy
@@ -5,7 +5,7 @@ import grails.gorm.transactions.Transactional
 
 class InstitutionController extends ProviderGroupController {
 
-    def authService, activityLogService, providerGroupService
+    def authService
 
     InstitutionController() {
         entityName = "Institution"
diff --git a/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy b/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
index 4bb9fb11..2226b2dd 100644
--- a/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ProviderGroupController.groovy
@@ -857,8 +857,11 @@ abstract class ProviderGroupController {
             return true
         } else {
             def email = RequestContextHolder.currentRequestAttributes()?.getUserPrincipal()?.name
-            if (email) {
-                return providerGroupService._get(uid)?.isAuthorised(email)
+            ProviderGroup pg = providerGroupService?._get(uid)
+            if (email && pg) {
+                if(pg){
+                    return pg.isAuthorised(email)
+                }
             }
         }
         return false

From 61f11256cf70c5c746f6716643d55781d0c2fadd Mon Sep 17 00:00:00 2001
From: Sushant <105408469+sughics@users.noreply.github.com>
Date: Thu, 19 Jan 2023 19:20:11 +1100
Subject: [PATCH 10/10] issue #175 updating adminroleinterceptor to include
 admin gbif functions, updating editroleinterceptor to include missing
 dataResource controller and licence/list, updating providerCodeController to
 enfore ROLE_ADMIN on deletion

---
 .../ala/collectory/AdminRoleInterceptor.groovy   | 16 +++++++++++++++-
 .../ala/collectory/EditRoleInterceptor.groovy    |  2 ++
 .../ala/collectory/ProviderCodeController.groovy |  6 ++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy
index 600cc532..110e7283 100644
--- a/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/AdminRoleInterceptor.groovy
@@ -24,10 +24,24 @@ class AdminRoleInterceptor {
 
     AdminRoleInterceptor(){
         match(controller: 'admin')
+        match(controller: 'manage')
+        match(controller: 'gbif', actionName:'healthCheck')
+        match(controller: 'gbif', actionName:'healthCheckLinked')
+        match(controller: 'gbif', actionName:'downloadCSV')
     }
 
     boolean before() {
-        // check roles using userInRoles.userInRole methods also returns true if user has the application configured ROLE_ADMIN.
+        // add an exception for /manage/  and /manage/index/ as it is redirected from  /admin/  - which should be visible to all logged in users.
+        if(controllerName == "manage" && (actionName == "index" || actionName == "list")){
+            return true
+        }
+
+        // check against configured gbifRegistrationRole for admin gbif methods
+        if(controllerName == "gbif" && collectoryAuthService?.userInRole(grailsApplication.config.gbifRegistrationRole)){
+            return true
+        }
+
+        // check roles using userInRoles.userInRole method. also returns true if user has the application configured ROLE_ADMIN.
         if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
             return true
         }
diff --git a/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy b/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
index 84eff8a5..4a8a4021 100644
--- a/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/EditRoleInterceptor.groovy
@@ -26,6 +26,7 @@ class EditRoleInterceptor {
         match(controller: 'collection')
         match(controller: 'institution')
         match(controller: 'contact')
+        match(controller: 'licence', action: 'list')
         match(controller: 'licence', action: 'create')
         match(controller: 'licence', action: 'edit')
         match(controller: 'licence', action: 'show')
@@ -33,6 +34,7 @@ class EditRoleInterceptor {
         match('controller':'providerGroup')
         match('controller':'providerMap')
         match('controller':'providerCode')
+        match('controller':'dataResource')
         match('controller':'dataProvider')
         match('controller':'dataHub')
         match('controller':'reports')
diff --git a/grails-app/controllers/au/org/ala/collectory/ProviderCodeController.groovy b/grails-app/controllers/au/org/ala/collectory/ProviderCodeController.groovy
index 67969b00..2aed73b4 100644
--- a/grails-app/controllers/au/org/ala/collectory/ProviderCodeController.groovy
+++ b/grails-app/controllers/au/org/ala/collectory/ProviderCodeController.groovy
@@ -6,6 +6,7 @@ import org.springframework.dao.DataIntegrityViolationException
 class ProviderCodeController {
 
     static allowedMethods = [save: "POST", update: "POST", delete: "POST"]
+    def collectoryAuthService
 
     def index() {
         redirect(action: "list", params: params)
@@ -87,6 +88,7 @@ class ProviderCodeController {
 
     @Transactional
     def delete(Long id) {
+        if (collectoryAuthService?.userInRole(grailsApplication.config.ROLE_ADMIN)) {
         def providerCodeInstance = ProviderCode.get(id)
         if (!providerCodeInstance) {
             flash.message = message(code: 'default.not.found.message', args: [message(code: 'providerCode.label', default: 'ProviderCode'), id])
@@ -103,5 +105,9 @@ class ProviderCodeController {
             flash.message = message(code: 'default.not.deleted.message', args: [message(code: 'providerCode.label', default: 'ProviderCode'), id])
             redirect(action: "show", id: id)
         }
+        } else{
+            response.setHeader("Content-type", "text/plain; charset=UTF-8")
+            render(message(code: "provider.group.controller.04", default: "You are not authorised to access this page."))
+        }
     }
 }