Skip to content

Commit 78517ca

Browse files
EmyrkEmyrk
authored
feat: add group allowlist for oidc (#11070)
* feat: group allow list in OIDC settings
1 parent cb89bc1 commit 78517ca

File tree

14 files changed

+141
-5
lines changed

14 files changed

+141
-5
lines changed

‎cli/server.go

+10
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,15 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
149149
}
150150
useCfg = pkiCfg
151151
}
152+
if len(vals.OIDC.GroupAllowList) > 0 && vals.OIDC.GroupField == "" {
153+
return nil, xerrors.Errorf("'oidc-group-field' must be set if 'oidc-allowed-groups' is set. Either unset 'oidc-allowed-groups' or set 'oidc-group-field'")
154+
}
155+
156+
groupAllowList := make(map[string]bool)
157+
for _, group := range vals.OIDC.GroupAllowList.Value() {
158+
groupAllowList[group] = true
159+
}
160+
152161
return &coderd.OIDCConfig{
153162
OAuth2Config: useCfg,
154163
Provider: oidcProvider,
@@ -163,6 +172,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
163172
IgnoreUserInfo: vals.OIDC.IgnoreUserInfo.Value(),
164173
GroupField: vals.OIDC.GroupField.String(),
165174
GroupFilter: vals.OIDC.GroupRegexFilter.Value(),
175+
GroupAllowList: groupAllowList,
166176
CreateMissingGroups: vals.OIDC.GroupAutoCreate.Value(),
167177
GroupMapping: vals.OIDC.GroupMapping.Value,
168178
UserRoleField: vals.OIDC.UserRoleField.String(),

‎cli/testdata/coder_server_--help.golden

+6
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,12 @@ OIDC OPTIONS:
333333
--oidc-allow-signups bool, $CODER_OIDC_ALLOW_SIGNUPS (default: true)
334334
Whether new users can sign up with OIDC.
335335

336+
--oidc-allowed-groups string-array, $CODER_OIDC_ALLOWED_GROUPS
337+
If provided any group name not in the list will not be allowed to
338+
authenticate. This allows for restricting access to a specific set of
339+
groups. This filter is applied after the group mapping and before the
340+
regex filter.
341+
336342
--oidc-auth-url-params struct[map[string]string], $CODER_OIDC_AUTH_URL_PARAMS (default: {"access_type": "offline"})
337343
OIDC auth URL parameters to pass to the upstream provider.
338344

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

+5
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,11 @@ oidc:
323323
# mapping.
324324
# (default: .*, type: regexp)
325325
groupRegexFilter: .*
326+
# If provided any group name not in the list will not be allowed to authenticate.
327+
# This allows for restricting access to a specific set of groups. This filter is
328+
# applied after the group mapping and before the regex filter.
329+
# (default: <unset>, type: string-array)
330+
groupAllowed: []
326331
# This field must be set if using the user roles sync feature. Set this to the
327332
# name of the claim used to store the user's role. The roles should be sent as an
328333
# array of strings.

‎coderd/apidoc/docs.go

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderdtest/oidctest/helper.go

+8
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ func (h *LoginHelper) Login(t *testing.T, idTokenClaims jwt.MapClaims) (*codersd
4848
return h.fake.Login(t, unauthenticatedClient, idTokenClaims)
4949
}
5050

51+
// AttemptLogin does not assert a successful login.
52+
func (h *LoginHelper) AttemptLogin(t *testing.T, idTokenClaims jwt.MapClaims) (*codersdk.Client, *http.Response) {
53+
t.Helper()
54+
unauthenticatedClient := codersdk.New(h.client.URL)
55+
56+
return h.fake.AttemptLogin(t, unauthenticatedClient, idTokenClaims)
57+
}
58+
5159
// ExpireOauthToken expires the oauth token for the given user.
5260
func (*LoginHelper) ExpireOauthToken(t *testing.T, db database.Store, user *codersdk.Client) database.UserLink {
5361
t.Helper()

‎coderd/userauth.go

+29
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,10 @@ type OIDCConfig struct {
697697
// the OIDC provider. Any group not matched by this regex will be ignored.
698698
// If the group filter is nil, then no group filtering will occur.
699699
GroupFilter *regexp.Regexp
700+
// GroupAllowList is a list of groups that are allowed to log in.
701+
// If the list length is 0, then the allow list will not be applied and
702+
// this feature is disabled.
703+
GroupAllowList map[string]bool
700704
// GroupMapping controls how groups returned by the OIDC provider get mapped
701705
// to groups within Coder.
702706
// map[oidcGroupName]coderGroupName
@@ -921,6 +925,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
921925
picture, _ = pictureRaw.(string)
922926
}
923927

928+
ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username))
924929
usingGroups, groups, groupErr := api.oidcGroups(ctx, mergedClaims)
925930
if groupErr != nil {
926931
groupErr.Write(rw, r)
@@ -1010,6 +1015,10 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
10101015
// If the GroupField is the empty string, then groups from OIDC are not used.
10111016
// This is so we can support manual group assignment.
10121017
if api.OIDCConfig.GroupField != "" {
1018+
// If the allow list is empty, then the user is allowed to log in.
1019+
// Otherwise, they must belong to at least 1 group in the allow list.
1020+
inAllowList := len(api.OIDCConfig.GroupAllowList) == 0
1021+
10131022
usingGroups = true
10141023
groupsRaw, ok := mergedClaims[api.OIDCConfig.GroupField]
10151024
if ok {
@@ -1036,9 +1045,29 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
10361045
if mappedGroup, ok := api.OIDCConfig.GroupMapping[group]; ok {
10371046
group = mappedGroup
10381047
}
1048+
if _, ok := api.OIDCConfig.GroupAllowList[group]; ok {
1049+
inAllowList = true
1050+
}
10391051
groups = append(groups, group)
10401052
}
10411053
}
1054+
1055+
if !inAllowList {
1056+
logger.Debug(ctx, "oidc group claim not in allow list, rejecting login",
1057+
slog.F("allow_list_count", len(api.OIDCConfig.GroupAllowList)),
1058+
slog.F("user_group_count", len(groups)),
1059+
)
1060+
detail := "Ask an administrator to add one of your groups to the whitelist"
1061+
if len(groups) == 0 {
1062+
detail = "You are currently not a member of any groups! Ask an administrator to add you to an authorized group to login."
1063+
}
1064+
return usingGroups, groups, &httpError{
1065+
code: http.StatusForbidden,
1066+
msg: "Not a member of an allowed group",
1067+
detail: detail,
1068+
renderStaticPage: true,
1069+
}
1070+
}
10421071
}
10431072

10441073
// This conditional is purely to warn the user they might have misconfigured their OIDC

‎codersdk/deployment.go

+11
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ type OIDCConfig struct {
291291
IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"`
292292
GroupAutoCreate clibase.Bool `json:"group_auto_create" typescript:",notnull"`
293293
GroupRegexFilter clibase.Regexp `json:"group_regex_filter" typescript:",notnull"`
294+
GroupAllowList clibase.StringArray `json:"group_allow_list" typescript:",notnull"`
294295
GroupField clibase.String `json:"groups_field" typescript:",notnull"`
295296
GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"`
296297
UserRoleField clibase.String `json:"user_role_field" typescript:",notnull"`
@@ -1187,6 +1188,16 @@ when required by your organization's security policy.`,
11871188
Group: &deploymentGroupOIDC,
11881189
YAML: "groupRegexFilter",
11891190
},
1191+
{
1192+
Name: "OIDC Allowed Groups",
1193+
Description: "If provided any group name not in the list will not be allowed to authenticate. This allows for restricting access to a specific set of groups. This filter is applied after the group mapping and before the regex filter.",
1194+
Flag: "oidc-allowed-groups",
1195+
Env: "CODER_OIDC_ALLOWED_GROUPS",
1196+
Default: "",
1197+
Value: &c.OIDC.GroupAllowList,
1198+
Group: &deploymentGroupOIDC,
1199+
YAML: "groupAllowed",
1200+
},
11901201
{
11911202
Name: "OIDC User Role Field",
11921203
Description: "This field must be set if using the user roles sync feature. Set this to the name of the claim used to store the user's role. The roles should be sent as an array of strings.",

‎docs/api/general.md

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/api/schemas.md

+4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/cli/server.md

+10
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎enterprise/cli/testdata/coder_server_--help.golden

+6
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,12 @@ OIDC OPTIONS:
334334
--oidc-allow-signups bool, $CODER_OIDC_ALLOW_SIGNUPS (default: true)
335335
Whether new users can sign up with OIDC.
336336

337+
--oidc-allowed-groups string-array, $CODER_OIDC_ALLOWED_GROUPS
338+
If provided any group name not in the list will not be allowed to
339+
authenticate. This allows for restricting access to a specific set of
340+
groups. This filter is applied after the group mapping and before the
341+
regex filter.
342+
337343
--oidc-auth-url-params struct[map[string]string], $CODER_OIDC_AUTH_URL_PARAMS (default: {"access_type": "offline"})
338344
OIDC auth URL parameters to pass to the upstream provider.
339345

‎enterprise/coderd/userauth_test.go

+38-5
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,37 @@ func TestUserOIDC(t *testing.T) {
387387
require.Equal(t, http.StatusOK, resp.StatusCode)
388388
runner.AssertGroups(t, "alice", []string{groupName})
389389
})
390+
391+
t.Run("GroupAllowList", func(t *testing.T) {
392+
t.Parallel()
393+
394+
const groupClaim = "custom-groups"
395+
const allowedGroup = "foo"
396+
runner := setupOIDCTest(t, oidcTestConfig{
397+
Config: func(cfg *coderd.OIDCConfig) {
398+
cfg.AllowSignups = true
399+
cfg.GroupField = groupClaim
400+
cfg.GroupAllowList = map[string]bool{allowedGroup: true}
401+
},
402+
})
403+
404+
// Test forbidden
405+
_, resp := runner.AttemptLogin(t, jwt.MapClaims{
406+
"email": "alice@coder.com",
407+
groupClaim: []string{"not-allowed"},
408+
})
409+
require.Equal(t, http.StatusForbidden, resp.StatusCode)
410+
411+
// Test allowed
412+
client, _ := runner.Login(t, jwt.MapClaims{
413+
"email": "alice@coder.com",
414+
groupClaim: []string{allowedGroup},
415+
})
416+
417+
ctx := testutil.Context(t, testutil.WaitShort)
418+
_, err := client.User(ctx, codersdk.Me)
419+
require.NoError(t, err)
420+
})
390421
})
391422

392423
t.Run("Refresh", func(t *testing.T) {
@@ -661,7 +692,8 @@ type oidcTestRunner struct {
661692

662693
// Login will call the OIDC flow with an unauthenticated client.
663694
// The IDP will return the idToken claims.
664-
Login func(t *testing.T, idToken jwt.MapClaims) (*codersdk.Client, *http.Response)
695+
Login func(t *testing.T, idToken jwt.MapClaims) (*codersdk.Client, *http.Response)
696+
AttemptLogin func(t *testing.T, idToken jwt.MapClaims) (*codersdk.Client, *http.Response)
665697
// ForceRefresh will use an authenticated codersdk.Client, and force their
666698
// OIDC token to be expired and require a refresh. The refresh will use the claims provided.
667699
// It just calls the /users/me endpoint to trigger the refresh.
@@ -751,10 +783,11 @@ func setupOIDCTest(t *testing.T, settings oidcTestConfig) *oidcTestRunner {
751783
helper := oidctest.NewLoginHelper(owner, fake)
752784

753785
return &oidcTestRunner{
754-
AdminClient: owner,
755-
AdminUser: admin,
756-
API: api,
757-
Login: helper.Login,
786+
AdminClient: owner,
787+
AdminUser: admin,
788+
API: api,
789+
Login: helper.Login,
790+
AttemptLogin: helper.AttemptLogin,
758791
ForceRefresh: func(t *testing.T, client *codersdk.Client, idToken jwt.MapClaims) {
759792
helper.ForceRefresh(t, api.Database, client, idToken)
760793
},

‎site/src/api/typesGenerated.ts

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)