Skip to content

Commit f4c5d00

Browse files
mabdhmabdh
authored
Merge branch 'main' into add-e2etest-edge-case
2 parents 62be0ef + 713e2e8 commit f4c5d00

File tree

10 files changed

+338
-32
lines changed

10 files changed

+338
-32
lines changed

‎cmd/serve.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,7 @@ func BuildAPIDependencies(
189189

190190
resourcePGRepository := postgres.NewResourceRepository(dbc)
191191
resourceService := resource.NewService(
192-
resourcePGRepository,
193-
resourceBlobRepository,
194-
relationService,
195-
userService,
196-
projectService)
192+
resourcePGRepository, resourceBlobRepository, relationService, userService, projectService, organizationService, groupService)
197193

198194
relationAdapter := adapter.NewRelation(groupService, userService, relationService)
199195

‎core/resource/service.go‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import (
55
"strings"
66

77
"github.com/goto/shield/core/action"
8+
"github.com/goto/shield/core/group"
89
"github.com/goto/shield/core/namespace"
910
"github.com/goto/shield/core/organization"
1011
"github.com/goto/shield/core/project"
1112
"github.com/goto/shield/core/relation"
1213
"github.com/goto/shield/core/user"
1314
"github.com/goto/shield/internal/schema"
15+
"github.com/goto/shield/pkg/uuid"
1416
)
1517

1618
type RelationService interface {
@@ -28,21 +30,33 @@ type ProjectService interface {
2830
Get(ctx context.Context, id string) (project.Project, error)
2931
}
3032

33+
type OrganizationService interface {
34+
Get(ctx context.Context, id string) (organization.Organization, error)
35+
}
36+
37+
type GroupService interface {
38+
Get(ctx context.Context, id string) (group.Group, error)
39+
}
40+
3141
type Service struct {
32-
repository Repository
33-
configRepository ConfigRepository
34-
relationService RelationService
35-
userService UserService
36-
projectService ProjectService
42+
repository Repository
43+
configRepository ConfigRepository
44+
relationService RelationService
45+
userService UserService
46+
projectService ProjectService
47+
organizationService OrganizationService
48+
groupService GroupService
3749
}
3850

39-
func NewService(repository Repository, configRepository ConfigRepository, relationService RelationService, userService UserService, projectService ProjectService) *Service {
51+
func NewService(repository Repository, configRepository ConfigRepository, relationService RelationService, userService UserService, projectService ProjectService, organizationService OrganizationService, groupService GroupService) *Service {
4052
return &Service{
41-
repository: repository,
42-
configRepository: configRepository,
43-
relationService: relationService,
44-
userService: userService,
45-
projectService: projectService,
53+
repository: repository,
54+
configRepository: configRepository,
55+
relationService: relationService,
56+
userService: userService,
57+
projectService: projectService,
58+
organizationService: organizationService,
59+
groupService: groupService,
4660
}
4761
}
4862

@@ -158,6 +172,28 @@ func (s Service) CheckAuthz(ctx context.Context, res Resource, act action.Action
158172
fetchedResource := res
159173

160174
if isSystemNS {
175+
if !uuid.IsValid(res.Name) {
176+
switch res.NamespaceID {
177+
case namespace.DefinitionProject.ID:
178+
project, err := s.projectService.Get(ctx, res.Name)
179+
if err != nil {
180+
return false, err
181+
}
182+
res.Name = project.ID
183+
case namespace.DefinitionOrg.ID:
184+
organization, err := s.organizationService.Get(ctx, res.Name)
185+
if err != nil {
186+
return false, err
187+
}
188+
res.Name = organization.ID
189+
case namespace.DefinitionTeam.ID:
190+
group, err := s.groupService.Get(ctx, res.Name)
191+
if err != nil {
192+
return false, err
193+
}
194+
res.Name = group.ID
195+
}
196+
}
161197
fetchedResource.Idxa = res.Name
162198
} else {
163199
fetchedResource, err = s.repository.GetByNamespace(ctx, res.Name, res.NamespaceID)

‎internal/proxy/middleware/authz/authz.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ func (c *Authz) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
257257
c.notAllowed(rw, err)
258258
return
259259
}
260+
c.log.Info("successfully checked permission", "permission", permission.Name, "result", isAuthorized)
260261
if isAuthorized {
261262
break
262263
}

‎internal/store/postgres/group_repository_test.go‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,16 @@ import (
99
"github.com/google/go-cmp/cmp/cmpopts"
1010
"github.com/google/uuid"
1111
"github.com/goto/salt/log"
12+
"github.com/ory/dockertest"
13+
"github.com/stretchr/testify/suite"
14+
1215
"github.com/goto/shield/core/group"
1316
"github.com/goto/shield/core/organization"
1417
"github.com/goto/shield/core/relation"
1518
"github.com/goto/shield/core/user"
1619
"github.com/goto/shield/internal/schema"
1720
"github.com/goto/shield/internal/store/postgres"
1821
"github.com/goto/shield/pkg/db"
19-
"github.com/ory/dockertest"
20-
"github.com/stretchr/testify/suite"
2122
)
2223

2324
type GroupRepositoryTestSuite struct {
@@ -712,6 +713,50 @@ func (s *GroupRepositoryTestSuite) TestListGroupRelations() {
712713
RoleID: "shield/group:member",
713714
},
714715
},
716+
{
717+
Object: relation.Object{
718+
ID: s.groups[0].ID,
719+
NamespaceID: schema.GroupNamespace,
720+
},
721+
Subject: relation.Subject{
722+
ID: s.users[2].ID,
723+
Namespace: schema.UserPrincipal,
724+
RoleID: "shield/group:member",
725+
},
726+
},
727+
{
728+
Object: relation.Object{
729+
ID: s.groups[0].ID,
730+
NamespaceID: schema.GroupNamespace,
731+
},
732+
Subject: relation.Subject{
733+
ID: s.users[3].ID,
734+
Namespace: schema.UserPrincipal,
735+
RoleID: "shield/group:member",
736+
},
737+
},
738+
{
739+
Object: relation.Object{
740+
ID: s.groups[0].ID,
741+
NamespaceID: schema.GroupNamespace,
742+
},
743+
Subject: relation.Subject{
744+
ID: s.users[4].ID,
745+
Namespace: schema.UserPrincipal,
746+
RoleID: "shield/group:member",
747+
},
748+
},
749+
{
750+
Object: relation.Object{
751+
ID: s.groups[0].ID,
752+
NamespaceID: schema.GroupNamespace,
753+
},
754+
Subject: relation.Subject{
755+
ID: s.users[5].ID,
756+
Namespace: schema.UserPrincipal,
757+
RoleID: "shield/group:member",
758+
},
759+
},
715760
},
716761
},
717762
}

‎internal/store/postgres/testdata/mock-user.json‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,29 @@
1414
"bar": "v2"
1515
}
1616
}
17+
},
18+
{
19+
"name": "Alex Xu",
20+
"email": "[email protected]"
21+
},
22+
{
23+
"name": "Alexa Xu",
24+
"email": "[email protected]",
25+
"metadata": {
26+
"k1": "value-string",
27+
"k2": 123,
28+
"k3": {
29+
"foo": "v1",
30+
"bar": "v2"
31+
}
32+
}
33+
},
34+
{
35+
"name": "Bella Anto",
36+
"email": "[email protected]"
37+
},
38+
{
39+
"name": "Balex Ro",
40+
"email": "[email protected]"
1741
}
1842
]

‎internal/store/postgres/user_repository.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ import (
1111
"time"
1212

1313
"github.com/doug-martin/goqu/v9"
14+
"github.com/jmoiron/sqlx"
15+
newrelic "github.com/newrelic/go-agent"
16+
1417
"github.com/goto/shield/core/user"
1518
"github.com/goto/shield/pkg/db"
1619
"github.com/goto/shield/pkg/uuid"
17-
"github.com/jmoiron/sqlx"
18-
newrelic "github.com/newrelic/go-agent"
1920
)
2021

2122
type UserRepository struct {
@@ -279,10 +280,20 @@ func (r UserRepository) List(ctx context.Context, flt user.Filter) ([]user.User,
279280

280281
query, params, err := dialect.From(TABLE_USERS).LeftOuterJoin(
281282
goqu.T(TABLE_METADATA),
282-
goqu.On(goqu.Ex{"users.id": goqu.I("metadata.user_id")})).Select("users.id", "name", "email", "key", "value", "users.created_at", "users.updated_at").Where(goqu.Or(
283-
goqu.C("name").ILike(fmt.Sprintf("%%%s%%", flt.Keyword)),
284-
goqu.C("email").ILike(fmt.Sprintf("%%%s%%", flt.Keyword)),
285-
)).Limit(uint(flt.Limit)).Offset(uint(offset)).ToSQL()
283+
goqu.On(goqu.Ex{"users.id": goqu.I("metadata.user_id")})).Select("users.id", "name", "email", "key", "value", "users.created_at", "users.updated_at").Where(
284+
goqu.I("users.email").In(
285+
goqu.From("users").
286+
Select(goqu.DISTINCT("email")).
287+
Where(
288+
goqu.Or(
289+
goqu.C("name").ILike(fmt.Sprintf("%%%s%%", flt.Keyword)),
290+
goqu.C("email").ILike(fmt.Sprintf("%%%s%%", flt.Keyword)),
291+
),
292+
).
293+
Limit(uint(flt.Limit)).
294+
Offset(uint(offset)),
295+
),
296+
).ToSQL()
286297
if err != nil {
287298
return []user.User{}, fmt.Errorf("%w: %s", queryErr, err)
288299
}

‎internal/store/postgres/user_repository_test.go‎

Lines changed: 89 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,20 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"sort"
78
"testing"
89

910
"github.com/google/go-cmp/cmp"
1011
"github.com/google/go-cmp/cmp/cmpopts"
1112
"github.com/google/uuid"
1213
"github.com/goto/salt/log"
14+
"github.com/ory/dockertest"
15+
"github.com/stretchr/testify/suite"
16+
1317
"github.com/goto/shield/core/user"
1418
"github.com/goto/shield/internal/store/postgres"
1519
"github.com/goto/shield/pkg/db"
1620
"github.com/goto/shield/pkg/metadata"
17-
"github.com/ory/dockertest"
18-
"github.com/stretchr/testify/suite"
1921
)
2022

2123
type UserRepositoryTestSuite struct {
@@ -241,7 +243,27 @@ func (s *UserRepositoryTestSuite) TestList() {
241243
{
242244
Description: "should return empty users if keyword not match any",
243245
Filter: user.Filter{
244-
Keyword: "some-keyword",
246+
Keyword: "random=keyword",
247+
},
248+
},
249+
{
250+
Description: "should return list of users if keyword match",
251+
Filter: user.Filter{
252+
Keyword: "alex",
253+
},
254+
ExpectedUsers: []user.User{
255+
{
256+
Name: s.users[2].Name,
257+
Email: s.users[2].Email,
258+
},
259+
{
260+
Name: s.users[3].Name,
261+
Email: s.users[3].Email,
262+
},
263+
{
264+
Name: s.users[5].Name,
265+
Email: s.users[5].Email,
266+
},
245267
},
246268
},
247269
{
@@ -252,8 +274,56 @@ func (s *UserRepositoryTestSuite) TestList() {
252274
},
253275
ExpectedUsers: []user.User{
254276
{
255-
Name: s.users[0].Name,
256-
Email: s.users[0].Email,
277+
Name: s.users[3].Name,
278+
Email: s.users[3].Email,
279+
},
280+
},
281+
},
282+
{
283+
Description: "should return 1st page after filtering the users based on keywords",
284+
Filter: user.Filter{
285+
Keyword: "alex",
286+
Page: 1,
287+
Limit: 2,
288+
},
289+
ExpectedUsers: []user.User{
290+
{
291+
Name: s.users[2].Name,
292+
Email: s.users[2].Email,
293+
},
294+
{
295+
Name: s.users[3].Name,
296+
Email: s.users[3].Email,
297+
},
298+
},
299+
},
300+
{
301+
Description: "should return 2nd page after filtering the users based on keywords",
302+
Filter: user.Filter{
303+
Keyword: "alex",
304+
Page: 2,
305+
Limit: 2,
306+
},
307+
ExpectedUsers: []user.User{
308+
{
309+
Name: s.users[5].Name,
310+
Email: s.users[5].Email,
311+
},
312+
},
313+
},
314+
{
315+
Description: "should return all users with keyword matching email or name",
316+
Filter: user.Filter{
317+
Keyword: "xu",
318+
},
319+
ExpectedUsers: []user.User{
320+
{
321+
Name: s.users[2].Name,
322+
Email: s.users[2].Email,
323+
},
324+
{
325+
Name: s.users[3].Name,
326+
Email: s.users[3].Email,
257327
},
258328
},
259329
},
@@ -270,6 +340,20 @@ func (s *UserRepositoryTestSuite) TestList() {
270340
if !(len(got) == len(tc.ExpectedUsers)) {
271341
s.T().Fatalf("got result %+v, expected was %+v", got, tc.ExpectedUsers)
272342
}
343+
344+
sort.Slice(got, func(i, j int) bool {
345+
return got[i].Email < got[j].Email
346+
})
347+
348+
sort.Slice(tc.ExpectedUsers, func(i, j int) bool {
349+
return tc.ExpectedUsers[i].Email < tc.ExpectedUsers[j].Email
350+
})
351+
352+
for idx := 0; idx < len(got); idx++ {
353+
if got[idx].Name != tc.ExpectedUsers[idx].Name || got[idx].Email != tc.ExpectedUsers[idx].Email {
354+
s.T().Fatalf("got user %+v, expected was %+v", got[idx], tc.ExpectedUsers[idx])
355+
}
356+
}
273357
})
274358
}
275359
}

0 commit comments

Comments
 (0)