Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/issue1058 #1059

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0bcae50
Updates to support new ala-security-project version/ java17 #1058
chrisala Jan 27, 2025
ab3b192
Simplified UserService.setUser and updated tests #1058
chrisala Jan 28, 2025
808d99e
Removed unused dependencies #1058
chrisala Jan 28, 2025
e0bde6f
Removed dependency on ala auth implementation #1058
chrisala Jan 28, 2025
89c76b7
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Jan 30, 2025
fdaece9
Updated user check to work with CAS #1058
chrisala Jan 31, 2025
0b1bc52
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 3, 2025
c2b053e
Bump travis to java17 #1058
chrisala Feb 3, 2025
b1f022a
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 6, 2025
b39ae2a
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 7, 2025
10de76b
Merge branch 'dev' into feature/issue1058
temi Feb 11, 2025
8a418ad
Check for clonable before calling clone for java17 #1058
chrisala Feb 11, 2025
58d1eed
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 14, 2025
86ddfc4
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 14, 2025
4e7ea0e
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 16, 2025
e4a5817
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 20, 2025
4d67e67
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 20, 2025
dd25a6b
Merge remote-tracking branch 'origin/dev' into feature/issue1058
chrisala Feb 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ name: ecodata build
on:
push:
branches:
- grails5java11
- dev
- master
- feature/**
Expand All @@ -20,18 +19,18 @@ jobs:
steps:
- uses: actions/checkout@v3

- name: Set up JDK 11
- name: Set up JDK 17
uses: actions/setup-java@v3
with:
java-version: '11'
java-version: '17'
distribution: 'adopt'

- name: Validate Gradle wrapper
uses: gradle/wrapper-validation-action@e6e38bacfdf1a337459f332974bb2327a31aaf4b

- name: Install and start elasticsearch
run: |
curl https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-7.16.3-amd64.deb -o elasticsearch.deb
curl https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-7.17.27-amd64.deb -o elasticsearch.deb
sudo dpkg -i --force-confnew elasticsearch.deb
sudo chown -R elasticsearch:elasticsearch /etc/default/elasticsearch
sudo sh -c 'echo ES_JAVA_OPTS=\"-Xmx1g -Xms1g\" >> /etc/default/elasticsearch'
Expand All @@ -40,7 +39,7 @@ jobs:
- name: Install and start mongodb
uses: supercharge/[email protected]
with:
mongodb-version: '5.0'
mongodb-version: '8.0'

- name: Build and run jacoco coverage report with Gradle
uses: gradle/[email protected]
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ plugins {
id "com.gorylenko.gradle-git-properties" version "2.4.1"
}

version "5.2-SNAPSHOT"
version "$ecodataVersion"
group "au.org.ala"
description "Ecodata"

Expand All @@ -38,7 +38,7 @@ if (Boolean.valueOf(enableJacoco)) {
}

sourceCompatibility = '11'
targetCompatibility = '11'
targetCompatibility = '17'

apply from: "${project.projectDir}/gradle/publish.gradle"

Expand Down
5 changes: 3 additions & 2 deletions gradle.properties
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
grailsVersion=6.2.0
ecodataVersion=5.2-java17-SNAPSHOT
grailsVersion=6.2.3
grailsGradlePluginVersion=6.1.2

gormVersion=8.1.2
gormMongoVersion=8.2.0
grailsViewsVersion=2.3.2
assetPipelineVersion=4.3.0
elasticsearchVersion=7.17.21
alaSecurityLibsVersion=6.3.0
alaSecurityLibsVersion=6.4.0-SNAPSHOT
#22.x+ causes issues with mongo / GORM javax.validation.spi, might need grails 5
geoToolsVersion=21.5
#jtsVersion must match the geotools version
Expand Down
5 changes: 5 additions & 0 deletions grails-app/conf/application.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -610,12 +610,17 @@ security {
requiredScopes = []
connectTimeoutMs = 20000
readTimeoutMs = 20000
callUserInfoEndpoint = false
alaUseridClaim = "username"
userIdClaim = "username"
}
}
webservice.jwt = false
webservice['jwt-scopes'] = "ala/internal users/read ala/attrs"
webservice['client-id']='changeMe'
webservice['client-secret'] = 'changeMe'
webservice['callUserInfoEndpoint'] = false


grails.gorm.graphql.browser = true

Expand Down
78 changes: 27 additions & 51 deletions grails-app/services/au/org/ala/ecodata/UserService.groovy
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
package au.org.ala.ecodata

import au.org.ala.userdetails.UserDetailsClient
import au.org.ala.web.AuthService
import au.org.ala.ws.security.client.AlaOidcClient
import grails.core.GrailsApplication
import org.grails.web.servlet.mvc.GrailsWebRequest
import org.pac4j.core.config.Config
import org.pac4j.core.context.WebContext
import org.pac4j.core.credentials.Credentials
import org.pac4j.core.util.FindBest
import org.pac4j.jee.context.JEEContextFactory
import org.springframework.beans.factory.annotation.Autowired

import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse

class UserService {

Expand All @@ -22,11 +13,7 @@ class UserService {
AuthService authService
WebService webService
GrailsApplication grailsApplication
UserDetailsClient userDetailsClient
@Autowired(required = false)
Config config
@Autowired(required = false)
AlaOidcClient alaOidcClient


/** Limit to the maximum number of Users returned by queries */
static final int MAX_QUERY_RESULT_SIZE = 1000
Expand Down Expand Up @@ -179,55 +166,44 @@ class UserService {
}


/**
* Get user from JWT.
* @param authorizationHeader
* @return
*/
au.org.ala.web.UserDetails getUserFromJWT(String authorizationHeader = null) {
if((config == null) || (alaOidcClient == null))
return
try {
GrailsWebRequest grailsWebRequest = GrailsWebRequest.lookup()
HttpServletRequest request = grailsWebRequest.getCurrentRequest()
HttpServletResponse response = grailsWebRequest.getCurrentResponse()
if (!authorizationHeader)
authorizationHeader = request?.getHeader(AUTHORIZATION_HEADER_FIELD)
if (authorizationHeader?.startsWith("Bearer")) {
final WebContext context = FindBest.webContextFactory(null, config, JEEContextFactory.INSTANCE).newContext(request, response)
def optCredentials = alaOidcClient.getCredentials(context, config.sessionStore)
if (optCredentials.isPresent()) {
Credentials credentials = optCredentials.get()
def optUserProfile = alaOidcClient.getUserProfile(credentials, context, config.sessionStore)
if (optUserProfile.isPresent()) {
def userProfile = optUserProfile.get()
String userId = userProfile?.userId ?: userProfile?.getAttribute(grailsApplication.config.getProperty('userProfile.userIdAttribute'))
if (userId) {
return authService.getUserForUserId(userId)
}
}
}
}
} catch (Throwable e) {
log.error("Failed to get user details from JWT", e)
private String checkForDelegatedUserId(HttpServletRequest request) {
// When BioCollect or MERIT calls ecodata, they use a M2M access token which contains custom scopes to
// enable access to the ecodata API.
// We can trust the requests containing bearer tokens with this scope and extract the userId from a header.
String scope = grailsApplication.config.getProperty('app.readScope')
if (!scope) {
log.error("No read scope specified in config.")
return null
}

if (request.isUserInRole(scope)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does isUserInRole check scope attribute or just role attribute?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for M2M tokens it applies the role check to the scopes rather than the user profile roles. The alternatives I could find to identify M2M was to check the principal (which is set to the clientId) or a check of the profile class (which introduces a dependency on the plugin implementation).
I thought since we otherwise were using the scopes it made sense to go with that approach?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The disadvantage of checking the client id is we need to manage a list of authorized clients)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me. I was just checking.

return request.getHeader(AuditInterceptor.httpRequestHeaderForUserId)
}
return null
}

def setUser() {
String userId
GrailsWebRequest grailsWebRequest = GrailsWebRequest.lookup()
HttpServletRequest request = grailsWebRequest.getCurrentRequest()

// First check if we've already saved the profile.
def userDetails = request.getAttribute(UserDetails.REQUEST_USER_DETAILS_KEY)

if (userDetails)
if (userDetails) {
return userDetails
}
// If the user has logged in interactively or supplies a bearer token which identifies the user
// (e.g. the Monitor app passes the user token) the authService will be able to resolve the user from the token.
String userId = authService.getUserId()

// userId is set from either the request param userId or failing that it tries to get it from
// the UserPrincipal (assumes ecodata is being accessed directly via admin page)
userId = getUserFromJWT()?.userId ?: authService.getUserId() ?: request.getHeader(AuditInterceptor.httpRequestHeaderForUserId)

// Otherwise, if the token is an ALA M2M token from MERIT or BioCollect, we can obtain the userId
// from a separate header.
if (!userId) {
userId = checkForDelegatedUserId(request)
}
if (userId) {
log.debug("Setting current user to ${userId}")

userDetails = setCurrentUser(userId)
if (userDetails) {
// We set the current user details in the request scope because
Expand Down
8 changes: 4 additions & 4 deletions src/main/groovy/au/org/ala/ecodata/IniReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ public int getIntegerValue(String section, String key) {
*/
public double getDoubleValue(String section, String key) {
String str = document.get(section + "\\" + key);
Double ret;
double ret;
try {
ret = new Double(str);
ret = Double.parseDouble(str);
} catch (Exception e) {
ret = new Double(0);
ret = 0d;
}
return ret.doubleValue();
return ret;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package au.org.ala.ecodata.metadata
import pl.touk.excel.export.getters.Getter
import au.org.ala.ecodata.Record

class SpeciesUrlGetter extends OutputDataGetter implements Getter<String> {
class SpeciesUrlGetter extends OutputDataGetter {
String biePrefix
SpeciesUrlGetter(String propertyName, Map dataNode, Map<String, Object> documentMap, TimeZone timeZone, String biePrefix) {
super(propertyName, dataNode, documentMap, timeZone)
Expand Down
59 changes: 36 additions & 23 deletions src/test/groovy/au/org/ala/ecodata/UserServiceSpec.groovy
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
package au.org.ala.ecodata

import au.org.ala.web.AuthService
import au.org.ala.ws.security.client.AlaOidcClient
import au.org.ala.ws.security.profile.AlaOidcUserProfile
import au.org.ala.ws.security.profile.AlaM2MUserProfile
import grails.test.mongodb.MongoSpec
import grails.testing.services.ServiceUnitTest
import grails.testing.web.GrailsWebUnitTest
import org.pac4j.core.config.Config
import org.pac4j.core.credentials.AnonymousCredentials
import org.pac4j.core.credentials.Credentials
import org.pac4j.core.profile.UserProfile
import spock.lang.Unroll

/**
Expand All @@ -20,7 +16,6 @@ class UserServiceSpec extends MongoSpec implements ServiceUnitTest<UserService>,

WebService webService = Mock(WebService)
AuthService authService = Mock(AuthService)
AlaOidcClient alaOidcClient
Config pack4jConfig

def user
Expand All @@ -43,6 +38,7 @@ class UserServiceSpec extends MongoSpec implements ServiceUnitTest<UserService>,
def cleanup() {
User.findAll().each{it.delete(flush:true)}
Hub.findAll().each{it.delete(flush:true)}
service.clearCurrentUser()
}

def "The recordLoginTime method requires a hubId and userId to be supplied"() {
Expand Down Expand Up @@ -160,28 +156,45 @@ class UserServiceSpec extends MongoSpec implements ServiceUnitTest<UserService>,
"h1" | "2021-04-15T00:00:00Z" | "2021-05-01T00:00:00Z" | 0
}

void "getUserFromJWT returns user when Authorization header is passed"() {
void "The user service can identify the user using the authService and make it available on a ThreadLocal"() {
when:
service.setUser()

then:
1 * authService.getUserId() >> user.userId
1 * authService.getUserForUserId(user.userId) >> userDetails

UserService.currentUser() == userDetails
}

void "The user service can identify an ALA M2M access token and identify the user from a request header"() {
setup:
def result
alaOidcClient = GroovyMock([global: true], AlaOidcClient)
pack4jConfig = GroovyMock([global: true], Config)
service.alaOidcClient = alaOidcClient
service.config = pack4jConfig
AlaOidcUserProfile person = new AlaOidcUserProfile(user.userId)
Optional<Credentials> credentials = new Optional<Credentials>(AnonymousCredentials.INSTANCE)
Optional<UserProfile> userProfile = new Optional<UserProfile>(person)
AuditInterceptor.httpRequestHeaderForUserId = 'userId'
request.addUserRole("ecodata/read_test")

when:
request.addHeader('Authorization', 'Bearer abcdef')
result = service.getUserFromJWT()
request.addHeader('userId', user.userId)
service.setUser()

then:
alaOidcClient.getCredentials(*_) >> credentials
alaOidcClient.getUserProfile(*_) >> userProfile
authService.getUserForUserId(user.userId) >> userDetails
result.userName == user.userName
result.displayName == "${user.firstName} ${user.lastName}"
result.userId == user.userId
1 * authService.getUserId() >> null

1 * authService.getUserForUserId(user.userId) >> userDetails

UserService.currentUser() == userDetails
}

void "The user service will not read the userId from the header if the caller isn't an ALA system"() {
when:
request.addHeader('userId', user.userId)
service.setUser()

then:
1 * authService.getUserId() >> null
0 * authService.getUserDetailsById(_)
UserService.currentUser() == null


}


Expand Down