Skip to content

Commit

Permalink
Epic/cognito/user service refactor&password reset (#140)
Browse files Browse the repository at this point in the history
* Userdetails Cognito:
 - Migrate signup page #122
 - Updates to my profile page #127
 - Delegate existing user service to an implementation provided by a Spring profile determined bean
 - Add *Record classes to mirror existing domain classes so that services aren't inherently coupled to GORM
 - Remove some direct GORM usage from Controller actions

* Add cognito password reset branch

* Add cognito password reset

* disassociated GORM domain objects from grails and dynamically register based on profile
merged UserService implementations from cognito develop branch using base domain objects

* Add cognito password reset

* Add cognito password reset

* Add create user account

* Fix logout issue

* Add banner config

* Fix reset password related issues

* moved domain object into new package and manually config as GORM artifacts for the gorm profile

* Fix gorm user service issues

* Add MFA set up

* Fix issues

* enable travis build

* Fix failing test cases

* Update domain

* Ignore failing tests

* Remove bulk user upload and export users admin features

* Update user update functionality

* Fix breadcrumbs

* Fix issues

* Update email label

* Update wordings

* Add logs

* Remove logs

* Add Capcha for reset password

* Clean up missed merge additions, add MFA service call return types

* Move GORM password reset into GORM user service, remove unused updateUser method

* Add assets directories for apps

* Fix for unit tests, extract authorised system lookup

* Move plugin Spring beans to separate @configuration file, fix Bootstrap syntax

Co-authored-by: Simon Bear <[email protected]>
Co-authored-by: Bruce Hyslop <[email protected]>
  • Loading branch information
3 people authored Dec 14, 2022
1 parent d10ab46 commit 542166e
Show file tree
Hide file tree
Showing 48 changed files with 982 additions and 327 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ branches:
- grails3
- experimental_jwt
- /^feature.*$/
- epic/cognito/userServiceRefactor&PasswordReset
before_cache:
- rm -f $HOME/.gradle/caches/modules-2/modules-2.lock
- rm -fr $HOME/.gradle/caches/*/plugin-resolution/
Expand Down
Empty file.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion userdetails-gorm/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ dependencies {
implementation "org.grails.plugins:ala-bootstrap3:4.1.0"
implementation "org.grails.plugins:ala-ws-plugin:3.1.1"
implementation "org.grails.plugins:ala-ws-security-plugin:4.3.3-SNAPSHOT"
implementation "org.grails.plugins:ala-auth:5.1.1"
implementation "org.grails.plugins:ala-auth:5.2.0-CognitoLogoutFix-SNAPSHOT"
implementation "org.grails.plugins:ala-admin-plugin:2.3.0"

implementation 'dk.glasius:external-config:3.1.0'
Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,12 @@ class User extends UserRecord implements Serializable {
def propsAsMap(){
def map = [:]
this.getUserProperties().each {
map.put(it.name.startsWith('custom:') ? it.name.substring(7) : it.name, it.value)
if(it.name == "enableMFA"){
map.put(it.name, it.value.toBoolean())
}
else {
map.put(it.name.startsWith('custom:') ? it.name.substring(7) : it.name, it.value)
}
}
map
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class UserProperty extends UserPropertyRecord implements Serializable {
User user
String name
String value
String id

static def addOrUpdateProperty(user, name, value){

Expand Down Expand Up @@ -55,4 +56,25 @@ class UserProperty extends UserPropertyRecord implements Serializable {
String toString(){
name + " : " + value
}

boolean equals(o) {
if (this.is(o)) return true
if (getClass() != o.class) return false

UserProperty that = (UserProperty) o

if (user?.id != that.user?.id) return false
if (name != that.name) return false
if (value != that.value) return false

return true
}

int hashCode() {
int result
result = (user != null ? user?.id?.hashCode() : 0)
result = 31 * result + (name != null ? name.hashCode() : 0)
result = 31 * result + (value != null ? value.hashCode() : 0)
return result
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package au.org.ala.userdetails.gorm

import au.org.ala.userdetails.EmailService
import au.org.ala.userdetails.IAuthorisedSystemRepository
import au.org.ala.userdetails.IUserService
import au.org.ala.userdetails.LocationService
import au.org.ala.userdetails.PasswordService
Expand Down Expand Up @@ -71,4 +72,9 @@ class Application extends GrailsAutoConfiguration {
return userService
}

@Bean('authorisedSystemRepository')
IAuthorisedSystemRepository authorisedSystemRepository() {
new GormAuthorisedSystemRepository()
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package au.org.ala.userdetails.gorm

import au.org.ala.userdetails.IAuthorisedSystemRepository

class GormAuthorisedSystemRepository implements IAuthorisedSystemRepository {

@Override
Boolean findByHost(String host) {
return AuthorisedSystem.findByHost(host) != null
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,19 @@ package au.org.ala.userdetails.gorm

import au.org.ala.auth.BulkUserLoadResults
import au.org.ala.auth.PasswordResetFailedException
import au.org.ala.cas.encoding.BcryptPasswordEncoder
import au.org.ala.cas.encoding.LegacyPasswordEncoder
import au.org.ala.userdetails.EmailService
import au.org.ala.userdetails.IUserService
import au.org.ala.userdetails.LocationService
import au.org.ala.userdetails.PasswordService
import au.org.ala.userdetails.ResultStreamer
//import au.org.ala.users.Password
import au.org.ala.users.RoleRecord
import au.org.ala.users.UserPropertyRecord
import au.org.ala.users.UserRecord
import au.org.ala.users.UserRoleRecord
import au.org.ala.web.AuthService
import au.org.ala.ws.service.WebService
import com.google.common.base.Preconditions
import grails.converters.JSON
import grails.core.GrailsApplication
import grails.plugin.cache.Cacheable
Expand All @@ -44,12 +44,16 @@ import org.grails.orm.hibernate.cfg.GrailsHibernateUtil
import org.hibernate.ScrollableResults
import org.springframework.beans.factory.annotation.Value
import org.springframework.context.MessageSource
import org.springframework.dao.DataIntegrityViolationException

import javax.servlet.http.HttpSession

@Slf4j
@Transactional
class GormUserService implements IUserService {

static final String BCRYPT_ENCODER_TYPE = 'bcrypt'
static final String LEGACY_ENCODER_TYPE = 'legacy'

EmailService emailService
PasswordService passwordService
AuthService authService
Expand All @@ -58,14 +62,18 @@ class GormUserService implements IUserService {
MessageSource messageSource
WebService webService

@Value('${password.encoder}')
String passwordEncoderType = 'bcrypt'
@Value('${bcrypt.strength}')
Integer bcryptStrength = 10
@Value('${encoding.algorithm}')
String legacyAlgorithm
@Value('${encoding.salt}')
String legacySalt

@Value('${attributes.affiliations.enabled:false}')
boolean affiliationsEnabled = false

@Override
boolean updateUser(GrailsParameterMap params) {

}

boolean updateUser(String userId, GrailsParameterMap params) {

User user = getUserById(userId)
Expand Down Expand Up @@ -132,17 +140,25 @@ class GormUserService implements IUserService {
}

@Transactional
void activateAccount(UserRecord user) {
boolean activateAccount(UserRecord user, GrailsParameterMap params) {
assert user instanceof User
Map resp = webService.post("${grailsApplication.config.getProperty('alerts.url')}/api/alerts/user/createAlerts", [:], [userId: user.id, email: user.email, firstName: user.firstName, lastName: user.lastName])
if (resp.statusCode == HttpStatus.SC_CREATED) {
emailService.sendAccountActivationSuccess(user, resp.resp)
} else if (resp.statusCode != HttpStatus.SC_OK) {
log.error("Alerts returned ${resp} when trying to create user alerts for " + user.id + " with email: " + user.email)
}
//check the activation key
if (user.tempAuthKey == params.authKey) {

Map resp = webService.post("${grailsApplication.config.getProperty('alerts.url')}/api/alerts/user/createAlerts", [:], [userId: user.id, email: user.email, firstName: user.firstName, lastName: user.lastName])
if (resp.statusCode == HttpStatus.SC_CREATED) {
emailService.sendAccountActivationSuccess(user, resp.resp)
} else if (resp.statusCode != HttpStatus.SC_OK) {
log.error("Alerts returned ${resp} when trying to create user alerts for " + user.id + " with email: " + user.email)
}

user.activated = true
user.save(flush:true)
user.activated = true
user.save(flush:true)
return true
} else {
log.error('Auth keys did not match for user : ' + params.userId + ", supplied: " + params.authKey + ", stored: " + user.tempAuthKey)
return false
}
}

@Override
Expand Down Expand Up @@ -237,7 +253,7 @@ class GormUserService implements IUserService {
}

roles?.each { role ->
def userRole = new UserRoleRecord(user: userInstance, role: role)
def userRole = new UserRole(user: userInstance, role: role)
userRole.save(failOnError: true)
}

Expand Down Expand Up @@ -277,7 +293,6 @@ class GormUserService implements IUserService {
def propValue = properties[propName] ?: ''
setUserProperty(user, propName, propValue)
}

}

private setUserProperty(UserRecord user, String propName, String propValue) {
Expand Down Expand Up @@ -346,6 +361,7 @@ class GormUserService implements IUserService {

}

@Override
void resetAndSendTemporaryPassword(UserRecord user, String emailSubject, String emailTitle, String emailBody, String password = null) throws PasswordResetFailedException {
assert user instanceof User
if (user) {
Expand All @@ -357,7 +373,9 @@ class GormUserService implements IUserService {
}
}

@Override
void clearTempAuthKey(UserRecord user) {
assert user instanceof User
if (user) {
//set the temp auth key
user.tempAuthKey = null
Expand Down Expand Up @@ -405,6 +423,7 @@ class GormUserService implements IUserService {
}

@NotTransactional
@Override
String getResetPasswordUrl(UserRecord user) {
assert user instanceof User
if(user.tempAuthKey){
Expand Down Expand Up @@ -668,4 +687,47 @@ class GormUserService implements IUserService {
}
resultStreamer.complete()
}

@Override
boolean resetPassword(UserRecord user, String newPassword, boolean isPermanent, String confirmationCode) {
assert user instanceof User
Password.findAllByUser(user).each {
it.delete()
}

boolean isBcrypt = passwordEncoderType.equalsIgnoreCase(BCRYPT_ENCODER_TYPE)

def encoder = isBcrypt ? new BcryptPasswordEncoder(bcryptStrength) : new LegacyPasswordEncoder(legacySalt, legacyAlgorithm, true)
def encodedPassword = encoder.encode(newPassword)

//reuse object if old password
def password = new Password()
password.user = user
password.password = encodedPassword
password.type = isBcrypt ? BCRYPT_ENCODER_TYPE : LEGACY_ENCODER_TYPE
password.created = new Date().toTimestamp()
password.expiry = null
password.status = "CURRENT"
password.save(failOnError: true)
return true
}

@Override
String getPasswordResetView() {
return "startPasswordReset"
}

@Override
def sendAccountActivation(UserRecord user) {
emailService.sendAccountActivation(user, user.tempAuthKey)
}

@Override
String getSecretForMfa() {}

@Override
boolean verifyUserCode(String userCode){}

@Override
void enableMfa(String userId, boolean enable){}
}
16 changes: 16 additions & 0 deletions userdetails-plugin/grails-app/assets/stylesheets/createAccount.css
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,20 @@
/* re-apply default highlight colour for chosen that is overridden by the above */
.chosen-container-active .chosen-single {
border-color: rgba(82, 168, 236, 0.8);
}

#qrcode, #instruction, #secret, #code, #buttonsDiv, #codeLabel {
padding-top: 10px;
}

#secret {
color: blue;
}

#code {
width: 20%;
}

#message {
color: red;
}
2 changes: 1 addition & 1 deletion userdetails-plugin/grails-app/conf/plugin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ security:
enabled: false
oidc:
enabled: true
scope: openid profile email ala roles
scope: openid profile email ala/attrs ala/roles aws.cognito.signin.user.admin
allowUnsignedIdTokens: true # Disable once CAS no longer suggests the none algorithm
jwt:
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ class AdminController {

def sendPasswordResetEmail(){

def user = userService.getUserByEmail(params.email) // UserRecord.findByEmail(params.email)
def user = userService.getUserById(params.email)
if (user) {
def password = passwordService.generatePassword(user)
//email to user
emailService.sendGeneratedPassword(user, password)
render(view:'userPasswordResetSuccess', model:[email:params.email])
render(view:'userPasswordResetSuccess', model:[email:params.email, password: password])
} else {
render(view:'resetPasswordForUser', model:[email:params.email, emailNotRecognised:true])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ExternalSiteController {
flickrUsers(flickrIds) { UserPropertyRecord flickrId ->
id flickrId.user.id.toString()
externalId flickrId.value
externalUsername flickrId.user.propsAsMap().flickrUsername
externalUsername flickrId.user.userProperties.find { it.name == 'flickrUsername' }?.value
externalUrl 'http://www.flickr.com/photos/' + flickrId.value
}
}
Expand Down
Loading

0 comments on commit 542166e

Please sign in to comment.