From fdee51605ba6a5a04bdb774caa97728b1d972cb2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 24 Jul 2023 12:11:27 -0400 Subject: [PATCH 1/2] fix: handle omitted role sync claim --- coderd/userauth.go | 2 +- enterprise/coderd/userauth_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 5b13a3d8d23ba..d46597a69d111 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -965,7 +965,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // a member. This is because there is no way to tell the difference // between []string{} and nil for OIDC claims. IDPs omit claims // if they are empty ([]string{}). - rolesRow = []string{} + rolesRow = []interface{} } rolesInterface, ok := rolesRow.([]interface{}) diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index d6b5903c16766..428cf91a6fef2 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -28,6 +28,45 @@ func TestUserOIDC(t *testing.T) { t.Run("RoleSync", func(t *testing.T) { t.Parallel() + t.Run("NoRoles", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + conf := coderdtest.NewOIDCConfig(t, "") + + oidcRoleName := "TemplateAuthor" + + config := conf.OIDCConfig(t, jwt.MapClaims{}, func(cfg *coderd.OIDCConfig) { + cfg.UserRoleMapping = map[string][]string{oidcRoleName: {rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin()}} + }) + config.AllowSignups = true + config.UserRoleField = "roles" + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + OIDCConfig: config, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureUserRoleManagement: 1}, + }, + }) + + admin, err := client.User(ctx, "me") + require.NoError(t, err) + require.Len(t, admin.OrganizationIDs, 1) + + resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "alice@coder.com", + })) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + user, err := client.User(ctx, "alice") + require.NoError(t, err) + + require.Len(t, user.Roles, 0) + roleNames := []string{} + require.ElementsMatch(t, roleNames, []string{}) + }) + t.Run("NewUserAndRemoveRoles", func(t *testing.T) { t.Parallel() From 453c72ddc27734ee4350bf755ba3581174ad4351 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 24 Jul 2023 12:16:27 -0400 Subject: [PATCH 2/2] fixup! fix: handle omitted role sync claim --- coderd/userauth.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index d46597a69d111..c6ab387cf203e 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -965,7 +965,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // a member. This is because there is no way to tell the difference // between []string{} and nil for OIDC claims. IDPs omit claims // if they are empty ([]string{}). - rolesRow = []interface{} + // Use []interface{}{} so the next typecast works. + rolesRow = []interface{}{} } rolesInterface, ok := rolesRow.([]interface{})