user->group->role validation fails and user cant be validated - defect in code plugin code I think

[woodmawa]

i think there is a bug in GormUserDetailsService.groovy:92, in the V3.1.1 plugin i'm using. I noted this on stackoverflow here stackoverlow ref .

essentially i wanted to use user groups and roles. i did the following to get started.

grails s2-quickstart org.softwood.security User Role --groupClassName=UserGroup

I used UserGroup as Group was getting mixed up with sql syntax and naming of tables. Avoided it by changing the domain class name.

i configured some users with a mixture of ROLE_ADMIN, and ROLE_USER in boostrap.groovy to give me some test data - you can see the details in my demo project on github here coffeShopApp

id did a bunch of asserts and checks an i think i set this all up - and dbconsole shows correct entries in tables.

i the created a a secureTest controller, with an open index() url, and protected secure url.

`class SecureTestController {

def index() {
    render "hello Will you passed the permit_any"
}

@Secured ('ROLE_ADMIN')
def secure () {
    render "hello Will you passed the ROLE_ADMIN"

}

}`

when i fire up the browser and connect to /secreTest/secure, the login screen is thrown and when i will in will/password it fails and you get stacktrace in the console

when i trawl through that and look in the code the thing i think is failing is this construct in the plugin

if (useGroups) { if (authorityGroupPropertyName) { authorities = userAuthorities.collect { it."$authorityGroupPropertyName" }.flatten().unique().collect { new SimpleGrantedAuthority(it."$authorityPropertyName") } }

i copied this out and put manually in my bootsrap.groovy so i could debug/walkthrough the steps. I also expanded the line in multiple calls so i could see which bit was failing like this in my boostrap

` if (useGroups) { if (authorityGroupPropertyName) { //userAuthorities returns Set println """ debug authoritiesPropertyName = $authoritiesPropertyName authorityPropertyName = $authorityPropertyName authorityGroupPropertyName = $authorityGroupPropertyName userAuthorities returns $userAuthorities of type ${userAuthorities.getClass()}

""" def roles = userAuthorities.collect { it."$authorityGroupPropertyName" }.flatten().unique()

            authorities = roles.collect { new SimpleGrantedAuthority(it."$authorityPropertyName") }`

If i have understand what this tries to do it calls on userAuthorities=userWill.authorities (), to get a HashSet, which is getting the Set from the groups the users is joined to.

(PS my implementation is getting the roles from groups, and any individual roles assigned directly to a user, from this in User.getAuthorities() ` Set getAuthorities() { //orig UserUserGroupBroken.findAllByUser(this)*.userGroup

    Set<Role> individualRoles = UserToRole.findAllByUser(this)*.role
    Set<UserGroup> groups = UserToUserGroup.findAllByUser(this)*.group
    Set<Role> groupRoles = groups.collect{it.getAuthorities() }
    Set<Role> aggregateRoles = new HashSet()
    aggregateRoles.addAll (groupRoles.flatten())
    aggregateRoles.addAll (individualRoles.flatten())
    aggregateRoles
}`

)

The key here is that the return from User class is HashSet. my asserts this worked as i expected in bootstrap.

the problem seems to me in the first part of the login authication code at line GormUserDeatilsService:92

def roleNames = userAuthorities.collect { it."$authorityGroupPropertyName" }.flatten().unique()

what that looks like to me is that with the returned set of roles, its building a Collection of strings of each Role in the set, flattens it and makes it unique. in the debugger this shows as an ArrayList of strings. when i look at this list it has my three three role names assigned to this user. the problem then comes in the last part of code

authorities = rolesNames.collect { new SimpleGrantedAuthority(it."$authorityPropertyName") }

this bit takes that arrayList of string and trys to read it."$authorityPropertyName" on it. but it is a string and doesnt have that property and so fails.

however i muck about with application.groovy settings this code construct doesn't work.

in my local copy i have 'fixed' the problem by writing and this returns a HashSet

authorities = userAuthorities.collect { it."$authorityGroupPropertyName" }.flatten().unique().collect { new SimpleGrantedAuthority(it) }

i'm stuck with the plugin at the mo - because this validation logic appears to be in error. Not brave enough to try and fix the plugin myself at this point - so reporting the issue/defect for the team to assess.

[woodmawa]

ok put an update up on stackoverflow thread, based on exchange.

In essence I was trying to do something that i had assumed should work, but the code inside plugin doesn't recognise.

So this is not a bug - its now a suggestion/improvement for consideration.

what i was trying to do was use groups (default assignment) but have individual role assignment as a sort of override feature.

The essential problem is between what the templates/code does for user/group/role and user/role setups in quickstart.

in user/role config, then User.'authorities' returns Set which is kind of obvious. However in user/group/role template build User.'authorities' returns Set which i wasnt expecting.

The code in GormUserDetailsService, double deferences to get the Roles before creating the Set when logging in.

I think this is confusing and somewhat opaque and anti intuitive (and just happened to break on my 'clever' code). In essence if User.'authorities' always returned a Set then coders wouldnt see a variance in the User domain class it would be the same in either generation strategy.

it would also make the code in GormUserDetailsService.loadAuthorites() clearer as your not switching models. and this would have left room for my clever model to work seamlessly just using generated domain classes as base. this would be cleaner and stop you having to peer into the plugin and find what it wasn't working.

I hope this makes sense. For now i 'll have scotch what i was doing its too far a deviation from the assumed pattern which may well be carried about in other areas i haven't got to yet.

i'll attach an alternative path tomorrow when i've cleared up my debug etc