From e034ebe41e605aead69c24370dd4693f8ac350da Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 6 Nov 2024 13:45:42 -0600 Subject: [PATCH 1/9] chore: move organizatinon sync to runtime configuration --- coderd/idpsync/group.go | 10 +- coderd/idpsync/group_test.go | 10 +- coderd/idpsync/idpsync.go | 14 +- coderd/idpsync/organization.go | 137 +++++++++++++++--- coderd/idpsync/organizations_test.go | 4 +- coderd/members.go | 26 +++- coderd/userauth.go | 33 ++--- codersdk/deployment.go | 3 + enterprise/coderd/enidpsync/groups.go | 6 +- enterprise/coderd/enidpsync/groups_test.go | 4 +- enterprise/coderd/enidpsync/organizations.go | 70 +++------ .../coderd/enidpsync/organizations_test.go | 12 +- 12 files changed, 201 insertions(+), 128 deletions(-) diff --git a/coderd/idpsync/group.go b/coderd/idpsync/group.go index 672bcb66da4cf..c14b7655e7e20 100644 --- a/coderd/idpsync/group.go +++ b/coderd/idpsync/group.go @@ -20,12 +20,12 @@ import ( ) type GroupParams struct { - // SyncEnabled if false will skip syncing the user's groups - SyncEnabled bool + // SyncEntitled if false will skip syncing the user's groups + SyncEntitled bool MergedClaims jwt.MapClaims } -func (AGPLIDPSync) GroupSyncEnabled() bool { +func (AGPLIDPSync) GroupSyncEntitled() bool { // AGPL does not support syncing groups. return false } @@ -73,13 +73,13 @@ func (s AGPLIDPSync) GroupSyncSettings(ctx context.Context, orgID uuid.UUID, db func (s AGPLIDPSync) ParseGroupClaims(_ context.Context, _ jwt.MapClaims) (GroupParams, *HTTPError) { return GroupParams{ - SyncEnabled: s.GroupSyncEnabled(), + SyncEntitled: s.GroupSyncEntitled(), }, nil } func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user database.User, params GroupParams) error { // Nothing happens if sync is not enabled - if !params.SyncEnabled { + if !params.SyncEntitled { return nil } diff --git a/coderd/idpsync/group_test.go b/coderd/idpsync/group_test.go index 1275dd4e48503..2baafd53ff03c 100644 --- a/coderd/idpsync/group_test.go +++ b/coderd/idpsync/group_test.go @@ -41,7 +41,7 @@ func TestParseGroupClaims(t *testing.T) { params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) // AllowList has no effect in AGPL @@ -61,7 +61,7 @@ func TestParseGroupClaims(t *testing.T) { params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) } @@ -276,7 +276,7 @@ func TestGroupSyncTable(t *testing.T) { // Do the group sync! err := s.SyncGroups(ctx, db, user, idpsync.GroupParams{ - SyncEnabled: true, + SyncEntitled: true, MergedClaims: userClaims, }) require.NoError(t, err) @@ -363,7 +363,7 @@ func TestGroupSyncTable(t *testing.T) { // Do the group sync! err = s.SyncGroups(ctx, db, user, idpsync.GroupParams{ - SyncEnabled: true, + SyncEntitled: true, MergedClaims: userClaims, }) require.NoError(t, err) @@ -420,7 +420,7 @@ func TestSyncDisabled(t *testing.T) { // Do the group sync! err := s.SyncGroups(ctx, db, user, idpsync.GroupParams{ - SyncEnabled: false, + SyncEntitled: false, MergedClaims: jwt.MapClaims{ "groups": []string{"baz", "bop"}, }, diff --git a/coderd/idpsync/idpsync.go b/coderd/idpsync/idpsync.go index f2c9e49ecc900..18f13df99631e 100644 --- a/coderd/idpsync/idpsync.go +++ b/coderd/idpsync/idpsync.go @@ -25,7 +25,11 @@ import ( // TODO: Move group + role sync into this interface. type IDPSync interface { AssignDefaultOrganization() bool - OrganizationSyncEnabled() bool + OrganizationSyncEntitled() bool + // OrganizationSyncEnabled returns true if all OIDC users are assigned + // to organizations via org sync settings. + // This is used to know when to disable manual org membership assignment. + OrganizationSyncEnabled(ctx context.Context, db database.Store) bool // ParseOrganizationClaims takes claims from an OIDC provider, and returns the // organization sync params for assigning users into organizations. ParseOrganizationClaims(ctx context.Context, mergedClaims jwt.MapClaims) (OrganizationParams, *HTTPError) @@ -33,7 +37,7 @@ type IDPSync interface { // provided params. SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error - GroupSyncEnabled() bool + GroupSyncEntitled() bool // ParseGroupClaims takes claims from an OIDC provider, and returns the params // for group syncing. Most of the logic happens in SyncGroups. ParseGroupClaims(ctx context.Context, mergedClaims jwt.MapClaims) (GroupParams, *HTTPError) @@ -147,8 +151,9 @@ func FromDeploymentValues(dv *codersdk.DeploymentValues) DeploymentSyncSettings type SyncSettings struct { DeploymentSyncSettings - Group runtimeconfig.RuntimeEntry[*GroupSyncSettings] - Role runtimeconfig.RuntimeEntry[*RoleSyncSettings] + Group runtimeconfig.RuntimeEntry[*GroupSyncSettings] + Role runtimeconfig.RuntimeEntry[*RoleSyncSettings] + Organization runtimeconfig.RuntimeEntry[*OrganizationSyncSettings] } func NewAGPLSync(logger slog.Logger, manager *runtimeconfig.Manager, settings DeploymentSyncSettings) *AGPLIDPSync { @@ -159,6 +164,7 @@ func NewAGPLSync(logger slog.Logger, manager *runtimeconfig.Manager, settings De DeploymentSyncSettings: settings, Group: runtimeconfig.MustNew[*GroupSyncSettings]("group-sync-settings"), Role: runtimeconfig.MustNew[*RoleSyncSettings]("role-sync-settings"), + Organization: runtimeconfig.MustNew[*OrganizationSyncSettings]("organization-sync-settings"), }, } } diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 3e2a0f84d5e5e..62c91a71a08a8 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -3,6 +3,8 @@ package idpsync import ( "context" "database/sql" + "encoding/json" + "fmt" "github.com/golang-jwt/jwt/v4" "github.com/google/uuid" @@ -13,35 +15,58 @@ import ( "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/runtimeconfig" "github.com/coder/coder/v2/coderd/util/slice" ) 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 + // SyncEntitled if false will skip syncing the user's organizations. + SyncEntitled bool + // MergedClaims are passed to the organization level for syncing + MergedClaims jwt.MapClaims } -func (AGPLIDPSync) OrganizationSyncEnabled() bool { +func (AGPLIDPSync) OrganizationSyncEntitled() bool { // AGPL does not support syncing organizations. return false } +func (AGPLIDPSync) OrganizationSyncEnabled(_ context.Context, _ database.Store) bool { + return false +} + func (s AGPLIDPSync) AssignDefaultOrganization() bool { return s.OrganizationAssignDefault } -func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) (OrganizationParams, *HTTPError) { +func (s AGPLIDPSync) OrganizationSyncSettings(ctx context.Context, db database.Store) (*OrganizationSyncSettings, error) { + rlv := s.Manager.Resolver(db) + orgSettings, err := s.SyncSettings.Organization.Resolve(ctx, rlv) + if err != nil { + if !xerrors.Is(err, runtimeconfig.ErrEntryNotFound) { + return nil, xerrors.Errorf("resolve org sync settings: %w", err) + } + + // Default to an empty config + orgSettings = &OrganizationSyncSettings{} + + if s.DeploymentSyncSettings.OrganizationField != "" { + // Use static settings if set + orgSettings = &OrganizationSyncSettings{ + OrganizationField: s.DeploymentSyncSettings.OrganizationField, + OrganizationMapping: s.DeploymentSyncSettings.OrganizationMapping, + OrganizationAssignDefault: s.DeploymentSyncSettings.OrganizationAssignDefault, + } + } + } + return orgSettings, nil +} + +func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, claims jwt.MapClaims) (OrganizationParams, *HTTPError) { // For AGPL we only sync the default organization. return OrganizationParams{ - SyncEnabled: s.OrganizationSyncEnabled(), - IncludeDefault: s.OrganizationAssignDefault, - Organizations: []uuid.UUID{}, + SyncEntitled: s.OrganizationSyncEntitled(), + MergedClaims: claims, }, nil } @@ -49,21 +74,24 @@ func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) // 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 { + if !params.SyncEntitled { return nil } + orgSettings, err := s.OrganizationSyncSettings(ctx, tx) + if err != nil { + return xerrors.Errorf("failed to get org sync settings: %w", err) + } + + if orgSettings.OrganizationField == "" { + return nil // No sync configured, nothing to do + } + // 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) + expectedOrgs, err := orgSettings.ParseClaims(ctx, tx, params.MergedClaims) + if err != nil { + return xerrors.Errorf("organization claims: %w", err) } existingOrgs, err := tx.GetOrganizationsByUserID(ctx, user.ID) @@ -77,7 +105,7 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u // 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) + add, remove := slice.SymmetricDifference(existingOrgIDs, expectedOrgs) notExists := make([]uuid.UUID, 0) for _, orgID := range add { //nolint:gocritic // System actor being used to assign orgs @@ -117,3 +145,64 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u } return nil } + +type OrganizationSyncSettings 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 +} + +func (s *OrganizationSyncSettings) Set(v string) error { + return json.Unmarshal([]byte(v), s) +} + +func (s *OrganizationSyncSettings) String() string { + return runtimeconfig.JSONString(s) +} + +// ParseClaims will parse the claims and return the list of organizations the user +// should sync to. +func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database.Store, mergedClaims jwt.MapClaims) ([]uuid.UUID, error) { + userOrganizations := make([]uuid.UUID, 0) + + if s.OrganizationAssignDefault { + // This is a bit hacky, but if AssignDefault is included, then always + // make sure to include the default org in the list of expected. + defaultOrg, err := db.GetDefaultOrganization(ctx) + if err != nil { + return nil, xerrors.Errorf("failed to get default organization: %w", err) + } + + // Always include default org. + userOrganizations = append(userOrganizations, defaultOrg.ID) + } + + organizationRaw, ok := mergedClaims[s.OrganizationField] + if !ok { + return userOrganizations, nil + } + + parsedOrganizations, err := ParseStringSliceClaim(organizationRaw) + if err != nil { + return userOrganizations, fmt.Errorf("failed to parese organizations OIDC claims: %w", err) + } + + // add any mapped organizations + for _, parsedOrg := range parsedOrganizations { + if mappedOrganization, ok := s.OrganizationMapping[parsedOrg]; ok { + // parsedOrg is in the mapping, so add the mapped organizations to the + // user's organizations. + userOrganizations = append(userOrganizations, mappedOrganization...) + } + } + + // Deduplicate the organizations + return slice.Unique(userOrganizations), nil +} diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 1670beaaedc75..5cecc98b1657a 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -34,7 +34,7 @@ func TestParseOrganizationClaims(t *testing.T) { require.Empty(t, params.Organizations) require.True(t, params.IncludeDefault) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) t.Run("AGPL", func(t *testing.T) { @@ -58,6 +58,6 @@ func TestParseOrganizationClaims(t *testing.T) { require.Empty(t, params.Organizations) require.False(t, params.IncludeDefault) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) } diff --git a/coderd/members.go b/coderd/members.go index 7f2acd982631b..b4d0d7f6374c9 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -45,11 +45,7 @@ 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), - }) + if !api.manualOrganizationMembership(ctx, rw, user) { return } @@ -104,6 +100,7 @@ func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request apiKey = httpmw.APIKey(r) organization = httpmw.OrganizationParam(r) member = httpmw.OrganizationMemberParam(r) + user = httpmw.UserParam(r) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationMember](rw, &audit.RequestParams{ OrganizationID: organization.ID, @@ -116,6 +113,10 @@ func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request aReq.Old = member.OrganizationMember.Auditable(member.Username) defer commitAudit() + if !api.manualOrganizationMembership(ctx, rw, user) { + return + } + if member.UserID == apiKey.UserID { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{Message: "cannot remove self from an organization"}) return @@ -372,3 +373,18 @@ func convertOrganizationMembersWithUserData(ctx context.Context, db database.Sto return converted, nil } + +// manualOrganizationMembership checks if the user is an OIDC user and if organization sync is enabled. +// If organization sync is enabled, manual organization assignment is not allowed, +// since all organization membership is controlled by the external IDP. +// TODO: Might be helpful to have an ignore list of organizations that can be manually assigned. +func (api *API) manualOrganizationMembership(ctx context.Context, rw http.ResponseWriter, user database.User) bool { + if user.LoginType == database.LoginTypeOIDC && api.IDPSync.OrganizationSyncEnabled(ctx, api.Database) { + 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 the membership in your external IDP.", user.Username), + }) + return false + } + return true +} diff --git a/coderd/userauth.go b/coderd/userauth.go index f6cf0e5292db7..e7db9e9719c35 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -968,12 +968,10 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { Name: normName, DebugContext: OauthDebugContext{}, GroupSync: idpsync.GroupParams{ - SyncEnabled: false, + SyncEntitled: false, }, OrganizationSync: idpsync.OrganizationParams{ - SyncEnabled: false, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: false, }, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) @@ -1513,14 +1511,6 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C // This can happen if a user is a built-in user but is signing in // with OIDC for the first time. if user.ID == uuid.Nil { - // Until proper multi-org support, all users will be added to the default organization. - // The default organization should always be present. - //nolint:gocritic - defaultOrganization, err := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) - if err != nil { - return xerrors.Errorf("unable to fetch default organization: %w", err) - } - //nolint:gocritic _, err = tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Username: params.Username, @@ -1555,19 +1545,22 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } } - // 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 + defaultOrganization, err := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) + if err != nil { + return xerrors.Errorf("unable to fetch default organization: %w", err) } //nolint:gocritic user, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{ CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{ - Email: params.Email, - Username: params.Username, - OrganizationIDs: orgIDs, + Email: params.Email, + Username: params.Username, + // This is a kludge, but all users are defaulted into the default + // organization. This exists as the default behavior. + // If org sync is enabled and configured, the user's groups + // will change based on the org sync settings. + OrganizationIDs: []uuid.UUID{defaultOrganization.ID}, UserStatus: ptr.Ref(codersdk.UserStatusActive), }, LoginType: params.LoginType, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 3ba09bd38d1a4..5959c35fa9d95 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1749,6 +1749,7 @@ when required by your organization's security policy.`, Value: &c.OIDC.OrganizationField, Group: &deploymentGroupOIDC, YAML: "organizationField", + Hidden: true, // Use db runtime config instead }, { Name: "OIDC Assign Default Organization", @@ -1762,6 +1763,7 @@ when required by your organization's security policy.`, Value: &c.OIDC.OrganizationAssignDefault, Group: &deploymentGroupOIDC, YAML: "organizationAssignDefault", + Hidden: true, // Use db runtime config instead }, { Name: "OIDC Organization Sync Mapping", @@ -1773,6 +1775,7 @@ when required by your organization's security policy.`, Value: &c.OIDC.OrganizationMapping, Group: &deploymentGroupOIDC, YAML: "organizationMapping", + Hidden: true, // Use db runtime config instead }, { Name: "OIDC Group Field", diff --git a/enterprise/coderd/enidpsync/groups.go b/enterprise/coderd/enidpsync/groups.go index dc8456fc6b1c9..7cabce412a1ea 100644 --- a/enterprise/coderd/enidpsync/groups.go +++ b/enterprise/coderd/enidpsync/groups.go @@ -10,7 +10,7 @@ import ( "github.com/coder/coder/v2/codersdk" ) -func (e EnterpriseIDPSync) GroupSyncEnabled() bool { +func (e EnterpriseIDPSync) GroupSyncEntitled() bool { return e.entitlements.Enabled(codersdk.FeatureTemplateRBAC) } @@ -20,7 +20,7 @@ func (e EnterpriseIDPSync) GroupSyncEnabled() bool { // GroupAllowList is implemented here to prevent login by unauthorized users. // TODO: GroupAllowList overlaps with the default organization group sync settings. func (e EnterpriseIDPSync) ParseGroupClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.GroupParams, *idpsync.HTTPError) { - if !e.GroupSyncEnabled() { + if !e.GroupSyncEntitled() { return e.AGPLIDPSync.ParseGroupClaims(ctx, mergedClaims) } @@ -64,7 +64,7 @@ func (e EnterpriseIDPSync) ParseGroupClaims(ctx context.Context, mergedClaims jw } return idpsync.GroupParams{ - SyncEnabled: true, + SyncEntitled: true, MergedClaims: mergedClaims, }, nil } diff --git a/enterprise/coderd/enidpsync/groups_test.go b/enterprise/coderd/enidpsync/groups_test.go index 278b647f29f14..652432c73f503 100644 --- a/enterprise/coderd/enidpsync/groups_test.go +++ b/enterprise/coderd/enidpsync/groups_test.go @@ -39,7 +39,7 @@ func TestEnterpriseParseGroupClaims(t *testing.T) { params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) t.Run("NotInAllowList", func(t *testing.T) { @@ -90,7 +90,7 @@ func TestEnterpriseParseGroupClaims(t *testing.T) { } params, err := s.ParseGroupClaims(ctx, claims) require.Nil(t, err) - require.True(t, params.SyncEnabled) + require.True(t, params.SyncEntitled) require.Equal(t, claims, params.MergedClaims) }) } diff --git a/enterprise/coderd/enidpsync/organizations.go b/enterprise/coderd/enidpsync/organizations.go index 2c7520fc412ee..95919f25613e3 100644 --- a/enterprise/coderd/enidpsync/organizations.go +++ b/enterprise/coderd/enidpsync/organizations.go @@ -2,72 +2,38 @@ 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/database" "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) OrganizationSyncEntitled() bool { + return e.entitlements.Enabled(codersdk.FeatureMultipleOrganizations) } -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) +func (e EnterpriseIDPSync) OrganizationSyncEnabled(ctx context.Context, db database.Store) bool { + if !e.OrganizationSyncEntitled() { + return false } - // 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) - } - } + settings, err := e.OrganizationSyncSettings(ctx, db) + if err == nil && settings.OrganizationField != "" { + return true + } + return false +} - e.Logger.Debug(ctx, "parsed organizations from claim", - slog.F("len", len(parsedOrganizations)), - slog.F("ignored", ignored), - slog.F("organizations", parsedOrganizations), - ) - } +func (e EnterpriseIDPSync) ParseOrganizationClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.OrganizationParams, *idpsync.HTTPError) { + if !e.OrganizationSyncEntitled() { + // Default to agpl if multi-org is not enabled + return e.AGPLIDPSync.ParseOrganizationClaims(ctx, mergedClaims) } 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), + // Return true if entitled + SyncEntitled: true, }, nil } diff --git a/enterprise/coderd/enidpsync/organizations_test.go b/enterprise/coderd/enidpsync/organizations_test.go index 6be2f597e382f..e82c643c65d21 100644 --- a/enterprise/coderd/enidpsync/organizations_test.go +++ b/enterprise/coderd/enidpsync/organizations_test.go @@ -100,7 +100,7 @@ func TestOrganizationSync(t *testing.T) { Name: "NoOrganizations", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: false, + SyncEntitled: false, IncludeDefault: true, Organizations: []uuid.UUID{}, }, @@ -112,7 +112,7 @@ func TestOrganizationSync(t *testing.T) { Name: "AlreadyInOrgs", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: false, + SyncEntitled: false, IncludeDefault: true, Organizations: []uuid.UUID{}, }, @@ -157,7 +157,7 @@ func TestOrganizationSync(t *testing.T) { Name: "NoOrganizations", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: true, + SyncEntitled: true, IncludeDefault: true, Organizations: []uuid.UUID{}, }, @@ -171,7 +171,7 @@ func TestOrganizationSync(t *testing.T) { "organizations": []string{"second", "extra"}, }, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: true, + SyncEntitled: true, IncludeDefault: true, Organizations: []uuid.UUID{two.ID}, }, @@ -196,7 +196,7 @@ func TestOrganizationSync(t *testing.T) { "organizations": []string{"second", "extra", "first", "third", "second", "second"}, }, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: true, + SyncEntitled: true, IncludeDefault: true, Organizations: []uuid.UUID{ two.ID, one.ID, three.ID, @@ -247,7 +247,7 @@ func TestOrganizationSync(t *testing.T) { } require.Nil(t, httpErr, "no parse error") - require.Equal(t, exp.ExpectedParams.SyncEnabled, params.SyncEnabled, "match enabled") + require.Equal(t, exp.ExpectedParams.SyncEntitled, params.SyncEntitled, "match enabled") require.Equal(t, exp.ExpectedParams.IncludeDefault, params.IncludeDefault, "match include default") if exp.ExpectedParams.Organizations == nil { exp.ExpectedParams.Organizations = []uuid.UUID{} From fce98c6463e4c2bcb6d5cd3d8dcf0bef111aaac1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 10:19:16 -0600 Subject: [PATCH 2/9] chore: update organization sync tests --- coderd/idpsync/organization.go | 10 ++ coderd/idpsync/organizations_test.go | 23 ---- enterprise/coderd/enidpsync/organizations.go | 1 + .../coderd/enidpsync/organizations_test.go | 116 ++++++++++++++---- 4 files changed, 100 insertions(+), 50 deletions(-) diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 62c91a71a08a8..4652a15d4e352 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -39,6 +39,16 @@ func (s AGPLIDPSync) AssignDefaultOrganization() bool { return s.OrganizationAssignDefault } +func (s AGPLIDPSync) UpdateOrganizationSettings(ctx context.Context, db database.Store, settings OrganizationSyncSettings) error { + rlv := s.Manager.Resolver(db) + err := s.SyncSettings.Organization.SetRuntimeValue(ctx, rlv, &settings) + if err != nil { + return xerrors.Errorf("update organization sync settings: %w", err) + } + + return nil +} + func (s AGPLIDPSync) OrganizationSyncSettings(ctx context.Context, db database.Store) (*OrganizationSyncSettings, error) { rlv := s.Manager.Resolver(db) orgSettings, err := s.SyncSettings.Organization.Resolve(ctx, rlv) diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 5cecc98b1657a..51c8a7365d22b 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -16,27 +16,6 @@ import ( func TestParseOrganizationClaims(t *testing.T) { t.Parallel() - t.Run("SingleOrgDeployment", func(t *testing.T) { - t.Parallel() - - s := idpsync.NewAGPLSync(slogtest.Make(t, &slogtest.Options{}), - runtimeconfig.NewManager(), - idpsync.DeploymentSyncSettings{ - 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.SyncEntitled) - }) - t.Run("AGPL", func(t *testing.T) { t.Parallel() @@ -56,8 +35,6 @@ func TestParseOrganizationClaims(t *testing.T) { 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.SyncEntitled) }) } diff --git a/enterprise/coderd/enidpsync/organizations.go b/enterprise/coderd/enidpsync/organizations.go index 95919f25613e3..f4b10bab2f701 100644 --- a/enterprise/coderd/enidpsync/organizations.go +++ b/enterprise/coderd/enidpsync/organizations.go @@ -35,5 +35,6 @@ func (e EnterpriseIDPSync) ParseOrganizationClaims(ctx context.Context, mergedCl return idpsync.OrganizationParams{ // Return true if entitled SyncEntitled: true, + MergedClaims: mergedClaims, }, nil } diff --git a/enterprise/coderd/enidpsync/organizations_test.go b/enterprise/coderd/enidpsync/organizations_test.go index e82c643c65d21..22dd3a2c4c7d7 100644 --- a/enterprise/coderd/enidpsync/organizations_test.go +++ b/enterprise/coderd/enidpsync/organizations_test.go @@ -34,17 +34,19 @@ type Expectations struct { Name string Claims jwt.MapClaims // Parse - ParseError func(t *testing.T, httpErr *idpsync.HTTPError) - ExpectedParams idpsync.OrganizationParams + ParseError func(t *testing.T, httpErr *idpsync.HTTPError) + ExpectedParams idpsync.OrganizationParams + ExpectedEnabled bool // 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.DeploymentSyncSettings - Entitlements *entitlements.Set - Exps []Expectations + Settings idpsync.DeploymentSyncSettings + RuntimeSettings *idpsync.OrganizationSyncSettings + Entitlements *entitlements.Set + Exps []Expectations } func TestOrganizationSync(t *testing.T) { @@ -100,10 +102,9 @@ func TestOrganizationSync(t *testing.T) { Name: "NoOrganizations", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEntitled: false, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: true, }, + ExpectedEnabled: false, Sync: ExpectedUser{ Organizations: []uuid.UUID{}, }, @@ -112,10 +113,9 @@ func TestOrganizationSync(t *testing.T) { Name: "AlreadyInOrgs", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEntitled: false, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: true, }, + ExpectedEnabled: false, Mutate: func(t *testing.T, db database.Store, user database.User) { dbgen.OrganizationMember(t, db, database.OrganizationMember{ UserID: user.ID, @@ -157,10 +157,9 @@ func TestOrganizationSync(t *testing.T) { Name: "NoOrganizations", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEntitled: true, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: true, }, + ExpectedEnabled: true, Sync: ExpectedUser{ Organizations: []uuid.UUID{def.ID}, }, @@ -171,10 +170,9 @@ func TestOrganizationSync(t *testing.T) { "organizations": []string{"second", "extra"}, }, ExpectedParams: idpsync.OrganizationParams{ - SyncEntitled: true, - IncludeDefault: true, - Organizations: []uuid.UUID{two.ID}, + SyncEntitled: true, }, + ExpectedEnabled: true, Mutate: func(t *testing.T, db database.Store, user database.User) { dbgen.OrganizationMember(t, db, database.OrganizationMember{ UserID: user.ID, @@ -196,12 +194,9 @@ func TestOrganizationSync(t *testing.T) { "organizations": []string{"second", "extra", "first", "third", "second", "second"}, }, ExpectedParams: idpsync.OrganizationParams{ - SyncEntitled: true, - IncludeDefault: true, - Organizations: []uuid.UUID{ - two.ID, one.ID, three.ID, - }, + SyncEntitled: true, }, + ExpectedEnabled: true, Mutate: func(t *testing.T, db database.Store, user database.User) { dbgen.OrganizationMember(t, db, database.OrganizationMember{ UserID: user.ID, @@ -220,6 +215,72 @@ func TestOrganizationSync(t *testing.T) { } }, }, + { + Name: "DynamicSettings", + 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.DeploymentSyncSettings{ + OrganizationField: "organizations", + OrganizationMapping: map[string][]uuid.UUID{ + "first": {one.ID}, + "second": {two.ID}, + "third": {three.ID}, + }, + OrganizationAssignDefault: true, + }, + // Override + RuntimeSettings: &idpsync.OrganizationSyncSettings{ + OrganizationField: "dynamic", + OrganizationMapping: map[string][]uuid.UUID{ + "third": {three.ID}, + }, + OrganizationAssignDefault: false, + }, + Exps: []Expectations{ + { + Name: "NoOrganizations", + Claims: jwt.MapClaims{}, + ExpectedParams: idpsync.OrganizationParams{ + SyncEntitled: true, + }, + ExpectedEnabled: true, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{}, + }, + }, + { + Name: "AlreadyInOrgs", + Claims: jwt.MapClaims{ + "organizations": []string{"second", "extra"}, + "dynamic": []string{"third"}, + }, + ExpectedParams: idpsync.OrganizationParams{ + SyncEntitled: true, + }, + ExpectedEnabled: true, + 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{three.ID}, + }, + }, + }, + } + }, + }, } for _, tc := range testCases { @@ -238,6 +299,11 @@ func TestOrganizationSync(t *testing.T) { // Create a new sync object sync := enidpsync.NewSync(logger, runtimeconfig.NewManager(), caseData.Entitlements, caseData.Settings) + if caseData.RuntimeSettings != nil { + err := sync.UpdateOrganizationSettings(ctx, rdb, *caseData.RuntimeSettings) + require.NoError(t, err) + } + for _, exp := range caseData.Exps { t.Run(exp.Name, func(t *testing.T) { params, httpErr := sync.ParseOrganizationClaims(ctx, exp.Claims) @@ -248,11 +314,7 @@ func TestOrganizationSync(t *testing.T) { require.Nil(t, httpErr, "no parse error") require.Equal(t, exp.ExpectedParams.SyncEntitled, params.SyncEntitled, "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") + require.Equal(t, exp.ExpectedEnabled, sync.OrganizationSyncEnabled(context.Background(), rdb)) user := dbgen.User(t, db, database.User{}) if exp.Mutate != nil { From 4c855047071f60cc2ff8b461350c0ef62ce0218e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 10:57:26 -0600 Subject: [PATCH 3/9] chore: make gen & golden files --- cli/testdata/coder_server_--help.golden | 13 -------- coderd/members.go | 16 ++++++---- docs/reference/cli/server.md | 32 ------------------- .../cli/testdata/coder_server_--help.golden | 13 -------- 4 files changed, 9 insertions(+), 65 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index cd647d0537a93..7e4088ccb6212 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -506,11 +506,6 @@ 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. @@ -557,14 +552,6 @@ 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/coderd/members.go b/coderd/members.go index b4d0d7f6374c9..97950b19e9137 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -100,7 +100,6 @@ func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request apiKey = httpmw.APIKey(r) organization = httpmw.OrganizationParam(r) member = httpmw.OrganizationMemberParam(r) - user = httpmw.UserParam(r) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationMember](rw, &audit.RequestParams{ OrganizationID: organization.ID, @@ -113,9 +112,13 @@ func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request aReq.Old = member.OrganizationMember.Auditable(member.Username) defer commitAudit() - if !api.manualOrganizationMembership(ctx, rw, user) { - return - } + // Note: we disallow adding OIDC users if organization sync is enabled. + // For removing members, do not have this same enforcement. As long as a user + // does not re-login, they will not be immediately removed from the organization. + // There might be an urgent need to revoke access. + // A user can re-login if they are removed in error. + // If we add a feature to force logout a user, then we can prevent manual + // member removal when organization sync is enabled, and use force logout instead. if member.UserID == apiKey.UserID { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{Message: "cannot remove self from an organization"}) @@ -273,7 +276,7 @@ func (api *API) allowChangingMemberRoles(ctx context.Context, rw http.ResponseWr } if orgSync { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Cannot modify roles for OIDC users when role sync is enabled. This organization member's roles are managed by the identity provider.", + Message: "Cannot modify roles for OIDC users when role sync is enabled. This organization member's roles are managed by the identity provider. Have the user re-login to refresh their roles.", Detail: "'User Role Field' is set in the organization settings. Ask an administrator to adjust or disable these settings.", }) return false @@ -377,11 +380,10 @@ func convertOrganizationMembersWithUserData(ctx context.Context, db database.Sto // manualOrganizationMembership checks if the user is an OIDC user and if organization sync is enabled. // If organization sync is enabled, manual organization assignment is not allowed, // since all organization membership is controlled by the external IDP. -// TODO: Might be helpful to have an ignore list of organizations that can be manually assigned. func (api *API) manualOrganizationMembership(ctx context.Context, rw http.ResponseWriter, user database.User) bool { if user.LoginType == database.LoginTypeOIDC && api.IDPSync.OrganizationSyncEnabled(ctx, api.Database) { 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.", + Message: "Organization sync is enabled for OIDC users, meaning manual organization assignment is not allowed for this user. Have the user re-login to refresh their organizations.", Detail: fmt.Sprintf("User %s is an OIDC user and organization sync is enabled. Ask an administrator to resolve the membership in your external IDP.", user.Username), }) return false diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 42ef7f7418b45..3b3d2376c9aab 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -559,38 +559,6 @@ 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 a6398586fa972..aaa4725c65181 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -507,11 +507,6 @@ 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. @@ -558,14 +553,6 @@ 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 From 88937d46fe668650afba04c176bc5749445a50de Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 11:02:34 -0600 Subject: [PATCH 4/9] linting fmt vs xerrors --- coderd/idpsync/organization.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 4652a15d4e352..6c9bb9191b302 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -4,7 +4,6 @@ import ( "context" "database/sql" "encoding/json" - "fmt" "github.com/golang-jwt/jwt/v4" "github.com/google/uuid" @@ -201,7 +200,7 @@ func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database. parsedOrganizations, err := ParseStringSliceClaim(organizationRaw) if err != nil { - return userOrganizations, fmt.Errorf("failed to parese organizations OIDC claims: %w", err) + return userOrganizations, xerrors.Errorf("failed to parese organizations OIDC claims: %w", err) } // add any mapped organizations From d1d041904cd79f8dc24642af1b4b954c2976ea2c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 12:03:07 -0600 Subject: [PATCH 5/9] chore: add endpoint to mutate org sync settings --- coderd/idpsync/idpsync.go | 3 +- coderd/idpsync/organization.go | 30 +++-- coderd/rbac/roles.go | 2 + coderd/rbac/roles_test.go | 19 +++- codersdk/idpsync.go | 40 +++++++ enterprise/coderd/coderd.go | 10 ++ enterprise/coderd/enidpsync/organizations.go | 2 +- .../coderd/enidpsync/organizations_test.go | 6 +- enterprise/coderd/idpsync.go | 103 +++++++++++++++++- enterprise/coderd/scim.go | 8 +- enterprise/coderd/userauth_test.go | 67 ++++++++---- site/src/api/typesGenerated.ts | 6 + 12 files changed, 242 insertions(+), 54 deletions(-) diff --git a/coderd/idpsync/idpsync.go b/coderd/idpsync/idpsync.go index 18f13df99631e..e936bada73752 100644 --- a/coderd/idpsync/idpsync.go +++ b/coderd/idpsync/idpsync.go @@ -24,8 +24,9 @@ import ( // claims to the internal representation of a user in Coder. // TODO: Move group + role sync into this interface. type IDPSync interface { - AssignDefaultOrganization() bool OrganizationSyncEntitled() bool + OrganizationSyncSettings(ctx context.Context, db database.Store) (*OrganizationSyncSettings, error) + UpdateOrganizationSettings(ctx context.Context, db database.Store, settings OrganizationSyncSettings) error // OrganizationSyncEnabled returns true if all OIDC users are assigned // to organizations via org sync settings. // This is used to know when to disable manual org membership assignment. diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 6c9bb9191b302..01f5dcb251b9e 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -34,10 +34,6 @@ func (AGPLIDPSync) OrganizationSyncEnabled(_ context.Context, _ database.Store) return false } -func (s AGPLIDPSync) AssignDefaultOrganization() bool { - return s.OrganizationAssignDefault -} - func (s AGPLIDPSync) UpdateOrganizationSettings(ctx context.Context, db database.Store, settings OrganizationSyncSettings) error { rlv := s.Manager.Resolver(db) err := s.SyncSettings.Organization.SetRuntimeValue(ctx, rlv, &settings) @@ -62,9 +58,9 @@ func (s AGPLIDPSync) OrganizationSyncSettings(ctx context.Context, db database.S if s.DeploymentSyncSettings.OrganizationField != "" { // Use static settings if set orgSettings = &OrganizationSyncSettings{ - OrganizationField: s.DeploymentSyncSettings.OrganizationField, - OrganizationMapping: s.DeploymentSyncSettings.OrganizationMapping, - OrganizationAssignDefault: s.DeploymentSyncSettings.OrganizationAssignDefault, + Field: s.DeploymentSyncSettings.OrganizationField, + Mapping: s.DeploymentSyncSettings.OrganizationMapping, + AssignDefault: s.DeploymentSyncSettings.OrganizationAssignDefault, } } } @@ -92,7 +88,7 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u return xerrors.Errorf("failed to get org sync settings: %w", err) } - if orgSettings.OrganizationField == "" { + if orgSettings.Field == "" { return nil // No sync configured, nothing to do } @@ -156,16 +152,16 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u } type OrganizationSyncSettings struct { - // OrganizationField selects the claim field to be used as the created user's + // Field 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 + Field string + // Mapping controls how organizations returned by the OIDC provider get mapped + Mapping map[string][]uuid.UUID + // AssignDefault will ensure all users that authenticate will be // placed into the default organization. This is mostly a hack to support // legacy deployments. - OrganizationAssignDefault bool + AssignDefault bool } func (s *OrganizationSyncSettings) Set(v string) error { @@ -181,7 +177,7 @@ func (s *OrganizationSyncSettings) String() string { func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database.Store, mergedClaims jwt.MapClaims) ([]uuid.UUID, error) { userOrganizations := make([]uuid.UUID, 0) - if s.OrganizationAssignDefault { + if s.AssignDefault { // This is a bit hacky, but if AssignDefault is included, then always // make sure to include the default org in the list of expected. defaultOrg, err := db.GetDefaultOrganization(ctx) @@ -193,7 +189,7 @@ func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database. userOrganizations = append(userOrganizations, defaultOrg.ID) } - organizationRaw, ok := mergedClaims[s.OrganizationField] + organizationRaw, ok := mergedClaims[s.Field] if !ok { return userOrganizations, nil } @@ -205,7 +201,7 @@ func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database. // add any mapped organizations for _, parsedOrg := range parsedOrganizations { - if mappedOrganization, ok := s.OrganizationMapping[parsedOrg]; ok { + if mappedOrganization, ok := s.Mapping[parsedOrg]; ok { // parsedOrg is in the mapping, so add the mapped organizations to the // user's organizations. userOrganizations = append(userOrganizations, mappedOrganization...) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 14700500266a1..a57bd071a8052 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -352,6 +352,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroup.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroupMember.Type: {policy.ActionRead}, + // Manage org membership based on OIDC claims + ResourceIdpsyncSettings.Type: {policy.ActionRead, policy.ActionUpdate}, }), Org: map[string][]Permission{}, User: []Permission{}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 954b5e9788c53..0172439829063 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -733,10 +733,25 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceIdpsyncSettings.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgUserAdmin}, + true: {owner, orgAdmin, orgUserAdmin, userAdmin}, false: { orgMemberMe, otherOrgAdmin, - memberMe, userAdmin, templateAdmin, + memberMe, templateAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + }, + }, + }, + { + Name: "OrganizationIDPSyncSettings", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceIdpsyncSettings, + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, userAdmin}, + false: { + orgAdmin, orgUserAdmin, + orgMemberMe, otherOrgAdmin, + memberMe, templateAdmin, orgAuditor, orgTemplateAdmin, otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, diff --git a/codersdk/idpsync.go b/codersdk/idpsync.go index 380b26336ad90..0226dc7f9eb5f 100644 --- a/codersdk/idpsync.go +++ b/codersdk/idpsync.go @@ -97,3 +97,43 @@ func (c *Client) PatchRoleIDPSyncSettings(ctx context.Context, orgID string, req var resp RoleSyncSettings return resp, json.NewDecoder(res.Body).Decode(&resp) } + +type OrganizationSyncSettings struct { + // Field 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. + Field string `json:"field"` + // Mapping maps from an OIDC claim --> Coder organization uuid + Mapping map[string][]uuid.UUID `json:"mapping"` + // AssignDefault will ensure the default org is always included + // for every user, regardless of their claims. This preserves legacy behavior. + AssignDefault bool `json:"organization_assign_default"` +} + +func (c *Client) OrganizationIDPSyncSettings(ctx context.Context) (OrganizationSyncSettings, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/settings/idpsync/organization", nil) + if err != nil { + return OrganizationSyncSettings{}, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return OrganizationSyncSettings{}, ReadBodyAsError(res) + } + var resp OrganizationSyncSettings + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + +func (c *Client) PatchOrganizationIDPSyncSettings(ctx context.Context, req OrganizationSyncSettings) (OrganizationSyncSettings, error) { + res, err := c.Request(ctx, http.MethodPatch, "/api/v2/settings/idpsync/organization", req) + if err != nil { + return OrganizationSyncSettings{}, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return OrganizationSyncSettings{}, ReadBodyAsError(res) + } + var resp OrganizationSyncSettings + return resp, json.NewDecoder(res.Body).Decode(&resp) +} diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index dddf619b34058..03d535f6ffb69 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -287,6 +287,16 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { r.Delete("/organizations/{organization}/members/roles/{roleName}", api.deleteOrgRole) }) + r.Group(func(r chi.Router) { + r.Use( + apiKeyMiddleware, + ) + r.Route("/settings/idpsync/organization", func(r chi.Router) { + r.Get("/", api.organizationIDPSyncSettings) + r.Patch("/", api.patchOrganizationIDPSyncSettings) + }) + }) + r.Group(func(r chi.Router) { r.Use( apiKeyMiddleware, diff --git a/enterprise/coderd/enidpsync/organizations.go b/enterprise/coderd/enidpsync/organizations.go index f4b10bab2f701..313d90fac8a9f 100644 --- a/enterprise/coderd/enidpsync/organizations.go +++ b/enterprise/coderd/enidpsync/organizations.go @@ -20,7 +20,7 @@ func (e EnterpriseIDPSync) OrganizationSyncEnabled(ctx context.Context, db datab } settings, err := e.OrganizationSyncSettings(ctx, db) - if err == nil && settings.OrganizationField != "" { + if err == nil && settings.Field != "" { return true } return false diff --git a/enterprise/coderd/enidpsync/organizations_test.go b/enterprise/coderd/enidpsync/organizations_test.go index 22dd3a2c4c7d7..36dbedf3a466d 100644 --- a/enterprise/coderd/enidpsync/organizations_test.go +++ b/enterprise/coderd/enidpsync/organizations_test.go @@ -235,11 +235,11 @@ func TestOrganizationSync(t *testing.T) { }, // Override RuntimeSettings: &idpsync.OrganizationSyncSettings{ - OrganizationField: "dynamic", - OrganizationMapping: map[string][]uuid.UUID{ + Field: "dynamic", + Mapping: map[string][]uuid.UUID{ "third": {three.ID}, }, - OrganizationAssignDefault: false, + AssignDefault: false, }, Exps: []Expectations{ { diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index 096209ffe8292..ece41112d5f56 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -46,6 +46,7 @@ func (api *API) groupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) +// @Param request body codersdk.GroupSyncSettings true // @Success 200 {object} codersdk.GroupSyncSettings // @Router /organizations/{organization}/settings/idpsync/groups [patch] func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { @@ -57,7 +58,7 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques return } - var req idpsync.GroupSyncSettings + var req codersdk.GroupSyncSettings if !httpapi.Read(ctx, rw, r, &req) { return } @@ -78,7 +79,13 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques //nolint:gocritic // Requires system context to update runtime config sysCtx := dbauthz.AsSystemRestricted(ctx) - err := api.IDPSync.UpdateGroupSettings(sysCtx, org.ID, api.Database, req) + err := api.IDPSync.UpdateGroupSettings(sysCtx, org.ID, api.Database, idpsync.GroupSyncSettings{ + Field: req.Field, + Mapping: req.Mapping, + RegexFilter: req.RegexFilter, + AutoCreateMissing: req.AutoCreateMissing, + LegacyNameMapping: req.LegacyNameMapping, + }) if err != nil { httpapi.InternalServerError(rw, err) return @@ -90,7 +97,13 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques return } - httpapi.Write(ctx, rw, http.StatusOK, settings) + httpapi.Write(ctx, rw, http.StatusOK, codersdk.GroupSyncSettings{ + Field: settings.Field, + Mapping: settings.Mapping, + RegexFilter: settings.RegexFilter, + AutoCreateMissing: settings.AutoCreateMissing, + LegacyNameMapping: settings.LegacyNameMapping, + }) } // @Summary Get role IdP Sync settings by organization @@ -127,6 +140,7 @@ func (api *API) roleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) +// @Param request body codersdk.RoleSyncSettings true // @Success 200 {object} codersdk.RoleSyncSettings // @Router /organizations/{organization}/settings/idpsync/roles [patch] func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { @@ -138,14 +152,17 @@ func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request return } - var req idpsync.RoleSyncSettings + var req codersdk.RoleSyncSettings if !httpapi.Read(ctx, rw, r, &req) { return } //nolint:gocritic // Requires system context to update runtime config sysCtx := dbauthz.AsSystemRestricted(ctx) - err := api.IDPSync.UpdateRoleSettings(sysCtx, org.ID, api.Database, req) + err := api.IDPSync.UpdateRoleSettings(sysCtx, org.ID, api.Database, idpsync.RoleSyncSettings{ + Field: req.Field, + Mapping: req.Mapping, + }) if err != nil { httpapi.InternalServerError(rw, err) return @@ -157,5 +174,81 @@ func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request return } + httpapi.Write(ctx, rw, http.StatusOK, codersdk.RoleSyncSettings{ + Field: settings.Field, + Mapping: settings.Mapping, + }) +} + +// @Summary Get organization IdP Sync settings +// @ID get-organization-idp-sync-settings +// @Security CoderSessionToken +// @Produce json +// @Tags Enterprise +// @Success 200 {object} codersdk.OrganizationSyncSettings +// @Router /settings/idpsync/organization [get] +func (api *API) organizationIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + if !api.Authorize(r, policy.ActionRead, rbac.ResourceIdpsyncSettings) { + httpapi.Forbidden(rw) + return + } + + //nolint:gocritic // Requires system context to read runtime config + sysCtx := dbauthz.AsSystemRestricted(ctx) + settings, err := api.IDPSync.OrganizationSyncSettings(sysCtx, api.Database) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + httpapi.Write(ctx, rw, http.StatusOK, settings) } + +// @Summary Update organization IdP Sync settings +// @ID update-organization-idp-sync-settings +// @Security CoderSessionToken +// @Produce json +// @Tags Enterprise +// @Success 200 {object} codersdk.OrganizationSyncSettings +// @Param request body codersdk.OrganizationSyncSettings true +// @Router /settings/idpsync/organization [patch] +func (api *API) patchOrganizationIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + if !api.Authorize(r, policy.ActionUpdate, rbac.ResourceIdpsyncSettings) { + httpapi.Forbidden(rw) + return + } + + var req codersdk.OrganizationSyncSettings + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + //nolint:gocritic // Requires system context to update runtime config + sysCtx := dbauthz.AsSystemRestricted(ctx) + err := api.IDPSync.UpdateOrganizationSettings(sysCtx, api.Database, idpsync.OrganizationSyncSettings{ + Field: req.Field, + // We do not check if the mappings point to actual organizations. + Mapping: req.Mapping, + AssignDefault: req.AssignDefault, + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + settings, err := api.IDPSync.OrganizationSyncSettings(sysCtx, api.Database) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, codersdk.OrganizationSyncSettings{ + Field: settings.Field, + Mapping: settings.Mapping, + AssignDefault: settings.AssignDefault, + }) +} diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index 439e6ca3225de..01d04626a6948 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -281,7 +281,13 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { // the default org, regardless if sync is enabled or not. // This is to preserve single org deployment behavior. organizations := []uuid.UUID{} - if api.IDPSync.AssignDefaultOrganization() { + //nolint:gocritic // SCIM operations are a system user + orgSync, err := api.IDPSync.OrganizationSyncSettings(dbauthz.AsSystemRestricted(ctx), api.Database) + if err != nil { + _ = handlerutil.WriteError(rw, xerrors.Errorf("failed to get organization sync settings: %w", err)) + return + } + if orgSync.AssignDefault { //nolint:gocritic // SCIM operations are a system user defaultOrganization, err := api.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) if err != nil { diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 538904cd5b428..af3cbe782bf03 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -102,62 +102,81 @@ func TestUserOIDC(t *testing.T) { 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 + // Will be overwritten by dynamic value + 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}, - }, + Value: map[string][]uuid.UUID{}, } }, }) - dbgen.Organization(t, runner.API.Database, database.Organization{ - ID: second, + + ctx := testutil.Context(t, testutil.WaitMedium) + orgOne, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "one", + DisplayName: "One", + Description: "", + Icon: "", }) - dbgen.Organization(t, runner.API.Database, database.Organization{ - ID: third, + require.NoError(t, err) + + orgTwo, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "two", + DisplayName: "two", + Description: "", + Icon: "", }) - fourth := dbgen.Organization(t, runner.API.Database, database.Organization{}) + require.NoError(t, err) + + orgThree, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "three", + DisplayName: "three", + }) + require.NoError(t, err) + + expectedSettings := codersdk.OrganizationSyncSettings{ + Field: "organization", + Mapping: map[string][]uuid.UUID{ + "first": {orgOne.ID}, + "second": {orgTwo.ID}, + }, + AssignDefault: true, + } + settings, err := runner.AdminClient.PatchOrganizationIDPSyncSettings(ctx, expectedSettings) + require.NoError(t, err) + require.Equal(t, expectedSettings.Field, settings.Field) - ctx := testutil.Context(t, testutil.WaitMedium) claims := jwt.MapClaims{ "email": "alice@coder.com", - "organization": []string{"second", "third"}, + "organization": []string{"first", "second"}, } // 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}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgOne.ID, orgTwo.ID}) 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") + _, err = runner.AdminClient.PostOrganizationMember(ctx, orgThree.ID, "alice") require.ErrorContains(t, err, "Organization sync is enabled") - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgOne.ID, orgTwo.ID}) // 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, + OrganizationID: orgThree.ID, }) - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third, fourth.ID}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgOne.ID, orgTwo.ID, orgThree.ID}) // Then: Log in again will resync the orgs to their updated // claims. @@ -165,7 +184,7 @@ func TestUserOIDC(t *testing.T) { "email": "alice@coder.com", "organization": []string{"third"}, }) - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{third}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgThree.ID}) }) t.Run("MultiOrgWithoutDefault", func(t *testing.T) { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index f89d9eada822c..bf8cc91b73ec3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -974,6 +974,12 @@ export interface OrganizationMemberWithUserData extends OrganizationMember { readonly global_roles: Readonly>; } +// From codersdk/idpsync.go +export interface OrganizationSyncSettings { + readonly field: string; + readonly mapping: Record>>; +} + // From codersdk/pagination.go export interface Pagination { readonly after_id?: string; From d4644d49bda7c173ba41f38247450c52669ea827 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 14:55:16 -0600 Subject: [PATCH 6/9] fixup org settings perms --- coderd/database/dbauthz/dbauthz.go | 5 ++--- coderd/idpsync/organization.go | 11 +++++------ enterprise/coderd/userauth_test.go | 4 ++-- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c855d5a1984df..9afa8afcb01d9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -33,9 +33,8 @@ var _ database.Store = (*querier)(nil) const wrapname = "dbauthz.querier" -// NoActorError wraps ErrNoRows for the api to return a 404. This is the correct -// response when the user is not authorized. -var NoActorError = xerrors.Errorf("no authorization actor in context: %w", sql.ErrNoRows) +// NoActorError is returned if no actor is present in the context. +var NoActorError = xerrors.Errorf("no authorization actor in context") // NotAuthorizedError is a sentinel error that unwraps to sql.ErrNoRows. // This allows the internal error to be read by the caller if needed. Otherwise diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 01f5dcb251b9e..2ac47838e0aee 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -83,6 +83,9 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u return nil } + // nolint:gocritic // all syncing is done as a system user + ctx = dbauthz.AsSystemRestricted(ctx) + orgSettings, err := s.OrganizationSyncSettings(ctx, tx) if err != nil { return xerrors.Errorf("failed to get org sync settings: %w", err) @@ -92,8 +95,6 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u return nil // No sync configured, nothing to do } - // nolint:gocritic // all syncing is done as a system user - ctx = dbauthz.AsSystemRestricted(ctx) expectedOrgs, err := orgSettings.ParseClaims(ctx, tx, params.MergedClaims) if err != nil { return xerrors.Errorf("organization claims: %w", err) @@ -113,8 +114,7 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u add, remove := slice.SymmetricDifference(existingOrgIDs, expectedOrgs) 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{ + _, err := tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{ OrganizationID: orgID, UserID: user.ID, CreatedAt: dbtime.Now(), @@ -131,8 +131,7 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u } for _, orgID := range remove { - //nolint:gocritic // System actor being used to assign orgs - err := tx.DeleteOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.DeleteOrganizationMemberParams{ + err := tx.DeleteOrganizationMember(ctx, database.DeleteOrganizationMemberParams{ OrganizationID: orgID, UserID: user.ID, }) diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index af3cbe782bf03..47ed424055ece 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -182,9 +182,9 @@ func TestUserOIDC(t *testing.T) { // claims. runner.Login(t, jwt.MapClaims{ "email": "alice@coder.com", - "organization": []string{"third"}, + "organization": []string{"second"}, }) - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgThree.ID}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgTwo.ID}) }) t.Run("MultiOrgWithoutDefault", func(t *testing.T) { From cb3c69ded216e5168df5109033ed9ca6d6cfb884 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 15:16:17 -0600 Subject: [PATCH 7/9] make gen --- coderd/apidoc/docs.go | 100 ++++++++++++++++++++++ coderd/apidoc/swagger.json | 92 +++++++++++++++++++++ docs/reference/api/enterprise.md | 138 +++++++++++++++++++++++++++++-- docs/reference/api/schemas.md | 22 +++++ enterprise/coderd/idpsync.go | 6 +- site/src/api/typesGenerated.ts | 1 + 6 files changed, 350 insertions(+), 9 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 6c770c18232ac..f2feb01b09e03 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3182,6 +3182,15 @@ const docTemplate = `{ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.GroupSyncSettings" + } } ], "responses": { @@ -3250,6 +3259,15 @@ const docTemplate = `{ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.RoleSyncSettings" + } } ], "responses": { @@ -3770,6 +3788,65 @@ const docTemplate = `{ } } }, + "/settings/idpsync/organization": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Get organization IdP Sync settings", + "operationId": "get-organization-idp-sync-settings", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + }, + "patch": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Update organization IdP Sync settings", + "operationId": "update-organization-idp-sync-settings", + "parameters": [ + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + } + }, "/tailnet": { "get": { "security": [ @@ -11807,6 +11884,29 @@ const docTemplate = `{ } } }, + "codersdk.OrganizationSyncSettings": { + "type": "object", + "properties": { + "field": { + "description": "Field selects the claim field to be used as the created user's\norganizations. If the field is the empty string, then no organization\nupdates will ever come from the OIDC provider.", + "type": "string" + }, + "mapping": { + "description": "Mapping maps from an OIDC claim --\u003e Coder organization uuid", + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "organization_assign_default": { + "description": "AssignDefault will ensure the default org is always included\nfor every user, regardless of their claims. This preserves legacy behavior.", + "type": "boolean" + } + } + }, "codersdk.PatchGroupRequest": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 4f5ca444f703e..ce15c11d53ece 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2796,6 +2796,15 @@ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.GroupSyncSettings" + } } ], "responses": { @@ -2856,6 +2865,15 @@ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.RoleSyncSettings" + } } ], "responses": { @@ -3316,6 +3334,57 @@ } } }, + "/settings/idpsync/organization": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Get organization IdP Sync settings", + "operationId": "get-organization-idp-sync-settings", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + }, + "patch": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Update organization IdP Sync settings", + "operationId": "update-organization-idp-sync-settings", + "parameters": [ + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + } + }, "/tailnet": { "get": { "security": [ @@ -10641,6 +10710,29 @@ } } }, + "codersdk.OrganizationSyncSettings": { + "type": "object", + "properties": { + "field": { + "description": "Field selects the claim field to be used as the created user's\norganizations. If the field is the empty string, then no organization\nupdates will ever come from the OIDC provider.", + "type": "string" + }, + "mapping": { + "description": "Mapping maps from an OIDC claim --\u003e Coder organization uuid", + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "organization_assign_default": { + "description": "AssignDefault will ensure the default org is always included\nfor every user, regardless of their claims. This preserves legacy behavior.", + "type": "boolean" + } + } + }, "codersdk.PatchGroupRequest": { "type": "object", "properties": { diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index 57ffa5260edde..17f55afcddd81 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -1831,17 +1831,37 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/settings/idpsync/groups \ + -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` `PATCH /organizations/{organization}/settings/idpsync/groups` +> Body parameter + +```json +{ + "auto_create_missing_groups": true, + "field": "string", + "legacy_group_name_mapping": { + "property1": "string", + "property2": "string" + }, + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "regex_filter": {} +} +``` + ### Parameters -| Name | In | Type | Required | Description | -| -------------- | ---- | ------------ | -------- | --------------- | -| `organization` | path | string(uuid) | true | Organization ID | +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------------------------------------------------------------ | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.GroupSyncSettings](schemas.md#codersdkgroupsyncsettings) | true | New settings | ### Example responses @@ -1919,17 +1939,31 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/settings/idpsync/roles \ + -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` `PATCH /organizations/{organization}/settings/idpsync/roles` +> Body parameter + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + } +} +``` + ### Parameters -| Name | In | Type | Required | Description | -| -------------- | ---- | ------------ | -------- | --------------- | -| `organization` | path | string(uuid) | true | Organization ID | +| Name | In | Type | Required | Description | +| -------------- | ---- | ---------------------------------------------------------------- | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.RoleSyncSettings](schemas.md#codersdkrolesyncsettings) | true | New settings | ### Example responses @@ -2239,6 +2273,98 @@ curl -X PATCH http://coder-server:8080/api/v2/scim/v2/Users/{id} \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get organization IdP Sync settings + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/settings/idpsync/organization \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /settings/idpsync/organization` + +### Example responses + +> 200 Response + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OrganizationSyncSettings](schemas.md#codersdkorganizationsyncsettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Update organization IdP Sync settings + +### Code samples + +```shell +# Example request using curl +curl -X PATCH http://coder-server:8080/api/v2/settings/idpsync/organization \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PATCH /settings/idpsync/organization` + +> Body parameter + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | -------------------------------------------------------------------------------- | -------- | ------------ | +| `body` | body | [codersdk.OrganizationSyncSettings](schemas.md#codersdkorganizationsyncsettings) | true | New settings | + +### Example responses + +> 200 Response + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OrganizationSyncSettings](schemas.md#codersdkorganizationsyncsettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get template ACLs ### Code samples diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index dab7703345b08..fe8db822aafb5 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3941,6 +3941,28 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `user_id` | string | false | | | | `username` | string | false | | | +## codersdk.OrganizationSyncSettings + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----------------------------- | --------------- | -------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `field` | string | false | | Field 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. | +| `mapping` | object | false | | Mapping maps from an OIDC claim --> Coder organization uuid | +| ยป `[any property]` | array of string | false | | | +| `organization_assign_default` | boolean | false | | Organization assign default will ensure the default org is always included for every user, regardless of their claims. This preserves legacy behavior. | + ## codersdk.PatchGroupRequest ```json diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index ece41112d5f56..e3439bcb25c54 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -46,7 +46,7 @@ func (api *API) groupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) -// @Param request body codersdk.GroupSyncSettings true +// @Param request body codersdk.GroupSyncSettings true "New settings" // @Success 200 {object} codersdk.GroupSyncSettings // @Router /organizations/{organization}/settings/idpsync/groups [patch] func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { @@ -140,7 +140,7 @@ func (api *API) roleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) -// @Param request body codersdk.RoleSyncSettings true +// @Param request body codersdk.RoleSyncSettings true "New settings" // @Success 200 {object} codersdk.RoleSyncSettings // @Router /organizations/{organization}/settings/idpsync/roles [patch] func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { @@ -212,7 +212,7 @@ func (api *API) organizationIDPSyncSettings(rw http.ResponseWriter, r *http.Requ // @Produce json // @Tags Enterprise // @Success 200 {object} codersdk.OrganizationSyncSettings -// @Param request body codersdk.OrganizationSyncSettings true +// @Param request body codersdk.OrganizationSyncSettings true "New settings" // @Router /settings/idpsync/organization [patch] func (api *API) patchOrganizationIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index bf8cc91b73ec3..bd00dbda353c3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -978,6 +978,7 @@ export interface OrganizationMemberWithUserData extends OrganizationMember { export interface OrganizationSyncSettings { readonly field: string; readonly mapping: Record>>; + readonly organization_assign_default: boolean; } // From codersdk/pagination.go From 4b1248c217e18ef0c2912034c17972ba76f4118e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 15:27:38 -0600 Subject: [PATCH 8/9] make gen --- coderd/apidoc/docs.go | 9 +++++++++ coderd/apidoc/swagger.json | 3 +++ enterprise/coderd/idpsync.go | 3 +++ 3 files changed, 15 insertions(+) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f2feb01b09e03..983abb61169c9 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3166,6 +3166,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], @@ -3243,6 +3246,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], @@ -3818,6 +3824,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index ce15c11d53ece..67cc92c71331d 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2784,6 +2784,7 @@ "CoderSessionToken": [] } ], + "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Enterprise"], "summary": "Update group IdP Sync settings by organization", @@ -2853,6 +2854,7 @@ "CoderSessionToken": [] } ], + "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Enterprise"], "summary": "Update role IdP Sync settings by organization", @@ -3360,6 +3362,7 @@ "CoderSessionToken": [] } ], + "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Enterprise"], "summary": "Update organization IdP Sync settings", diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index e3439bcb25c54..a3e2e184e9598 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -44,6 +44,7 @@ func (api *API) groupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @ID update-group-idp-sync-settings-by-organization // @Security CoderSessionToken // @Produce json +// @Accept json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) // @Param request body codersdk.GroupSyncSettings true "New settings" @@ -138,6 +139,7 @@ func (api *API) roleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @ID update-role-idp-sync-settings-by-organization // @Security CoderSessionToken // @Produce json +// @Accept json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) // @Param request body codersdk.RoleSyncSettings true "New settings" @@ -210,6 +212,7 @@ func (api *API) organizationIDPSyncSettings(rw http.ResponseWriter, r *http.Requ // @ID update-organization-idp-sync-settings // @Security CoderSessionToken // @Produce json +// @Accept json // @Tags Enterprise // @Success 200 {object} codersdk.OrganizationSyncSettings // @Param request body codersdk.OrganizationSyncSettings true "New settings" From 71daa455e6c9f13fad9247d5a7e5ac09c9ed1f92 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Nov 2024 15:40:17 -0600 Subject: [PATCH 9/9] Fix default org sync behavior --- coderd/idpsync/organization.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 2ac47838e0aee..66d8ab08495cc 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -52,16 +52,11 @@ func (s AGPLIDPSync) OrganizationSyncSettings(ctx context.Context, db database.S return nil, xerrors.Errorf("resolve org sync settings: %w", err) } - // Default to an empty config - orgSettings = &OrganizationSyncSettings{} - - if s.DeploymentSyncSettings.OrganizationField != "" { - // Use static settings if set - orgSettings = &OrganizationSyncSettings{ - Field: s.DeploymentSyncSettings.OrganizationField, - Mapping: s.DeploymentSyncSettings.OrganizationMapping, - AssignDefault: s.DeploymentSyncSettings.OrganizationAssignDefault, - } + // Default to the statically assigned settings if they exist. + orgSettings = &OrganizationSyncSettings{ + Field: s.DeploymentSyncSettings.OrganizationField, + Mapping: s.DeploymentSyncSettings.OrganizationMapping, + AssignDefault: s.DeploymentSyncSettings.OrganizationAssignDefault, } } return orgSettings, nil