Skip to content

Commit

Permalink
Merge pull request #193 from AtlasOfLivingAustralia/feature/biosecuri…
Browse files Browse the repository at this point in the history
…ty_admin_features

Add biosecurity admin features #191
  • Loading branch information
yasima-csiro authored Jun 19, 2024
2 parents 192c9f4 + 1b85662 commit 9aa8299
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import com.opencsv.RFC4180ParserBuilder
import org.springframework.web.multipart.MultipartFile
import org.springframework.web.multipart.MultipartHttpServletRequest

import static org.apache.http.HttpStatus.SC_UNAUTHORIZED

@PreAuthorise
class AdminController {

Expand All @@ -32,7 +34,18 @@ class AdminController {
def profileService
def authorisedSystemService

def index() {}
@PreAuthorise(allowedRoles = ["ROLE_ADMIN", "ROLE_USER_CREATOR"])
def index() {
def user = userService.currentUser

if (user) {
def isBiosecurityAdmin = request.isUserInRole("ROLE_USER_CREATOR")
[isBiosecurityAdmin: isBiosecurityAdmin]
} else {
log.info('my-profile without a user?')
render(status: SC_UNAUTHORIZED)
}
}

def resetPasswordForUser(){
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ class ProfileController {
if (user) {
def props = user.propsAsMap()
def isAdmin = request.isUserInRole("ROLE_ADMIN")
render(view: "myprofile", model: [user: user, props: props, isAdmin: isAdmin])
def isBiosecurityAdmin = request.isUserInRole("ROLE_USER_CREATOR")
render(view: "myprofile", model: [user: user, props: props, isAdmin: isAdmin, isBiosecurityAdmin: isBiosecurityAdmin])
} else {
log.info('my-profile without a user?')
render(status: SC_UNAUTHORIZED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class PropertyController extends BaseController {
)
@Path("getProperty")
@Produces("application/json")
@PreAuthorise(requiredScope = 'users/read', requiredRole = '')
@PreAuthorise(requiredScope = 'users/read', allowedRoles = [])
def getProperty() {
String name = params.name
Long alaId = params.long('alaId')
Expand Down Expand Up @@ -166,7 +166,7 @@ class PropertyController extends BaseController {
)
@Path("saveProperty")
@Produces("application/json")
@PreAuthorise(requiredScope = 'users/write', requiredRole = '')
@PreAuthorise(requiredScope = 'users/write', allowedRoles = [])
def saveProperty(){
String name = params.name;
String value = params.value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class RoleBasedInterceptor {
PreAuthorise pa = method.getAnnotation(PreAuthorise) ?: controllerClass.getAnnotation(PreAuthorise)
response.withFormat {
json {
if (!authorisedSystemService.isAuthorisedRequest(request, response, pa.requiredRole(), pa.requiredScope())) {
if (!authorisedSystemService.isAuthorisedRequest(request, response, pa.allowedRoles(), 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)
Expand All @@ -51,8 +51,8 @@ class RoleBasedInterceptor {
}
}
'*' {
def requiredRole = pa.requiredRole()
def inRole = request?.isUserInRole(requiredRole)
def allowedRoles = pa.allowedRoles()
def inRole = allowedRoles.any { role -> request?.isUserInRole(role) }

if (!inRole) {
log.warn("Denying access to $controllerName, $actionName to ${request?.userPrincipal?.name}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@ class UserController {
}
}

@PreAuthorise(allowedRoles = ["ROLE_ADMIN", "ROLE_USER_CREATOR"])
def create() {
[userInstance: new User(params)]
def isBiosecurityAdmin = request.isUserInRole("ROLE_USER_CREATOR")
[userInstance: new User(params), isBiosecurityAdmin: isBiosecurityAdmin]
}

@PreAuthorise(allowedRoles = ["ROLE_ADMIN", "ROLE_USER_CREATOR"])
@Transactional
def save() {
def userInstance = new User(params)
Expand All @@ -58,9 +61,18 @@ class UserController {
}

userService.updateProperties(userInstance, params)
userService.addUserRole(userInstance, "ROLE_USER")

def isBiosecurityAdmin = request.isUserInRole("ROLE_USER_CREATOR")

flash.message = message(code: 'default.created.message', args: [message(code: 'user.label', default: 'User'), userInstance.id])
redirect(action: "show", id: userInstance.id)
if(!isBiosecurityAdmin) {
redirect(action: "show", id: userInstance.id)
}
else{
//ROLE_USER_CREATOR role does not have permission to show(id) action
redirect(controller: "user", action: 'create')
}
}

def show(Long id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ class AuthorisedSystemService {
/**
* Validate a JWT Bearer token instead of the API key.
* @param fallbackToLegacy Whether to fall back to legacy authorised systems if the JWT is not present.
* @param role The user role required to continue
* @param roles The user roles required to continue. The user must have at least one role to be authorized.
* @param scope The JWT scope required for the request to be authorized
* @return true
*/
def isAuthorisedRequest(HttpServletRequest request, HttpServletResponse response, String role, String scope) {
def isAuthorisedRequest(HttpServletRequest request, HttpServletResponse response, String[] roles, String scope) {
def result = false

if (jwtProperties.enabled) {
Expand All @@ -73,8 +73,8 @@ class AuthorisedSystemService {
)

result = true
if (role) {
result = userProfile.roles.contains(role)
if (roles) {
result = roles.any { role -> userProfile.roles.contains(role) }
}

if (result && scope) {
Expand Down
11 changes: 11 additions & 0 deletions grails-app/services/au/org/ala/userdetails/UserService.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,17 @@ class UserService {
}
}

def addUserRole(User user, String roleString) {

def role = Role.findByRole(roleString)

def userRole = UserRole.findByUserAndRole(user, role)

if(!userRole) {
new UserRole(user:user, role:role).save(flush:true, failOnError: true)
}
}

def deleteUser(User user) {

if (user) {
Expand Down
23 changes: 14 additions & 9 deletions grails-app/views/admin/index.gsp
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,21 @@
<div class="col-md-12" id="page-body" role="main">
<h1>User Administration</h1>
<ul class="userdetails-menu">
<li><g:link controller="user" action="list">Find a user</g:link></li>
<li><g:link controller="admin" action="resetPasswordForUser">Reset user password</g:link></li>
<li><g:link controller="role" action="list">Roles</g:link></li>
<li><g:link controller="authorisedSystem" action="list">Authorised systems</g:link></li>
<li><g:link controller="admin" action="bulkUploadUsers">Bulk create user accounts</g:link></li>
<li><g:link controller="admin" action="exportUsers">Export users to CSV file</g:link></li>
<g:if test="${grailsApplication.config.getProperty('attributes.affiliations.enabled', Boolean, false)}">
<li><g:link controller="admin" action="surveyResults">Get user survey results</g:link></li>
<g:if test="${!isBiosecurityAdmin}">
<li><g:link controller="user" action="list">Find a user</g:link></li>
<li><g:link controller="admin" action="resetPasswordForUser">Reset user password</g:link></li>
<li><g:link controller="role" action="list">Roles</g:link></li>
<li><g:link controller="authorisedSystem" action="list">Authorised systems</g:link></li>
<li><g:link controller="admin" action="bulkUploadUsers">Bulk create user accounts</g:link></li>
<li><g:link controller="admin" action="exportUsers">Export users to CSV file</g:link></li>
<g:if test="${grailsApplication.config.getProperty('attributes.affiliations.enabled', Boolean, false)}">
<li><g:link controller="admin" action="surveyResults">Get user survey results</g:link></li>
</g:if>
<li><g:link controller="alaAdmin" action="index">ALA admin page</g:link></li>
</g:if>
<li><g:link controller="alaAdmin" action="index">ALA admin page</g:link></li>
<g:else>
<li><g:link controller="user" action="create">Create a user</g:link></li>
</g:else>
</ul>

<h2>Web services (HTTP POST)</h2>
Expand Down
2 changes: 1 addition & 1 deletion grails-app/views/profile/myprofile.gsp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
</div>
</div>

<g:if test="${isAdmin}">
<g:if test="${isAdmin || isBiosecurityAdmin}">
<div class="d-flex">
<div class="image">
<i class="glyphicon glyphicon-cog"></i>
Expand Down
2 changes: 2 additions & 0 deletions grails-app/views/user/_form.gsp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@

<hr/>

<g:if test="${!isBiosecurityAdmin}">
<div class="fieldcontain ${hasErrors(bean: userInstance, field: 'userRoles', 'error')} ">
<label for="userRoles">
<g:message code="user.userRoles.label" default=" Roles"/>
Expand Down Expand Up @@ -119,6 +120,7 @@
params="['user.id': userInstance?.id]">${message(code: 'default.add.label', args: [message(code: 'userRole.label', default: 'UserRole')])}</g:link>

</div>
</g:if>

</div>

Expand Down
2 changes: 1 addition & 1 deletion grails-app/views/user/create.gsp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
</g:hasErrors>
<g:form action="save">
<fieldset class="form">
<g:render template="form"/>
<g:render template="form" model="[isBiosecurityAdmin: isBiosecurityAdmin]"/>
</fieldset>
<fieldset class="buttons">
<g:submitButton name="create" class="btn btn-primary"
Expand Down
6 changes: 5 additions & 1 deletion src/main/groovy/au/org/ala/auth/PreAuthorise.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ import java.lang.annotation.Target
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface PreAuthorise {
String requiredRole() default "ROLE_ADMIN"
/***
* A list of roles that is allowed to access the method. The user must have at least one role to access the method.
* @return
*/
String[] allowedRoles() default ["ROLE_ADMIN"]
String redirectController() default "userdetails"
String redirectAction() default "index"
String requiredScope() default "users/read"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ class RegistrationControllerSpec extends UserDetailsSpec implements ControllerUn

then:
1 * userService.currentUser >> user
1 * userService.isEmailInUse('[email protected]', user) >> false
1 * passwordService.checkUserPassword(user, password) >> true
1 * userService.updateUser(user, params) >> true
0 * _ // no other interactions
Expand Down Expand Up @@ -398,6 +399,7 @@ class RegistrationControllerSpec extends UserDetailsSpec implements ControllerUn

then:
1 * userService.currentUser >> user
1 * userService.isEmailInUse('[email protected]', user) >> false
1 * passwordService.checkUserPassword(user, wrongPassword) >> false
0 * _ // no other interactions
flash.message == 'Incorrect password. Could not update account details. Please try again.'
Expand Down Expand Up @@ -451,6 +453,7 @@ class RegistrationControllerSpec extends UserDetailsSpec implements ControllerUn

then:
1 * userService.currentUser >> user
1 * userService.isEmailInUse('[email protected]', user) >> false
1 * passwordService.checkUserPassword(user, password) >> true
1 * userService.updateUser(user, params) >> false
0 * _ // no other interactions
Expand Down Expand Up @@ -489,13 +492,15 @@ class RegistrationControllerSpec extends UserDetailsSpec implements ControllerUn
params.city = 'Canberra'
params.password = 'password'
params.reenteredPassword = 'password'
params.confirmUserPassword = 'password'

when:
controller.update()

then:
1 * userService.updateUser(_, _) >> true
1 * userService.isEmailInUse(params.email, currentUser) >> false
1 * passwordService.checkUserPassword(_, 'password') >> true
response.redirectedUrl == '/profile'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,49 @@ class RoleBasedInterceptorSpec extends UserDetailsSpec implements InterceptorUni
response.status == HttpStatus.SC_OK

}

void "ROLE_USER_CREATOR users should not be able to access the user role UI"(String action, boolean result) {

setup:
request.addUserRole("ROLE_USER_CREATOR")

when:
request.setAttribute(GrailsApplicationAttributes.CONTROLLER_NAME_ATTRIBUTE, 'userRole')
request.setAttribute(GrailsApplicationAttributes.ACTION_NAME_ATTRIBUTE, action)
withRequest(action: action)

then:
interceptor.before() == result

where:
action | result
'list' | false
'create' | false
'addRole' | false
'deleteRole' | false
}

void "ROLE_USER_CREATOR users should be able to access the user UI"(String action, boolean result) {

setup:
request.addUserRole("ROLE_USER_CREATOR")
controller = new UserController()
grailsApplication.addArtefact("Controller", UserController)

when:
request.setAttribute(GrailsApplicationAttributes.CONTROLLER_NAME_ATTRIBUTE, 'user')
request.setAttribute(GrailsApplicationAttributes.ACTION_NAME_ATTRIBUTE, action)
withRequest(action: action)

then:
interceptor.before() == result

where:
action | result
'list' | false
'create' | true
'save' | true
'edit' | false
'show' | false
}
}

0 comments on commit 9aa8299

Please sign in to comment.