Skip to content

Commit

Permalink
feat(oauth2): support roles from userInfo mapping (#830)
Browse files Browse the repository at this point in the history
If present in userAuthentication details, extract roles and use for login

Note: potentially breaking if you had extended
SpinnakerUserInfoTokenServices and expected the
OAuth2SsoConfig.UserInfoMapping.roles to equal 'roles'.

Otherwise unless explicitly configured by supplying a value for the
userInfoMapping.roles no roles will be extracted from
the userAuthentication.details which preserves existing behaviour.
  • Loading branch information
cfieber committed Jun 19, 2019
1 parent 3a94ee7 commit 2d70a50
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class OAuth2SsoConfig extends WebSecurityConfigurerAdapter {
String lastName = "family_name"
String username = "email"
String serviceAccountEmail = "client_email"
String roles = "roles"
String roles = null
}

@Component
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices {
OAuth2Authentication loadAuthentication(String accessToken) throws AuthenticationException, InvalidTokenException {
OAuth2Authentication oAuth2Authentication = userInfoTokenServices.loadAuthentication(accessToken)

Map details = oAuth2Authentication.userAuthentication.details as Map
Map<String, Object> details = oAuth2Authentication.userAuthentication.details as Map

if (log.isDebugEnabled()) {
log.debug("UserInfo details: " + entries(details))
Expand Down Expand Up @@ -147,16 +147,29 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices {
return false
}

boolean hasAllUserInfoRequirements(Map details) {
private static boolean valueMatchesConstraint(Object value, String requiredVal) {
if (value == null) {
return false
}

if (isRegexExpression(requiredVal)) {
return String.valueOf(value).matches(mutateRegexPattern(requiredVal))
}

return value == requiredVal
}

boolean hasAllUserInfoRequirements(Map<String, Object> details) {
if (!userInfoRequirements) {
return true
}

def invalidFields = userInfoRequirements.findAll { String reqKey, String reqVal ->
if (details[reqKey] && isRegexExpression(reqVal)) {
return !String.valueOf(details[reqKey]).matches(mutateRegexPattern(reqVal))
def value = details[reqKey]
if (value instanceof Collection) {
return !value.any { valueMatchesConstraint(it, reqVal) }
}
return details[reqKey] != reqVal
return !valueMatchesConstraint(value, reqVal)
}
if (invalidFields && log.debugEnabled) {
log.debug "Invalid userInfo response: " + invalidFields.collect({k, v -> "got $k=${details[k]}, wanted $v"}).join(", ")
Expand All @@ -182,7 +195,18 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices {
val.substring(1, val.length() - 1)
}

protected List<String> getRoles(Map<String, String> details) {
protected List<String> getRoles(Map<String, Object> details) {
if (!userInfoMapping.roles) {
return []
}
def roles = details[userInfoMapping.roles] ?: []
if (roles instanceof Collection) {
return roles as List<String>
}
if (roles instanceof String) {
return roles.split(/[, ]+/) as List<String>
}
log.warn("unsupported roles value in details, type: ${roles.class}, value: ${roles}")
return []
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,42 +21,89 @@ import spock.lang.Subject

class SpinnakerUserInfoTokenServicesSpec extends Specification {

def "should check restricted domain flag if set"() {
def "should evaluate userInfoRequirements against authentication details"() {
setup:
def userInfoRequirements = new OAuth2SsoConfig.UserInfoRequirements();
@Subject tokenServices = new SpinnakerUserInfoTokenServices(userInfoRequirements: userInfoRequirements)
def userInfoRequirements = new OAuth2SsoConfig.UserInfoRequirements()
@Subject tokenServices = new SpinnakerUserInfoTokenServices(userInfoRequirements: userInfoRequirements)

expect: "no domain restriction means everything matches"
tokenServices.hasAllUserInfoRequirements([:])
tokenServices.hasAllUserInfoRequirements(["hd": "foo.com"])
tokenServices.hasAllUserInfoRequirements(["bar": "foo.com"])
tokenServices.hasAllUserInfoRequirements(["bar": "bar.com"])
tokenServices.hasAllUserInfoRequirements([:])
tokenServices.hasAllUserInfoRequirements(["hd": "foo.com"])
tokenServices.hasAllUserInfoRequirements(["bar": "foo.com"])
tokenServices.hasAllUserInfoRequirements(["bar": "bar.com"])

when: "domain restricted but not found on userAuthorizationUri"
userInfoRequirements.hd = "foo.com"
userInfoRequirements.hd = "foo.com"

then:
!tokenServices.hasAllUserInfoRequirements([:])
tokenServices.hasAllUserInfoRequirements(["hd": "foo.com"])
!tokenServices.hasAllUserInfoRequirements(["bar": "foo.com"])
!tokenServices.hasAllUserInfoRequirements(["bar": "bar.com"])
!tokenServices.hasAllUserInfoRequirements([:])
tokenServices.hasAllUserInfoRequirements(["hd": "foo.com"])
!tokenServices.hasAllUserInfoRequirements(["bar": "foo.com"])
!tokenServices.hasAllUserInfoRequirements(["bar": "bar.com"])

when: 'domain restricted by regex expression'
userInfoRequirements.hd = "/foo\\.com|bar\\.com/"
userInfoRequirements.hd = "/foo\\.com|bar\\.com/"

then:
!tokenServices.hasAllUserInfoRequirements([:])
tokenServices.hasAllUserInfoRequirements(['hd': 'foo.com'])
tokenServices.hasAllUserInfoRequirements(['hd': 'bar.com'])
!tokenServices.hasAllUserInfoRequirements(['hd': 'baz.com'])
!tokenServices.hasAllUserInfoRequirements(['bar': 'foo.com'])
!tokenServices.hasAllUserInfoRequirements([:])
tokenServices.hasAllUserInfoRequirements(['hd': 'foo.com'])
tokenServices.hasAllUserInfoRequirements(['hd': 'bar.com'])
!tokenServices.hasAllUserInfoRequirements(['hd': 'baz.com'])
!tokenServices.hasAllUserInfoRequirements(['bar': 'foo.com'])

when: "multiple restriction values"
userInfoRequirements.bar = "bar.com"
userInfoRequirements.bar = "bar.com"

then:
!tokenServices.hasAllUserInfoRequirements(["hd": "foo.com"])
!tokenServices.hasAllUserInfoRequirements(["bar": "bar.com"])
tokenServices.hasAllUserInfoRequirements(["hd": "foo.com", "bar": "bar.com"])
!tokenServices.hasAllUserInfoRequirements(["hd": "foo.com"])
!tokenServices.hasAllUserInfoRequirements(["bar": "bar.com"])
tokenServices.hasAllUserInfoRequirements(["hd": "foo.com", "bar": "bar.com"])

when: "evaluating a list, any match is accepted"
userInfoRequirements.clear()
userInfoRequirements.roles = "expected-role"

then:
tokenServices.hasAllUserInfoRequirements("roles": "expected-role")
tokenServices.hasAllUserInfoRequirements("roles": ["expected-role", "unexpected-role"])
!tokenServices.hasAllUserInfoRequirements([:])
!tokenServices.hasAllUserInfoRequirements("roles": "unexpected-role")
!tokenServices.hasAllUserInfoRequirements("roles": ["unexpected-role"])

when: "evaluating a regex in a list, any match is accepted"
userInfoRequirements.roles = "/^.+_ADMIN\$/"

then:
tokenServices.hasAllUserInfoRequirements(roles: "foo_ADMIN")
tokenServices.hasAllUserInfoRequirements(roles: ["foo_ADMIN"])
!tokenServices.hasAllUserInfoRequirements(roles: ["_ADMIN", "foo_USER"])
!tokenServices.hasAllUserInfoRequirements(roles: ["foo_ADMINISTRATOR", "bar_USER"])


}

def "should extract roles from details"() {
given:
def tokenServices = new SpinnakerUserInfoTokenServices(userInfoMapping: new OAuth2SsoConfig.UserInfoMapping(roles: 'roles'))
def details = [
roles: rolesValue
]

expect:
tokenServices.getRoles(details) == expectedRoles

where:
rolesValue || expectedRoles
null || []
"" || []
["foo", "bar"] || ["foo", "bar"]
"foo,bar" || ["foo", "bar"]
"foo bar" || ["foo", "bar"]
"foo" || ["foo"]
"foo bar" || ["foo", "bar"]
"foo,,,bar" || ["foo", "bar"]
"foo, bar" || ["foo", "bar"]
1 || []
[blergh: "blarg"] || []
}
}

0 comments on commit 2d70a50

Please sign in to comment.