From 3040caeb2bfd3ddcbdb405e95a95a67c0afc74f3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 19:14:31 -0600 Subject: [PATCH 1/8] feat: Allow changing the 'group' oidc claim field --- cli/server.go | 1 + coderd/userauth.go | 8 ++++++-- codersdk/deployment.go | 11 +++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/cli/server.go b/cli/server.go index 94f4188aa2934..35716ea5b5f63 100644 --- a/cli/server.go +++ b/cli/server.go @@ -779,6 +779,7 @@ flags, and YAML configuration. The precedence is as follows: EmailDomain: cfg.OIDC.EmailDomain, AllowSignups: cfg.OIDC.AllowSignups.Value(), UsernameField: cfg.OIDC.UsernameField.String(), + GroupField: cfg.OIDC.GroupField.String(), SignInText: cfg.OIDC.SignInText.String(), IconURL: cfg.OIDC.IconURL.String(), IgnoreEmailVerified: cfg.OIDC.IgnoreEmailVerified.Value(), diff --git a/coderd/userauth.go b/coderd/userauth.go index 7fab1df83fda1..b426a533c2a27 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -477,6 +477,10 @@ type OIDCConfig struct { // UsernameField selects the claim field to be used as the created user's // username. UsernameField string + // GroupField selects the claim field to be used as the created user's + // groups. If the group field is the empty string, then no group updates + // will ever come from the OIDC provider. + GroupField string // SignInText is the text to display on the OIDC login button SignInText string // IconURL points to the URL of an icon to display on the OIDC login button @@ -610,8 +614,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } var groups []string - groupsRaw, ok := claims["groups"] - if ok { + groupsRaw, ok := claims[api.OIDCConfig.GroupField] + if ok && api.OIDCConfig.GroupField != "" { // Convert the []interface{} we get to a []string. groupsInterface, ok := groupsRaw.([]interface{}) if ok { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 5066d9ff11f45..938fd1f67262c 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -220,6 +220,7 @@ type OIDCConfig struct { Scopes clibase.Strings `json:"scopes" typescript:",notnull"` IgnoreEmailVerified clibase.Bool `json:"ignore_email_verified" typescript:",notnull"` UsernameField clibase.String `json:"username_field" typescript:",notnull"` + GroupField clibase.String `json:"group" typescript:",notnull"` SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` IconURL clibase.URL `json:"icon_url" typescript:",notnull"` } @@ -818,6 +819,16 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "usernameField", }, + { + Name: "OIDC Group Field", + Description: "OIDC claim field to use as the user's groups. Set to \"\" to disable OIDC group support.", + Flag: "oidc-group-field", + Env: "OIDC_GROUP_FIELD", + Default: "group", + Value: &c.OIDC.GroupField, + Group: &deploymentGroupOIDC, + YAML: "groupField", + }, { Name: "OpenID Connect sign in text", Description: "The text to show on the OpenID Connect sign in button", From 62ef8d9399f40195c5a430cabd8eea3b1c00c572 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 19:24:36 -0600 Subject: [PATCH 2/8] Add unit test --- coderd/coderdtest/coderdtest.go | 9 +++++++-- codersdk/deployment.go | 2 +- enterprise/coderd/userauth_test.go | 10 +++++++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 0b4222763dd1a..e05e00649f879 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -939,7 +939,7 @@ func (o *OIDCConfig) EncodeClaims(t *testing.T, claims jwt.MapClaims) string { return base64.StdEncoding.EncodeToString([]byte(signed)) } -func (o *OIDCConfig) OIDCConfig(t *testing.T, userInfoClaims jwt.MapClaims) *coderd.OIDCConfig { +func (o *OIDCConfig) OIDCConfig(t *testing.T, userInfoClaims jwt.MapClaims, opts ...func(cfg *coderd.OIDCConfig)) *coderd.OIDCConfig { // By default, the provider can be empty. // This means it won't support any endpoints! provider := &oidc.Provider{} @@ -956,7 +956,7 @@ func (o *OIDCConfig) OIDCConfig(t *testing.T, userInfoClaims jwt.MapClaims) *cod } provider = cfg.NewProvider(context.Background()) } - return &coderd.OIDCConfig{ + cfg := &coderd.OIDCConfig{ OAuth2Config: o, Verifier: oidc.NewVerifier(o.issuer, &oidc.StaticKeySet{ PublicKeys: []crypto.PublicKey{o.key.Public()}, @@ -965,7 +965,12 @@ func (o *OIDCConfig) OIDCConfig(t *testing.T, userInfoClaims jwt.MapClaims) *cod }), Provider: provider, UsernameField: "preferred_username", + GroupField: "groups", } + for _, opt := range opts { + opt(cfg) + } + return cfg } // NewAzureInstanceIdentity returns a metadata client and ID token validator for faking diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 938fd1f67262c..a746bac91a508 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -824,7 +824,7 @@ when required by your organization's security policy.`, Description: "OIDC claim field to use as the user's groups. Set to \"\" to disable OIDC group support.", Flag: "oidc-group-field", Env: "OIDC_GROUP_FIELD", - Default: "group", + Default: "groups", Value: &c.OIDC.GroupField, Group: &deploymentGroupOIDC, YAML: "groupField", diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index a2dbd051d7714..55ddb888c12da 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/codersdk" "github.com/coder/coder/enterprise/coderd/coderdenttest" @@ -28,7 +29,10 @@ func TestUserOIDC(t *testing.T) { ctx, _ := testutil.Context(t) conf := coderdtest.NewOIDCConfig(t, "") - config := conf.OIDCConfig(t, jwt.MapClaims{}) + const groupClaim = "custom-groups" + config := conf.OIDCConfig(t, jwt.MapClaims{}, func(cfg *coderd.OIDCConfig) { + cfg.GroupField = groupClaim + }) config.AllowSignups = true client := coderdenttest.New(t, &coderdenttest.Options{ @@ -53,8 +57,8 @@ func TestUserOIDC(t *testing.T) { require.Len(t, group.Members, 0) resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ - "email": "colin@coder.com", - "groups": []string{groupName}, + "email": "colin@coder.com", + groupClaim: []string{groupName}, })) assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) From b1ffebe4a8aef9e9be70ee5e86c940cc1f5b08ae Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 21:31:51 -0600 Subject: [PATCH 3/8] Make gen --- coderd/apidoc/docs.go | 3 +++ coderd/apidoc/swagger.json | 3 +++ codersdk/deployment.go | 2 +- docs/api/general.md | 1 + docs/api/schemas.md | 4 ++++ site/src/api/typesGenerated.ts | 1 + 6 files changed, 13 insertions(+), 1 deletion(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 966bff2bf4d45..6b8943b4d7812 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7078,6 +7078,9 @@ const docTemplate = `{ "type": "string" } }, + "groups_field": { + "type": "string" + }, "icon_url": { "$ref": "#/definitions/clibase.URL" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index b64e2cd4d79d2..1d7a821dee173 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6338,6 +6338,9 @@ "type": "string" } }, + "groups_field": { + "type": "string" + }, "icon_url": { "$ref": "#/definitions/clibase.URL" }, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index a746bac91a508..0f6b881d04a39 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -220,7 +220,7 @@ type OIDCConfig struct { Scopes clibase.Strings `json:"scopes" typescript:",notnull"` IgnoreEmailVerified clibase.Bool `json:"ignore_email_verified" typescript:",notnull"` UsernameField clibase.String `json:"username_field" typescript:",notnull"` - GroupField clibase.String `json:"group" typescript:",notnull"` + GroupField clibase.String `json:"groups_field" typescript:",notnull"` SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` IconURL clibase.URL `json:"icon_url" typescript:",notnull"` } diff --git a/docs/api/general.md b/docs/api/general.md index 2516d79e995c2..679b6405af156 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -230,6 +230,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "groups_field": "string", "icon_url": { "forceQuery": true, "fragment": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index bfe21043b84b9..8ce9eac55540b 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1762,6 +1762,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "groups_field": "string", "icon_url": { "forceQuery": true, "fragment": "string", @@ -2101,6 +2102,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "groups_field": "string", "icon_url": { "forceQuery": true, "fragment": "string", @@ -2761,6 +2763,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "groups_field": "string", "icon_url": { "forceQuery": true, "fragment": "string", @@ -2790,6 +2793,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a | `client_id` | string | false | | | | `client_secret` | string | false | | | | `email_domain` | array of string | false | | | +| `groups_field` | string | false | | | | `icon_url` | [clibase.URL](#clibaseurl) | false | | | | `ignore_email_verified` | boolean | false | | | | `issuer_url` | string | false | | | diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index f9bd7bcf48b19..18647dcd68161 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -502,6 +502,7 @@ export interface OIDCConfig { readonly scopes: string[] readonly ignore_email_verified: boolean readonly username_field: string + readonly groups_field: string readonly sign_in_text: string readonly icon_url: string } From c207351ce92128a6bc67280ad3b698e8de1f6115 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 22:01:02 -0600 Subject: [PATCH 4/8] Enable empty groups support --- cli/server.go | 6 ++++++ coderd/userauth.go | 40 +++++++++++++++++++++++++--------------- codersdk/deployment.go | 15 ++++++++++----- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/cli/server.go b/cli/server.go index 35716ea5b5f63..fe94ce1611ba2 100644 --- a/cli/server.go +++ b/cli/server.go @@ -75,6 +75,7 @@ import ( "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/coderd/tracing" "github.com/coder/coder/coderd/updatecheck" + "github.com/coder/coder/coderd/util/slice" "github.com/coder/coder/codersdk" "github.com/coder/coder/cryptorand" "github.com/coder/coder/provisioner/echo" @@ -764,6 +765,11 @@ flags, and YAML configuration. The precedence is as follows: if err != nil { return xerrors.Errorf("parse oidc oauth callback url: %w", err) } + // If the scopes contain 'groups', we enable group support. + // Do not override any custom value set by the user. + if slice.Contains(cfg.OIDC.Scopes, "groups") && cfg.OIDC.GroupField == "" { + cfg.OIDC.GroupField = "groups" + } options.OIDCConfig = &coderd.OIDCConfig{ OAuth2Config: &oauth2.Config{ ClientID: cfg.OIDC.ClientID.String(), diff --git a/coderd/userauth.go b/coderd/userauth.go index b426a533c2a27..9ca7dfec53c3b 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -613,21 +613,27 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } } + var usingGroups bool var groups []string - groupsRaw, ok := claims[api.OIDCConfig.GroupField] - if ok && api.OIDCConfig.GroupField != "" { - // Convert the []interface{} we get to a []string. - groupsInterface, ok := groupsRaw.([]interface{}) - if ok { - for _, groupInterface := range groupsInterface { - group, ok := groupInterface.(string) - if !ok { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Invalid group type. Expected string, got: %t", emailRaw), - }) - return + // 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 != "" { + usingGroups = true + groupsRaw, ok := claims[api.OIDCConfig.GroupField] + if ok && api.OIDCConfig.GroupField != "" { + // Convert the []interface{} we get to a []string. + groupsInterface, ok := groupsRaw.([]interface{}) + if ok { + for _, groupInterface := range groupsInterface { + group, ok := groupInterface.(string) + if !ok { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Invalid group type. Expected string, got: %t", emailRaw), + }) + return + } + groups = append(groups, group) } - groups = append(groups, group) } } } @@ -688,6 +694,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { Email: email, Username: username, AvatarURL: picture, + UsingGroups: usingGroups, Groups: groups, }) var httpErr httpError @@ -729,7 +736,10 @@ type oauthLoginParams struct { Email string Username string AvatarURL string - Groups []string + // Is UsingGroups is true, then the user will be assigned + // to the Groups provided. + UsingGroups bool + Groups []string } type httpError struct { @@ -869,7 +879,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook } // Ensure groups are correct. - if len(params.Groups) > 0 { + if params.UsingGroups { //nolint:gocritic err := api.Options.SetUserGroups(dbauthz.AsSystemRestricted(ctx), tx, user.ID, params.Groups) if err != nil { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 0f6b881d04a39..4135a95bfec46 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -821,13 +821,18 @@ when required by your organization's security policy.`, }, { Name: "OIDC Group Field", - Description: "OIDC claim field to use as the user's groups. Set to \"\" to disable OIDC group support.", + Description: "Change the OIDC default 'groups' claim field. By default, will be 'groups' if present in the oidc scopes argument.", Flag: "oidc-group-field", Env: "OIDC_GROUP_FIELD", - Default: "groups", - Value: &c.OIDC.GroupField, - Group: &deploymentGroupOIDC, - YAML: "groupField", + // This value is intentionally blank. If this is empty, then OIDC group + // behavior is disabled. If 'oidc-scopes' contains 'groups', then the + // default value will be 'groups'. If the user wants to use a different claim + // such as 'memberOf', they can override the default 'groups' claim value + // that comes from the oidc scopes. + Default: "", + Value: &c.OIDC.GroupField, + Group: &deploymentGroupOIDC, + YAML: "groupField", }, { Name: "OpenID Connect sign in text", From 0c47e3b3548d63a2350601d6ef61c874bc2614f6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 22:21:08 -0600 Subject: [PATCH 5/8] fix: Delete was wiping all groups, not just the single user's groups --- coderd/database/queries.sql.go | 16 +++++----------- coderd/database/queries/groupmembers.sql | 12 +++--------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 707e87ee6f4e7..81245f43795e5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -960,25 +960,19 @@ func (q *sqlQuerier) DeleteGroupMemberFromGroup(ctx context.Context, arg DeleteG const deleteGroupMembersByOrgAndUser = `-- name: DeleteGroupMembersByOrgAndUser :exec DELETE FROM - group_members -USING - group_members AS gm -LEFT JOIN - groups -ON - groups.id = gm.group_id + group_members WHERE - groups.organization_id = $1 AND - gm.user_id = $2 + group_members.user_id = $1 + AND group_id = ANY(SELECT id FROM groups WHERE organization_id = $2) ` type DeleteGroupMembersByOrgAndUserParams struct { - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` UserID uuid.UUID `db:"user_id" json:"user_id"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` } func (q *sqlQuerier) DeleteGroupMembersByOrgAndUser(ctx context.Context, arg DeleteGroupMembersByOrgAndUserParams) error { - _, err := q.db.ExecContext(ctx, deleteGroupMembersByOrgAndUser, arg.OrganizationID, arg.UserID) + _, err := q.db.ExecContext(ctx, deleteGroupMembersByOrgAndUser, arg.UserID, arg.OrganizationID) return err } diff --git a/coderd/database/queries/groupmembers.sql b/coderd/database/queries/groupmembers.sql index 2ffa1e4f5caca..6e3081803dff4 100644 --- a/coderd/database/queries/groupmembers.sql +++ b/coderd/database/queries/groupmembers.sql @@ -35,16 +35,10 @@ FROM -- name: DeleteGroupMembersByOrgAndUser :exec DELETE FROM - group_members -USING - group_members AS gm -LEFT JOIN - groups -ON - groups.id = gm.group_id + group_members WHERE - groups.organization_id = @organization_id AND - gm.user_id = @user_id; + group_members.user_id = @user_id + AND group_id = ANY(SELECT id FROM groups WHERE organization_id = @organization_id); -- name: InsertGroupMember :exec INSERT INTO From 2fce66a2fe41b2373a2b4ca5828caed85478ef67 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 22:31:02 -0600 Subject: [PATCH 6/8] Update docs --- docs/admin/auth.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/admin/auth.md b/docs/admin/auth.md index 8cf905fb3022f..fb632fffddb19 100644 --- a/docs/admin/auth.md +++ b/docs/admin/auth.md @@ -183,7 +183,9 @@ CODER_TLS_CLIENT_KEY_FILE=/path/to/key.pem If your OpenID Connect provider supports group claims, you can configure Coder to synchronize groups in your auth provider to groups within Coder. -To enable group sync, ensure that the `group` claim is set: +To enable group sync, ensure that the `groups` claim is set. If group sync is +enabled, the user's groups will be controlled by the OIDC provider. This means +manual group additions/removals will be overwritten on the next login. ```console # as an environment variable From 8d3caa94c90cdc66f6a0c03e41f975120c54ec39 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 23:04:15 -0600 Subject: [PATCH 7/8] fix: Dbfake delete group member fixed --- coderd/database/dbfake/databasefake.go | 19 +++++-- coderd/database/queries.sql.go | 4 +- coderd/database/queries/groupmembers.sql | 4 +- enterprise/coderd/userauth_test.go | 68 ++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index 40e004e157527..8a8eb652ddc7e 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -3905,13 +3905,22 @@ func (q *fakeQuerier) DeleteGroupMembersByOrgAndUser(_ context.Context, arg data newMembers := q.groupMembers[:0] for _, member := range q.groupMembers { - if member.UserID == arg.UserID { + if member.UserID != arg.UserID { + // Do not delete the other members + newMembers = append(newMembers, member) + } else if member.UserID == arg.UserID { + // We only want to delete from groups in the organization in the args. for _, group := range q.groups { - if group.ID == member.GroupID && group.OrganizationID == arg.OrganizationID { - continue + // Find the group that the member is apartof. + if group.ID == member.GroupID { + // Only add back the member if the organization ID does not match + // the arg organization ID. Since the arg is saying which + // org to delete. + if group.OrganizationID != arg.OrganizationID { + newMembers = append(newMembers, member) + } + break } - - newMembers = append(newMembers, member) } } } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 81245f43795e5..b13551978fad6 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -962,8 +962,8 @@ const deleteGroupMembersByOrgAndUser = `-- name: DeleteGroupMembersByOrgAndUser DELETE FROM group_members WHERE - group_members.user_id = $1 - AND group_id = ANY(SELECT id FROM groups WHERE organization_id = $2) + group_members.user_id = $1 + AND group_id = ANY(SELECT id FROM groups WHERE organization_id = $2) ` type DeleteGroupMembersByOrgAndUserParams struct { diff --git a/coderd/database/queries/groupmembers.sql b/coderd/database/queries/groupmembers.sql index 6e3081803dff4..a7c04d85f20a8 100644 --- a/coderd/database/queries/groupmembers.sql +++ b/coderd/database/queries/groupmembers.sql @@ -37,8 +37,8 @@ FROM DELETE FROM group_members WHERE - group_members.user_id = @user_id - AND group_id = ANY(SELECT id FROM groups WHERE organization_id = @organization_id); + group_members.user_id = @user_id + AND group_id = ANY(SELECT id FROM groups WHERE organization_id = @organization_id); -- name: InsertGroupMember :exec INSERT INTO diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 55ddb888c12da..0832a9fd3565d 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -7,6 +7,8 @@ import ( "net/http" "testing" + "github.com/google/uuid" + "github.com/golang-jwt/jwt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -66,6 +68,72 @@ func TestUserOIDC(t *testing.T) { require.NoError(t, err) require.Len(t, group.Members, 1) }) + t.Run("AddThenRemove", func(t *testing.T) { + t.Parallel() + + ctx, _ := testutil.Context(t) + conf := coderdtest.NewOIDCConfig(t, "") + + config := conf.OIDCConfig(t, jwt.MapClaims{}) + config.AllowSignups = true + + client := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + OIDCConfig: config, + }, + }) + firstUser := coderdtest.CreateFirstUser(t, client) + coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + AllFeatures: true, + }) + + // Add some extra users/groups that should be asserted after. + // Adding this user as there was a bug that removing 1 user removed + // all users from the group. + _, extra := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) + groupName := "bingbong" + group, err := client.CreateGroup(ctx, firstUser.OrganizationID, codersdk.CreateGroupRequest{ + Name: groupName, + }) + require.NoError(t, err, "create group") + + group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{ + AddUsers: []string{ + firstUser.UserID.String(), + extra.ID.String(), + }, + }) + require.NoError(t, err, "patch group") + require.Len(t, group.Members, 2, "expect both members") + + // Now add OIDC user into the group + resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "colin@coder.com", + "groups": []string{groupName}, + })) + assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + group, err = client.Group(ctx, group.ID) + require.NoError(t, err) + require.Len(t, group.Members, 3) + + // Login to remove the OIDC user from the group + resp = oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "colin@coder.com", + "groups": []string{}, + })) + assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + group, err = client.Group(ctx, group.ID) + require.NoError(t, err) + require.Len(t, group.Members, 2) + var expected []uuid.UUID + for _, mem := range group.Members { + expected = append(expected, mem.ID) + } + require.ElementsMatchf(t, expected, []uuid.UUID{firstUser.UserID, extra.ID}, "expected members") + }) + t.Run("NoneMatch", func(t *testing.T) { t.Parallel() From 4cca9b88ece6c6a00fca3be1072e19250e7032f4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 9 Mar 2023 23:06:55 -0600 Subject: [PATCH 8/8] Import order --- enterprise/coderd/userauth_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 0832a9fd3565d..5e93b63611795 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -7,9 +7,8 @@ import ( "net/http" "testing" - "github.com/google/uuid" - "github.com/golang-jwt/jwt" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require"