proxy: Rework oidc role mapper to allow multiple matching roles

If multiple claims values have a valid matching for ocis roles, we'll pick
the ocis role that appears first in the mapping configuration.

changelog/unreleased/role-assignment-from-oidc.md

@@ -0,0 +1,23 @@
+Enhancement: Added possiblity to assign roles based on OIDC claims
+
+oCIS can now be configured to update a user's role assignment from the values of a claim provided
+via the IDPs userinfo endpoint. The claim name and the mapping between claim values and ocis role
+name can be configured via the configuration of the proxy service. Example:
+
+```yaml
+role_assignment:
+    driver: oidc
+    oidc_role_mapper:
+        role_claim: ocisRoles
+        role_mapping:
+            - role_name: admin
+              claim_value: myAdminRole
+            - role_name: spaceadmin
+              claim_value: mySpaceAdminRole
+            - role_name: user
+              claim_value: myUserRole
+            - role_name: guest:
+              claim_value: myGuestRole
+```
+
+https://github.com/owncloud/ocis/pull/xxxx

services/proxy/README.md

@@ -50,10 +50,14 @@ role_assignment:
     oidc_role_mapper:
         role_claim: ocisRoles
         role_mapping:
-            admin: myAdminRole
-            user: myUserRole
-            spaceadmin: mySpaceAdminRole
-            guest: myGuestRole
+            - role_name: admin
+              claim_value: myAdminRole
+            - role_name: spaceadmin
+              claim_value: mySpaceAdminRole
+            - role_name: user
+              claim_value: myUserRole
+            - role_name: guest:
+              claim_value: myGuestRole
 ```
 
 This would assign the role `admin` to users with the value `myAdminRole` in the claim `ocisRoles`.
@@ -62,16 +66,27 @@ The role `user` to users with the values `myUserRole` in the claims `ocisRoles`
 Claim values that are not mapped to a specific ownCloud Infinite Scale role will be ignored.
 
 Note: An ownCloud Infinite Scale user can only have a single role assigned. If the configured
-`role_mapping` and a user's claim values result in multiple possible roles for a user, an error
-will be logged and the user will not be able to login.
+`role_mapping` and a user's claim values result in multiple possible roles for a user, the order in
+which the role mappings are defined in the configuration is important. The first role in the
+`role_mappings` where the `claim_value` matches a value from the user's roles claim will be assigned
+to the user. So if e.g. a user's `ocisRoles` claim has the values `myUserRole` and
+`mySpaceAdminRole` that user will get the ocis role `spaceadmin` assigned (because `spaceadmin`
+appears before `user` in the above sample configuration).
 
-The default `role_claim` (or `PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM`) is `roles`. The `role_mapping` is:
+If a user's claim values don't match any of the configured role mappings an error will be logged and
+the user will not be able to login.
+
+The default `role_claim` (or `PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM`) is `roles`. The default `role_mapping` is:
 
 ```yaml
-admin: ocisAdmin
-user: ocisUser
-spaceadmin: ocisSpaceAdmin
-guest: ocisGuest
+- role_name: admin
+  claim_value: ocisAdmin
+- role_name: spaceadmin
+  claim_value: ocisSpaceAdmin
+- role_name: user
+  claim_value: ocisUser
+- role_name: guest:
+  claim_value: ocisGuest
 ```
 
 ## Recommendations for Production Deployments

services/proxy/pkg/command/server.go

@@ -178,7 +178,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config)
 			userroles.WithRoleService(rolesClient),
 			userroles.WithLogger(logger),
 			userroles.WithRolesClaim(cfg.RoleAssignment.OIDCRoleMapper.RoleClaim),
-			userroles.WithRoleMapping(cfg.RoleAssignment.OIDCRoleMapper.RoleMapping),
+			userroles.WithRoleMapping(cfg.RoleAssignment.OIDCRoleMapper.RolesMap),
 			userroles.WithAutoProvisonCreator(autoProvsionCreator),
 		)
 	default:

services/proxy/pkg/config/config.go

@@ -135,8 +135,14 @@ type RoleAssignment struct {
 
 // OIDCRoleMapper contains the configuration for the "oidc" role assignment driber
 type OIDCRoleMapper struct {
-	RoleClaim   string            `yaml:"role_claim" env:"PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM" desc:"The OIDC claim used to create the users role assignment."`
-	RoleMapping map[string]string `yaml:"role_mapping" desc:"A mapping of ocis role names to PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM claim values. This setting can only be configured in the configuration file and not via environment variables."`
+	RoleClaim string        `yaml:"role_claim" env:"PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM" desc:"The OIDC claim used to create the users role assignment."`
+	RolesMap  []RoleMapping `yaml:"role_mapping" desc:"A list of mappings of ocis role names to PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM claim values. This setting can only be configured in the configuration file and not via environment variables."`
+}
+
+// RoleMapping defines which ocis role matches a specific claim value
+type RoleMapping struct {
+	RoleName   string `yaml:"role_name" desc:"The name of an ocis role that this mapping should apply for."`
+	ClaimValue string `yaml:"claim_value" desc:"The value of the 'PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM' that matches the role defined in 'role_name'."`
 }
 
 // PolicySelector is the toplevel-configuration for different selectors

services/proxy/pkg/config/defaults/defaultconfig.go

@@ -60,11 +60,11 @@ func DefaultConfig() *config.Config {
 			// this default is only relevant when Driver is set to "oidc"
 			OIDCRoleMapper: config.OIDCRoleMapper{
 				RoleClaim: "roles",
-				RoleMapping: map[string]string{
-					"admin":      "ocisAdmin",
-					"spaceadmin": "ocisSpaceAdmin",
-					"user":       "ocisUser",
-					"guest":      "ocisGuest",
+				RolesMap: []config.RoleMapping{
+					config.RoleMapping{RoleName: "admin", ClaimValue: "ocisAdmin"},
+					config.RoleMapping{RoleName: "spaceadmin", ClaimValue: "ocisSpaceAdmin"},
+					config.RoleMapping{RoleName: "user", ClaimValue: "ocisUser"},
+					config.RoleMapping{RoleName: "guest", ClaimValue: "ocisGuest"},
 				},
 			},
 		},

services/proxy/pkg/userroles/oidcroles.go

@@ -34,47 +34,55 @@ func NewOIDCRoleAssigner(opts ...Option) UserRoleAssigner {
 // already has a different role assigned.
 func (ra oidcRoleAssigner) UpdateUserRoleAssignment(ctx context.Context, user *cs3.User, claims map[string]interface{}) (*cs3.User, error) {
 	logger := ra.logger.SubloggerWithRequestID(ctx).With().Str("userid", user.GetId().GetOpaqueId()).Logger()
-	claimValueToRoleID, err := ra.oidcClaimvaluesToRoleIDs()
+	roleNamesToRoleIDs, err := ra.roleNamesToRoleIDs()
 	if err != nil {
-		logger.Error().Err(err).Msg("Error mapping claims to roles ids")
+		logger.Error().Err(err).Msg("Error mapping role names to role ids")
 		return nil, err
 	}
 
-	roleIDsFromClaim := make([]string, 0, 1)
-	claimRoles, ok := claims[ra.rolesClaim].([]interface{})
+	claimRolesRaw, ok := claims[ra.rolesClaim].([]interface{})
 	if !ok {
-		logger.Error().Err(err).Str("rolesClaim", ra.rolesClaim).Msg("No roles in user claims")
-		return nil, err
+		logger.Error().Str("rolesClaim", ra.rolesClaim).Msg("No roles in user claims")
+		return nil, errors.New("no roles in user claims")
 	}
+
 	logger.Debug().Str("rolesClaim", ra.rolesClaim).Interface("rolesInClaim", claims[ra.rolesClaim]).Msg("got roles in claim")
-	for _, cri := range claimRoles {
+	claimRoles := map[string]struct{}{}
+	for _, cri := range claimRolesRaw {
 		cr, ok := cri.(string)
 		if !ok {
 			err := errors.New("invalid role in claims")
 			logger.Error().Err(err).Interface("claimValue", cri).Msg("Is not a valid string.")
 			return nil, err
 		}
-		id, ok := claimValueToRoleID[cr]
-		if !ok {
-			logger.Debug().Str("role", cr).Msg("No mapping for claim role. Skipped.")
-			continue
-		}
-		roleIDsFromClaim = append(roleIDsFromClaim, id)
+
+		claimRoles[cr] = struct{}{}
 	}
-	logger.Debug().Interface("roleIDs", roleIDsFromClaim).Msg("Mapped claim roles to roleids")
 
-	switch len(roleIDsFromClaim) {
-	default:
-		err := errors.New("too many roles found in claims")
-		logger.Error().Err(err).Msg("Only one role per user is allowed.")
+	if len(claimRoles) == 0 {
+		err := errors.New("no roles set in claim")
+		logger.Error().Err(err).Msg("")
 		return nil, err
-	case 0:
-		err := errors.New("no role in claim, maps to a ocis role")
+	}
+
+	// the roleMapping config is supposed to have the role mappings ordered from the highest privileged role
+	// down to the lowest privileged role. Since ocis currently only can handle a single role assignment we
+	// pick the highest privileged role that matches a value from the claims
+	roleIDFromClaim := ""
+	for _, mapping := range ra.Options.roleMapping {
+		if _, ok := claimRoles[mapping.ClaimValue]; ok {
+			logger.Debug().Str("ocisRole", mapping.RoleName).Str("role id", roleNamesToRoleIDs[mapping.RoleName]).Msg("first matching role")
+			roleIDFromClaim = roleNamesToRoleIDs[mapping.RoleName]
+			break
+		}
+	}
+
+	if roleIDFromClaim == "" {
+		err := errors.New("no role in claim maps to an ocis role")
 		logger.Error().Err(err).Msg("")
 		return nil, err
-	case 1:
-		// exactly one mapping. This is right
 	}
+
 	assignedRoles, err := loadRolesIDs(ctx, user.GetId().GetOpaqueId(), ra.roleService)
 	if err != nil {
 		logger.Error().Err(err).Msg("Could not load roles")
@@ -86,23 +94,24 @@ func (ra oidcRoleAssigner) UpdateUserRoleAssignment(ctx context.Context, user *c
 		return nil, err
 	}
 	logger.Debug().Interface("assignedRoleIds", assignedRoles).Msg("Currently assigned roles")
-	if len(assignedRoles) == 0 || (assignedRoles[0] != roleIDsFromClaim[0]) {
-		logger.Debug().Interface("assignedRoleIds", assignedRoles).Interface("newRoleIds", roleIDsFromClaim).Msg("Updating role assignment for user")
+
+	if len(assignedRoles) == 0 || (assignedRoles[0] != roleIDFromClaim) {
+		logger.Debug().Interface("assignedRoleIds", assignedRoles).Interface("newRoleId", roleIDFromClaim).Msg("Updating role assignment for user")
 		newctx, err := ra.prepareAdminContext()
 		if err != nil {
 			logger.Error().Err(err).Msg("Error creating admin context")
 			return nil, err
 		}
 		if _, err = ra.roleService.AssignRoleToUser(newctx, &settingssvc.AssignRoleToUserRequest{
 			AccountUuid: user.GetId().GetOpaqueId(),
-			RoleId:      roleIDsFromClaim[0],
+			RoleId:      roleIDFromClaim,
 		}); err != nil {
 			logger.Error().Err(err).Msg("Role assignment failed")
 			return nil, err
 		}
 	}
 
-	user.Opaque = utils.AppendJSONToOpaque(user.Opaque, "roles", roleIDsFromClaim)
+	user.Opaque = utils.AppendJSONToOpaque(user.Opaque, "roles", []string{roleIDFromClaim})
 	return user, nil
 }
 
@@ -136,32 +145,32 @@ func (ra oidcRoleAssigner) prepareAdminContext() (context.Context, error) {
 	return newctx, nil
 }
 
-type roleClaimToIDCache struct {
-	roleClaimToID map[string]string
-	lastRead      time.Time
-	lock          sync.RWMutex
+type roleNameToIDCache struct {
+	roleNameToID map[string]string
+	lastRead     time.Time
+	lock         sync.RWMutex
 }
 
-var roleClaimToID roleClaimToIDCache
+var roleNameToID roleNameToIDCache
 
-func (ra oidcRoleAssigner) oidcClaimvaluesToRoleIDs() (map[string]string, error) {
+func (ra oidcRoleAssigner) roleNamesToRoleIDs() (map[string]string, error) {
 	cacheTTL := 5 * time.Minute
-	roleClaimToID.lock.RLock()
+	roleNameToID.lock.RLock()
 
-	if !roleClaimToID.lastRead.IsZero() && time.Since(roleClaimToID.lastRead) < cacheTTL {
-		defer roleClaimToID.lock.RUnlock()
-		return roleClaimToID.roleClaimToID, nil
+	if !roleNameToID.lastRead.IsZero() && time.Since(roleNameToID.lastRead) < cacheTTL {
+		defer roleNameToID.lock.RUnlock()
+		return roleNameToID.roleNameToID, nil
 	}
 	ra.logger.Debug().Msg("refreshing roles ids")
 
 	// cache needs Refresh get a write lock
-	roleClaimToID.lock.RUnlock()
-	roleClaimToID.lock.Lock()
-	defer roleClaimToID.lock.Unlock()
+	roleNameToID.lock.RUnlock()
+	roleNameToID.lock.Lock()
+	defer roleNameToID.lock.Unlock()
 
 	// check again, another goroutine might have updated while we "upgraded" the lock
-	if !roleClaimToID.lastRead.IsZero() && time.Since(roleClaimToID.lastRead) < cacheTTL {
-		return roleClaimToID.roleClaimToID, nil
+	if !roleNameToID.lastRead.IsZero() && time.Since(roleNameToID.lastRead) < cacheTTL {
+		return roleNameToID.roleNameToID, nil
 	}
 
 	// Get all roles to find the role IDs.
@@ -183,16 +192,10 @@ func (ra oidcRoleAssigner) oidcClaimvaluesToRoleIDs() (map[string]string, error)
 	newIDs := map[string]string{}
 	for _, role := range res.Bundles {
 		ra.logger.Debug().Str("role", role.Name).Str("id", role.Id).Msg("Got Role")
-		roleClaim, ok := ra.roleMapping[role.Name]
-		if !ok {
-			err := errors.New("Incomplete role mapping")
-			ra.logger.Error().Err(err).Str("role", role.Name).Msg("Role not mapped to a claim value")
-			return map[string]string{}, err
-		}
-		newIDs[roleClaim] = role.Id
+		newIDs[role.Name] = role.Id
 	}
-	ra.logger.Debug().Interface("roleMap", newIDs).Msg("Claim Role to role ID map")
-	roleClaimToID.roleClaimToID = newIDs
-	roleClaimToID.lastRead = time.Now()
-	return roleClaimToID.roleClaimToID, nil
+	ra.logger.Debug().Interface("roleMap", newIDs).Msg("Role Name to role ID map")
+	roleNameToID.roleNameToID = newIDs
+	roleNameToID.lastRead = time.Now()
+	return roleNameToID.roleNameToID, nil
 }

services/proxy/pkg/userroles/userroles.go

@@ -7,6 +7,7 @@ import (
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/autoprovision"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
 )
 
 //go:generate mockery --name=UserRoleAssigner
@@ -26,7 +27,7 @@ type UserRoleAssigner interface {
 type Options struct {
 	roleService         settingssvc.RoleService
 	rolesClaim          string
-	roleMapping         map[string]string
+	roleMapping         []config.RoleMapping
 	autoProvsionCreator autoprovision.Creator
 	logger              log.Logger
 }
@@ -56,7 +57,7 @@ func WithRolesClaim(claim string) Option {
 }
 
 // WithRoleMapping configures the map of ocis role names to claims values
-func WithRoleMapping(roleMap map[string]string) Option {
+func WithRoleMapping(roleMap []config.RoleMapping) Option {
 	return func(o *Options) {
 		o.roleMapping = roleMap
 	}