Skip to content

Commit 1e6bd37

Browse files
EmyrkEmyrk
committed
allow mapping 1:many
1 parent 81feea5 commit 1e6bd37

File tree

8 files changed

+179
-81
lines changed

8 files changed

+179
-81
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ var (
207207
rbac.ResourceWildcard.Type: {rbac.ActionRead},
208208
rbac.ResourceAPIKey.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
209209
rbac.ResourceGroup.Type: {rbac.ActionCreate, rbac.ActionUpdate},
210-
rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate},
210+
rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate, rbac.ActionDelete},
211211
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
212212
rbac.ResourceOrganization.Type: {rbac.ActionCreate},
213213
rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate},

‎coderd/rbac/roles.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -283,10 +283,13 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
283283
// map[actor_role][assign_role]<can_assign>
284284
var assignRoles = map[string]map[string]bool{
285285
"system": {
286-
owner: true,
287-
member: true,
288-
orgAdmin: true,
289-
orgMember: true,
286+
owner: true,
287+
auditor: true,
288+
member: true,
289+
orgAdmin: true,
290+
orgMember: true,
291+
templateAdmin: true,
292+
userAdmin: true,
290293
},
291294
owner: {
292295
owner: true,

‎coderd/userauth.go

Lines changed: 83 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ type OIDCConfig struct {
691691
// UserRoleMapping controls how groups returned by the OIDC provider get mapped
692692
// to roles within Coder.
693693
// map[oidcRoleName]coderRoleName
694-
UserRoleMapping map[string]string
694+
UserRoleMapping map[string][]string
695695
// UserRolesDefault is the default set of roles to assign to a user if role sync
696696
// is enabled.
697697
UserRolesDefault []string
@@ -905,55 +905,6 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
905905
}
906906
}
907907

908-
roles := api.OIDCConfig.UserRolesDefault
909-
if api.OIDCConfig.RoleSyncEnabled() {
910-
rolesRow, ok := claims[api.OIDCConfig.UserRoleField]
911-
if !ok {
912-
logger.Error(ctx, "oidc user roles are missing from claim")
913-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
914-
Message: "Login disabled until OIDC config is fixed, contact your administrator. Missing OIDC user roles in claim.",
915-
Detail: "If role sync is enabled, then the OIDC user roles must be present in the claim. Disabling role sync will allow login to proceed.",
916-
})
917-
return
918-
}
919-
920-
// Convert the []interface{} we get to a []string.
921-
rolesInterface, ok := rolesRow.([]interface{})
922-
if !ok {
923-
api.Logger.Error(ctx, "oidc claim user roles field was an unknown type",
924-
slog.F("type", fmt.Sprintf("%T", rolesRow)),
925-
)
926-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
927-
Message: "Login disabled until OIDC config is fixed, contact your administrator. Missing OIDC user roles in claim.",
928-
Detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
929-
})
930-
return
931-
}
932-
933-
api.Logger.Debug(ctx, "roles returned in oidc claims",
934-
slog.F("len", len(rolesInterface)),
935-
slog.F("roles", rolesInterface),
936-
)
937-
for _, roleInterface := range rolesInterface {
938-
role, ok := roleInterface.(string)
939-
if !ok {
940-
api.Logger.Error(ctx, "invalid oidc user role type",
941-
slog.F("type", fmt.Sprintf("%T", rolesRow)),
942-
)
943-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
944-
Message: fmt.Sprintf("Invalid user role type. Expected string, got: %T", roleInterface),
945-
})
946-
return
947-
}
948-
949-
if mappedRole, ok := api.OIDCConfig.UserRoleMapping[role]; ok {
950-
role = mappedRole
951-
}
952-
953-
roles = append(roles, role)
954-
}
955-
}
956-
957908
// This conditional is purely to warn the user they might have misconfigured their OIDC
958909
// configuration.
959910
if _, groupClaimExists := claims["groups"]; !usingGroups && groupClaimExists {
@@ -1006,6 +957,63 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
1006957
return
1007958
}
1008959

960+
roles := api.OIDCConfig.UserRolesDefault
961+
if api.OIDCConfig.RoleSyncEnabled() {
962+
rolesRow, ok := claims[api.OIDCConfig.UserRoleField]
963+
if !ok {
964+
// If no claim is provided than we can assume the user is just
965+
// a member. This is because there is no way to tell the difference
966+
// between []string{} and nil for OIDC claims. IDPs omit claims
967+
// if they are empty ([]string{}).
968+
rolesRow = []string{}
969+
}
970+
971+
// Convert the []interface{} we get to a []string.
972+
rolesInterface, ok := rolesRow.([]interface{})
973+
if !ok {
974+
api.Logger.Error(ctx, "oidc claim user roles field was an unknown type",
975+
slog.F("type", fmt.Sprintf("%T", rolesRow)),
976+
)
977+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
978+
Status: http.StatusInternalServerError,
979+
HideStatus: true,
980+
Title: "Login disabled until OIDC config is fixed",
981+
Description: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
982+
RetryEnabled: false,
983+
DashboardURL: "/login",
984+
})
985+
return
986+
}
987+
988+
api.Logger.Debug(ctx, "roles returned in oidc claims",
989+
slog.F("len", len(rolesInterface)),
990+
slog.F("roles", rolesInterface),
991+
)
992+
for _, roleInterface := range rolesInterface {
993+
role, ok := roleInterface.(string)
994+
if !ok {
995+
api.Logger.Error(ctx, "invalid oidc user role type",
996+
slog.F("type", fmt.Sprintf("%T", rolesRow)),
997+
)
998+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
999+
Message: fmt.Sprintf("Invalid user role type. Expected string, got: %T", roleInterface),
1000+
})
1001+
return
1002+
}
1003+
1004+
if mappedRoles, ok := api.OIDCConfig.UserRoleMapping[role]; ok {
1005+
if len(mappedRoles) == 0 {
1006+
continue
1007+
}
1008+
// Mapped roles are added to the list of roles
1009+
roles = append(roles, mappedRoles...)
1010+
continue
1011+
}
1012+
1013+
roles = append(roles, role)
1014+
}
1015+
}
1016+
10091017
// If a new user is authenticating for the first time
10101018
// the audit action is 'register', not 'login'
10111019
if user.ID == uuid.Nil {
@@ -1178,6 +1186,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
11781186
ctx = r.Context()
11791187
user database.User
11801188
cookies []*http.Cookie
1189+
logger = api.Logger.Named(userAuthLoggerName)
11811190
)
11821191

11831192
var isConvertLoginType bool
@@ -1320,10 +1329,32 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
13201329

13211330
// Ensure roles are correct.
13221331
if params.UsingRoles {
1332+
ignored := make([]string, 0)
1333+
filtered := make([]string, 0, len(params.Roles))
1334+
for _, role := range params.Roles {
1335+
if _, err := rbac.RoleByName(role); err == nil {
1336+
filtered = append(filtered, role)
1337+
} else {
1338+
ignored = append(ignored, role)
1339+
}
1340+
}
1341+
13231342
//nolint:gocritic
1324-
err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, params.Roles)
1343+
err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, filtered)
13251344
if err != nil {
1326-
return xerrors.Errorf("set user groups: %w", err)
1345+
return httpError{
1346+
code: http.StatusBadRequest,
1347+
msg: "Invalid roles through OIDC claim",
1348+
detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()),
1349+
renderStaticPage: true,
1350+
}
1351+
}
1352+
if len(ignored) > 0 {
1353+
logger.Debug(ctx, "OIDC roles ignored in assignment",
1354+
slog.F("ignored", ignored),
1355+
slog.F("assigned", filtered),
1356+
slog.F("user_id", user.ID),
1357+
)
13271358
}
13281359
}
13291360

‎coderd/users.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) {
909909
return
910910
}
911911

912-
updatedUser, err := api.UpdateSiteUserRoles(ctx, database.UpdateUserRolesParams{
912+
updatedUser, err := UpdateSiteUserRoles(ctx, api.Database, database.UpdateUserRolesParams{
913913
GrantedRoles: params.Roles,
914914
ID: user.ID,
915915
})
@@ -939,7 +939,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) {
939939

940940
// UpdateSiteUserRoles will ensure only site wide roles are passed in as arguments.
941941
// If an organization role is included, an error is returned.
942-
func (api *API) UpdateSiteUserRoles(ctx context.Context, args database.UpdateUserRolesParams) (database.User, error) {
942+
func UpdateSiteUserRoles(ctx context.Context, db database.Store, args database.UpdateUserRolesParams) (database.User, error) {
943943
// Enforce only site wide roles.
944944
for _, r := range args.GrantedRoles {
945945
if _, ok := rbac.IsOrgRole(r); ok {
@@ -951,7 +951,7 @@ func (api *API) UpdateSiteUserRoles(ctx context.Context, args database.UpdateUse
951951
}
952952
}
953953

954-
updatedUser, err := api.Database.UpdateUserRoles(ctx, args)
954+
updatedUser, err := db.UpdateUserRoles(ctx, args)
955955
if err != nil {
956956
return database.User{}, xerrors.Errorf("update site roles: %w", err)
957957
}

‎codersdk/deployment.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -256,24 +256,24 @@ type OAuth2GithubConfig struct {
256256
}
257257

258258
type OIDCConfig struct {
259-
AllowSignups clibase.Bool `json:"allow_signups" typescript:",notnull"`
260-
ClientID clibase.String `json:"client_id" typescript:",notnull"`
261-
ClientSecret clibase.String `json:"client_secret" typescript:",notnull"`
262-
EmailDomain clibase.StringArray `json:"email_domain" typescript:",notnull"`
263-
IssuerURL clibase.String `json:"issuer_url" typescript:",notnull"`
264-
Scopes clibase.StringArray `json:"scopes" typescript:",notnull"`
265-
IgnoreEmailVerified clibase.Bool `json:"ignore_email_verified" typescript:",notnull"`
266-
UsernameField clibase.String `json:"username_field" typescript:",notnull"`
267-
EmailField clibase.String `json:"email_field" typescript:",notnull"`
268-
AuthURLParams clibase.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"`
269-
IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"`
270-
GroupField clibase.String `json:"groups_field" typescript:",notnull"`
271-
GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"`
272-
UserRoleField clibase.String `json:"user_role_field" typescript:",notnull"`
273-
UserRoleMapping clibase.Struct[map[string]string] `json:"user_role_mapping" typescript:",notnull"`
274-
UserRolesDefault clibase.StringArray `json:"user_roles_default" typescript:",notnull"`
275-
SignInText clibase.String `json:"sign_in_text" typescript:",notnull"`
276-
IconURL clibase.URL `json:"icon_url" typescript:",notnull"`
259+
AllowSignups clibase.Bool `json:"allow_signups" typescript:",notnull"`
260+
ClientID clibase.String `json:"client_id" typescript:",notnull"`
261+
ClientSecret clibase.String `json:"client_secret" typescript:",notnull"`
262+
EmailDomain clibase.StringArray `json:"email_domain" typescript:",notnull"`
263+
IssuerURL clibase.String `json:"issuer_url" typescript:",notnull"`
264+
Scopes clibase.StringArray `json:"scopes" typescript:",notnull"`
265+
IgnoreEmailVerified clibase.Bool `json:"ignore_email_verified" typescript:",notnull"`
266+
UsernameField clibase.String `json:"username_field" typescript:",notnull"`
267+
EmailField clibase.String `json:"email_field" typescript:",notnull"`
268+
AuthURLParams clibase.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"`
269+
IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"`
270+
GroupField clibase.String `json:"groups_field" typescript:",notnull"`
271+
GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"`
272+
UserRoleField clibase.String `json:"user_role_field" typescript:",notnull"`
273+
UserRoleMapping clibase.Struct[map[string][]string] `json:"user_role_mapping" typescript:",notnull"`
274+
UserRolesDefault clibase.StringArray `json:"user_roles_default" typescript:",notnull"`
275+
SignInText clibase.String `json:"sign_in_text" typescript:",notnull"`
276+
IconURL clibase.URL `json:"icon_url" typescript:",notnull"`
277277
}
278278

279279
type TelemetryConfig struct {
@@ -1046,7 +1046,7 @@ when required by your organization's security policy.`,
10461046
},
10471047
{
10481048
Name: "OIDC User Role Mapping",
1049-
Description: "A map of the OIDC passed in user roles and the groups in Coder it should map to. This is useful if the group names do not match.",
1049+
Description: "A map of the OIDC passed in user roles and the groups in Coder it should map to. This is useful if the group names do not match. If mapped to the empty string, the role will ignored.",
10501050
Flag: "oidc-user-role-mapping",
10511051
Env: "CODER_OIDC_USER_ROLE_MAPPING",
10521052
Default: "{}",

‎enterprise/coderd/userauth.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package coderd
33
import (
44
"context"
55

6+
"github.com/coder/coder/coderd"
7+
68
"github.com/google/uuid"
79
"golang.org/x/xerrors"
810

@@ -54,12 +56,12 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui
5456
func (api *API) setUserSiteRoles(ctx context.Context, db database.Store, userID uuid.UUID, roles []string) error {
5557
// Should this be feature protected?
5658
return db.InTx(func(tx database.Store) error {
57-
_, err := api.AGPL.UpdateSiteUserRoles(ctx, database.UpdateUserRolesParams{
59+
_, err := coderd.UpdateSiteUserRoles(ctx, db, database.UpdateUserRolesParams{
5860
GrantedRoles: roles,
5961
ID: userID,
6062
})
6163
if err != nil {
62-
return xerrors.Errorf("set user roles: %w", err)
64+
return xerrors.Errorf("set user roles(%s): %w", userID.String(), err)
6365
}
6466

6567
return nil

‎enterprise/coderd/userauth_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/coder/coder/coderd"
1818
"github.com/coder/coder/coderd/coderdtest"
19+
"github.com/coder/coder/coderd/rbac"
1920
"github.com/coder/coder/codersdk"
2021
"github.com/coder/coder/enterprise/coderd/coderdenttest"
2122
"github.com/coder/coder/testutil"
@@ -24,6 +25,61 @@ import (
2425
// nolint:bodyclose
2526
func TestUserOIDC(t *testing.T) {
2627
t.Parallel()
28+
t.Run("Roles", func(t *testing.T) {
29+
t.Parallel()
30+
31+
t.Run("NewUser", func(t *testing.T) {
32+
t.Parallel()
33+
34+
ctx := testutil.Context(t, testutil.WaitMedium)
35+
conf := coderdtest.NewOIDCConfig(t, "")
36+
37+
oidcRoleName := "TemplateAuthor"
38+
39+
config := conf.OIDCConfig(t, jwt.MapClaims{}, func(cfg *coderd.OIDCConfig) {
40+
cfg.UserRoleMapping = map[string][]string{oidcRoleName: {rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin()}}
41+
})
42+
config.AllowSignups = true
43+
config.UserRoleField = "roles"
44+
45+
client, _ := coderdenttest.New(t, &coderdenttest.Options{
46+
Options: &coderdtest.Options{
47+
OIDCConfig: config,
48+
},
49+
LicenseOptions: &coderdenttest.LicenseOptions{
50+
Features: license.Features{codersdk.FeatureTemplateRBAC: 1},
51+
},
52+
})
53+
54+
admin, err := client.User(ctx, "me")
55+
require.NoError(t, err)
56+
require.Len(t, admin.OrganizationIDs, 1)
57+
58+
resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{
59+
"email": "alice@coder.com",
60+
"roles": []string{"random", oidcRoleName, rbac.RoleOwner()},
61+
}))
62+
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
63+
user, err := client.User(ctx, "alice")
64+
require.NoError(t, err)
65+
66+
require.Len(t, user.Roles, 3)
67+
roleNames := []string{user.Roles[0].Name, user.Roles[1].Name, user.Roles[2].Name}
68+
require.ElementsMatch(t, roleNames, []string{rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin(), rbac.RoleOwner()})
69+
70+
// Now remove the roles with a new oidc login
71+
resp = oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{
72+
"email": "alice@coder.com",
73+
"roles": []string{"random"},
74+
}))
75+
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
76+
user, err = client.User(ctx, "alice")
77+
require.NoError(t, err)
78+
79+
require.Len(t, user.Roles, 0)
80+
})
81+
})
82+
2783
t.Run("Groups", func(t *testing.T) {
2884
t.Parallel()
2985
t.Run("Assigns", func(t *testing.T) {

‎site/src/api/typesGenerated.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,12 @@ export interface OIDCConfig {
605605
// Named type "github.com/coder/coder/cli/clibase.Struct[map[string]string]" unknown, using "any"
606606
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type
607607
readonly group_mapping: any
608+
readonly user_role_field: string
609+
// Named type "github.com/coder/coder/cli/clibase.Struct[map[string][]string]" unknown, using "any"
610+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type
611+
readonly user_role_mapping: any
612+
// This is likely an enum in an external package ("github.com/coder/coder/cli/clibase.StringArray")
613+
readonly user_roles_default: string[]
608614
readonly sign_in_text: string
609615
readonly icon_url: string
610616
}

0 commit comments

Comments
 (0)