Skip to content

Commit f827829

Browse files
EmyrkEmyrk
authored
feat: synchronize oidc user roles (#8595)
* feat: oidc user role sync User roles come from oidc claims. Prevent manual user role changes if set. * allow mapping 1:many
1 parent 94541d2 commit f827829

38 files changed

+596
-46
lines changed

‎.gitattributes

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Generated files
22
coderd/apidoc/docs.go linguist-generated=true
3+
docs/api/*.md linguist-generated=true
4+
docs/cli/*.md linguist-generated=true
35
coderd/apidoc/swagger.json linguist-generated=true
46
coderd/database/dump.sql linguist-generated=true
57
peerbroker/proto/*.go linguist-generated=true
@@ -9,3 +11,4 @@ provisionersdk/proto/*.go linguist-generated=true
911
*.tfstate.json linguist-generated=true
1012
*.tfstate.dot linguist-generated=true
1113
*.tfplan.dot linguist-generated=true
14+

‎cli/server.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,9 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
596596
IgnoreUserInfo: cfg.OIDC.IgnoreUserInfo.Value(),
597597
GroupField: cfg.OIDC.GroupField.String(),
598598
GroupMapping: cfg.OIDC.GroupMapping.Value,
599+
UserRoleField: cfg.OIDC.UserRoleField.String(),
600+
UserRoleMapping: cfg.OIDC.UserRoleMapping.Value,
601+
UserRolesDefault: cfg.OIDC.UserRolesDefault.GetSlice(),
599602
SignInText: cfg.OIDC.SignInText.String(),
600603
IconURL: cfg.OIDC.IconURL.String(),
601604
IgnoreEmailVerified: cfg.OIDC.IgnoreEmailVerified.Value(),

‎cli/server_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,8 @@ func TestServer(t *testing.T) {
10951095
require.False(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
10961096
require.Empty(t, deploymentConfig.Values.OIDC.GroupField.Value())
10971097
require.Empty(t, deploymentConfig.Values.OIDC.GroupMapping.Value)
1098+
require.Empty(t, deploymentConfig.Values.OIDC.UserRoleField.Value())
1099+
require.Empty(t, deploymentConfig.Values.OIDC.UserRoleMapping.Value)
10981100
require.Equal(t, "OpenID Connect", deploymentConfig.Values.OIDC.SignInText.Value())
10991101
require.Empty(t, deploymentConfig.Values.OIDC.IconURL.Value())
11001102
})

‎cli/testdata/coder_server_--help.golden

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,20 @@ can safely ignore these settings.
337337
--oidc-scopes string-array, $CODER_OIDC_SCOPES (default: openid,profile,email)
338338
Scopes to grant when authenticating with OIDC.
339339

340+
--oidc-user-role-default string-array, $CODER_OIDC_USER_ROLE_DEFAULT
341+
If user role sync is enabled, these roles are always included for all
342+
authenticated users. The 'member' role is always assigned.
343+
344+
--oidc-user-role-field string, $CODER_OIDC_USER_ROLE_FIELD
345+
This field must be set if using the user roles sync feature. Set this
346+
to the name of the claim used to store the user's role. The roles
347+
should be sent as an array of strings.
348+
349+
--oidc-user-role-mapping struct[map[string][]string], $CODER_OIDC_USER_ROLE_MAPPING (default: {})
350+
A map of the OIDC passed in user roles and the groups in Coder it
351+
should map to. This is useful if the group names do not match. If
352+
mapped to the empty string, the role will ignored.
353+
340354
--oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username)
341355
OIDC claim field to use as the username.
342356

‎cli/testdata/coder_users_list_--output_json.golden

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
"display_name": "Owner"
1616
}
1717
],
18-
"avatar_url": ""
18+
"avatar_url": "",
19+
"login_type": "password"
1920
},
2021
{
2122
"id": "[second user ID]",
@@ -28,6 +29,7 @@
2829
"[first org ID]"
2930
],
3031
"roles": [],
31-
"avatar_url": ""
32+
"avatar_url": "",
33+
"login_type": "password"
3234
}
3335
]

‎cli/testdata/server-config.yaml.golden

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,20 @@ oidc:
268268
# for when OIDC providers only return group IDs.
269269
# (default: {}, type: struct[map[string]string])
270270
groupMapping: {}
271+
# This field must be set if using the user roles sync feature. Set this to the
272+
# name of the claim used to store the user's role. The roles should be sent as an
273+
# array of strings.
274+
# (default: <unset>, type: string)
275+
userRoleField: ""
276+
# A map of the OIDC passed in user roles and the groups in Coder it should map to.
277+
# This is useful if the group names do not match. If mapped to the empty string,
278+
# the role will ignored.
279+
# (default: {}, type: struct[map[string][]string])
280+
userRoleMapping: {}
281+
# If user role sync is enabled, these roles are always included for all
282+
# authenticated users. The 'member' role is always assigned.
283+
# (default: <unset>, type: string-array)
284+
userRoleDefault: []
271285
# The text to show on the OpenID Connect sign in button.
272286
# (default: OpenID Connect, type: string)
273287
signInText: OpenID Connect

‎coderd/apidoc/docs.go

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ type Options struct {
124124
DERPMap *tailcfg.DERPMap
125125
SwaggerEndpoint bool
126126
SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error
127+
SetUserSiteRoles func(ctx context.Context, tx database.Store, userID uuid.UUID, roles []string) error
127128
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
128129
UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore]
129130
// AppSecurityKey is the crypto key used to sign and encrypt tokens related to
@@ -258,6 +259,14 @@ func New(options *Options) *API {
258259
return nil
259260
}
260261
}
262+
if options.SetUserSiteRoles == nil {
263+
options.SetUserSiteRoles = func(ctx context.Context, _ database.Store, userID uuid.UUID, roles []string) error {
264+
options.Logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise license",
265+
slog.F("user_id", userID), slog.F("roles", roles),
266+
)
267+
return nil
268+
}
269+
}
261270
if options.TemplateScheduleStore == nil {
262271
options.TemplateScheduleStore = &atomic.Pointer[schedule.TemplateScheduleStore]{}
263272
}

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ func User(user database.User, organizationIDs []uuid.UUID) codersdk.User {
115115
OrganizationIDs: organizationIDs,
116116
Roles: make([]codersdk.Role, 0, len(user.RBACRoles)),
117117
AvatarURL: user.AvatarURL.String,
118+
LoginType: codersdk.LoginType(user.LoginType),
118119
}
119120

120121
for _, roleName := range user.RBACRoles {

‎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: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,12 +684,27 @@ type OIDCConfig struct {
684684
// to groups within Coder.
685685
// map[oidcGroupName]coderGroupName
686686
GroupMapping map[string]string
687+
// UserRoleField selects the claim field to be used as the created user's
688+
// roles. If the field is the empty string, then no role updates
689+
// will ever come from the OIDC provider.
690+
UserRoleField string
691+
// UserRoleMapping controls how groups returned by the OIDC provider get mapped
692+
// to roles within Coder.
693+
// map[oidcRoleName][]coderRoleName
694+
UserRoleMapping map[string][]string
695+
// UserRolesDefault is the default set of roles to assign to a user if role sync
696+
// is enabled.
697+
UserRolesDefault []string
687698
// SignInText is the text to display on the OIDC login button
688699
SignInText string
689700
// IconURL points to the URL of an icon to display on the OIDC login button
690701
IconURL string
691702
}
692703

704+
func (cfg OIDCConfig) RoleSyncEnabled() bool {
705+
return cfg.UserRoleField != ""
706+
}
707+
693708
// @Summary OpenID Connect Callback
694709
// @ID openid-connect-callback
695710
// @Security CoderSessionToken
@@ -942,6 +957,62 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
942957
return
943958
}
944959

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+
rolesInterface, ok := rolesRow.([]interface{})
972+
if !ok {
973+
api.Logger.Error(ctx, "oidc claim user roles field was an unknown type",
974+
slog.F("type", fmt.Sprintf("%T", rolesRow)),
975+
)
976+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
977+
Status: http.StatusInternalServerError,
978+
HideStatus: true,
979+
Title: "Login disabled until OIDC config is fixed",
980+
Description: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
981+
RetryEnabled: false,
982+
DashboardURL: "/login",
983+
})
984+
return
985+
}
986+
987+
api.Logger.Debug(ctx, "roles returned in oidc claims",
988+
slog.F("len", len(rolesInterface)),
989+
slog.F("roles", rolesInterface),
990+
)
991+
for _, roleInterface := range rolesInterface {
992+
role, ok := roleInterface.(string)
993+
if !ok {
994+
api.Logger.Error(ctx, "invalid oidc user role type",
995+
slog.F("type", fmt.Sprintf("%T", rolesRow)),
996+
)
997+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
998+
Message: fmt.Sprintf("Invalid user role type. Expected string, got: %T", roleInterface),
999+
})
1000+
return
1001+
}
1002+
1003+
if mappedRoles, ok := api.OIDCConfig.UserRoleMapping[role]; ok {
1004+
if len(mappedRoles) == 0 {
1005+
continue
1006+
}
1007+
// Mapped roles are added to the list of roles
1008+
roles = append(roles, mappedRoles...)
1009+
continue
1010+
}
1011+
1012+
roles = append(roles, role)
1013+
}
1014+
}
1015+
9451016
// If a new user is authenticating for the first time
9461017
// the audit action is 'register', not 'login'
9471018
if user.ID == uuid.Nil {
@@ -959,6 +1030,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
9591030
Username: username,
9601031
AvatarURL: picture,
9611032
UsingGroups: usingGroups,
1033+
UsingRoles: api.OIDCConfig.RoleSyncEnabled(),
1034+
Roles: roles,
9621035
Groups: groups,
9631036
}).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) {
9641037
return audit.InitRequest[database.User](rw, params)
@@ -1045,6 +1118,10 @@ type oauthLoginParams struct {
10451118
// to the Groups provided.
10461119
UsingGroups bool
10471120
Groups []string
1121+
// Is UsingRoles is true, then the user will be assigned
1122+
// the roles provided.
1123+
UsingRoles bool
1124+
Roles []string
10481125

10491126
commitLock sync.Mutex
10501127
initAuditRequest func(params *audit.RequestParams) *audit.Request[database.User]
@@ -1108,6 +1185,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
11081185
ctx = r.Context()
11091186
user database.User
11101187
cookies []*http.Cookie
1188+
logger = api.Logger.Named(userAuthLoggerName)
11111189
)
11121190

11131191
var isConvertLoginType bool
@@ -1248,6 +1326,37 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
12481326
}
12491327
}
12501328

1329+
// Ensure roles are correct.
1330+
if params.UsingRoles {
1331+
ignored := make([]string, 0)
1332+
filtered := make([]string, 0, len(params.Roles))
1333+
for _, role := range params.Roles {
1334+
if _, err := rbac.RoleByName(role); err == nil {
1335+
filtered = append(filtered, role)
1336+
} else {
1337+
ignored = append(ignored, role)
1338+
}
1339+
}
1340+
1341+
//nolint:gocritic
1342+
err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, filtered)
1343+
if err != nil {
1344+
return httpError{
1345+
code: http.StatusBadRequest,
1346+
msg: "Invalid roles through OIDC claim",
1347+
detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()),
1348+
renderStaticPage: true,
1349+
}
1350+
}
1351+
if len(ignored) > 0 {
1352+
logger.Debug(ctx, "OIDC roles ignored in assignment",
1353+
slog.F("ignored", ignored),
1354+
slog.F("assigned", filtered),
1355+
slog.F("user_id", user.ID),
1356+
)
1357+
}
1358+
}
1359+
12511360
needsUpdate := false
12521361
if user.AvatarURL.String != params.AvatarURL {
12531362
user.AvatarURL = sql.NullString{

0 commit comments

Comments
 (0)