Skip to content

Commit 601652c

Browse files
EmyrkEmyrk
committed
test enterprise parse
1 parent 5d0f729 commit 601652c

File tree

4 files changed

+238
-7
lines changed

4 files changed

+238
-7
lines changed

‎coderd/idpsync/role.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,19 @@ func (s AGPLIDPSync) syncSiteWideRoles(ctx context.Context, tx database.Store, u
214214
)
215215
}
216216

217-
_, err := tx.UpdateUserRoles(ctx, database.UpdateUserRolesParams{
218-
GrantedRoles: filtered,
219-
ID: user.ID,
220-
})
221-
if err != nil {
222-
return xerrors.Errorf("set site wide roles: %w", err)
217+
filtered = slice.Unique(filtered)
218+
slices.Sort(filtered)
219+
220+
existing := slice.Unique(user.RBACRoles)
221+
slices.Sort(existing)
222+
if !slices.Equal(existing, filtered) {
223+
_, err := tx.UpdateUserRoles(ctx, database.UpdateUserRolesParams{
224+
GrantedRoles: filtered,
225+
ID: user.ID,
226+
})
227+
if err != nil {
228+
return xerrors.Errorf("set site wide roles: %w", err)
229+
}
223230
}
224231
return nil
225232
}

‎coderd/idpsync/role_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
package idpsync_test
22

33
import (
4+
"context"
5+
"database/sql"
6+
"encoding/json"
47
"testing"
58

69
"github.com/golang-jwt/jwt/v4"
710
"github.com/google/uuid"
811
"github.com/stretchr/testify/require"
12+
"go.uber.org/mock/gomock"
913
"golang.org/x/exp/slices"
1014

1115
"cdr.dev/slog/sloggers/slogtest"
1216
"github.com/coder/coder/v2/coderd/database"
1317
"github.com/coder/coder/v2/coderd/database/dbauthz"
1418
"github.com/coder/coder/v2/coderd/database/dbgen"
19+
"github.com/coder/coder/v2/coderd/database/dbmock"
1520
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1621
"github.com/coder/coder/v2/coderd/idpsync"
1722
"github.com/coder/coder/v2/coderd/rbac"
@@ -281,6 +286,7 @@ func TestRoleSyncTable(t *testing.T) {
281286
allRoleIDs, err := allRoles.RoleNames()
282287
require.NoError(t, err)
283288

289+
// Remove the org roles
284290
siteRoles := slices.DeleteFunc(allRoleIDs, func(r rbac.RoleIdentifier) bool {
285291
return r.IsOrgRole()
286292
})
@@ -290,3 +296,76 @@ func TestRoleSyncTable(t *testing.T) {
290296
}, siteRoles)
291297
})
292298
}
299+
300+
// TestNoopNoDiff verifies if no role change occurs, no database call is taken
301+
// per organization. This limits the number of db calls to O(1) if there
302+
// are no changes. Which is the usual case, as user's roles do not change often.
303+
func TestNoopNoDiff(t *testing.T) {
304+
ctx := context.Background()
305+
ctrl := gomock.NewController(t)
306+
mDB := dbmock.NewMockStore(ctrl)
307+
308+
mgr := runtimeconfig.NewManager()
309+
s := idpsync.NewAGPLSync(slogtest.Make(t, &slogtest.Options{}), mgr, idpsync.DeploymentSyncSettings{
310+
SiteRoleField: "",
311+
SiteRoleMapping: nil,
312+
SiteDefaultRoles: nil,
313+
})
314+
315+
userID := uuid.New()
316+
orgID := uuid.New()
317+
siteRoles := []string{rbac.RoleTemplateAdmin().Name, rbac.RoleAuditor().Name}
318+
orgRoles := []string{rbac.RoleOrgAuditor(), rbac.RoleOrgAdmin()}
319+
// The DB mock expects.
320+
// If this test fails, feel free to add more expectations.
321+
// The primary expectations to avoid is 'UpdateUserRoles'
322+
// and 'UpdateMemberRoles'.
323+
mDB.EXPECT().InTx(
324+
gomock.Any(), gomock.Any(),
325+
).DoAndReturn(func(f func(database.Store) error, _ *sql.TxOptions) error {
326+
err := f(mDB)
327+
return err
328+
})
329+
330+
mDB.EXPECT().OrganizationMembers(gomock.Any(), database.OrganizationMembersParams{
331+
UserID: userID,
332+
}).Return([]database.OrganizationMembersRow{
333+
{
334+
OrganizationMember: database.OrganizationMember{
335+
UserID: userID,
336+
OrganizationID: orgID,
337+
Roles: orgRoles,
338+
},
339+
},
340+
}, nil)
341+
342+
mDB.EXPECT().GetRuntimeConfig(gomock.Any(), gomock.Any()).Return(
343+
string(must(json.Marshal(idpsync.RoleSyncSettings{
344+
Field: "roles",
345+
Mapping: nil,
346+
}))), nil)
347+
348+
err := s.SyncRoles(ctx, mDB, database.User{
349+
ID: userID,
350+
Email: "alice@email.com",
351+
Username: "alice",
352+
Status: database.UserStatusActive,
353+
RBACRoles: siteRoles,
354+
LoginType: database.LoginTypePassword,
355+
}, idpsync.RoleParams{
356+
SyncEnabled: true,
357+
SyncSiteWide: true,
358+
SiteWideRoles: siteRoles,
359+
MergedClaims: jwt.MapClaims{
360+
"roles": orgRoles,
361+
},
362+
})
363+
require.NoError(t, err)
364+
}
365+
366+
func must[T any](value T, err error) T {
367+
if err != nil {
368+
panic(err)
369+
}
370+
return value
371+
}

‎enterprise/coderd/enidpsync/role.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"cdr.dev/slog"
1111
"github.com/coder/coder/v2/coderd/idpsync"
12+
"github.com/coder/coder/v2/coderd/util/slice"
1213
"github.com/coder/coder/v2/codersdk"
1314
)
1415

@@ -61,7 +62,7 @@ func (e EnterpriseIDPSync) ParseRoleClaims(ctx context.Context, mergedClaims jwt
6162
return idpsync.RoleParams{
6263
SyncEnabled: e.RoleSyncEnabled(),
6364
SyncSiteWide: e.AGPLIDPSync.SiteRoleField != "",
64-
SiteWideRoles: siteRoles,
65+
SiteWideRoles: slice.Unique(siteRoles),
6566
MergedClaims: mergedClaims,
6667
}, nil
6768
}

‎enterprise/coderd/enidpsync/role_test.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
package enidpsync_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/golang-jwt/jwt/v4"
8+
"github.com/stretchr/testify/require"
9+
10+
"cdr.dev/slog/sloggers/slogtest"
11+
"github.com/coder/coder/v2/coderd/entitlements"
12+
"github.com/coder/coder/v2/coderd/idpsync"
13+
"github.com/coder/coder/v2/coderd/rbac"
14+
"github.com/coder/coder/v2/coderd/runtimeconfig"
15+
"github.com/coder/coder/v2/codersdk"
16+
"github.com/coder/coder/v2/enterprise/coderd/enidpsync"
17+
)
18+
19+
func TestEnterpriseParseRoleClaims(t *testing.T) {
20+
t.Parallel()
21+
22+
entitled := entitlements.New()
23+
entitled.Update(func(en *codersdk.Entitlements) {
24+
en.Features[codersdk.FeatureUserRoleManagement] = codersdk.Feature{
25+
Entitlement: codersdk.EntitlementEntitled,
26+
Enabled: true,
27+
}
28+
})
29+
30+
t.Run("NotEntitled", func(t *testing.T) {
31+
t.Parallel()
32+
33+
mgr := runtimeconfig.NewManager()
34+
s := enidpsync.NewSync(slogtest.Make(t, nil), mgr, entitlements.New(), idpsync.DeploymentSyncSettings{})
35+
36+
params, err := s.ParseRoleClaims(context.Background(), jwt.MapClaims{})
37+
require.Nil(t, err)
38+
require.False(t, params.SyncEnabled)
39+
require.False(t, params.SyncSiteWide)
40+
})
41+
42+
t.Run("NotEntitledButEnabled", func(t *testing.T) {
43+
t.Parallel()
44+
// Since it is not entitled, it should not be enabled
45+
46+
mgr := runtimeconfig.NewManager()
47+
s := enidpsync.NewSync(slogtest.Make(t, nil), mgr, entitlements.New(), idpsync.DeploymentSyncSettings{
48+
SiteRoleField: "roles",
49+
})
50+
51+
params, err := s.ParseRoleClaims(context.Background(), jwt.MapClaims{})
52+
require.Nil(t, err)
53+
require.False(t, params.SyncEnabled)
54+
require.False(t, params.SyncSiteWide)
55+
})
56+
57+
t.Run("SiteDisabled", func(t *testing.T) {
58+
t.Parallel()
59+
60+
mgr := runtimeconfig.NewManager()
61+
s := enidpsync.NewSync(slogtest.Make(t, nil), mgr, entitled, idpsync.DeploymentSyncSettings{})
62+
63+
params, err := s.ParseRoleClaims(context.Background(), jwt.MapClaims{})
64+
require.Nil(t, err)
65+
require.True(t, params.SyncEnabled)
66+
require.False(t, params.SyncSiteWide)
67+
})
68+
69+
t.Run("SiteEnabled", func(t *testing.T) {
70+
t.Parallel()
71+
72+
mgr := runtimeconfig.NewManager()
73+
s := enidpsync.NewSync(slogtest.Make(t, nil), mgr, entitled, idpsync.DeploymentSyncSettings{
74+
SiteRoleField: "roles",
75+
SiteRoleMapping: map[string][]string{},
76+
SiteDefaultRoles: []string{rbac.RoleTemplateAdmin().Name},
77+
})
78+
79+
params, err := s.ParseRoleClaims(context.Background(), jwt.MapClaims{
80+
"roles": []string{rbac.RoleAuditor().Name},
81+
})
82+
require.Nil(t, err)
83+
require.True(t, params.SyncEnabled)
84+
require.True(t, params.SyncSiteWide)
85+
require.ElementsMatch(t, []string{
86+
rbac.RoleTemplateAdmin().Name,
87+
rbac.RoleAuditor().Name,
88+
}, params.SiteWideRoles)
89+
})
90+
91+
t.Run("SiteMapping", func(t *testing.T) {
92+
t.Parallel()
93+
94+
mgr := runtimeconfig.NewManager()
95+
s := enidpsync.NewSync(slogtest.Make(t, nil), mgr, entitled, idpsync.DeploymentSyncSettings{
96+
SiteRoleField: "roles",
97+
SiteRoleMapping: map[string][]string{
98+
"foo": {rbac.RoleAuditor().Name, rbac.RoleUserAdmin().Name},
99+
"bar": {rbac.RoleOwner().Name},
100+
},
101+
SiteDefaultRoles: []string{rbac.RoleTemplateAdmin().Name},
102+
})
103+
104+
params, err := s.ParseRoleClaims(context.Background(), jwt.MapClaims{
105+
"roles": []string{"foo", "bar", "random"},
106+
})
107+
require.Nil(t, err)
108+
require.True(t, params.SyncEnabled)
109+
require.True(t, params.SyncSiteWide)
110+
require.ElementsMatch(t, []string{
111+
rbac.RoleTemplateAdmin().Name,
112+
rbac.RoleAuditor().Name,
113+
rbac.RoleUserAdmin().Name,
114+
rbac.RoleOwner().Name,
115+
// Invalid claims are still passed at this point
116+
"random",
117+
}, params.SiteWideRoles)
118+
})
119+
120+
t.Run("DuplicateRoles", func(t *testing.T) {
121+
t.Parallel()
122+
123+
mgr := runtimeconfig.NewManager()
124+
s := enidpsync.NewSync(slogtest.Make(t, nil), mgr, entitled, idpsync.DeploymentSyncSettings{
125+
SiteRoleField: "roles",
126+
SiteRoleMapping: map[string][]string{
127+
"foo": {rbac.RoleOwner().Name, rbac.RoleAuditor().Name},
128+
"bar": {rbac.RoleOwner().Name},
129+
},
130+
SiteDefaultRoles: []string{rbac.RoleAuditor().Name},
131+
})
132+
133+
params, err := s.ParseRoleClaims(context.Background(), jwt.MapClaims{
134+
"roles": []string{"foo", "bar", rbac.RoleAuditor().Name, rbac.RoleOwner().Name},
135+
})
136+
require.Nil(t, err)
137+
require.True(t, params.SyncEnabled)
138+
require.True(t, params.SyncSiteWide)
139+
require.ElementsMatch(t, []string{
140+
rbac.RoleAuditor().Name,
141+
rbac.RoleOwner().Name,
142+
}, params.SiteWideRoles)
143+
})
144+
}

0 commit comments

Comments
 (0)