diff --git a/cli/server.go b/cli/server.go index 53c974f373cd4..6773b62f57832 100644 --- a/cli/server.go +++ b/cli/server.go @@ -55,6 +55,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" + "github.com/coder/coder/v2/coderd/entitlements" "github.com/coder/pretty" "github.com/coder/quartz" "github.com/coder/retry" @@ -605,6 +606,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. SSHConfigOptions: configSSHOptions, }, AllowWorkspaceRenames: vals.AllowWorkspaceRenames.Value(), + Entitlements: entitlements.New(), NotificationsEnqueuer: notifications.NewNoopEnqueuer(), // Changed further down if notifications enabled. } if httpServers.TLSConfig != nil { diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 54e9eeffc1248..d7bc620b32517 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -433,6 +433,11 @@ OIDC OPTIONS: groups. This filter is applied after the group mapping and before the regex filter. + --oidc-organization-assign-default bool, $CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT (default: true) + If set to true, users will always be added to the default + organization. If organization sync is enabled, then the default org is + always added to the user's set of expectedorganizations. + --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. @@ -479,6 +484,14 @@ OIDC OPTIONS: --oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name) OIDC claim field to use as the name. + --oidc-organization-field string, $CODER_OIDC_ORGANIZATION_FIELD + This field must be set if using the organization sync feature. Set to + the claim to be used for organizations. + + --oidc-organization-mapping struct[map[string][]uuid.UUID], $CODER_OIDC_ORGANIZATION_MAPPING (default: {}) + A map of OIDC claims and the organizations in Coder it should map to. + This is required because organization IDs must be used within Coder. + --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index b050b662c56fa..21308e00cca60 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -319,6 +319,19 @@ oidc: # Ignore the userinfo endpoint and only use the ID token for user information. # (default: false, type: bool) ignoreUserInfo: false + # This field must be set if using the organization sync feature. Set to the claim + # to be used for organizations. + # (default: , type: string) + organizationField: "" + # If set to true, users will always be added to the default organization. If + # organization sync is enabled, then the default org is always added to the user's + # set of expectedorganizations. + # (default: true, type: bool) + organizationAssignDefault: true + # A map of OIDC claims and the organizations in Coder it should map to. This is + # required because organization IDs must be used within Coder. + # (default: {}, type: struct[map[string][]uuid.UUID]) + organizationMapping: {} # This field must be set if using the group sync feature and the scope name is not # 'groups'. Set to the claim to be used for groups. # (default: , type: string) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 08447abb22c50..23f465ce27d69 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11077,6 +11077,15 @@ const docTemplate = `{ "name_field": { "type": "string" }, + "organization_assign_default": { + "type": "boolean" + }, + "organization_field": { + "type": "string" + }, + "organization_mapping": { + "type": "object" + }, "scopes": { "type": "array", "items": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 11126e672609a..6cd8d84a38eb7 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9972,6 +9972,15 @@ "name_field": { "type": "string" }, + "organization_assign_default": { + "type": "boolean" + }, + "organization_field": { + "type": "string" + }, + "organization_mapping": { + "type": "object" + }, "scopes": { "type": "array", "items": { diff --git a/coderd/coderd.go b/coderd/coderd.go index 939a7141e6df5..20ce616eab5ba 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -38,6 +38,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/entitlements" + "github.com/coder/coder/v2/coderd/idpsync" "github.com/coder/quartz" "github.com/coder/serpent" @@ -243,6 +244,9 @@ type Options struct { WorkspaceUsageTracker *workspacestats.UsageTracker // NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc. NotificationsEnqueuer notifications.Enqueuer + + // IDPSync holds all configured values for syncing external IDP users into Coder. + IDPSync idpsync.IDPSync } // @title Coder API @@ -270,6 +274,13 @@ func New(options *Options) *API { if options.Entitlements == nil { options.Entitlements = entitlements.New() } + if options.IDPSync == nil { + options.IDPSync = idpsync.NewAGPLSync(options.Logger, idpsync.SyncSettings{ + OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(), + OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value, + OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(), + }) + } if options.NewTicker == nil { options.NewTicker = func(duration time.Duration) (tick <-chan time.Time, done func()) { ticker := time.NewTicker(duration) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 8dd903a2a9137..2da1462e8af00 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -243,7 +243,7 @@ var ( rbac.ResourceAssignOrgRole.Type: rbac.ResourceAssignOrgRole.AvailableActions(), rbac.ResourceSystem.Type: {policy.WildcardSymbol}, rbac.ResourceOrganization.Type: {policy.ActionCreate, policy.ActionRead}, - rbac.ResourceOrganizationMember.Type: {policy.ActionCreate}, + rbac.ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionDelete, policy.ActionRead}, rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionUpdate}, rbac.ResourceProvisionerKeys.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionDelete}, rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(), diff --git a/coderd/idpsync/idpsync.go b/coderd/idpsync/idpsync.go new file mode 100644 index 0000000000000..73a7b9b6f530d --- /dev/null +++ b/coderd/idpsync/idpsync.go @@ -0,0 +1,172 @@ +package idpsync + +import ( + "context" + "net/http" + "strings" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/site" +) + +// IDPSync is an interface, so we can implement this as AGPL and as enterprise, +// and just swap the underlying implementation. +// IDPSync exists to contain all the logic for mapping a user's external IDP +// claims to the internal representation of a user in Coder. +// TODO: Move group + role sync into this interface. +type IDPSync interface { + OrganizationSyncEnabled() bool + // ParseOrganizationClaims takes claims from an OIDC provider, and returns the + // organization sync params for assigning users into organizations. + ParseOrganizationClaims(ctx context.Context, _ jwt.MapClaims) (OrganizationParams, *HTTPError) + // SyncOrganizations assigns and removed users from organizations based on the + // provided params. + SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error +} + +// AGPLIDPSync is the configuration for syncing user information from an external +// IDP. All related code to syncing user information should be in this package. +type AGPLIDPSync struct { + Logger slog.Logger + + SyncSettings +} + +type SyncSettings struct { + // OrganizationField selects the claim field to be used as the created user's + // organizations. If the field is the empty string, then no organization updates + // will ever come from the OIDC provider. + OrganizationField string + // OrganizationMapping controls how organizations returned by the OIDC provider get mapped + OrganizationMapping map[string][]uuid.UUID + // OrganizationAssignDefault will ensure all users that authenticate will be + // placed into the default organization. This is mostly a hack to support + // legacy deployments. + OrganizationAssignDefault bool +} + +type OrganizationParams struct { + // SyncEnabled if false will skip syncing the user's organizations. + SyncEnabled bool + // IncludeDefault is primarily for single org deployments. It will ensure + // a user is always inserted into the default org. + IncludeDefault bool + // Organizations is the list of organizations the user should be a member of + // assuming syncing is turned on. + Organizations []uuid.UUID +} + +func NewAGPLSync(logger slog.Logger, settings SyncSettings) *AGPLIDPSync { + return &AGPLIDPSync{ + Logger: logger.Named("idp-sync"), + SyncSettings: settings, + } +} + +// ParseStringSliceClaim parses the claim for groups and roles, expected []string. +// +// Some providers like ADFS return a single string instead of an array if there +// is only 1 element. So this function handles the edge cases. +func ParseStringSliceClaim(claim interface{}) ([]string, error) { + groups := make([]string, 0) + if claim == nil { + return groups, nil + } + + // The simple case is the type is exactly what we expected + asStringArray, ok := claim.([]string) + if ok { + return asStringArray, nil + } + + asArray, ok := claim.([]interface{}) + if ok { + for i, item := range asArray { + asString, ok := item.(string) + if !ok { + return nil, xerrors.Errorf("invalid claim type. Element %d expected a string, got: %T", i, item) + } + groups = append(groups, asString) + } + return groups, nil + } + + asString, ok := claim.(string) + if ok { + if asString == "" { + // Empty string should be 0 groups. + return []string{}, nil + } + // If it is a single string, first check if it is a csv. + // If a user hits this, it is likely a misconfiguration and they need + // to reconfigure their IDP to send an array instead. + if strings.Contains(asString, ",") { + return nil, xerrors.Errorf("invalid claim type. Got a csv string (%q), change this claim to return an array of strings instead.", asString) + } + return []string{asString}, nil + } + + // Not sure what the user gave us. + return nil, xerrors.Errorf("invalid claim type. Expected an array of strings, got: %T", claim) +} + +// IsHTTPError handles us being inconsistent with returning errors as values or +// pointers. +func IsHTTPError(err error) *HTTPError { + var httpErr HTTPError + if xerrors.As(err, &httpErr) { + return &httpErr + } + + var httpErrPtr *HTTPError + if xerrors.As(err, &httpErrPtr) { + return httpErrPtr + } + return nil +} + +// HTTPError is a helper struct for returning errors from the IDP sync process. +// A regular error is not sufficient because many of these errors are surfaced +// to a user logging in, and the errors should be descriptive. +type HTTPError struct { + Code int + Msg string + Detail string + RenderStaticPage bool + RenderDetailMarkdown bool +} + +func (e HTTPError) Write(rw http.ResponseWriter, r *http.Request) { + if e.RenderStaticPage { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: e.Code, + HideStatus: true, + Title: e.Msg, + Description: e.Detail, + RetryEnabled: false, + DashboardURL: "/login", + + RenderDescriptionMarkdown: e.RenderDetailMarkdown, + }) + return + } + httpapi.Write(r.Context(), rw, e.Code, codersdk.Response{ + Message: e.Msg, + Detail: e.Detail, + }) +} + +func (e HTTPError) Error() string { + if e.Detail != "" { + return e.Detail + } + + return e.Msg +} diff --git a/coderd/userauth_internal_test.go b/coderd/idpsync/idpsync_test.go similarity index 89% rename from coderd/userauth_internal_test.go rename to coderd/idpsync/idpsync_test.go index 421e654995fdf..7dc29d903af3f 100644 --- a/coderd/userauth_internal_test.go +++ b/coderd/idpsync/idpsync_test.go @@ -1,10 +1,12 @@ -package coderd +package idpsync_test import ( "encoding/json" "testing" "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/idpsync" ) func TestParseStringSliceClaim(t *testing.T) { @@ -123,7 +125,7 @@ func TestParseStringSliceClaim(t *testing.T) { require.NoError(t, err, "unmarshal json claim") } - found, err := parseStringSliceClaim(c.GoClaim) + found, err := idpsync.ParseStringSliceClaim(c.GoClaim) if c.ErrorExpected { require.Error(t, err) } else { @@ -133,3 +135,13 @@ func TestParseStringSliceClaim(t *testing.T) { }) } } + +func TestIsHTTPError(t *testing.T) { + t.Parallel() + + herr := idpsync.HTTPError{} + require.NotNil(t, idpsync.IsHTTPError(herr)) + require.NotNil(t, idpsync.IsHTTPError(&herr)) + + require.Nil(t, error(nil)) +} diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go new file mode 100644 index 0000000000000..6d475f28ea0ef --- /dev/null +++ b/coderd/idpsync/organization.go @@ -0,0 +1,104 @@ +package idpsync + +import ( + "context" + "database/sql" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/util/slice" +) + +func (AGPLIDPSync) OrganizationSyncEnabled() bool { + // AGPL does not support syncing organizations. + return false +} + +func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) (OrganizationParams, *HTTPError) { + // For AGPL we only sync the default organization. + return OrganizationParams{ + SyncEnabled: s.OrganizationSyncEnabled(), + IncludeDefault: s.OrganizationAssignDefault, + Organizations: []uuid.UUID{}, + }, nil +} + +// SyncOrganizations if enabled will ensure the user is a member of the provided +// organizations. It will add and remove their membership to match the expected set. +func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error { + // Nothing happens if sync is not enabled + if !params.SyncEnabled { + return nil + } + + // nolint:gocritic // all syncing is done as a system user + ctx = dbauthz.AsSystemRestricted(ctx) + + // This is a bit hacky, but if AssignDefault is included, then always + // make sure to include the default org in the list of expected. + if s.OrganizationAssignDefault { + defaultOrg, err := tx.GetDefaultOrganization(ctx) + if err != nil { + return xerrors.Errorf("failed to get default organization: %w", err) + } + params.Organizations = append(params.Organizations, defaultOrg.ID) + } + + existingOrgs, err := tx.GetOrganizationsByUserID(ctx, user.ID) + if err != nil { + return xerrors.Errorf("failed to get user organizations: %w", err) + } + + existingOrgIDs := db2sdk.List(existingOrgs, func(org database.Organization) uuid.UUID { + return org.ID + }) + + // Find the difference in the expected and the existing orgs, and + // correct the set of orgs the user is a member of. + add, remove := slice.SymmetricDifference(existingOrgIDs, params.Organizations) + notExists := make([]uuid.UUID, 0) + for _, orgID := range add { + //nolint:gocritic // System actor being used to assign orgs + _, err := tx.InsertOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.InsertOrganizationMemberParams{ + OrganizationID: orgID, + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + Roles: []string{}, + }) + if err != nil { + if xerrors.Is(err, sql.ErrNoRows) { + notExists = append(notExists, orgID) + continue + } + return xerrors.Errorf("add user to organization: %w", err) + } + } + + for _, orgID := range remove { + //nolint:gocritic // System actor being used to assign orgs + err := tx.DeleteOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.DeleteOrganizationMemberParams{ + OrganizationID: orgID, + UserID: user.ID, + }) + if err != nil { + return xerrors.Errorf("remove user from organization: %w", err) + } + } + + if len(notExists) > 0 { + s.Logger.Debug(ctx, "organizations do not exist but attempted to use in org sync", + slog.F("not_found", notExists), + slog.F("user_id", user.ID), + slog.F("username", user.Username), + ) + } + return nil +} diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go new file mode 100644 index 0000000000000..03b1ebfa4b27b --- /dev/null +++ b/coderd/idpsync/organizations_test.go @@ -0,0 +1,58 @@ +package idpsync_test + +import ( + "testing" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/idpsync" + "github.com/coder/coder/v2/testutil" +) + +func TestParseOrganizationClaims(t *testing.T) { + t.Parallel() + + t.Run("SingleOrgDeployment", func(t *testing.T) { + t.Parallel() + + s := idpsync.NewAGPLSync(slogtest.Make(t, &slogtest.Options{}), idpsync.SyncSettings{ + OrganizationField: "", + OrganizationMapping: nil, + OrganizationAssignDefault: true, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + + params, err := s.ParseOrganizationClaims(ctx, jwt.MapClaims{}) + require.Nil(t, err) + + require.Empty(t, params.Organizations) + require.True(t, params.IncludeDefault) + require.False(t, params.SyncEnabled) + }) + + t.Run("AGPL", func(t *testing.T) { + t.Parallel() + + // AGPL has limited behavior + s := idpsync.NewAGPLSync(slogtest.Make(t, &slogtest.Options{}), idpsync.SyncSettings{ + OrganizationField: "orgs", + OrganizationMapping: map[string][]uuid.UUID{ + "random": {uuid.New()}, + }, + OrganizationAssignDefault: false, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + + params, err := s.ParseOrganizationClaims(ctx, jwt.MapClaims{}) + require.Nil(t, err) + + require.Empty(t, params.Organizations) + require.False(t, params.IncludeDefault) + require.False(t, params.SyncEnabled) + }) +} diff --git a/coderd/members.go b/coderd/members.go index 805bdafbd0447..6f5c0c5864f08 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -2,6 +2,7 @@ package coderd import ( "context" + "fmt" "net/http" "github.com/google/uuid" @@ -43,6 +44,14 @@ func (api *API) postOrganizationMember(rw http.ResponseWriter, r *http.Request) aReq.Old = database.AuditableOrganizationMember{} defer commitAudit() + if user.LoginType == database.LoginTypeOIDC && api.IDPSync.OrganizationSyncEnabled() { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Organization sync is enabled for OIDC users, meaning manual organization assignment is not allowed for this user.", + Detail: fmt.Sprintf("User %s is an OIDC user and organization sync is enabled. Ask an administrator to resolve this in your external IDP.", user.ID), + }) + return + } + member, err := api.Database.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{ OrganizationID: organization.ID, UserID: user.ID, diff --git a/coderd/userauth.go b/coderd/userauth.go index 1a5488d2d6ded..9c72af249cf3a 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -25,6 +25,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/idpsync" "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" @@ -40,7 +41,6 @@ import ( "github.com/coder/coder/v2/coderd/userpassword" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" - "github.com/coder/coder/v2/site" ) const ( @@ -659,17 +659,21 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { AvatarURL: ghUser.GetAvatarURL(), Name: normName, DebugContext: OauthDebugContext{}, + OrganizationSync: idpsync.OrganizationParams{ + SyncEnabled: false, + IncludeDefault: true, + Organizations: []uuid.UUID{}, + }, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) }) cookies, user, key, err := api.oauthLogin(r, params) defer params.CommitAuditLogs() - var httpErr httpError - if xerrors.As(err, &httpErr) { - httpErr.Write(rw, r) - return - } if err != nil { + if httpErr := idpsync.IsHTTPError(err); httpErr != nil { + httpErr.Write(rw, r) + return + } logger.Error(ctx, "oauth2: login failed", slog.F("user", user.Username), slog.Error(err)) httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to process OAuth login.", @@ -737,6 +741,8 @@ type OIDCConfig struct { // support the userinfo endpoint, or if the userinfo endpoint causes // undesirable behavior. IgnoreUserInfo bool + + // TODO: Move all idp fields into the IDPSync struct // 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. @@ -1020,6 +1026,12 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { return } + orgSync, orgSyncErr := api.IDPSync.ParseOrganizationClaims(ctx, mergedClaims) + if orgSyncErr != nil { + orgSyncErr.Write(rw, r) + return + } + // If a new user is authenticating for the first time // the audit action is 'register', not 'login' if user.ID == uuid.Nil { @@ -1041,6 +1053,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { Roles: roles, UsingGroups: usingGroups, Groups: groups, + OrganizationSync: orgSync, CreateMissingGroups: api.OIDCConfig.CreateMissingGroups, GroupFilter: api.OIDCConfig.GroupFilter, DebugContext: OauthDebugContext{ @@ -1052,12 +1065,11 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { }) cookies, user, key, err := api.oauthLogin(r, params) defer params.CommitAuditLogs() - var httpErr httpError - if xerrors.As(err, &httpErr) { - httpErr.Write(rw, r) - return - } if err != nil { + if hErr := idpsync.IsHTTPError(err); hErr != nil { + hErr.Write(rw, r) + return + } logger.Error(ctx, "oauth2: login failed", slog.F("user", user.Username), slog.Error(err)) httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to process OAuth login.", @@ -1080,7 +1092,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } // oidcGroups returns the groups for the user from the OIDC claims. -func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interface{}) (bool, []string, *httpError) { +func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interface{}) (bool, []string, *idpsync.HTTPError) { logger := api.Logger.Named(userAuthLoggerName) usingGroups := false var groups []string @@ -1095,17 +1107,17 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac usingGroups = true groupsRaw, ok := mergedClaims[api.OIDCConfig.GroupField] if ok { - parsedGroups, err := parseStringSliceClaim(groupsRaw) + parsedGroups, err := idpsync.ParseStringSliceClaim(groupsRaw) if err != nil { api.Logger.Debug(ctx, "groups field was an unknown type in oidc claims", slog.F("type", fmt.Sprintf("%T", groupsRaw)), slog.Error(err), ) - return false, nil, &httpError{ - code: http.StatusBadRequest, - msg: "Failed to sync groups from OIDC claims", - detail: err.Error(), - renderStaticPage: false, + return false, nil, &idpsync.HTTPError{ + Code: http.StatusBadRequest, + Msg: "Failed to sync groups from OIDC claims", + Detail: err.Error(), + RenderStaticPage: false, } } @@ -1134,11 +1146,11 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac 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, + return usingGroups, groups, &idpsync.HTTPError{ + Code: http.StatusForbidden, + Msg: "Not a member of an allowed group", + Detail: detail, + RenderStaticPage: true, } } } @@ -1158,7 +1170,7 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac // It would be preferred to just return an error, however this function // decorates returned errors with the appropriate HTTP status codes and details // that are hard to carry in a standard `error` without more work. -func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface{}) ([]string, *httpError) { +func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface{}) ([]string, *idpsync.HTTPError) { roles := api.OIDCConfig.UserRolesDefault if !api.OIDCConfig.RoleSyncEnabled() { return roles, nil @@ -1174,17 +1186,17 @@ func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface rolesRow = []interface{}{} } - parsedRoles, err := parseStringSliceClaim(rolesRow) + parsedRoles, err := idpsync.ParseStringSliceClaim(rolesRow) if err != nil { api.Logger.Error(ctx, "oidc claims user roles field was an unknown type", slog.F("type", fmt.Sprintf("%T", rolesRow)), slog.Error(err), ) - return nil, &httpError{ - code: http.StatusInternalServerError, - msg: "Login disabled until OIDC config is fixed", - detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow), - renderStaticPage: false, + return nil, &idpsync.HTTPError{ + Code: http.StatusInternalServerError, + Msg: "Login disabled until OIDC config is fixed", + Detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow), + RenderStaticPage: false, } } @@ -1264,6 +1276,8 @@ type oauthLoginParams struct { Username string Name string AvatarURL string + // OrganizationSync has the organizations that the user will be assigned to. + OrganizationSync idpsync.OrganizationParams // Is UsingGroups is true, then the user will be assigned // to the Groups provided. UsingGroups bool @@ -1303,43 +1317,6 @@ func (p *oauthLoginParams) CommitAuditLogs() { } } -type httpError struct { - code int - msg string - detail string - renderStaticPage bool - - renderDetailMarkdown bool -} - -func (e httpError) Write(rw http.ResponseWriter, r *http.Request) { - if e.renderStaticPage { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: e.code, - HideStatus: true, - Title: e.msg, - Description: e.detail, - RetryEnabled: false, - DashboardURL: "/login", - - RenderDescriptionMarkdown: e.renderDetailMarkdown, - }) - return - } - httpapi.Write(r.Context(), rw, e.code, codersdk.Response{ - Message: e.msg, - Detail: e.detail, - }) -} - -func (e httpError) Error() string { - if e.detail != "" { - return e.detail - } - - return e.msg -} - func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.Cookie, database.User, database.APIKey, error) { var ( ctx = r.Context() @@ -1376,13 +1353,12 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C if api.OIDCConfig != nil && api.OIDCConfig.SignupsDisabledText != "" { signupsDisabledText = render.HTMLFromMarkdown(api.OIDCConfig.SignupsDisabledText) } - return httpError{ - code: http.StatusForbidden, - msg: "Signups are disabled", - detail: signupsDisabledText, - renderStaticPage: true, - - renderDetailMarkdown: true, + return &idpsync.HTTPError{ + Code: http.StatusForbidden, + Msg: "Signups are disabled", + Detail: signupsDisabledText, + RenderStaticPage: true, + RenderDetailMarkdown: true, } } @@ -1428,19 +1404,26 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } } if !validUsername { - return httpError{ - code: http.StatusConflict, - msg: fmt.Sprintf("exhausted alternatives for taken username %q", original), + return &idpsync.HTTPError{ + Code: http.StatusConflict, + Msg: fmt.Sprintf("exhausted alternatives for taken username %q", original), } } } + // Even if org sync is disabled, single org deployments will always + // have this set to true. + orgIDs := []uuid.UUID{} + if params.OrganizationSync.IncludeDefault { + orgIDs = append(orgIDs, defaultOrganization.ID) + } + //nolint:gocritic user, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{ CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{ Email: params.Email, Username: params.Username, - OrganizationIDs: []uuid.UUID{defaultOrganization.ID}, + OrganizationIDs: orgIDs, }, LoginType: params.LoginType, }) @@ -1503,6 +1486,11 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } } + err = api.IDPSync.SyncOrganizations(ctx, tx, user, params.OrganizationSync) + if err != nil { + return xerrors.Errorf("sync organizations: %w", err) + } + // Ensure groups are correct. // This places all groups into the default organization. // To go multi-org, we need to add a mapping feature here to know which @@ -1569,11 +1557,11 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C //nolint:gocritic err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), logger, tx, user.ID, filtered) if err != nil { - return httpError{ - code: http.StatusBadRequest, - msg: "Invalid roles through OIDC claims", - detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()), - renderStaticPage: true, + return &idpsync.HTTPError{ + Code: http.StatusBadRequest, + Msg: "Invalid roles through OIDC claims", + Detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()), + RenderStaticPage: true, } } if len(ignored) > 0 { @@ -1684,17 +1672,17 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data // Trying to convert to OIDC, but the email does not match. // So do not make a new user, just block the request. if user.ID == uuid.Nil { - return database.User{}, httpError{ - code: http.StatusBadRequest, - msg: fmt.Sprintf("The oidc account with the email %q does not match the email of the account you are trying to convert. Contact your administrator to resolve this issue.", params.Email), + return database.User{}, idpsync.HTTPError{ + Code: http.StatusBadRequest, + Msg: fmt.Sprintf("The oidc account with the email %q does not match the email of the account you are trying to convert. Contact your administrator to resolve this issue.", params.Email), } } jwtCookie, err := r.Cookie(OAuthConvertCookieValue) if err != nil { - return database.User{}, httpError{ - code: http.StatusBadRequest, - msg: fmt.Sprintf("Convert to oauth cookie not found. Missing signed jwt to authorize this action. " + + return database.User{}, idpsync.HTTPError{ + Code: http.StatusBadRequest, + Msg: fmt.Sprintf("Convert to oauth cookie not found. Missing signed jwt to authorize this action. " + "Please try again."), } } @@ -1704,15 +1692,15 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data }) if xerrors.Is(err, jwt.ErrSignatureInvalid) || !token.Valid { // These errors are probably because the user is mixing 2 coder deployments. - return database.User{}, httpError{ - code: http.StatusBadRequest, - msg: "Using an invalid jwt to authorize this action. Ensure there is only 1 coder deployment and try again.", + return database.User{}, idpsync.HTTPError{ + Code: http.StatusBadRequest, + Msg: "Using an invalid jwt to authorize this action. Ensure there is only 1 coder deployment and try again.", } } if err != nil { - return database.User{}, httpError{ - code: http.StatusInternalServerError, - msg: fmt.Sprintf("Error parsing jwt: %v", err), + return database.User{}, idpsync.HTTPError{ + Code: http.StatusInternalServerError, + Msg: fmt.Sprintf("Error parsing jwt: %v", err), } } @@ -1732,16 +1720,16 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data oauthConvertAudit.Old = user if claims.RegisteredClaims.Issuer != api.DeploymentID { - return database.User{}, httpError{ - code: http.StatusForbidden, - msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.", + return database.User{}, idpsync.HTTPError{ + Code: http.StatusForbidden, + Msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.", } } if params.State.StateString != claims.State { - return database.User{}, httpError{ - code: http.StatusForbidden, - msg: "Request to convert login type failed. State mismatch.", + return database.User{}, idpsync.HTTPError{ + Code: http.StatusForbidden, + Msg: "Request to convert login type failed. State mismatch.", } } @@ -1751,9 +1739,9 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data if user.ID != claims.UserID || codersdk.LoginType(user.LoginType) != claims.FromLoginType || codersdk.LoginType(params.LoginType) != claims.ToLoginType { - return database.User{}, httpError{ - code: http.StatusForbidden, - msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType), + return database.User{}, idpsync.HTTPError{ + Code: http.StatusForbidden, + Msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType), } } @@ -1767,9 +1755,9 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data UserID: user.ID, }) if err != nil { - return database.User{}, httpError{ - code: http.StatusInternalServerError, - msg: "Failed to convert user to new login type", + return database.User{}, idpsync.HTTPError{ + Code: http.StatusInternalServerError, + Msg: "Failed to convert user to new login type", } } oauthConvertAudit.New = user @@ -1855,63 +1843,16 @@ func clearOAuthConvertCookie() *http.Cookie { } } -func wrongLoginTypeHTTPError(user database.LoginType, params database.LoginType) httpError { +func wrongLoginTypeHTTPError(user database.LoginType, params database.LoginType) idpsync.HTTPError { addedMsg := "" if user == database.LoginTypePassword { addedMsg = " You can convert your account to use this login type by visiting your account settings." } - return httpError{ - code: http.StatusForbidden, - renderStaticPage: true, - msg: "Incorrect login type", - detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", + return idpsync.HTTPError{ + Code: http.StatusForbidden, + RenderStaticPage: true, + Msg: "Incorrect login type", + Detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", params, user, addedMsg), } } - -// parseStringSliceClaim parses the claim for groups and roles, expected []string. -// -// Some providers like ADFS return a single string instead of an array if there -// is only 1 element. So this function handles the edge cases. -func parseStringSliceClaim(claim interface{}) ([]string, error) { - groups := make([]string, 0) - if claim == nil { - return groups, nil - } - - // The simple case is the type is exactly what we expected - asStringArray, ok := claim.([]string) - if ok { - return asStringArray, nil - } - - asArray, ok := claim.([]interface{}) - if ok { - for i, item := range asArray { - asString, ok := item.(string) - if !ok { - return nil, xerrors.Errorf("invalid claim type. Element %d expected a string, got: %T", i, item) - } - groups = append(groups, asString) - } - return groups, nil - } - - asString, ok := claim.(string) - if ok { - if asString == "" { - // Empty string should be 0 groups. - return []string{}, nil - } - // If it is a single string, first check if it is a csv. - // If a user hits this, it is likely a misconfiguration and they need - // to reconfigure their IDP to send an array instead. - if strings.Contains(asString, ",") { - return nil, xerrors.Errorf("invalid claim type. Got a csv string (%q), change this claim to return an array of strings instead.", asString) - } - return []string{asString}, nil - } - - // Not sure what the user gave us. - return nil, xerrors.Errorf("invalid claim type. Expected an array of strings, got: %T", claim) -} diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 8603cfcfb439a..8e1f07e24d1b8 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -366,6 +366,7 @@ func TestUserOAuth2Github(t *testing.T) { require.Equal(t, "kyle", user.Username) require.Equal(t, "Kylium Carbonate", user.Name) require.Equal(t, "/hello-world", user.AvatarURL) + require.Equal(t, 1, len(user.OrganizationIDs), "in the default org") require.Len(t, auditor.AuditLogs(), numLogs) require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil) @@ -419,6 +420,7 @@ func TestUserOAuth2Github(t *testing.T) { require.Equal(t, "kyle", user.Username) require.Equal(t, strings.Repeat("a", 128), user.Name) require.Equal(t, "/hello-world", user.AvatarURL) + require.Equal(t, 1, len(user.OrganizationIDs), "in the default org") require.Len(t, auditor.AuditLogs(), numLogs) require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil) @@ -474,6 +476,7 @@ func TestUserOAuth2Github(t *testing.T) { require.Equal(t, "kyle", user.Username) require.Equal(t, "Kylium Carbonate", user.Name) require.Equal(t, "/hello-world", user.AvatarURL) + require.Equal(t, 1, len(user.OrganizationIDs), "in the default org") require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) @@ -536,6 +539,7 @@ func TestUserOAuth2Github(t *testing.T) { require.Equal(t, "mathias@coder.com", user.Email) require.Equal(t, "mathias", user.Username) require.Equal(t, "Mathias Mathias", user.Name) + require.Equal(t, 1, len(user.OrganizationIDs), "in the default org") require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) @@ -598,6 +602,7 @@ func TestUserOAuth2Github(t *testing.T) { require.Equal(t, "mathias@coder.com", user.Email) require.Equal(t, "mathias", user.Username) require.Equal(t, "Mathias Mathias", user.Name) + require.Equal(t, 1, len(user.OrganizationIDs), "in the default org") require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) @@ -1270,6 +1275,7 @@ func TestUserOIDC(t *testing.T) { require.Len(t, auditor.AuditLogs(), numLogs) require.NotEqual(t, uuid.Nil, auditor.AuditLogs()[numLogs-1].UserID) require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) + require.Equal(t, 1, len(user.OrganizationIDs), "in the default org") } }) } diff --git a/coderd/users.go b/coderd/users.go index 3ae2d916e1de8..38dcc1bd82f6e 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1294,10 +1294,6 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create var user database.User err := store.InTx(func(tx database.Store) error { orgRoles := make([]string, 0) - // Organization is required to know where to allocate the user. - if len(req.OrganizationIDs) == 0 { - return xerrors.Errorf("organization ID must be provided") - } params := database.InsertUserParams{ ID: uuid.New(), diff --git a/codersdk/deployment.go b/codersdk/deployment.go index d7c1ddce4fec9..c0acf9ca01e6e 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/google/uuid" "golang.org/x/mod/semver" "golang.org/x/xerrors" @@ -512,29 +513,32 @@ type OIDCConfig struct { ClientID serpent.String `json:"client_id" typescript:",notnull"` ClientSecret serpent.String `json:"client_secret" typescript:",notnull"` // ClientKeyFile & ClientCertFile are used in place of ClientSecret for PKI auth. - ClientKeyFile serpent.String `json:"client_key_file" typescript:",notnull"` - ClientCertFile serpent.String `json:"client_cert_file" typescript:",notnull"` - EmailDomain serpent.StringArray `json:"email_domain" typescript:",notnull"` - IssuerURL serpent.String `json:"issuer_url" typescript:",notnull"` - Scopes serpent.StringArray `json:"scopes" typescript:",notnull"` - IgnoreEmailVerified serpent.Bool `json:"ignore_email_verified" typescript:",notnull"` - UsernameField serpent.String `json:"username_field" typescript:",notnull"` - NameField serpent.String `json:"name_field" typescript:",notnull"` - EmailField serpent.String `json:"email_field" typescript:",notnull"` - AuthURLParams serpent.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"` - IgnoreUserInfo serpent.Bool `json:"ignore_user_info" typescript:",notnull"` - GroupAutoCreate serpent.Bool `json:"group_auto_create" typescript:",notnull"` - GroupRegexFilter serpent.Regexp `json:"group_regex_filter" typescript:",notnull"` - GroupAllowList serpent.StringArray `json:"group_allow_list" typescript:",notnull"` - GroupField serpent.String `json:"groups_field" typescript:",notnull"` - GroupMapping serpent.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"` - UserRoleField serpent.String `json:"user_role_field" typescript:",notnull"` - UserRoleMapping serpent.Struct[map[string][]string] `json:"user_role_mapping" typescript:",notnull"` - UserRolesDefault serpent.StringArray `json:"user_roles_default" typescript:",notnull"` - SignInText serpent.String `json:"sign_in_text" typescript:",notnull"` - IconURL serpent.URL `json:"icon_url" typescript:",notnull"` - SignupsDisabledText serpent.String `json:"signups_disabled_text" typescript:",notnull"` - SkipIssuerChecks serpent.Bool `json:"skip_issuer_checks" typescript:",notnull"` + ClientKeyFile serpent.String `json:"client_key_file" typescript:",notnull"` + ClientCertFile serpent.String `json:"client_cert_file" typescript:",notnull"` + EmailDomain serpent.StringArray `json:"email_domain" typescript:",notnull"` + IssuerURL serpent.String `json:"issuer_url" typescript:",notnull"` + Scopes serpent.StringArray `json:"scopes" typescript:",notnull"` + IgnoreEmailVerified serpent.Bool `json:"ignore_email_verified" typescript:",notnull"` + UsernameField serpent.String `json:"username_field" typescript:",notnull"` + NameField serpent.String `json:"name_field" typescript:",notnull"` + EmailField serpent.String `json:"email_field" typescript:",notnull"` + AuthURLParams serpent.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"` + IgnoreUserInfo serpent.Bool `json:"ignore_user_info" typescript:",notnull"` + OrganizationField serpent.String `json:"organization_field" typescript:",notnull"` + OrganizationMapping serpent.Struct[map[string][]uuid.UUID] `json:"organization_mapping" typescript:",notnull"` + OrganizationAssignDefault serpent.Bool `json:"organization_assign_default" typescript:",notnull"` + GroupAutoCreate serpent.Bool `json:"group_auto_create" typescript:",notnull"` + GroupRegexFilter serpent.Regexp `json:"group_regex_filter" typescript:",notnull"` + GroupAllowList serpent.StringArray `json:"group_allow_list" typescript:",notnull"` + GroupField serpent.String `json:"groups_field" typescript:",notnull"` + GroupMapping serpent.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"` + UserRoleField serpent.String `json:"user_role_field" typescript:",notnull"` + UserRoleMapping serpent.Struct[map[string][]string] `json:"user_role_mapping" typescript:",notnull"` + UserRolesDefault serpent.StringArray `json:"user_roles_default" typescript:",notnull"` + SignInText serpent.String `json:"sign_in_text" typescript:",notnull"` + IconURL serpent.URL `json:"icon_url" typescript:",notnull"` + SignupsDisabledText serpent.String `json:"signups_disabled_text" typescript:",notnull"` + SkipIssuerChecks serpent.Bool `json:"skip_issuer_checks" typescript:",notnull"` } type TelemetryConfig struct { @@ -1541,6 +1545,42 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "ignoreUserInfo", }, + { + Name: "OIDC Organization Field", + Description: "This field must be set if using the organization sync feature." + + " Set to the claim to be used for organizations.", + Flag: "oidc-organization-field", + Env: "CODER_OIDC_ORGANIZATION_FIELD", + // Empty value means sync is disabled + Default: "", + Value: &c.OIDC.OrganizationField, + Group: &deploymentGroupOIDC, + YAML: "organizationField", + }, + { + Name: "OIDC Assign Default Organization", + Description: "If set to true, users will always be added to the default organization. " + + "If organization sync is enabled, then the default org is always added to the user's set of expected" + + "organizations.", + Flag: "oidc-organization-assign-default", + Env: "CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT", + // Single org deployments should always have this enabled. + Default: "true", + Value: &c.OIDC.OrganizationAssignDefault, + Group: &deploymentGroupOIDC, + YAML: "organizationAssignDefault", + }, + { + Name: "OIDC Organization Sync Mapping", + Description: "A map of OIDC claims and the organizations in Coder it should map to. " + + "This is required because organization IDs must be used within Coder.", + Flag: "oidc-organization-mapping", + Env: "CODER_OIDC_ORGANIZATION_MAPPING", + Default: "{}", + Value: &c.OIDC.OrganizationMapping, + Group: &deploymentGroupOIDC, + YAML: "organizationMapping", + }, { Name: "OIDC Group Field", Description: "This field must be set if using the group sync feature and the scope name is not 'groups'. Set to the claim to be used for groups.", diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index cf85bbf672057..c70c6e006f6d2 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -345,6 +345,9 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "ignore_user_info": true, "issuer_url": "string", "name_field": "string", + "organization_assign_default": true, + "organization_field": "string", + "organization_mapping": {}, "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index ee229a8cc4914..1c2898394a9be 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1849,6 +1849,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_user_info": true, "issuer_url": "string", "name_field": "string", + "organization_assign_default": true, + "organization_field": "string", + "organization_mapping": {}, "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", @@ -2272,6 +2275,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_user_info": true, "issuer_url": "string", "name_field": "string", + "organization_assign_default": true, + "organization_field": "string", + "organization_mapping": {}, "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", @@ -3639,6 +3645,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_user_info": true, "issuer_url": "string", "name_field": "string", + "organization_assign_default": true, + "organization_field": "string", + "organization_mapping": {}, "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", @@ -3652,34 +3661,37 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------------- | -------------------------------- | -------- | ------------ | -------------------------------------------------------------------------------- | -| `allow_signups` | boolean | false | | | -| `auth_url_params` | object | false | | | -| `client_cert_file` | string | false | | | -| `client_id` | string | false | | | -| `client_key_file` | string | false | | Client key file & ClientCertFile are used in place of ClientSecret for PKI auth. | -| `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` | [serpent.Regexp](#serpentregexp) | false | | | -| `groups_field` | string | false | | | -| `icon_url` | [serpent.URL](#serpenturl) | false | | | -| `ignore_email_verified` | boolean | false | | | -| `ignore_user_info` | boolean | false | | | -| `issuer_url` | string | false | | | -| `name_field` | string | false | | | -| `scopes` | array of string | false | | | -| `sign_in_text` | string | false | | | -| `signups_disabled_text` | string | false | | | -| `skip_issuer_checks` | boolean | false | | | -| `user_role_field` | string | false | | | -| `user_role_mapping` | object | false | | | -| `user_roles_default` | array of string | false | | | -| `username_field` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------------------- | -------------------------------- | -------- | ------------ | -------------------------------------------------------------------------------- | +| `allow_signups` | boolean | false | | | +| `auth_url_params` | object | false | | | +| `client_cert_file` | string | false | | | +| `client_id` | string | false | | | +| `client_key_file` | string | false | | Client key file & ClientCertFile are used in place of ClientSecret for PKI auth. | +| `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` | [serpent.Regexp](#serpentregexp) | false | | | +| `groups_field` | string | false | | | +| `icon_url` | [serpent.URL](#serpenturl) | false | | | +| `ignore_email_verified` | boolean | false | | | +| `ignore_user_info` | boolean | false | | | +| `issuer_url` | string | false | | | +| `name_field` | string | false | | | +| `organization_assign_default` | boolean | false | | | +| `organization_field` | string | false | | | +| `organization_mapping` | object | false | | | +| `scopes` | array of string | false | | | +| `sign_in_text` | string | false | | | +| `signups_disabled_text` | string | false | | | +| `skip_issuer_checks` | boolean | false | | | +| `user_role_field` | string | false | | | +| `user_role_mapping` | object | false | | | +| `user_roles_default` | array of string | false | | | +| `username_field` | string | false | | | ## codersdk.Organization diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 1ecdd326f1b17..2291a8b7b4915 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -558,6 +558,38 @@ OIDC auth URL parameters to pass to the upstream provider. Ignore the userinfo endpoint and only use the ID token for user information. +### --oidc-organization-field + +| | | +| ----------- | ------------------------------------------- | +| Type | string | +| Environment | $CODER_OIDC_ORGANIZATION_FIELD | +| YAML | oidc.organizationField | + +This field must be set if using the organization sync feature. Set to the claim to be used for organizations. + +### --oidc-organization-assign-default + +| | | +| ----------- | ---------------------------------------------------- | +| Type | bool | +| Environment | $CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT | +| YAML | oidc.organizationAssignDefault | +| Default | true | + +If set to true, users will always be added to the default organization. If organization sync is enabled, then the default org is always added to the user's set of expectedorganizations. + +### --oidc-organization-mapping + +| | | +| ----------- | --------------------------------------------- | +| Type | struct[map[string][]uuid.UUID] | +| Environment | $CODER_OIDC_ORGANIZATION_MAPPING | +| YAML | oidc.organizationMapping | +| Default | {} | + +A map of OIDC claims and the organizations in Coder it should map to. This is required because organization IDs must be used within Coder. + ### --oidc-group-field | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 0337e239d54a4..58e7ee2400eb1 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -434,6 +434,11 @@ OIDC OPTIONS: groups. This filter is applied after the group mapping and before the regex filter. + --oidc-organization-assign-default bool, $CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT (default: true) + If set to true, users will always be added to the default + organization. If organization sync is enabled, then the default org is + always added to the user's set of expectedorganizations. + --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. @@ -480,6 +485,14 @@ OIDC OPTIONS: --oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name) OIDC claim field to use as the name. + --oidc-organization-field string, $CODER_OIDC_ORGANIZATION_FIELD + This field must be set if using the organization sync feature. Set to + the claim to be used for organizations. + + --oidc-organization-mapping struct[map[string][]uuid.UUID], $CODER_OIDC_ORGANIZATION_MAPPING (default: {}) + A map of OIDC claims and the organizations in Coder it should map to. + This is required because organization IDs must be used within Coder. + --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 92225f3e44ecb..971b675f20d02 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -16,8 +16,10 @@ import ( "github.com/coder/coder/v2/coderd/appearance" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/entitlements" + "github.com/coder/coder/v2/coderd/idpsync" agplportsharing "github.com/coder/coder/v2/coderd/portsharing" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/enterprise/coderd/enidpsync" "github.com/coder/coder/v2/enterprise/coderd/portsharing" "golang.org/x/xerrors" @@ -78,6 +80,13 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { if options.Entitlements == nil { options.Entitlements = entitlements.New() } + if options.IDPSync == nil { + options.IDPSync = enidpsync.NewSync(options.Logger, options.Entitlements, idpsync.SyncSettings{ + OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(), + OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value, + OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(), + }) + } ctx, cancelFunc := context.WithCancel(ctx) diff --git a/enterprise/coderd/enidpsync/enidpsync.go b/enterprise/coderd/enidpsync/enidpsync.go new file mode 100644 index 0000000000000..bb21c68501e1b --- /dev/null +++ b/enterprise/coderd/enidpsync/enidpsync.go @@ -0,0 +1,25 @@ +package enidpsync + +import ( + "cdr.dev/slog" + + "github.com/coder/coder/v2/coderd/entitlements" + "github.com/coder/coder/v2/coderd/idpsync" +) + +// EnterpriseIDPSync enabled syncing user information from an external IDP. +// The sync is an enterprise feature, so this struct wraps the AGPL implementation +// and extends it with enterprise capabilities. These capabilities can entirely +// be changed in the Parsing, and leaving the "syncing" part (which holds the +// more complex logic) to the shared AGPL implementation. +type EnterpriseIDPSync struct { + entitlements *entitlements.Set + *idpsync.AGPLIDPSync +} + +func NewSync(logger slog.Logger, set *entitlements.Set, settings idpsync.SyncSettings) *EnterpriseIDPSync { + return &EnterpriseIDPSync{ + entitlements: set, + AGPLIDPSync: idpsync.NewAGPLSync(logger.With(slog.F("enterprise_capable", "true")), settings), + } +} diff --git a/enterprise/coderd/enidpsync/organizations.go b/enterprise/coderd/enidpsync/organizations.go new file mode 100644 index 0000000000000..2c7520fc412ee --- /dev/null +++ b/enterprise/coderd/enidpsync/organizations.go @@ -0,0 +1,73 @@ +package enidpsync + +import ( + "context" + "net/http" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/idpsync" + "github.com/coder/coder/v2/coderd/util/slice" + "github.com/coder/coder/v2/codersdk" +) + +func (e EnterpriseIDPSync) OrganizationSyncEnabled() bool { + return e.entitlements.Enabled(codersdk.FeatureMultipleOrganizations) && e.OrganizationField != "" +} + +func (e EnterpriseIDPSync) ParseOrganizationClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.OrganizationParams, *idpsync.HTTPError) { + if !e.OrganizationSyncEnabled() { + // Default to agpl if multi-org is not enabled + return e.AGPLIDPSync.ParseOrganizationClaims(ctx, mergedClaims) + } + + // nolint:gocritic // all syncing is done as a system user + ctx = dbauthz.AsSystemRestricted(ctx) + userOrganizations := make([]uuid.UUID, 0) + + // Pull extra organizations from the claims. + if e.OrganizationField != "" { + organizationRaw, ok := mergedClaims[e.OrganizationField] + if ok { + parsedOrganizations, err := idpsync.ParseStringSliceClaim(organizationRaw) + if err != nil { + return idpsync.OrganizationParams{}, &idpsync.HTTPError{ + Code: http.StatusBadRequest, + Msg: "Failed to sync organizations from the OIDC claims", + Detail: err.Error(), + RenderStaticPage: false, + RenderDetailMarkdown: false, + } + } + + // Keep track of which claims are not mapped for debugging purposes. + var ignored []string + for _, parsedOrg := range parsedOrganizations { + if mappedOrganization, ok := e.OrganizationMapping[parsedOrg]; ok { + // parsedOrg is in the mapping, so add the mapped organizations to the + // user's organizations. + userOrganizations = append(userOrganizations, mappedOrganization...) + } else { + ignored = append(ignored, parsedOrg) + } + } + + e.Logger.Debug(ctx, "parsed organizations from claim", + slog.F("len", len(parsedOrganizations)), + slog.F("ignored", ignored), + slog.F("organizations", parsedOrganizations), + ) + } + } + + return idpsync.OrganizationParams{ + // If the field is not set, then sync is not enabled. + SyncEnabled: e.OrganizationField != "", + IncludeDefault: e.OrganizationAssignDefault, + // Do not return duplicates + Organizations: slice.Unique(userOrganizations), + }, nil +} diff --git a/enterprise/coderd/enidpsync/organizations_test.go b/enterprise/coderd/enidpsync/organizations_test.go new file mode 100644 index 0000000000000..0b2ed1ef6521f --- /dev/null +++ b/enterprise/coderd/enidpsync/organizations_test.go @@ -0,0 +1,272 @@ +package enidpsync_test + +import ( + "context" + "testing" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/entitlements" + "github.com/coder/coder/v2/coderd/idpsync" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/enidpsync" + "github.com/coder/coder/v2/testutil" +) + +type ExpectedUser struct { + SyncError bool + Organizations []uuid.UUID +} + +type Expectations struct { + Name string + Claims jwt.MapClaims + // Parse + ParseError func(t *testing.T, httpErr *idpsync.HTTPError) + ExpectedParams idpsync.OrganizationParams + // Mutate allows mutating the user before syncing + Mutate func(t *testing.T, db database.Store, user database.User) + Sync ExpectedUser +} + +type OrganizationSyncTestCase struct { + Settings idpsync.SyncSettings + Entitlements *entitlements.Set + Exps []Expectations +} + +func TestOrganizationSync(t *testing.T) { + t.Parallel() + + if dbtestutil.WillUsePostgres() { + t.Skip("Skipping test because it populates a lot of db entries, which is slow on postgres") + } + + requireUserOrgs := func(t *testing.T, db database.Store, user database.User, expected []uuid.UUID) { + t.Helper() + + // nolint:gocritic // in testing + members, err := db.OrganizationMembers(dbauthz.AsSystemRestricted(context.Background()), database.OrganizationMembersParams{ + UserID: user.ID, + }) + require.NoError(t, err) + + foundIDs := db2sdk.List(members, func(m database.OrganizationMembersRow) uuid.UUID { + return m.OrganizationMember.OrganizationID + }) + require.ElementsMatch(t, expected, foundIDs, "match user organizations") + } + + entitled := entitlements.New() + entitled.Update(func(entitlements *codersdk.Entitlements) { + entitlements.Features[codersdk.FeatureMultipleOrganizations] = codersdk.Feature{ + Entitlement: codersdk.EntitlementEntitled, + Enabled: true, + Limit: nil, + Actual: nil, + } + }) + + testCases := []struct { + Name string + Case func(t *testing.T, db database.Store) OrganizationSyncTestCase + }{ + { + Name: "SingleOrgDeployment", + Case: func(t *testing.T, db database.Store) OrganizationSyncTestCase { + def, _ := db.GetDefaultOrganization(context.Background()) + other := dbgen.Organization(t, db, database.Organization{}) + return OrganizationSyncTestCase{ + Entitlements: entitled, + Settings: idpsync.SyncSettings{ + OrganizationField: "", + OrganizationMapping: nil, + OrganizationAssignDefault: true, + }, + Exps: []Expectations{ + { + Name: "NoOrganizations", + Claims: jwt.MapClaims{}, + ExpectedParams: idpsync.OrganizationParams{ + SyncEnabled: false, + IncludeDefault: true, + Organizations: []uuid.UUID{}, + }, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{}, + }, + }, + { + Name: "AlreadyInOrgs", + Claims: jwt.MapClaims{}, + ExpectedParams: idpsync.OrganizationParams{ + SyncEnabled: false, + IncludeDefault: true, + Organizations: []uuid.UUID{}, + }, + Mutate: func(t *testing.T, db database.Store, user database.User) { + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: def.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: other.ID, + }) + }, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{def.ID, other.ID}, + }, + }, + }, + } + }, + }, + { + Name: "MultiOrgWithDefault", + Case: func(t *testing.T, db database.Store) OrganizationSyncTestCase { + def, _ := db.GetDefaultOrganization(context.Background()) + one := dbgen.Organization(t, db, database.Organization{}) + two := dbgen.Organization(t, db, database.Organization{}) + three := dbgen.Organization(t, db, database.Organization{}) + return OrganizationSyncTestCase{ + Entitlements: entitled, + Settings: idpsync.SyncSettings{ + OrganizationField: "organizations", + OrganizationMapping: map[string][]uuid.UUID{ + "first": {one.ID}, + "second": {two.ID}, + "third": {three.ID}, + }, + OrganizationAssignDefault: true, + }, + Exps: []Expectations{ + { + Name: "NoOrganizations", + Claims: jwt.MapClaims{}, + ExpectedParams: idpsync.OrganizationParams{ + SyncEnabled: true, + IncludeDefault: true, + Organizations: []uuid.UUID{}, + }, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{def.ID}, + }, + }, + { + Name: "AlreadyInOrgs", + Claims: jwt.MapClaims{ + "organizations": []string{"second", "extra"}, + }, + ExpectedParams: idpsync.OrganizationParams{ + SyncEnabled: true, + IncludeDefault: true, + Organizations: []uuid.UUID{two.ID}, + }, + Mutate: func(t *testing.T, db database.Store, user database.User) { + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: def.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: one.ID, + }) + }, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{def.ID, two.ID}, + }, + }, + { + Name: "ManyClaims", + Claims: jwt.MapClaims{ + // Add some repeats + "organizations": []string{"second", "extra", "first", "third", "second", "second"}, + }, + ExpectedParams: idpsync.OrganizationParams{ + SyncEnabled: true, + IncludeDefault: true, + Organizations: []uuid.UUID{ + two.ID, one.ID, three.ID, + }, + }, + Mutate: func(t *testing.T, db database.Store, user database.User) { + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: def.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: one.ID, + }) + }, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{def.ID, one.ID, two.ID, three.ID}, + }, + }, + }, + } + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + logger := slogtest.Make(t, &slogtest.Options{}) + + rdb, _ := dbtestutil.NewDB(t) + db := dbauthz.New(rdb, rbac.NewAuthorizer(prometheus.NewRegistry()), logger, coderdtest.AccessControlStorePointer()) + caseData := tc.Case(t, rdb) + if caseData.Entitlements == nil { + caseData.Entitlements = entitlements.New() + } + + // Create a new sync object + sync := enidpsync.NewSync(logger, caseData.Entitlements, caseData.Settings) + for _, exp := range caseData.Exps { + t.Run(exp.Name, func(t *testing.T) { + params, httpErr := sync.ParseOrganizationClaims(ctx, exp.Claims) + if exp.ParseError != nil { + exp.ParseError(t, httpErr) + return + } + require.Nil(t, httpErr, "no parse error") + + require.Equal(t, exp.ExpectedParams.SyncEnabled, params.SyncEnabled, "match enabled") + require.Equal(t, exp.ExpectedParams.IncludeDefault, params.IncludeDefault, "match include default") + if exp.ExpectedParams.Organizations == nil { + exp.ExpectedParams.Organizations = []uuid.UUID{} + } + require.ElementsMatch(t, exp.ExpectedParams.Organizations, params.Organizations, "match organizations") + + user := dbgen.User(t, db, database.User{}) + if exp.Mutate != nil { + exp.Mutate(t, rdb, user) + } + + err := sync.SyncOrganizations(ctx, rdb, user, params) + if exp.Sync.SyncError { + require.Error(t, err) + return + } + require.NoError(t, err) + requireUserOrgs(t, db, user, exp.Sync.Organizations) + }) + } + }) + } +} diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index e5ca9c9180290..3e94a25a1c013 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" "github.com/stretchr/testify/require" "golang.org/x/xerrors" @@ -14,7 +15,9 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/coderdtest/oidctest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/util/slice" @@ -23,11 +26,208 @@ import ( "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" "github.com/coder/coder/v2/testutil" + "github.com/coder/serpent" ) // nolint:bodyclose func TestUserOIDC(t *testing.T) { t.Parallel() + + t.Run("OrganizationSync", func(t *testing.T) { + t.Parallel() + + t.Run("SingleOrgDeployment", func(t *testing.T) { + t.Parallel() + + runner := setupOIDCTest(t, oidcTestConfig{ + Config: func(cfg *coderd.OIDCConfig) { + cfg.AllowSignups = true + cfg.UserRoleField = "roles" + }, + }) + + claims := jwt.MapClaims{ + "email": "alice@coder.com", + } + + // Login a new client that signs up + client, resp := runner.Login(t, claims) + require.Equal(t, http.StatusOK, resp.StatusCode) + runner.AssertOrganizations(t, "alice", true, nil) + + // Force a refresh, and assert nothing has changes + runner.ForceRefresh(t, client, claims) + runner.AssertOrganizations(t, "alice", true, nil) + }) + + t.Run("MultiOrgNoSync", func(t *testing.T) { + t.Parallel() + + runner := setupOIDCTest(t, oidcTestConfig{ + Config: func(cfg *coderd.OIDCConfig) { + cfg.AllowSignups = true + }, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + second, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "second", + DisplayName: "", + Description: "", + Icon: "", + }) + require.NoError(t, err) + + claims := jwt.MapClaims{ + "email": "alice@coder.com", + } + + // Login a new client that signs up + _, resp := runner.Login(t, claims) + require.Equal(t, http.StatusOK, resp.StatusCode) + runner.AssertOrganizations(t, "alice", true, nil) + + // Add alice to new org + _, err = runner.AdminClient.PostOrganizationMember(ctx, second.ID, "alice") + require.NoError(t, err) + + // Log in again to refresh the sync. The user should not be removed + // from the second organization. + runner.Login(t, claims) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second.ID}) + }) + + t.Run("MultiOrgWithDefault", func(t *testing.T) { + t.Parallel() + + // Chicken and egg problem. Config is at startup, but orgs are + // created at runtime. We should add a runtime configuration of + // this. + second := uuid.New() + third := uuid.New() + + // Given: 4 organizations: default, second, third, and fourth + runner := setupOIDCTest(t, oidcTestConfig{ + Config: func(cfg *coderd.OIDCConfig) { + cfg.AllowSignups = true + }, + DeploymentValues: func(dv *codersdk.DeploymentValues) { + dv.OIDC.OrganizationAssignDefault = true + dv.OIDC.OrganizationField = "organization" + dv.OIDC.OrganizationMapping = serpent.Struct[map[string][]uuid.UUID]{ + Value: map[string][]uuid.UUID{ + "second": {second}, + "third": {third}, + }, + } + }, + }) + dbgen.Organization(t, runner.API.Database, database.Organization{ + ID: second, + }) + dbgen.Organization(t, runner.API.Database, database.Organization{ + ID: third, + }) + fourth := dbgen.Organization(t, runner.API.Database, database.Organization{}) + + ctx := testutil.Context(t, testutil.WaitMedium) + claims := jwt.MapClaims{ + "email": "alice@coder.com", + "organization": []string{"second", "third"}, + } + + // Then: a new user logs in with claims "second" and "third", they + // should belong to [default, second, third]. + userClient, resp := runner.Login(t, claims) + require.Equal(t, http.StatusOK, resp.StatusCode) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third}) + user, err := userClient.User(ctx, codersdk.Me) + require.NoError(t, err) + + // When: they are manually added to the fourth organization, a new sync + // should remove them. + _, err = runner.AdminClient.PostOrganizationMember(ctx, fourth.ID, "alice") + require.ErrorContains(t, err, "Organization sync is enabled") + + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third}) + // Go around the block to add the user to see if they are removed. + dbgen.OrganizationMember(t, runner.API.Database, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: fourth.ID, + }) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third, fourth.ID}) + + // Then: Log in again will resync the orgs to their updated + // claims. + runner.Login(t, jwt.MapClaims{ + "email": "alice@coder.com", + "organization": []string{"third"}, + }) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{third}) + }) + + t.Run("MultiOrgWithoutDefault", func(t *testing.T) { + t.Parallel() + + second := uuid.New() + third := uuid.New() + + // Given: 4 organizations: default, second, third, and fourth + runner := setupOIDCTest(t, oidcTestConfig{ + Config: func(cfg *coderd.OIDCConfig) { + cfg.AllowSignups = true + }, + DeploymentValues: func(dv *codersdk.DeploymentValues) { + dv.OIDC.OrganizationAssignDefault = false + dv.OIDC.OrganizationField = "organization" + dv.OIDC.OrganizationMapping = serpent.Struct[map[string][]uuid.UUID]{ + Value: map[string][]uuid.UUID{ + "second": {second}, + "third": {third}, + }, + } + }, + }) + dbgen.Organization(t, runner.API.Database, database.Organization{ + ID: second, + }) + dbgen.Organization(t, runner.API.Database, database.Organization{ + ID: third, + }) + fourth := dbgen.Organization(t, runner.API.Database, database.Organization{}) + + ctx := testutil.Context(t, testutil.WaitMedium) + claims := jwt.MapClaims{ + "email": "alice@coder.com", + "organization": []string{"second", "third"}, + } + + // Then: a new user logs in with claims "second" and "third", they + // should belong to [ second, third]. + userClient, resp := runner.Login(t, claims) + require.Equal(t, http.StatusOK, resp.StatusCode) + runner.AssertOrganizations(t, "alice", false, []uuid.UUID{second, third}) + user, err := userClient.User(ctx, codersdk.Me) + require.NoError(t, err) + + // When: they are manually added to the fourth organization, a new sync + // should remove them. + dbgen.OrganizationMember(t, runner.API.Database, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: fourth.ID, + }) + runner.AssertOrganizations(t, "alice", false, []uuid.UUID{second, third, fourth.ID}) + + // Then: Log in again will resync the orgs to their updated + // claims. + runner.Login(t, jwt.MapClaims{ + "email": "alice@coder.com", + "organization": []string{"third"}, + }) + runner.AssertOrganizations(t, "alice", false, []uuid.UUID{third}) + }) + }) + t.Run("RoleSync", func(t *testing.T) { t.Parallel() @@ -54,6 +254,8 @@ func TestUserOIDC(t *testing.T) { // Force a refresh, and assert nothing has changes runner.ForceRefresh(t, client, claims) runner.AssertRoles(t, "alice", []string{}) + + runner.AssertOrganizations(t, "alice", true, nil) }) // Some IDPs (ADFS) send the "string" type vs "[]string" if only @@ -81,6 +283,7 @@ func TestUserOIDC(t *testing.T) { }) require.Equal(t, http.StatusOK, resp.StatusCode) runner.AssertRoles(t, "alice", []string{rbac.RoleTemplateAdmin().String()}) + runner.AssertOrganizations(t, "alice", true, nil) }) // A user has some roles, then on an oauth refresh will lose said @@ -118,6 +321,7 @@ func TestUserOIDC(t *testing.T) { "roles": []string{"random"}, }) runner.AssertRoles(t, "alice", []string{}) + runner.AssertOrganizations(t, "alice", true, nil) }) // A user has some roles, then on another oauth login will lose said @@ -153,6 +357,7 @@ func TestUserOIDC(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) runner.AssertRoles(t, "alice", []string{}) + runner.AssertOrganizations(t, "alice", true, nil) }) // All manual role updates should fail when role sync is enabled. @@ -214,6 +419,7 @@ func TestUserOIDC(t *testing.T) { }) require.Equal(t, http.StatusOK, resp.StatusCode) runner.AssertGroups(t, "alice", []string{groupName}) + runner.AssertOrganizations(t, "alice", true, nil) }) // Tests the group mapping feature. @@ -245,6 +451,7 @@ func TestUserOIDC(t *testing.T) { }) require.Equal(t, http.StatusOK, resp.StatusCode) runner.AssertGroups(t, "alice", []string{coderGroupName}) + runner.AssertOrganizations(t, "alice", true, nil) }) // User is in a group, then on an oauth refresh will lose said @@ -284,6 +491,7 @@ func TestUserOIDC(t *testing.T) { "email": "alice@coder.com", }) runner.AssertGroups(t, "alice", []string{}) + runner.AssertOrganizations(t, "alice", true, nil) }) t.Run("AddThenRemoveOnReAuth", func(t *testing.T) { @@ -318,6 +526,7 @@ func TestUserOIDC(t *testing.T) { }) require.Equal(t, http.StatusOK, resp.StatusCode) runner.AssertGroups(t, "alice", []string{}) + runner.AssertOrganizations(t, "alice", true, nil) }) // Updating groups where the claimed group does not exist. @@ -795,8 +1004,31 @@ type oidcTestConfig struct { Userinfo jwt.MapClaims // Config allows modifying the Coderd OIDC configuration. - Config func(cfg *coderd.OIDCConfig) - FakeOpts []oidctest.FakeIDPOpt + Config func(cfg *coderd.OIDCConfig) + DeploymentValues func(dv *codersdk.DeploymentValues) + FakeOpts []oidctest.FakeIDPOpt +} + +func (r *oidcTestRunner) AssertOrganizations(t *testing.T, userIdent string, includeDefault bool, expected []uuid.UUID) { + t.Helper() + + ctx := testutil.Context(t, testutil.WaitMedium) + userOrgs, err := r.AdminClient.OrganizationsByUser(ctx, userIdent) + require.NoError(t, err) + + cpy := make([]uuid.UUID, 0, len(expected)) + cpy = append(cpy, expected...) + hasDefault := false + userOrgIDs := db2sdk.List(userOrgs, func(o codersdk.Organization) uuid.UUID { + if o.IsDefault { + hasDefault = true + cpy = append(cpy, o.ID) + } + return o.ID + }) + + require.Equal(t, includeDefault, hasDefault, "expected default org") + require.ElementsMatch(t, cpy, userOrgIDs, "expected orgs") } func (r *oidcTestRunner) AssertRoles(t *testing.T, userIdent string, roles []string) { @@ -856,14 +1088,21 @@ func setupOIDCTest(t *testing.T, settings oidcTestConfig) *oidcTestRunner { ctx := testutil.Context(t, testutil.WaitMedium) cfg := fake.OIDCConfig(t, nil, settings.Config) + dv := coderdtest.DeploymentValues(t) + if settings.DeploymentValues != nil { + settings.DeploymentValues(dv) + } + dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} owner, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ Options: &coderdtest.Options{ - OIDCConfig: cfg, + OIDCConfig: cfg, + DeploymentValues: dv, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ - codersdk.FeatureUserRoleManagement: 1, - codersdk.FeatureTemplateRBAC: 1, + codersdk.FeatureUserRoleManagement: 1, + codersdk.FeatureTemplateRBAC: 1, + codersdk.FeatureMultipleOrganizations: 1, }, }, }) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 262e58d77c76c..1bdf3578fb077 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -879,6 +879,9 @@ export interface OIDCConfig { readonly email_field: string; readonly auth_url_params: Record; readonly ignore_user_info: boolean; + readonly organization_field: string; + readonly organization_mapping: Record>>; + readonly organization_assign_default: boolean; readonly group_auto_create: boolean; readonly group_regex_filter: string; readonly group_allow_list: string[];