diff --git a/cli/server.go b/cli/server.go index 99dccc1088646..8b325e409078d 100644 --- a/cli/server.go +++ b/cli/server.go @@ -147,6 +147,15 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co } useCfg = pkiCfg } + if len(vals.OIDC.GroupAllowList) > 0 && vals.OIDC.GroupField == "" { + 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'") + } + + groupAllowList := make(map[string]bool) + for _, group := range vals.OIDC.GroupAllowList.Value() { + groupAllowList[group] = true + } + return &coderd.OIDCConfig{ OAuth2Config: useCfg, Provider: oidcProvider, @@ -161,6 +170,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co IgnoreUserInfo: vals.OIDC.IgnoreUserInfo.Value(), GroupField: vals.OIDC.GroupField.String(), GroupFilter: vals.OIDC.GroupRegexFilter.Value(), + GroupAllowList: groupAllowList, CreateMissingGroups: vals.OIDC.GroupAutoCreate.Value(), GroupMapping: vals.OIDC.GroupMapping.Value, UserRoleField: vals.OIDC.UserRoleField.String(), diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 4d1b9609aa6f4..72c1c2bee7334 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -333,6 +333,12 @@ OIDC OPTIONS: --oidc-allow-signups bool, $CODER_OIDC_ALLOW_SIGNUPS (default: true) Whether new users can sign up with OIDC. + --oidc-allowed-groups string-array, $CODER_OIDC_ALLOWED_GROUPS + 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. + --oidc-auth-url-params struct[map[string]string], $CODER_OIDC_AUTH_URL_PARAMS (default: {"access_type": "offline"}) OIDC auth URL parameters to pass to the upstream provider. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 3163a6e2cab33..2346f73d112d1 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -323,6 +323,11 @@ oidc: # mapping. # (default: .*, type: regexp) groupRegexFilter: .* + # 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. + # (default: , type: string-array) + groupAllowed: [] # 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. diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 8b9a47c224549..40d48562d47c6 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -9408,6 +9408,12 @@ const docTemplate = `{ "email_field": { "type": "string" }, + "group_allow_list": { + "type": "array", + "items": { + "type": "string" + } + }, "group_auto_create": { "type": "boolean" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 85fe382918ec4..dc6f847e5b46a 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -8445,6 +8445,12 @@ "email_field": { "type": "string" }, + "group_allow_list": { + "type": "array", + "items": { + "type": "string" + } + }, "group_auto_create": { "type": "boolean" }, diff --git a/coderd/coderdtest/oidctest/helper.go b/coderd/coderdtest/oidctest/helper.go index 1c434f1fcdf34..abf29d4fa2b46 100644 --- a/coderd/coderdtest/oidctest/helper.go +++ b/coderd/coderdtest/oidctest/helper.go @@ -48,6 +48,14 @@ func (h *LoginHelper) Login(t *testing.T, idTokenClaims jwt.MapClaims) (*codersd return h.fake.Login(t, unauthenticatedClient, idTokenClaims) } +// AttemptLogin does not assert a successful login. +func (h *LoginHelper) AttemptLogin(t *testing.T, idTokenClaims jwt.MapClaims) (*codersdk.Client, *http.Response) { + t.Helper() + unauthenticatedClient := codersdk.New(h.client.URL) + + return h.fake.AttemptLogin(t, unauthenticatedClient, idTokenClaims) +} + // ExpireOauthToken expires the oauth token for the given user. func (*LoginHelper) ExpireOauthToken(t *testing.T, db database.Store, user *codersdk.Client) database.UserLink { t.Helper() diff --git a/coderd/userauth.go b/coderd/userauth.go index b4c16ebdba98a..796ea806d9719 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -697,6 +697,10 @@ type OIDCConfig struct { // the OIDC provider. Any group not matched by this regex will be ignored. // If the group filter is nil, then no group filtering will occur. GroupFilter *regexp.Regexp + // GroupAllowList is a list of groups that are allowed to log in. + // If the list length is 0, then the allow list will not be applied and + // this feature is disabled. + GroupAllowList map[string]bool // GroupMapping controls how groups returned by the OIDC provider get mapped // to groups within Coder. // map[oidcGroupName]coderGroupName @@ -921,6 +925,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { picture, _ = pictureRaw.(string) } + ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username)) usingGroups, groups, groupErr := api.oidcGroups(ctx, mergedClaims) if groupErr != nil { groupErr.Write(rw, r) @@ -1010,6 +1015,10 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac // If the GroupField is the empty string, then groups from OIDC are not used. // This is so we can support manual group assignment. if api.OIDCConfig.GroupField != "" { + // If the allow list is empty, then the user is allowed to log in. + // Otherwise, they must belong to at least 1 group in the allow list. + inAllowList := len(api.OIDCConfig.GroupAllowList) == 0 + usingGroups = true groupsRaw, ok := mergedClaims[api.OIDCConfig.GroupField] if ok { @@ -1036,9 +1045,29 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac if mappedGroup, ok := api.OIDCConfig.GroupMapping[group]; ok { group = mappedGroup } + if _, ok := api.OIDCConfig.GroupAllowList[group]; ok { + inAllowList = true + } groups = append(groups, group) } } + + if !inAllowList { + logger.Debug(ctx, "oidc group claim not in allow list, rejecting login", + slog.F("allow_list_count", len(api.OIDCConfig.GroupAllowList)), + slog.F("user_group_count", len(groups)), + ) + detail := "Ask an administrator to add one of your groups to the whitelist" + if len(groups) == 0 { + detail = "You are currently not a member of any groups! Ask an administrator to add you to an authorized group to login." + } + return usingGroups, groups, &httpError{ + code: http.StatusForbidden, + msg: "Not a member of an allowed group", + detail: detail, + renderStaticPage: true, + } + } } // This conditional is purely to warn the user they might have misconfigured their OIDC diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 56dd5b38f0c8d..45d1d7601ef2f 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -291,6 +291,7 @@ type OIDCConfig struct { IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"` GroupAutoCreate clibase.Bool `json:"group_auto_create" typescript:",notnull"` GroupRegexFilter clibase.Regexp `json:"group_regex_filter" typescript:",notnull"` + GroupAllowList clibase.StringArray `json:"group_allow_list" typescript:",notnull"` GroupField clibase.String `json:"groups_field" typescript:",notnull"` GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"` UserRoleField clibase.String `json:"user_role_field" typescript:",notnull"` @@ -1187,6 +1188,16 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "groupRegexFilter", }, + { + Name: "OIDC Allowed Groups", + 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.", + Flag: "oidc-allowed-groups", + Env: "CODER_OIDC_ALLOWED_GROUPS", + Default: "", + Value: &c.OIDC.GroupAllowList, + Group: &deploymentGroupOIDC, + YAML: "groupAllowed", + }, { Name: "OIDC User Role Field", 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.", diff --git a/docs/api/general.md b/docs/api/general.md index de89e07e558c5..8aa124a18368f 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -271,6 +271,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_allow_list": ["string"], "group_auto_create": true, "group_mapping": {}, "group_regex_filter": {}, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index bcf54626d4952..0ccafbea41872 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2196,6 +2196,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_allow_list": ["string"], "group_auto_create": true, "group_mapping": {}, "group_regex_filter": {}, @@ -2571,6 +2572,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_allow_list": ["string"], "group_auto_create": true, "group_mapping": {}, "group_regex_filter": {}, @@ -3577,6 +3579,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_allow_list": ["string"], "group_auto_create": true, "group_mapping": {}, "group_regex_filter": {}, @@ -3618,6 +3621,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `client_secret` | string | false | | | | `email_domain` | array of string | false | | | | `email_field` | string | false | | | +| `group_allow_list` | array of string | false | | | | `group_auto_create` | boolean | false | | | | `group_mapping` | object | false | | | | `group_regex_filter` | [clibase.Regexp](#clibaseregexp) | false | | | diff --git a/docs/cli/server.md b/docs/cli/server.md index 3fb0c57b3ee48..2b700e09568f8 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -449,6 +449,16 @@ Base URL of a GitHub Enterprise deployment to use for Login with GitHub. Whether new users can sign up with OIDC. +### --oidc-allowed-groups + +| | | +| ----------- | --------------------------------------- | +| Type | string-array | +| Environment | $CODER_OIDC_ALLOWED_GROUPS | +| YAML | oidc.groupAllowed | + +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. + ### --oidc-auth-url-params | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 85c924474c206..6997e74260317 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -334,6 +334,12 @@ OIDC OPTIONS: --oidc-allow-signups bool, $CODER_OIDC_ALLOW_SIGNUPS (default: true) Whether new users can sign up with OIDC. + --oidc-allowed-groups string-array, $CODER_OIDC_ALLOWED_GROUPS + 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. + --oidc-auth-url-params struct[map[string]string], $CODER_OIDC_AUTH_URL_PARAMS (default: {"access_type": "offline"}) OIDC auth URL parameters to pass to the upstream provider. diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 70e63f6a1e3b6..bb3b37e943101 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -387,6 +387,37 @@ func TestUserOIDC(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) runner.AssertGroups(t, "alice", []string{groupName}) }) + + t.Run("GroupAllowList", func(t *testing.T) { + t.Parallel() + + const groupClaim = "custom-groups" + const allowedGroup = "foo" + runner := setupOIDCTest(t, oidcTestConfig{ + Config: func(cfg *coderd.OIDCConfig) { + cfg.AllowSignups = true + cfg.GroupField = groupClaim + cfg.GroupAllowList = map[string]bool{allowedGroup: true} + }, + }) + + // Test forbidden + _, resp := runner.AttemptLogin(t, jwt.MapClaims{ + "email": "alice@coder.com", + groupClaim: []string{"not-allowed"}, + }) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + + // Test allowed + client, _ := runner.Login(t, jwt.MapClaims{ + "email": "alice@coder.com", + groupClaim: []string{allowedGroup}, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + _, err := client.User(ctx, codersdk.Me) + require.NoError(t, err) + }) }) t.Run("Refresh", func(t *testing.T) { @@ -661,7 +692,8 @@ type oidcTestRunner struct { // Login will call the OIDC flow with an unauthenticated client. // The IDP will return the idToken claims. - Login func(t *testing.T, idToken jwt.MapClaims) (*codersdk.Client, *http.Response) + Login func(t *testing.T, idToken jwt.MapClaims) (*codersdk.Client, *http.Response) + AttemptLogin func(t *testing.T, idToken jwt.MapClaims) (*codersdk.Client, *http.Response) // ForceRefresh will use an authenticated codersdk.Client, and force their // OIDC token to be expired and require a refresh. The refresh will use the claims provided. // It just calls the /users/me endpoint to trigger the refresh. @@ -751,10 +783,11 @@ func setupOIDCTest(t *testing.T, settings oidcTestConfig) *oidcTestRunner { helper := oidctest.NewLoginHelper(owner, fake) return &oidcTestRunner{ - AdminClient: owner, - AdminUser: admin, - API: api, - Login: helper.Login, + AdminClient: owner, + AdminUser: admin, + API: api, + Login: helper.Login, + AttemptLogin: helper.AttemptLogin, ForceRefresh: func(t *testing.T, client *codersdk.Client, idToken jwt.MapClaims) { helper.ForceRefresh(t, api.Database, client, idToken) }, diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index af34c7d602e4e..dedb421dab568 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -689,6 +689,7 @@ export interface OIDCConfig { readonly ignore_user_info: boolean; readonly group_auto_create: boolean; readonly group_regex_filter: string; + readonly group_allow_list: string[]; readonly groups_field: string; readonly group_mapping: Record; readonly user_role_field: string;