From b53d353dca4caebd821724b1e888bac8006e4587 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Tue, 21 Mar 2023 13:14:55 -0500 Subject: [PATCH 1/2] feat: add group mapping option for group sync --- cli/server.go | 1 + coderd/apidoc/docs.go | 3 ++ coderd/apidoc/swagger.json | 3 ++ coderd/coderd.go | 7 ++++- coderd/userauth.go | 9 ++++++ codersdk/deployment.go | 35 ++++++++++++++--------- docs/admin/auth.md | 16 +++++++++++ docs/api/general.md | 1 + docs/api/schemas.md | 4 +++ enterprise/coderd/userauth_test.go | 45 ++++++++++++++++++++++++++++++ go.sum | 4 --- site/src/api/typesGenerated.ts | 3 ++ 12 files changed, 113 insertions(+), 18 deletions(-) diff --git a/cli/server.go b/cli/server.go index db5b7ae09be82..288e44ac3d6e1 100644 --- a/cli/server.go +++ b/cli/server.go @@ -796,6 +796,7 @@ flags, and YAML configuration. The precedence is as follows: AllowSignups: cfg.OIDC.AllowSignups.Value(), UsernameField: cfg.OIDC.UsernameField.String(), GroupField: cfg.OIDC.GroupField.String(), + GroupMapping: cfg.OIDC.GroupMapping.Value, SignInText: cfg.OIDC.SignInText.String(), IconURL: cfg.OIDC.IconURL.String(), IgnoreEmailVerified: cfg.OIDC.IgnoreEmailVerified.Value(), diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d3e5b48d58db0..0b8533a65d3a4 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7138,6 +7138,9 @@ const docTemplate = `{ "type": "string" } }, + "group_mapping": { + "type": "object" + }, "groups_field": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 913e02eb22c37..062716eabc969 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6392,6 +6392,9 @@ "type": "string" } }, + "group_mapping": { + "type": "object" + }, "groups_field": { "type": "string" }, diff --git a/coderd/coderd.go b/coderd/coderd.go index ac277d057bb71..a5c7b127d04b0 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -223,7 +223,12 @@ func New(options *Options) *API { options.SSHConfig.HostnamePrefix = "coder." } if options.SetUserGroups == nil { - options.SetUserGroups = func(context.Context, database.Store, uuid.UUID, []string) error { return nil } + options.SetUserGroups = func(ctx context.Context, _ database.Store, id uuid.UUID, groups []string) error { + options.Logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license", + slog.F("id", id), slog.F("groups", groups), + ) + return nil + } } if options.TemplateScheduleStore == nil { options.TemplateScheduleStore = schedule.NewAGPLTemplateScheduleStore() diff --git a/coderd/userauth.go b/coderd/userauth.go index 9418d384833cc..92a1aa9f82406 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -481,6 +481,10 @@ type OIDCConfig struct { // groups. If the group field is the empty string, then no group updates // will ever come from the OIDC provider. GroupField string + // GroupMapping controls how groups returned by the OIDC provider get mapped + // to groups within Coder. + // map[oidcGroupName]coderGroupName + GroupMapping map[string]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 @@ -651,6 +655,11 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { }) return } + + if mappedGroup, ok := api.OIDCConfig.GroupMapping[group]; ok { + group = mappedGroup + } + groups = append(groups, group) } } else { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index dba09776a87b1..d522e89da0645 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "flag" - "fmt" "math" "net/http" "os" @@ -199,7 +198,7 @@ func ParseSSHConfigOption(opt string) (key string, value string, err error) { return r == ' ' || r == '=' }) if idx == -1 { - return "", "", fmt.Errorf("invalid config-ssh option %q", opt) + return "", "", xerrors.Errorf("invalid config-ssh option %q", opt) } return opt[:idx], opt[idx+1:], nil } @@ -248,17 +247,18 @@ type OAuth2GithubConfig struct { } type OIDCConfig struct { - AllowSignups clibase.Bool `json:"allow_signups" typescript:",notnull"` - ClientID clibase.String `json:"client_id" typescript:",notnull"` - ClientSecret clibase.String `json:"client_secret" typescript:",notnull"` - EmailDomain clibase.Strings `json:"email_domain" typescript:",notnull"` - IssuerURL clibase.String `json:"issuer_url" typescript:",notnull"` - 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:"groups_field" typescript:",notnull"` - SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` - IconURL clibase.URL `json:"icon_url" typescript:",notnull"` + AllowSignups clibase.Bool `json:"allow_signups" typescript:",notnull"` + ClientID clibase.String `json:"client_id" typescript:",notnull"` + ClientSecret clibase.String `json:"client_secret" typescript:",notnull"` + EmailDomain clibase.Strings `json:"email_domain" typescript:",notnull"` + IssuerURL clibase.String `json:"issuer_url" typescript:",notnull"` + 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:"groups_field" typescript:",notnull"` + GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"` + SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` + IconURL clibase.URL `json:"icon_url" typescript:",notnull"` } type TelemetryConfig struct { @@ -875,6 +875,15 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "groupField", }, + { + Name: "OIDC Group Mapping", + Description: "A map of OIDC group IDs and the group in Coder it should map to. This is useful for when OIDC providers only return group IDs.", + Flag: "oidc-group-mapping", + Default: "{}", + Value: &c.OIDC.GroupMapping, + Group: &deploymentGroupOIDC, + YAML: "groupMapping", + }, { Name: "OpenID Connect sign in text", Description: "The text to show on the OpenID Connect sign in button", diff --git a/docs/admin/auth.md b/docs/admin/auth.md index fb632fffddb19..da070cf45f014 100644 --- a/docs/admin/auth.md +++ b/docs/admin/auth.md @@ -197,4 +197,20 @@ CODER_OIDC_SCOPES=openid,profile,email,groups On login, users will automatically be assigned to groups that have matching names in Coder and removed from groups that the user no longer belongs to. +For cases when an OIDC provider only returns group IDs ([Azure AD][azure-gids]) +or you want to have different group names in Coder than in your OIDC provider, +you can configure mapping between the two. + +```console +# as an environment variable +CODER_OIDC_GROUP_MAPPING='{"myOIDCGroupID": "myCoderGroupName"}' +# as a flag +--oidc-group-mapping '{"myOIDCGroupID": "myCoderGroupName"}' +``` + +From the example above, users that belong to the `myOIDCGroupID` group in your +OIDC provider will be added to the `myCoderGroupName` group in Coder. + > **Note:** Groups are only updated on login. + +[azure-gids]: https://github.com/MicrosoftDocs/azure-docs/issues/59766#issuecomment-664387195 diff --git a/docs/api/general.md b/docs/api/general.md index 0676876b4f277..4a61e71983e19 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -234,6 +234,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "group_mapping": {}, "groups_field": "string", "icon_url": { "forceQuery": true, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 441feb2201b44..65bf324ba27fb 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1766,6 +1766,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "group_mapping": {}, "groups_field": "string", "icon_url": { "forceQuery": true, @@ -2110,6 +2111,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "group_mapping": {}, "groups_field": "string", "icon_url": { "forceQuery": true, @@ -2771,6 +2773,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "client_id": "string", "client_secret": "string", "email_domain": ["string"], + "group_mapping": {}, "groups_field": "string", "icon_url": { "forceQuery": true, @@ -2801,6 +2804,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 | | | +| `group_mapping` | object | false | | | | `groups_field` | string | false | | | | `icon_url` | [clibase.URL](#clibaseurl) | false | | | | `ignore_email_verified` | boolean | false | | | diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 5e93b63611795..f9251cb69779e 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -67,6 +67,51 @@ func TestUserOIDC(t *testing.T) { require.NoError(t, err) require.Len(t, group.Members, 1) }) + t.Run("AssignsMapped", func(t *testing.T) { + t.Parallel() + + ctx, _ := testutil.Context(t) + conf := coderdtest.NewOIDCConfig(t, "") + + oidcGroupName := "pingpong" + coderGroupName := "bingbong" + + config := conf.OIDCConfig(t, jwt.MapClaims{}, func(cfg *coderd.OIDCConfig) { + cfg.GroupMapping = map[string]string{oidcGroupName: coderGroupName} + }) + config.AllowSignups = true + + client := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + OIDCConfig: config, + }, + }) + _ = coderdtest.CreateFirstUser(t, client) + coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + AllFeatures: true, + }) + + admin, err := client.User(ctx, "me") + require.NoError(t, err) + require.Len(t, admin.OrganizationIDs, 1) + + group, err := client.CreateGroup(ctx, admin.OrganizationIDs[0], codersdk.CreateGroupRequest{ + Name: coderGroupName, + }) + require.NoError(t, err) + require.Len(t, group.Members, 0) + + resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "colin@coder.com", + "groups": []string{oidcGroupName}, + })) + assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + group, err = client.Group(ctx, group.ID) + require.NoError(t, err) + require.Len(t, group.Members, 1) + }) + t.Run("AddThenRemove", func(t *testing.T) { t.Parallel() diff --git a/go.sum b/go.sum index 23c1364ed8c86..5c69b3e10b415 100644 --- a/go.sum +++ b/go.sum @@ -376,10 +376,6 @@ github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d h1:09JG37IgTB6n3ouX9 github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d/go.mod h1:r+1J5i/989wt6CUeNSuvFKKA9hHuKKPMxdzDbTuvwwk= github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko= github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914= -github.com/coder/tailscale v1.1.1-0.20230314023417-d9efcc0ac972 h1:193YGsJz8hc4yxqAclE36paKl+9CQ6KGLgdleIguCVE= -github.com/coder/tailscale v1.1.1-0.20230314023417-d9efcc0ac972/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA= -github.com/coder/tailscale v1.1.1-0.20230321164649-3362540e3026 h1:6YnWw08eQEGc/7KyweGWP8urOb9TDlo6S35ZqNm8qsQ= -github.com/coder/tailscale v1.1.1-0.20230321164649-3362540e3026/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA= github.com/coder/tailscale v1.1.1-0.20230321171725-fed359a0cafa h1:EjRGgTz7BUECmbV8jHTi1/rKdDjJESGSlm1Jp7evvCQ= github.com/coder/tailscale v1.1.1-0.20230321171725-fed359a0cafa/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA= github.com/coder/terraform-provider-coder v0.6.20 h1:bVyITX9JlbnGzKzTj0qi/JziUCGqD2DiN3cXaWyDcxE= diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4b31afb1f0ee0..6d25ab5fe475d 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -507,6 +507,9 @@ export interface OIDCConfig { readonly ignore_email_verified: boolean readonly username_field: string readonly groups_field: string + // Named type "github.com/coder/coder/cli/clibase.Struct[map[string]string]" unknown, using "any" + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type + readonly group_mapping: any readonly sign_in_text: string readonly icon_url: string } From 7109286c3e2e542fe3926c3b3af5d967e0335ad6 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Tue, 21 Mar 2023 13:50:42 -0500 Subject: [PATCH 2/2] fixup! feat: add group mapping option for group sync --- codersdk/deployment.go | 1 + 1 file changed, 1 insertion(+) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index d522e89da0645..c0d8959232bdc 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -879,6 +879,7 @@ when required by your organization's security policy.`, Name: "OIDC Group Mapping", Description: "A map of OIDC group IDs and the group in Coder it should map to. This is useful for when OIDC providers only return group IDs.", Flag: "oidc-group-mapping", + Env: "OIDC_GROUP_MAPPING", Default: "{}", Value: &c.OIDC.GroupMapping, Group: &deploymentGroupOIDC,