Skip to content

Commit 1093e83

Browse files
EmyrkEmyrk
committed
add unit test for legacy group settings
1 parent c8982a4 commit 1093e83

File tree

5 files changed

+107
-40
lines changed

5 files changed

+107
-40
lines changed

‎coderd/idpsync/group.go

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,45 @@ func (AGPLIDPSync) GroupSyncEnabled() bool {
3030
return false
3131
}
3232

33-
func (s AGPLIDPSync) GroupSyncSettings() runtimeconfig.RuntimeEntry[*GroupSyncSettings] {
34-
return s.Group
33+
func (s AGPLIDPSync) UpdateGroupSettings(ctx context.Context, orgID uuid.UUID, db database.Store, settings GroupSyncSettings) error {
34+
orgResolver := s.Manager.OrganizationResolver(db, orgID)
35+
err := s.SyncSettings.Group.SetRuntimeValue(ctx, orgResolver, &settings)
36+
if err != nil {
37+
return xerrors.Errorf("update group sync settings: %w", err)
38+
}
39+
40+
return nil
41+
}
42+
43+
func (s AGPLIDPSync) GroupSyncSettings(ctx context.Context, orgID uuid.UUID, db database.Store) (*GroupSyncSettings, error) {
44+
orgResolver := s.Manager.OrganizationResolver(db, orgID)
45+
settings, err := s.SyncSettings.Group.Resolve(ctx, orgResolver)
46+
if err != nil {
47+
if !xerrors.Is(err, runtimeconfig.ErrEntryNotFound) {
48+
return nil, xerrors.Errorf("resolve group sync settings: %w", err)
49+
}
50+
51+
// Default to not being configured
52+
settings = &GroupSyncSettings{}
53+
}
54+
55+
// Check for legacy settings if the default org.
56+
if s.DeploymentSyncSettings.Legacy.GroupField != "" && settings.Field == "" {
57+
defaultOrganization, err := db.GetDefaultOrganization(ctx)
58+
if err != nil {
59+
return nil, xerrors.Errorf("get default organization: %w", err)
60+
}
61+
if defaultOrganization.ID == orgID {
62+
settings = ptr.Ref(GroupSyncSettings(codersdk.GroupSyncSettings{
63+
Field: s.Legacy.GroupField,
64+
LegacyNameMapping: s.Legacy.GroupMapping,
65+
RegexFilter: s.Legacy.GroupFilter,
66+
AutoCreateMissing: s.Legacy.CreateMissingGroups,
67+
}))
68+
}
69+
}
70+
71+
return settings, nil
3572
}
3673

3774
func (s AGPLIDPSync) ParseGroupClaims(_ context.Context, _ jwt.MapClaims) (GroupParams, *HTTPError) {
@@ -49,18 +86,6 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
4986
// nolint:gocritic // all syncing is done as a system user
5087
ctx = dbauthz.AsSystemRestricted(ctx)
5188

52-
// Only care about the default org for deployment settings if the
53-
// legacy deployment settings exist.
54-
defaultOrgID := uuid.Nil
55-
// Default organization is configured via legacy deployment values
56-
if s.DeploymentSyncSettings.Legacy.GroupField != "" {
57-
defaultOrganization, err := db.GetDefaultOrganization(ctx)
58-
if err != nil {
59-
return xerrors.Errorf("get default organization: %w", err)
60-
}
61-
defaultOrgID = defaultOrganization.ID
62-
}
63-
6489
err := db.InTx(func(tx database.Store) error {
6590
userGroups, err := tx.GetGroups(ctx, database.GetGroupsParams{
6691
HasMemberID: user.ID,
@@ -83,25 +108,21 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
83108
// organization.
84109
orgSettings := make(map[uuid.UUID]GroupSyncSettings)
85110
for orgID := range userOrgs {
86-
orgResolver := s.Manager.OrganizationResolver(tx, orgID)
87-
settings, err := s.SyncSettings.Group.Resolve(ctx, orgResolver)
111+
def, _ := tx.GetDefaultOrganization(ctx)
112+
if def.ID == orgID {
113+
fmt.Println("as")
114+
}
115+
settings, err := s.GroupSyncSettings(ctx, orgID, tx)
88116
if err != nil {
89-
if !xerrors.Is(err, runtimeconfig.ErrEntryNotFound) {
90-
return xerrors.Errorf("resolve group sync settings: %w", err)
91-
}
92-
// Default to not being configured
117+
// TODO: This error is currently silent to org admins.
118+
// We need to come up with a way to notify the org admin of this
119+
// error.
120+
s.Logger.Error(ctx, "failed to get group sync settings",
121+
slog.F("organization_id", orgID),
122+
slog.Error(err),
123+
)
93124
settings = &GroupSyncSettings{}
94125
}
95-
96-
// Legacy deployment settings will override empty settings.
97-
if orgID == defaultOrgID && settings.Field == "" {
98-
settings = ptr.Ref(GroupSyncSettings(codersdk.GroupSyncSettings{
99-
Field: s.Legacy.GroupField,
100-
LegacyNameMapping: s.Legacy.GroupMapping,
101-
RegexFilter: s.Legacy.GroupFilter,
102-
AutoCreateMissing: s.Legacy.CreateMissingGroups,
103-
}))
104-
}
105126
orgSettings[orgID] = *settings
106127
}
107128

‎coderd/idpsync/group_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,15 @@ func TestGroupSyncTable(t *testing.T) {
242242
manager,
243243
idpsync.DeploymentSyncSettings{
244244
GroupField: "groups",
245+
Legacy: idpsync.DefaultOrgLegacySettings{
246+
GroupField: "groups",
247+
GroupMapping: map[string]string{
248+
"foo": "legacy-foo",
249+
"baz": "legacy-baz",
250+
},
251+
GroupFilter: regexp.MustCompile("^legacy"),
252+
CreateMissingGroups: true,
253+
},
245254
},
246255
)
247256

@@ -275,6 +284,8 @@ func TestGroupSyncTable(t *testing.T) {
275284
// Also sync the default org!
276285
idpsync.DeploymentSyncSettings{
277286
GroupField: "groups",
287+
// This legacy field will fail any tests if the legacy override code
288+
// has any bugs.
278289
Legacy: idpsync.DefaultOrgLegacySettings{
279290
GroupField: "groups",
280291
GroupMapping: map[string]string{

‎coderd/idpsync/idpsync.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ type IDPSync interface {
4141
// GroupSyncSettings is exposed for the API to implement CRUD operations
4242
// on the settings used by IDPSync. This entry is thread safe and can be
4343
// accessed concurrently. The settings are stored in the database.
44-
GroupSyncSettings() runtimeconfig.RuntimeEntry[*GroupSyncSettings]
44+
GroupSyncSettings(ctx context.Context, orgID uuid.UUID, db database.Store) (*GroupSyncSettings, error)
45+
UpdateGroupSettings(ctx context.Context, orgID uuid.UUID, db database.Store, settings GroupSyncSettings) error
4546
}
4647

4748
// AGPLIDPSync is the configuration for syncing user information from an external

‎enterprise/coderd/idpsync.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,9 @@ func (api *API) groupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) {
2828
return
2929
}
3030

31-
rlv := api.Options.RuntimeConfig.OrganizationResolver(api.Database, org.ID)
32-
runtimeConfigEntry := api.IDPSync.GroupSyncSettings()
33-
3431
//nolint:gocritic // Requires system context to read runtime config
3532
sysCtx := dbauthz.AsSystemRestricted(ctx)
36-
settings, err := runtimeConfigEntry.Resolve(sysCtx, rlv)
33+
settings, err := api.IDPSync.GroupSyncSettings(sysCtx, org.ID, api.Database)
3734
if err != nil {
3835
httpapi.InternalServerError(rw, err)
3936
return
@@ -64,18 +61,15 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques
6461
return
6562
}
6663

67-
rlv := api.Options.RuntimeConfig.OrganizationResolver(api.Database, org.ID)
68-
runtimeConfigEntry := api.IDPSync.GroupSyncSettings()
69-
7064
//nolint:gocritic // Requires system context to update runtime config
7165
sysCtx := dbauthz.AsSystemRestricted(ctx)
72-
err := runtimeConfigEntry.SetRuntimeValue(sysCtx, rlv, &req)
66+
err := api.IDPSync.UpdateGroupSettings(sysCtx, org.ID, api.Database, req)
7367
if err != nil {
7468
httpapi.InternalServerError(rw, err)
7569
return
7670
}
7771

78-
settings, err := runtimeConfigEntry.Resolve(sysCtx, rlv)
72+
settings, err := api.IDPSync.GroupSyncSettings(sysCtx, org.ID, api.Database)
7973
if err != nil {
8074
httpapi.InternalServerError(rw, err)
8175
return

‎enterprise/coderd/idpsync_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package coderd_test
22

33
import (
44
"net/http"
5+
"regexp"
56
"testing"
67

78
"github.com/stretchr/testify/require"
@@ -15,6 +16,7 @@ import (
1516
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
1617
"github.com/coder/coder/v2/enterprise/coderd/license"
1718
"github.com/coder/coder/v2/testutil"
19+
"github.com/coder/serpent"
1820
)
1921

2022
func TestGetGroupSyncConfig(t *testing.T) {
@@ -52,6 +54,44 @@ func TestGetGroupSyncConfig(t *testing.T) {
5254
require.NoError(t, err)
5355
require.Equal(t, "august", settings.Field)
5456
})
57+
58+
t.Run("Legacy", func(t *testing.T) {
59+
t.Parallel()
60+
61+
dv := coderdtest.DeploymentValues(t)
62+
dv.Experiments = []string{
63+
string(codersdk.ExperimentCustomRoles),
64+
string(codersdk.ExperimentMultiOrganization),
65+
}
66+
dv.OIDC.GroupField = "legacy-group"
67+
dv.OIDC.GroupRegexFilter = serpent.Regexp(*regexp.MustCompile("legacy-filter"))
68+
dv.OIDC.GroupMapping = serpent.Struct[map[string]string]{
69+
Value: map[string]string{
70+
"foo": "bar",
71+
},
72+
}
73+
74+
owner, user := coderdenttest.New(t, &coderdenttest.Options{
75+
Options: &coderdtest.Options{
76+
DeploymentValues: dv,
77+
},
78+
LicenseOptions: &coderdenttest.LicenseOptions{
79+
Features: license.Features{
80+
codersdk.FeatureCustomRoles: 1,
81+
codersdk.FeatureMultipleOrganizations: 1,
82+
},
83+
},
84+
})
85+
orgAdmin, _ := coderdtest.CreateAnotherUser(t, owner, user.OrganizationID, rbac.ScopedRoleOrgAdmin(user.OrganizationID))
86+
87+
ctx := testutil.Context(t, testutil.WaitShort)
88+
89+
settings, err := orgAdmin.GroupIDPSyncSettings(ctx, user.OrganizationID.String())
90+
require.NoError(t, err)
91+
require.Equal(t, dv.OIDC.GroupField.Value(), settings.Field)
92+
require.Equal(t, dv.OIDC.GroupRegexFilter.String(), settings.RegexFilter.String())
93+
require.Equal(t, dv.OIDC.GroupMapping.Value, settings.LegacyNameMapping)
94+
})
5595
}
5696

5797
func TestPostGroupSyncConfig(t *testing.T) {

0 commit comments

Comments
 (0)