From 0cb0908f325d191a81274675877b61ab07521ce2 Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Mon, 7 Oct 2024 12:15:15 -0400 Subject: [PATCH] Group permissions api relationships (#324) * init events Signed-off-by: Bailin He * add and remove members Signed-off-by: Bailin He * group members replace Signed-off-by: Bailin He * add tests Signed-off-by: Bailin He * applied review suggestions * enhance rollback errors * use diff for put * use permissions-api mock Signed-off-by: Bailin He * Update internal/api/httpsrv/handler.go Co-authored-by: Mike Mason Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --------- Signed-off-by: Bailin He Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> Co-authored-by: Mike Mason --- .gitignore | 15 + cmd/serve.go | 23 +- go.mod | 1 + internal/api/httpsrv/errors.go | 3 + internal/api/httpsrv/handler.go | 25 +- internal/api/httpsrv/handler_group.go | 31 +- internal/api/httpsrv/handler_group_members.go | 29 +- .../api/httpsrv/handler_group_members_test.go | 358 ++++++++++++++++-- internal/api/httpsrv/handler_group_test.go | 238 +++++++++++- internal/config/config.go | 2 + internal/events/doc.go | 3 + internal/events/events.go | 34 ++ internal/events/groups_event.go | 84 ++++ internal/events/service.go | 24 ++ internal/storage/groups.go | 101 +++-- internal/storage/helpers.go | 29 ++ internal/types/groups.go | 18 +- 17 files changed, 917 insertions(+), 101 deletions(-) create mode 100644 internal/events/doc.go create mode 100644 internal/events/events.go create mode 100644 internal/events/groups_event.go create mode 100644 internal/events/service.go create mode 100644 internal/storage/helpers.go diff --git a/.gitignore b/.gitignore index 61618e3a..7321972f 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,18 @@ audit.log identity-api !chart/identity-api +# vscode stuff +.vscode/* +.vscode/settings.json +!.vscode/tasks.json +!.vscode/extensions.json +!.vscode/*.code-snippets + +# Local History for Visual Studio Code +.history/ + +# Built Visual Studio Code Extensions +*.vsix + +# binary files +tmp diff --git a/cmd/serve.go b/cmd/serve.go index 6184733c..6b1945ae 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -16,6 +16,7 @@ import ( "go.infratographer.com/x/crdbx" "go.infratographer.com/x/echojwtx" "go.infratographer.com/x/echox" + eventsx "go.infratographer.com/x/events" "go.infratographer.com/x/otelx" "go.infratographer.com/x/versionx" "go.uber.org/zap" @@ -23,6 +24,7 @@ import ( "go.infratographer.com/identity-api/internal/api/httpsrv" "go.infratographer.com/identity-api/internal/auditx" "go.infratographer.com/identity-api/internal/config" + "go.infratographer.com/identity-api/internal/events" "go.infratographer.com/identity-api/internal/fositex" "go.infratographer.com/identity-api/internal/jwks" "go.infratographer.com/identity-api/internal/oauth2" @@ -42,9 +44,7 @@ var serveCmd = &cobra.Command{ }, } -var ( - defaultListen = ":8080" -) +var defaultListen = ":8080" func init() { rootCmd.AddCommand(serveCmd) @@ -56,6 +56,7 @@ func init() { echox.MustViperFlags(v, flags, defaultListen) otelx.MustViperFlags(v, flags) auditx.MustViperFlags(v, flags) + eventsx.MustViperFlags(v, flags, appName) } func serve(ctx context.Context) { @@ -73,8 +74,18 @@ func serve(ctx context.Context) { defer auditCloseFn() //nolint:errcheck // Not needed to check returned error. } - perms, err := permissions.New(config.Config.Permissions, + nc, err := eventsx.NewNATSConnection( + config.Config.Events.NATS, + eventsx.WithNATSLogger(logger), + ) + if err != nil { + logger.Fatal("failed to initialize NATS connection", zap.Error(err)) + } + + perms, err := permissions.New( + config.Config.Permissions, permissions.WithLogger(logger), + permissions.WithEventsPublisher(nc), ) if err != nil { logger.Fatal("failed to initialize permissions", zap.Error(err)) @@ -113,7 +124,9 @@ func serve(ctx context.Context) { oauth2.NewClientCredentialsHandlerFactory, ) - apiHandler, err := httpsrv.NewAPIHandler(storageEngine, auditMiddleware, perms.Middleware()) + es := events.NewEvents(events.WithLogger(logger.Desugar())) + + apiHandler, err := httpsrv.NewAPIHandler(storageEngine, es, auditMiddleware, perms.Middleware()) if err != nil { logger.Fatal("error initializing API server: %s", err) } diff --git a/go.mod b/go.mod index d7ec26cc..4822d809 100644 --- a/go.mod +++ b/go.mod @@ -110,6 +110,7 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.6.0 // indirect github.com/stoewer/go-strcase v1.2.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasttemplate v1.2.2 // indirect diff --git a/internal/api/httpsrv/errors.go b/internal/api/httpsrv/errors.go index 6a7a0fa3..89120223 100644 --- a/internal/api/httpsrv/errors.go +++ b/internal/api/httpsrv/errors.go @@ -22,6 +22,9 @@ var ( status: http.StatusNotFound, message: "not found", } + + // ErrDBRollbackFailed is returned when a database rollback fails + ErrDBRollbackFailed = errors.New("failed to rollback database transaction") ) func permissionsError(err error) error { diff --git a/internal/api/httpsrv/handler.go b/internal/api/httpsrv/handler.go index 4f284f90..8af5ed7b 100644 --- a/internal/api/httpsrv/handler.go +++ b/internal/api/httpsrv/handler.go @@ -1,11 +1,14 @@ package httpsrv import ( + "context" + "fmt" "net/http" "github.com/labstack/echo/v4" "github.com/metal-toolbox/auditevent/middleware/echoaudit" + "go.infratographer.com/identity-api/internal/events" "go.infratographer.com/identity-api/internal/storage" ) @@ -43,7 +46,8 @@ func storageMiddleware(engine storage.Engine) echo.MiddlewareFunc { // apiHandler represents an API handler. type apiHandler struct { - engine storage.Engine + engine storage.Engine + eventService events.Service } // APIHandler represents an identity-api management API handler. @@ -55,14 +59,18 @@ type APIHandler struct { } // NewAPIHandler creates an API handler with the given storage engine. -func NewAPIHandler(engine storage.Engine, amw *echoaudit.Middleware, middleware ...echo.MiddlewareFunc) (*APIHandler, error) { +func NewAPIHandler( + engine storage.Engine, es events.Service, + amw *echoaudit.Middleware, middleware ...echo.MiddlewareFunc, +) (*APIHandler, error) { validationMiddleware, err := oapiValidationMiddleware() if err != nil { return nil, err } handler := apiHandler{ - engine: engine, + engine: engine, + eventService: es, } out := &APIHandler{ @@ -94,3 +102,14 @@ func (h *APIHandler) Routes(rg *echo.Group) { RegisterHandlers(rg, strictHandler) } + +func (h *apiHandler) rollbackAndReturnError(ctx context.Context, httpcode int, msg string) *echo.HTTPError { + if err := h.engine.RollbackContext(ctx); err != nil { + return echo.NewHTTPError( + http.StatusInternalServerError, + fmt.Errorf("%s and %w", msg, ErrDBRollbackFailed), + ).SetInternal(err) + } + + return echo.NewHTTPError(httpcode, msg) +} diff --git a/internal/api/httpsrv/handler_group.go b/internal/api/httpsrv/handler_group.go index 9d4a05cc..0dd64cbe 100644 --- a/internal/api/httpsrv/handler_group.go +++ b/internal/api/httpsrv/handler_group.go @@ -73,6 +73,11 @@ func (h *apiHandler) CreateGroup(ctx context.Context, req CreateGroupRequestObje return nil, err } + if err := h.eventService.CreateGroup(ctx, ownerID, id); err != nil { + resperr := h.rollbackAndReturnError(ctx, http.StatusInternalServerError, "failed to create group in permissions API") + return nil, resperr + } + groupResp, err := g.ToV1Group() if err != nil { return nil, err @@ -230,7 +235,7 @@ func (h *apiHandler) DeleteGroup(ctx context.Context, req DeleteGroupRequestObje return nil, permissionsError(err) } - err := h.engine.DeleteGroup(ctx, gid) + group, err := h.engine.GetGroupByID(ctx, gid) if err != nil { if errors.Is(err, types.ErrNotFound) { err = echo.NewHTTPError( @@ -241,6 +246,25 @@ func (h *apiHandler) DeleteGroup(ctx context.Context, req DeleteGroupRequestObje return nil, err } + return nil, err + } + + mc, err := h.engine.GroupMembersCount(ctx, gid) + if err != nil { + return nil, err + } + + if mc > 0 { + err := echo.NewHTTPError( + http.StatusBadRequest, + fmt.Sprintf("cannot delete group %s: still has members", gid), + ) + + return nil, err + } + + err = h.engine.DeleteGroup(ctx, group.ID) + if err != nil { if errors.Is(err, types.ErrInvalidArgument) { return nil, echo.NewHTTPError(http.StatusBadRequest, err.Error()) } @@ -248,5 +272,10 @@ func (h *apiHandler) DeleteGroup(ctx context.Context, req DeleteGroupRequestObje return nil, err } + if err := h.eventService.DeleteGroup(ctx, group.OwnerID, group.ID); err != nil { + resperr := h.rollbackAndReturnError(ctx, http.StatusInternalServerError, "failed to remove group in permissions API") + return nil, resperr + } + return DeleteGroup200JSONResponse{true}, nil } diff --git a/internal/api/httpsrv/handler_group_members.go b/internal/api/httpsrv/handler_group_members.go index c46c0859..cb692d07 100644 --- a/internal/api/httpsrv/handler_group_members.go +++ b/internal/api/httpsrv/handler_group_members.go @@ -49,7 +49,7 @@ func (h *apiHandler) AddGroupMembers(ctx context.Context, req AddGroupMembersReq return nil, permissionsError(err) } - if err := h.engine.AddMembers(ctx, gid, reqbody.MemberIDs...); err != nil { + if err := h.engine.AddGroupMembers(ctx, gid, reqbody.MemberIDs...); err != nil { if errors.Is(err, types.ErrNotFound) { err = echo.NewHTTPError(http.StatusNotFound, err.Error()) } @@ -57,6 +57,11 @@ func (h *apiHandler) AddGroupMembers(ctx context.Context, req AddGroupMembersReq return nil, err } + if err := h.eventService.AddGroupMembers(ctx, gid, reqbody.MemberIDs...); err != nil { + resperr := h.rollbackAndReturnError(ctx, http.StatusInternalServerError, "failed to add group members in permissions API") + return nil, resperr + } + return AddGroupMembers200JSONResponse{Success: true}, nil } @@ -77,7 +82,7 @@ func (h *apiHandler) ListGroupMembers(ctx context.Context, req ListGroupMembersR return nil, permissionsError(err) } - members, err := h.engine.ListMembers(ctx, gid, req.Params) + members, err := h.engine.ListGroupMembers(ctx, gid, req.Params) if err != nil { if errors.Is(err, types.ErrNotFound) { err = echo.NewHTTPError(http.StatusNotFound, err.Error()) @@ -133,7 +138,7 @@ func (h *apiHandler) RemoveGroupMember(ctx context.Context, req RemoveGroupMembe return nil, permissionsError(err) } - if err := h.engine.RemoveMember(ctx, gid, sid); err != nil { + if err := h.engine.RemoveGroupMember(ctx, gid, sid); err != nil { if errors.Is(err, types.ErrNotFound) { err = echo.NewHTTPError(http.StatusNotFound, err.Error()) } @@ -141,6 +146,11 @@ func (h *apiHandler) RemoveGroupMember(ctx context.Context, req RemoveGroupMembe return nil, err } + if err := h.eventService.RemoveGroupMembers(ctx, gid, sid); err != nil { + resperr := h.rollbackAndReturnError(ctx, http.StatusInternalServerError, "failed to remove group member in permissions API") + return nil, resperr + } + return RemoveGroupMember200JSONResponse{true}, nil } @@ -173,7 +183,8 @@ func (h *apiHandler) ReplaceGroupMembers(ctx context.Context, req ReplaceGroupMe return nil, permissionsError(err) } - if err := h.engine.ReplaceMembers(ctx, gid, reqbody.MemberIDs...); err != nil { + add, rm, err := h.engine.ReplaceGroupMembers(ctx, gid, reqbody.MemberIDs...) + if err != nil { if errors.Is(err, types.ErrNotFound) { err = echo.NewHTTPError(http.StatusNotFound, err.Error()) } @@ -181,6 +192,16 @@ func (h *apiHandler) ReplaceGroupMembers(ctx context.Context, req ReplaceGroupMe return nil, err } + if err := h.eventService.RemoveGroupMembers(ctx, gid, rm...); err != nil { + resperr := h.rollbackAndReturnError(ctx, http.StatusInternalServerError, "failed to replace group members in permissions API") + return nil, resperr + } + + if err := h.eventService.AddGroupMembers(ctx, gid, add...); err != nil { + resperr := h.rollbackAndReturnError(ctx, http.StatusInternalServerError, "failed to replace group members in permissions API") + return nil, resperr + } + return ReplaceGroupMembers200JSONResponse{true}, nil } diff --git a/internal/api/httpsrv/handler_group_members_test.go b/internal/api/httpsrv/handler_group_members_test.go index a42ef83b..f595c68a 100644 --- a/internal/api/httpsrv/handler_group_members_test.go +++ b/internal/api/httpsrv/handler_group_members_test.go @@ -2,17 +2,22 @@ package httpsrv import ( "context" + "fmt" "net/http" "testing" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" pagination "go.infratographer.com/identity-api/internal/crdbx" + "go.infratographer.com/identity-api/internal/events" "go.infratographer.com/identity-api/internal/storage" "go.infratographer.com/identity-api/internal/testingx" "go.infratographer.com/identity-api/internal/types" v1 "go.infratographer.com/identity-api/pkg/api/v1" + "go.infratographer.com/permissions-api/pkg/permissions/mockpermissions" "go.infratographer.com/x/crdbx" + eventsx "go.infratographer.com/x/events" "go.infratographer.com/x/gidx" ) @@ -44,13 +49,16 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.FailNow(t, "initialization failed") } - setupFn := func(ctx context.Context) context.Context { - ctx, err := store.BeginContext(ctx) + es := events.NewEvents() + + beginTx := func(ctx context.Context) context.Context { + tx, err := store.BeginContext(ctx) + if !assert.NoError(t, err) { assert.FailNow(t, "setup failed") } - return ctx + return tx } cleanupFn := func(ctx context.Context) { @@ -61,7 +69,10 @@ func TestGroupMembersAPIHandler(t *testing.T) { t.Run("ListGroupMembers", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } testGroup := &types.Group{ ID: gidx.MustNewID(types.IdentityGroupIDPrefix), @@ -81,7 +92,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { { Name: "Invalid group id", Input: ListGroupMembersRequestObject{GroupID: "definitely not a valid group id"}, - SetupFn: setupFn, + SetupFn: beginTx, CleanupFn: cleanupFn, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[ListGroupMembersResponseObject]) { assert.Nil(t, res.Success) @@ -92,7 +103,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { { Name: "Group not found", Input: ListGroupMembersRequestObject{GroupID: gidx.MustNewID(types.IdentityGroupIDPrefix)}, - SetupFn: setupFn, + SetupFn: beginTx, CleanupFn: cleanupFn, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[ListGroupMembersResponseObject]) { assert.Nil(t, res.Success) @@ -103,7 +114,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { { Name: "Success default pagination", Input: ListGroupMembersRequestObject{GroupID: testGroup.ID}, - SetupFn: setupFn, + SetupFn: beginTx, CleanupFn: cleanupFn, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[ListGroupMembersResponseObject]) { assert.Nil(t, res.Err) @@ -122,7 +133,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { Limit: ptr(1), }, }, - SetupFn: setupFn, + SetupFn: beginTx, CleanupFn: cleanupFn, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[ListGroupMembersResponseObject]) { assert.Nil(t, res.Err) @@ -149,16 +160,36 @@ func TestGroupMembersAPIHandler(t *testing.T) { t.Run("AddGroupMembers", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } - testGroupWithNoMembers := &types.Group{ - ID: gidx.MustNewID(types.IdentityGroupIDPrefix), + m := &mockpermissions.MockPermissions{} + m.On("CreateAuthRelationships").Return(nil) + m.On("DeleteAuthRelationships").Return(nil) + + setupFn := func(ctx context.Context) context.Context { + ctx = m.ContextWithHandler(ctx) + return beginTx(ctx) + } + + notfoundGroupID := gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "notfound")) + + testGroupWithNoMember := &types.Group{ + ID: gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "test-with-no-member")), OwnerID: ownerID, - Name: "test-add-group-members", + Name: "test-add-group-member", + } + + theOtherTestGroupWithNoMember := &types.Group{ + ID: gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "the-other-test-with-no-member")), + OwnerID: ownerID, + Name: "test-add-group-member-1", } testGroupWithSomeMembers := &types.Group{ - ID: gidx.MustNewID(types.IdentityGroupIDPrefix), + ID: gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "test-with-some-members")), OwnerID: ownerID, Name: "test-add-group-members-with-some-members", } @@ -169,7 +200,8 @@ func TestGroupMembersAPIHandler(t *testing.T) { gidx.MustNewID(types.IdentityUserIDPrefix), } - withStoredGroupAndMembers(t, store, testGroupWithNoMembers) + withStoredGroupAndMembers(t, store, testGroupWithNoMember) + withStoredGroupAndMembers(t, store, theOtherTestGroupWithNoMember) withStoredGroupAndMembers(t, store, testGroupWithSomeMembers, someMembers...) tc := []testingx.TestCase[AddGroupMembersRequestObject, []gidx.PrefixedID]{ @@ -182,12 +214,14 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + + m.AssertNotCalled(t, "CreateAuthRelationships", events.GroupTopic, "definitely not a valid group id", mock.Anything) }, }, { Name: "Group not found", Input: AddGroupMembersRequestObject{ - GroupID: gidx.MustNewID(types.IdentityGroupIDPrefix), + GroupID: notfoundGroupID, Body: &v1.AddGroupMembersJSONRequestBody{ MemberIDs: []gidx.PrefixedID{gidx.MustNewID(types.IdentityUserIDPrefix)}, }, @@ -198,12 +232,13 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusNotFound, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, "CreateAuthRelationships", events.GroupTopic, notfoundGroupID, mock.Anything) }, }, { Name: "Invalid member id", Input: AddGroupMembersRequestObject{ - GroupID: testGroupWithNoMembers.ID, + GroupID: testGroupWithNoMember.ID, Body: &v1.AddGroupMembersJSONRequestBody{ MemberIDs: []gidx.PrefixedID{"definitely not a valid member id"}, }, @@ -214,21 +249,76 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled( + t, "CreateAuthRelationships", events.GroupTopic, testGroupWithNoMember.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: "definitely not a valid member id", + }, + ) + }, + }, + + { + Name: "Failed to publish event", + Input: AddGroupMembersRequestObject{ + GroupID: theOtherTestGroupWithNoMember.ID, + Body: &v1.AddGroupMembersJSONRequestBody{ + MemberIDs: []gidx.PrefixedID{gidx.MustNewID(types.IdentityUserIDPrefix)}, + }, + }, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "CreateAuthRelationships", events.GroupTopic, + theOtherTestGroupWithNoMember.ID, + mock.Anything, + ).Return(fmt.Errorf("you bad bad")) // nolint: goerr113 + + return setupFn(ctx) + }, + CleanupFn: func(_ context.Context) {}, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { + assert.Error(t, res.Err) + assert.ErrorContains(t, res.Err, "failed to add group members in permissions API") + + m.AssertCalled( + t, "CreateAuthRelationships", events.GroupTopic, + theOtherTestGroupWithNoMember.ID, mock.Anything, + ) + + // ensure no members were added + mc, err := store.GroupMembersCount(context.Background(), theOtherTestGroupWithNoMember.ID) + assert.NoError(t, err) + assert.Equal(t, 0, mc) }, }, { Name: "Success", Input: AddGroupMembersRequestObject{ - GroupID: testGroupWithNoMembers.ID, + GroupID: testGroupWithNoMember.ID, Body: &v1.AddGroupMembersJSONRequestBody{ MemberIDs: []gidx.PrefixedID{gidx.MustNewID(types.IdentityUserIDPrefix)}, }, }, - SetupFn: setupFn, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "CreateAuthRelationships", events.GroupTopic, + testGroupWithNoMember.ID, + mock.Anything, + ).Return(nil) + + return setupFn(ctx) + }, CleanupFn: func(_ context.Context) {}, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { assert.Nil(t, res.Err) assert.Len(t, res.Success, 1) + + m.AssertCalled( + t, "CreateAuthRelationships", events.GroupTopic, + testGroupWithNoMember.ID, + mock.Anything, + ) }, }, { @@ -239,11 +329,31 @@ func TestGroupMembersAPIHandler(t *testing.T) { MemberIDs: []gidx.PrefixedID{someMembers[0]}, }, }, - SetupFn: setupFn, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "CreateAuthRelationships", events.GroupTopic, + testGroupWithSomeMembers.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: someMembers[0], + }, + ).Return(nil) + + return setupFn(ctx) + }, CleanupFn: func(_ context.Context) {}, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { assert.Nil(t, res.Err) assert.Len(t, res.Success, len(someMembers)) + + m.AssertCalled( + t, "CreateAuthRelationships", events.GroupTopic, + testGroupWithSomeMembers.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: someMembers[0], + }, + ) }, }, } @@ -259,9 +369,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { } ctx = context.Background() - ctx = pagination.AsOfSystemTime(ctx, "") - p := v1.ListGroupMembersParams{} - mm, err := store.ListMembers(ctx, input.GroupID, p) + mm, err := store.ListGroupMembers(ctx, input.GroupID, nil) return testingx.TestResult[[]gidx.PrefixedID]{Success: mm, Err: err} } @@ -272,7 +380,22 @@ func TestGroupMembersAPIHandler(t *testing.T) { t.Run("RemoveGroupMember", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } + + m := &mockpermissions.MockPermissions{} + m.On("CreateAuthRelationships").Return(nil) + m.On("DeleteAuthRelationships").Return(nil) + + setupFn := func(ctx context.Context) context.Context { + ctx = m.ContextWithHandler(ctx) + return beginTx(ctx) + } + + notfoundGroupID := gidx.MustNewID(types.IdentityGroupIDPrefix) + notfoundSubjectID := gidx.MustNewID(types.IdentityUserIDPrefix) testGroup := &types.Group{ ID: gidx.MustNewID(types.IdentityGroupIDPrefix), @@ -280,6 +403,12 @@ func TestGroupMembersAPIHandler(t *testing.T) { Name: "test-remove-group-member", } + theOtherTestGroup := &types.Group{ + ID: gidx.MustNewID(types.IdentityGroupIDPrefix), + OwnerID: ownerID, + Name: "test-remove-group-member-1", + } + someMembers := []gidx.PrefixedID{ gidx.MustNewID(types.IdentityUserIDPrefix), gidx.MustNewID(types.IdentityUserIDPrefix), @@ -287,6 +416,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { } withStoredGroupAndMembers(t, store, testGroup, someMembers...) + withStoredGroupAndMembers(t, store, theOtherTestGroup, someMembers...) tc := []testingx.TestCase[RemoveGroupMemberRequestObject, []gidx.PrefixedID]{ { @@ -301,6 +431,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, "DeleteAuthRelationships", events.GroupTopic, "definitely not a valid group id", mock.Anything) }, }, { @@ -315,12 +446,19 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled( + t, "DeleteAuthRelationships", events.GroupTopic, testGroup.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: "definitely not a valid member id", + }, + ) }, }, { Name: "Group not found", Input: RemoveGroupMemberRequestObject{ - GroupID: gidx.MustNewID(types.IdentityGroupIDPrefix), + GroupID: notfoundGroupID, SubjectID: gidx.MustNewID(types.IdentityUserIDPrefix), }, SetupFn: setupFn, @@ -329,13 +467,14 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusNotFound, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, "DeleteAuthRelationships", events.GroupTopic, notfoundGroupID, mock.Anything) }, }, { Name: "Member not found", Input: RemoveGroupMemberRequestObject{ GroupID: testGroup.ID, - SubjectID: gidx.MustNewID(types.IdentityUserIDPrefix), + SubjectID: notfoundSubjectID, }, SetupFn: setupFn, CleanupFn: cleanupFn, @@ -343,6 +482,48 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusNotFound, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled( + t, "DeleteAuthRelationships", events.GroupTopic, testGroup.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: notfoundSubjectID, + }) + }, + }, + { + Name: "Failed to publish event", + Input: RemoveGroupMemberRequestObject{ + GroupID: theOtherTestGroup.ID, + SubjectID: someMembers[0], + }, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "DeleteAuthRelationships", events.GroupTopic, + theOtherTestGroup.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: someMembers[0], + }, + ).Return(fmt.Errorf("you bad bad")) // nolint: goerr113 + + return setupFn(ctx) + }, + CleanupFn: func(_ context.Context) {}, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { + assert.Error(t, res.Err) + + m.AssertCalled( + t, "DeleteAuthRelationships", events.GroupTopic, + theOtherTestGroup.ID, eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: someMembers[0], + }, + ) + + // ensure the member is still in the group + mc, err := store.GroupMembersCount(context.Background(), theOtherTestGroup.ID) + assert.NoError(t, err) + assert.Len(t, someMembers, mc) }, }, { @@ -351,11 +532,29 @@ func TestGroupMembersAPIHandler(t *testing.T) { GroupID: testGroup.ID, SubjectID: someMembers[0], }, - SetupFn: setupFn, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "DeleteAuthRelationships", events.GroupTopic, + testGroup.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: someMembers[0], + }, + ).Return(nil) + + return setupFn(ctx) + }, CleanupFn: func(_ context.Context) {}, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { assert.Nil(t, res.Err) assert.Len(t, res.Success, len(someMembers)-1) + m.AssertCalled( + t, "DeleteAuthRelationships", events.GroupTopic, + testGroup.ID, eventsx.AuthRelationshipRelation{ + Relation: events.DirectMemberRelationship, + SubjectID: someMembers[0], + }, + ) }, }, } @@ -371,9 +570,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { } ctx = context.Background() - ctx = pagination.AsOfSystemTime(ctx, "") - p := v1.ListGroupMembersParams{} - mm, err := store.ListMembers(ctx, input.GroupID, p) + mm, err := store.ListGroupMembers(ctx, input.GroupID, nil) return testingx.TestResult[[]gidx.PrefixedID]{Success: mm, Err: err} } @@ -384,14 +581,35 @@ func TestGroupMembersAPIHandler(t *testing.T) { t.Run("PutGroupMembers", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } + + m := &mockpermissions.MockPermissions{} + m.On("CreateAuthRelationships").Return(nil) + m.On("DeleteAuthRelationships").Return(nil) + + setupFn := func(ctx context.Context) context.Context { + ctx = m.ContextWithHandler(ctx) + return beginTx(ctx) + } + + notfoundGroupID := gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "notfound-group")) + newSubjectID := gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityUserIDPrefix, "new-subject")) testGroup := &types.Group{ - ID: gidx.MustNewID(types.IdentityGroupIDPrefix), + ID: gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "test-put-group-members")), OwnerID: ownerID, Name: "test-put-group-members", } + theOtherTestGroup := &types.Group{ + ID: gidx.PrefixedID(fmt.Sprintf("%s-%s", types.IdentityGroupIDPrefix, "the-other-test-put-group-members")), + OwnerID: ownerID, + Name: "test-put-group-members-1", + } + someMembers := []gidx.PrefixedID{ gidx.MustNewID(types.IdentityUserIDPrefix), gidx.MustNewID(types.IdentityUserIDPrefix), @@ -399,6 +617,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { } withStoredGroupAndMembers(t, store, testGroup, someMembers...) + withStoredGroupAndMembers(t, store, theOtherTestGroup, someMembers...) tc := []testingx.TestCase[ReplaceGroupMembersRequestObject, []gidx.PrefixedID]{ { @@ -415,6 +634,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, mock.Anything, events.GroupTopic, "definitely not a valid group id", mock.Anything) }, }, { @@ -431,12 +651,13 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, mock.Anything, events.GroupTopic, testGroup.ID, mock.Anything) }, }, { Name: "Group not found", Input: ReplaceGroupMembersRequestObject{ - GroupID: gidx.MustNewID(types.IdentityGroupIDPrefix), + GroupID: notfoundGroupID, Body: &v1.ReplaceGroupMembersJSONRequestBody{ MemberIDs: []gidx.PrefixedID{gidx.MustNewID(types.IdentityUserIDPrefix)}, }, @@ -447,6 +668,39 @@ func TestGroupMembersAPIHandler(t *testing.T) { assert.Nil(t, res.Success) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusNotFound, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, mock.Anything, events.GroupTopic, notfoundGroupID, mock.Anything) + }, + }, + { + Name: "Failed to publish event", + Input: ReplaceGroupMembersRequestObject{ + GroupID: theOtherTestGroup.ID, + Body: &v1.ReplaceGroupMembersJSONRequestBody{ + MemberIDs: []gidx.PrefixedID{gidx.MustNewID(types.IdentityUserIDPrefix)}, + }, + }, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "DeleteAuthRelationships", events.GroupTopic, + theOtherTestGroup.ID, mock.Anything, mock.Anything, mock.Anything, + ).Return(fmt.Errorf("you bad bad")) // nolint: goerr113 + + m.On( + "CreateAuthRelationships", events.GroupTopic, + theOtherTestGroup.ID, mock.Anything, mock.Anything, mock.Anything, + ).Return(fmt.Errorf("you bad bad")) // nolint: goerr113 + + return setupFn(ctx) + }, + CleanupFn: func(_ context.Context) {}, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { + assert.Error(t, res.Err) + assert.ErrorContains(t, res.Err, "failed to replace group members in permissions API") + + // ensure no members were added + mc, err := store.GroupMembersCount(context.Background(), theOtherTestGroup.ID) + assert.NoError(t, err) + assert.Equal(t, len(someMembers), mc) }, }, { @@ -454,14 +708,42 @@ func TestGroupMembersAPIHandler(t *testing.T) { Input: ReplaceGroupMembersRequestObject{ GroupID: testGroup.ID, Body: &v1.ReplaceGroupMembersJSONRequestBody{ - MemberIDs: []gidx.PrefixedID{gidx.MustNewID(types.IdentityUserIDPrefix)}, + MemberIDs: []gidx.PrefixedID{ + someMembers[0], + newSubjectID, + }, }, }, - SetupFn: setupFn, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "DeleteAuthRelationships", events.GroupTopic, testGroup.ID, + mock.Anything, + mock.Anything, + ).Return(nil) + + m.On( + "CreateAuthRelationships", events.GroupTopic, testGroup.ID, + mock.Anything, + ).Return(nil) + + return setupFn(ctx) + }, CleanupFn: func(_ context.Context) {}, CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]gidx.PrefixedID]) { assert.Nil(t, res.Err) - assert.Len(t, res.Success, 1) + assert.Len(t, res.Success, 2) + assert.Contains(t, res.Success, someMembers[0]) + assert.Contains(t, res.Success, newSubjectID) + + m.AssertCalled( + t, "DeleteAuthRelationships", events.GroupTopic, testGroup.ID, + mock.Anything, mock.Anything, + ) + + m.AssertCalled( + t, "CreateAuthRelationships", events.GroupTopic, testGroup.ID, + mock.Anything, + ) }, }, } @@ -477,9 +759,7 @@ func TestGroupMembersAPIHandler(t *testing.T) { } ctx = context.Background() - ctx = pagination.AsOfSystemTime(ctx, "") - p := v1.ListGroupMembersParams{} - mm, err := store.ListMembers(ctx, input.GroupID, p) + mm, err := store.ListGroupMembers(ctx, input.GroupID, nil) return testingx.TestResult[[]gidx.PrefixedID]{Success: mm, Err: err} } @@ -501,7 +781,7 @@ func withStoredGroupAndMembers(t *testing.T, s storage.Engine, group *types.Grou *group = *g - if err := s.AddMembers(seedCtx, group.ID, m...); !assert.NoError(t, err) { + if err := s.AddGroupMembers(seedCtx, group.ID, m...); !assert.NoError(t, err) { assert.FailNow(t, "failed to add members") } diff --git a/internal/api/httpsrv/handler_group_test.go b/internal/api/httpsrv/handler_group_test.go index 0797d6ee..b2e56c25 100644 --- a/internal/api/httpsrv/handler_group_test.go +++ b/internal/api/httpsrv/handler_group_test.go @@ -8,14 +8,18 @@ import ( "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" pagination "go.infratographer.com/identity-api/internal/crdbx" + "go.infratographer.com/identity-api/internal/events" "go.infratographer.com/identity-api/internal/storage" "go.infratographer.com/identity-api/internal/testingx" "go.infratographer.com/identity-api/internal/types" v1 "go.infratographer.com/identity-api/pkg/api/v1" + "go.infratographer.com/permissions-api/pkg/permissions/mockpermissions" "go.infratographer.com/x/crdbx" + eventsx "go.infratographer.com/x/events" "go.infratographer.com/x/gidx" ) @@ -37,6 +41,7 @@ func TestGroupAPIHandler(t *testing.T) { }) ownerID := gidx.MustNewID("testten") + ownerIDFailure := gidx.MustNewID("testten") config := crdbx.Config{ URI: testServer.PGURL().String(), @@ -47,11 +52,14 @@ func TestGroupAPIHandler(t *testing.T) { assert.FailNow(t, "initialization failed") } + es := events.NewEvents() + t.Run("GetGroup", func(t *testing.T) { t.Parallel() handler := apiHandler{ - engine: store, + engine: store, + eventService: es, } getGroupTestGroup := &types.Group{ @@ -133,7 +141,12 @@ func TestGroupAPIHandler(t *testing.T) { t.Run("CreateGroup", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } + + successOwnerID := gidx.MustNewID("testten") createGroupTestGroup := &types.Group{ ID: gidx.MustNewID(types.IdentityGroupIDPrefix), @@ -144,13 +157,19 @@ func TestGroupAPIHandler(t *testing.T) { withStoredGroups(t, store, createGroupTestGroup) + m := &mockpermissions.MockPermissions{} + m.On("CreateAuthRelationships").Return(nil) + m.On("DeleteAuthRelationships").Return(nil) + setupFn := func(ctx context.Context) context.Context { - ctx, err := store.BeginContext(ctx) + ctx = m.ContextWithHandler(ctx) + tx, err := store.BeginContext(ctx) + if !assert.NoError(t, err) { assert.FailNow(t, "setup failed") } - return ctx + return tx } cleanupFn := func(ctx context.Context) { @@ -160,6 +179,7 @@ func TestGroupAPIHandler(t *testing.T) { runFn := func(ctx context.Context, input CreateGroupRequestObject) testingx.TestResult[CreateGroupResponseObject] { resp, err := handler.CreateGroup(ctx, input) + return testingx.TestResult[CreateGroupResponseObject]{Success: resp, Err: err} } @@ -175,6 +195,12 @@ func TestGroupAPIHandler(t *testing.T) { assert.Error(t, res.Err) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled( + t, "CreateAuthRelationships", events.GroupTopic, mock.Anything, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: ownerID, + }) }, }, { @@ -191,6 +217,13 @@ func TestGroupAPIHandler(t *testing.T) { assert.Error(t, res.Err) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled( + t, "CreateAuthRelationships", events.GroupTopic, mock.Anything, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: ownerID, + }, + ) }, }, { @@ -207,18 +240,80 @@ func TestGroupAPIHandler(t *testing.T) { assert.Error(t, res.Err) assert.IsType(t, &echo.HTTPError{}, res.Err, "unexpected error type", res.Err.Error()) assert.Equal(t, http.StatusConflict, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled( + t, "CreateAuthRelationships", events.GroupTopic, mock.Anything, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: ownerID, + }, + ) + }, + }, + { + Name: "Fail to publish group relationship", + Input: CreateGroupRequestObject{ + OwnerID: ownerIDFailure, + Body: &v1.CreateGroupJSONRequestBody{ + Name: "test-creategroup-1", + Description: ptr("it's a group for testing create group"), + }, + }, + CleanupFn: func(_ context.Context) {}, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "CreateAuthRelationships", + events.GroupTopic, + mock.Anything, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: ownerIDFailure, + }, + ).Return(fmt.Errorf("you bad bad")) // nolint: goerr113 + + return setupFn(ctx) + }, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[CreateGroupResponseObject]) { + assert.NotNil(t, res.Err) + assert.Nil(t, res.Success) + + m.AssertCalled(t, "CreateAuthRelationships", events.GroupTopic, mock.Anything, mock.Anything) + + // ensure group will not be created + ctx := context.Background() + ctx = pagination.AsOfSystemTime(ctx, "") + p := v1.ListGroupsParams{} + g, err := store.ListGroupsByOwner(ctx, ownerIDFailure, p) + assert.NoError(t, err) + + for _, gg := range g { + if gg.OwnerID == ownerIDFailure { + assert.NotEqual(t, "test-creategroup-1", gg.Name) + } + } }, }, { Name: "Success", Input: CreateGroupRequestObject{ - OwnerID: ownerID, + OwnerID: successOwnerID, Body: &v1.CreateGroupJSONRequestBody{ Name: "test-creategroup-2", Description: ptr("new group description"), }, }, - SetupFn: setupFn, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "CreateAuthRelationships", + events.GroupTopic, + mock.Anything, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: successOwnerID, + }, + ).Return(nil) + + return setupFn(ctx) + }, CleanupFn: cleanupFn, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[CreateGroupResponseObject]) { assert.NoError(t, res.Err) @@ -233,6 +328,15 @@ func TestGroupAPIHandler(t *testing.T) { assert.Equal(t, item.ID, group.ID) assert.Equal(t, item.Name, group.Name) assert.Equal(t, *item.Description, group.Description) + + m.AssertCalled( + t, "CreateAuthRelationships", + events.GroupTopic, item.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: successOwnerID, + }, + ) }, }, } @@ -245,9 +349,12 @@ func TestGroupAPIHandler(t *testing.T) { const numOfGroups = 5 - theOtherOwnerID := gidx.MustNewID("testten") - handler := apiHandler{engine: store} listGroupsTestGroups := make([]*types.Group, numOfGroups) + theOtherOwnerID := gidx.MustNewID("testten") + handler := apiHandler{ + engine: store, + eventService: es, + } for i := 0; i < numOfGroups; i++ { listGroupsTestGroups[i] = &types.Group{ @@ -351,7 +458,10 @@ func TestGroupAPIHandler(t *testing.T) { t.Run("UpdateGroup", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } theOtherOwnerID := gidx.MustNewID("testten") @@ -494,7 +604,13 @@ func TestGroupAPIHandler(t *testing.T) { t.Run("DeleteGroup", func(t *testing.T) { t.Parallel() - handler := apiHandler{engine: store} + handler := apiHandler{ + engine: store, + eventService: es, + } + + notfoundID := gidx.MustNewID(types.IdentityGroupIDPrefix) + deleteGroupTestGroup := &types.Group{ ID: gidx.MustNewID(types.IdentityGroupIDPrefix), OwnerID: ownerID, @@ -502,15 +618,44 @@ func TestGroupAPIHandler(t *testing.T) { Description: "it's a group for testing delete group", } + theOtherTestGroup := &types.Group{ + ID: gidx.MustNewID(types.IdentityGroupIDPrefix), + OwnerID: ownerID, + Name: "test-deletegroup-2", + Description: "it's a group for testing delete group", + } + withStoredGroups(t, store, deleteGroupTestGroup) + withStoredGroups(t, store, theOtherTestGroup) + + testGroupWithSomeMembers := &types.Group{ + ID: gidx.MustNewID(types.IdentityGroupIDPrefix), + OwnerID: ownerID, + Name: "test-deletegroup-3", + Description: "it's a group for testing delete group", + } + + someMembers := []gidx.PrefixedID{ + gidx.MustNewID(types.IdentityUserIDPrefix), + gidx.MustNewID(types.IdentityUserIDPrefix), + gidx.MustNewID(types.IdentityUserIDPrefix), + } + + withStoredGroupAndMembers(t, store, testGroupWithSomeMembers, someMembers...) + + m := &mockpermissions.MockPermissions{} + m.On("CreateAuthRelationships").Return(nil) + m.On("DeleteAuthRelationships").Return(nil) setupFn := func(ctx context.Context) context.Context { - ctx, err := store.BeginContext(ctx) + ctx = m.ContextWithHandler(ctx) + tx, err := store.BeginContext(ctx) + if !assert.NoError(t, err) { assert.FailNow(t, "setup failed") } - return ctx + return tx } cleanupFn := func(ctx context.Context) { @@ -535,12 +680,13 @@ func TestGroupAPIHandler(t *testing.T) { assert.Error(t, res.Err) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, "DeleteAuthRelationships", events.GroupTopic, "notavalidid", mock.Anything) }, }, { Name: "Group not found", Input: DeleteGroupRequestObject{ - GroupID: gidx.MustNewID(types.IdentityGroupIDPrefix), + GroupID: notfoundID, }, SetupFn: setupFn, CleanupFn: cleanupFn, @@ -548,6 +694,52 @@ func TestGroupAPIHandler(t *testing.T) { assert.Error(t, res.Err) assert.IsType(t, &echo.HTTPError{}, res.Err) assert.Equal(t, http.StatusNotFound, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, "DeleteAuthRelationships", events.GroupTopic, notfoundID, mock.Anything) + }, + }, + { + Name: "Group with members", + Input: DeleteGroupRequestObject{ + GroupID: testGroupWithSomeMembers.ID, + }, + SetupFn: setupFn, + CleanupFn: cleanupFn, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[DeleteGroupResponseObject]) { + assert.Error(t, res.Err) + assert.IsType(t, &echo.HTTPError{}, res.Err) + assert.Equal(t, http.StatusBadRequest, res.Err.(*echo.HTTPError).Code) + m.AssertNotCalled(t, "DeleteAuthRelationships", events.GroupTopic, testGroupWithSomeMembers.ID, mock.Anything) + }, + }, + { + Name: "Fail to publish group relationship", + Input: DeleteGroupRequestObject{ + GroupID: theOtherTestGroup.ID, + }, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "DeleteAuthRelationships", + events.GroupTopic, + theOtherTestGroup.ID, + mock.Anything, + ).Return(fmt.Errorf("you bad bad")) // nolint: goerr113 + + return setupFn(ctx) + }, + CleanupFn: func(_ context.Context) {}, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[DeleteGroupResponseObject]) { + assert.NotNil(t, res.Err) + assert.Nil(t, res.Success) + + m.AssertCalled( + t, "DeleteAuthRelationships", events.GroupTopic, + theOtherTestGroup.ID, mock.Anything, + ) + + // ensure group will not be deleted + group, err := store.GetGroupByID(context.Background(), deleteGroupTestGroup.ID) + assert.NoError(t, err) + assert.NotNil(t, group) }, }, { @@ -555,7 +747,16 @@ func TestGroupAPIHandler(t *testing.T) { Input: DeleteGroupRequestObject{ GroupID: deleteGroupTestGroup.ID, }, - SetupFn: setupFn, + SetupFn: func(ctx context.Context) context.Context { + m.On( + "DeleteAuthRelationships", + events.GroupTopic, + deleteGroupTestGroup.ID, + mock.Anything, + ).Return(nil) // nolint: goerr113 + + return setupFn(ctx) + }, CleanupFn: cleanupFn, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[DeleteGroupResponseObject]) { assert.NoError(t, res.Err) @@ -565,6 +766,15 @@ func TestGroupAPIHandler(t *testing.T) { assert.Error(t, err) assert.IsType(t, &echo.HTTPError{}, err) assert.Equal(t, http.StatusNotFound, err.(*echo.HTTPError).Code) + + m.AssertCalled( + t, "DeleteAuthRelationships", + events.GroupTopic, deleteGroupTestGroup.ID, + eventsx.AuthRelationshipRelation{ + Relation: events.GroupParentRelationship, + SubjectID: ownerID, + }, + ) }, }, } diff --git a/internal/config/config.go b/internal/config/config.go index 8aacc6f3..7a270fbd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,6 +5,7 @@ import ( "go.infratographer.com/permissions-api/pkg/permissions" "go.infratographer.com/x/crdbx" "go.infratographer.com/x/echox" + eventsx "go.infratographer.com/x/events" "go.infratographer.com/x/loggingx" "go.infratographer.com/x/otelx" @@ -21,4 +22,5 @@ var Config struct { CRDB crdbx.Config Audit auditx.Config Permissions permissions.Config + Events eventsx.Config } diff --git a/internal/events/doc.go b/internal/events/doc.go new file mode 100644 index 00000000..cb38771e --- /dev/null +++ b/internal/events/doc.go @@ -0,0 +1,3 @@ +// Package events contains methods to publish group memberships +// info to the permissions-api +package events diff --git a/internal/events/events.go b/internal/events/events.go new file mode 100644 index 00000000..36c993cc --- /dev/null +++ b/internal/events/events.go @@ -0,0 +1,34 @@ +package events + +import ( + "go.uber.org/zap" +) + +// Events represents a collection of relationships. +type Events struct { + logger *zap.Logger +} + +// Events implements the Service interface. +var _ Service = (*Events)(nil) + +// Opt represents an option for configuring Relationships. +type Opt func(*Events) + +// NewEvents creates a new Relationships instance with the given NATS URL and options. +func NewEvents(opts ...Opt) *Events { + r := &Events{} + + for _, opt := range opts { + opt(r) + } + + return r +} + +// WithLogger is an option to set the logger for Relationships. +func WithLogger(logger *zap.Logger) Opt { + return func(e *Events) { + e.logger = logger + } +} diff --git a/internal/events/groups_event.go b/internal/events/groups_event.go new file mode 100644 index 00000000..339778e2 --- /dev/null +++ b/internal/events/groups_event.go @@ -0,0 +1,84 @@ +package events + +import ( + "context" + + "go.infratographer.com/permissions-api/pkg/permissions" + eventsx "go.infratographer.com/x/events" + "go.infratographer.com/x/gidx" +) + +const ( + // DirectMemberRelationship is the direct member relationship. + DirectMemberRelationship = "direct_member" + // GroupParentRelationship is the group parent relationship. + GroupParentRelationship = "parent" + // GroupTopic is the group topic. + GroupTopic = "group" +) + +// AddGroupMembers adds subjects to a group. +func (e *Events) AddGroupMembers(ctx context.Context, gid gidx.PrefixedID, subjIDs ...gidx.PrefixedID) error { + if len(subjIDs) == 0 { + return nil + } + + rels := make([]eventsx.AuthRelationshipRelation, 0, len(subjIDs)) + + for _, subj := range subjIDs { + if subj == "" { + continue + } + + rels = append(rels, + eventsx.AuthRelationshipRelation{ + Relation: DirectMemberRelationship, + SubjectID: subj, + }, + ) + } + + return permissions.CreateAuthRelationships(ctx, GroupTopic, gid, rels...) +} + +// RemoveGroupMembers removes subjects from a group. +func (e *Events) RemoveGroupMembers(ctx context.Context, gid gidx.PrefixedID, subjIDs ...gidx.PrefixedID) error { + rels := make([]eventsx.AuthRelationshipRelation, 0, len(subjIDs)) + + for _, subj := range subjIDs { + if subj == "" { + continue + } + + rels = append(rels, + eventsx.AuthRelationshipRelation{ + Relation: DirectMemberRelationship, + SubjectID: subj, + }, + ) + } + + return permissions.DeleteAuthRelationships(ctx, GroupTopic, gid, rels...) +} + +// CreateGroup creates a group. +func (e *Events) CreateGroup(ctx context.Context, parentID, gid gidx.PrefixedID) error { + return permissions.CreateAuthRelationships( + ctx, GroupTopic, gid, + eventsx.AuthRelationshipRelation{ + Relation: GroupParentRelationship, + SubjectID: parentID, + }, + ) +} + +// DeleteGroup deletes a group. +func (e *Events) DeleteGroup(ctx context.Context, parentID, gid gidx.PrefixedID) error { + return permissions.DeleteAuthRelationships( + ctx, GroupTopic, gid, + eventsx.AuthRelationshipRelation{ + Relation: GroupParentRelationship, + SubjectID: parentID, + }, + ) +} diff --git a/internal/events/service.go b/internal/events/service.go new file mode 100644 index 00000000..9f931d6c --- /dev/null +++ b/internal/events/service.go @@ -0,0 +1,24 @@ +package events + +import ( + "context" + + "go.infratographer.com/x/gidx" +) + +// Service is the interface for the events service. +type Service interface { + GroupService +} + +// GroupService provides group-related event publishing and handling. +type GroupService interface { + // AddGroupMembers adds subjects to a group. + AddGroupMembers(ctx context.Context, gid gidx.PrefixedID, subjIDs ...gidx.PrefixedID) error + // RemoveGroupMembers removes subjects from a group. + RemoveGroupMembers(ctx context.Context, gid gidx.PrefixedID, subjIDs ...gidx.PrefixedID) error + // CreateGroup creates a group. + CreateGroup(ctx context.Context, parentID, gid gidx.PrefixedID) error + // DeleteGroup deletes a group. + DeleteGroup(ctx context.Context, parentID, gid gidx.PrefixedID) error +} diff --git a/internal/storage/groups.go b/internal/storage/groups.go index ad08bd14..3b3c6171 100644 --- a/internal/storage/groups.go +++ b/internal/storage/groups.go @@ -7,6 +7,7 @@ import ( "fmt" "strings" + "github.com/lib/pq" "go.infratographer.com/identity-api/internal/crdbx" "go.infratographer.com/identity-api/internal/types" "go.infratographer.com/x/gidx" @@ -37,6 +38,11 @@ var groupColsStr = strings.Join([]string{ groupCols.Name, groupCols.Description, }, ", ") +const ( + membersTable = "group_members" + groupsTable = "groups" +) + type groupService struct { db *sql.DB } @@ -237,7 +243,7 @@ func (gs *groupService) DeleteGroup(ctx context.Context, id gidx.PrefixedID) err return err } -func (gs *groupService) AddMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) error { +func (gs *groupService) AddGroupMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) error { if len(subjects) == 0 { return nil } @@ -276,22 +282,40 @@ func (gs *groupService) AddMembers(ctx context.Context, groupID gidx.PrefixedID, return err } -func (gs *groupService) ListMembers(ctx context.Context, groupID gidx.PrefixedID, pagination crdbx.Paginator) ([]gidx.PrefixedID, error) { - paginate := crdbx.Paginate(pagination, crdbx.ContextAsOfSystemTime(ctx, "-1m")) - +func (gs *groupService) ListGroupMembers(ctx context.Context, groupID gidx.PrefixedID, pagination crdbx.Paginator) ([]gidx.PrefixedID, error) { if _, err := gs.fetchGroupByID(ctx, groupID); err != nil { return nil, err } - q := fmt.Sprintf( - "SELECT %s FROM group_members %s WHERE %s = $1 %s %s %s", - groupMemberCols.SubjectID, paginate.AsOfSystemTime(), groupMemberCols.GroupID, - paginate.AndWhere(2), //nolint:gomnd - paginate.OrderClause(), - paginate.LimitClause(), - ) + var ex func() (*sql.Rows, error) - rows, err := gs.db.QueryContext(ctx, q, paginate.Values(groupID)...) + if pagination != nil { + paginate := crdbx.Paginate(pagination, crdbx.ContextAsOfSystemTime(ctx, "-1m")) + + q := fmt.Sprintf( + "SELECT %s FROM %s %s WHERE %s = $1 %s %s %s", + groupMemberCols.SubjectID, membersTable, + paginate.AsOfSystemTime(), groupMemberCols.GroupID, + paginate.AndWhere(2), //nolint:gomnd + paginate.OrderClause(), + paginate.LimitClause(), + ) + + ex = func() (*sql.Rows, error) { + return gs.db.QueryContext(ctx, q, paginate.Values(groupID)...) + } + } else { + q := fmt.Sprintf( + "SELECT %s FROM %s WHERE %s = $1", + groupMemberCols.SubjectID, membersTable, groupMemberCols.GroupID, + ) + + ex = func() (*sql.Rows, error) { + return gs.db.QueryContext(ctx, q, groupID) + } + } + + rows, err := ex() if err != nil { return nil, err } @@ -313,7 +337,7 @@ func (gs *groupService) ListMembers(ctx context.Context, groupID gidx.PrefixedID return members, nil } -func (gs *groupService) RemoveMember(ctx context.Context, groupID gidx.PrefixedID, subject gidx.PrefixedID) error { +func (gs *groupService) RemoveGroupMember(ctx context.Context, groupID gidx.PrefixedID, subject gidx.PrefixedID) error { tx, err := getContextTx(ctx) if err != nil { return err @@ -343,37 +367,43 @@ func (gs *groupService) RemoveMember(ctx context.Context, groupID gidx.PrefixedI return err } -func (gs *groupService) ReplaceMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) error { +func (gs *groupService) ReplaceGroupMembers( + ctx context.Context, groupID gidx.PrefixedID, incoming ...gidx.PrefixedID, +) ([]gidx.PrefixedID, []gidx.PrefixedID, error) { tx, err := getContextTx(ctx) if err != nil { - return err + return nil, nil, err } - if _, err := gs.fetchGroupByID(ctx, groupID); err != nil { - return err + current, err := gs.ListGroupMembers(ctx, groupID, nil) + if err != nil { + return nil, nil, err } + valFn := func(x gidx.PrefixedID) string { return x.String() } + add, rm := Diff(current, incoming, valFn) + delq := fmt.Sprintf( - "DELETE FROM group_members WHERE %s = $1", - groupMemberCols.GroupID, + "DELETE FROM %s WHERE %s = $1 AND %s = ANY($2)", + membersTable, + groupMemberCols.GroupID, groupMemberCols.SubjectID, ) - _, err = tx.ExecContext(ctx, delq, groupID) + _, err = tx.ExecContext(ctx, delq, groupID, pq.Array(rm)) if err != nil { - return err + return nil, nil, err } - return gs.AddMembers(ctx, groupID, subjects...) + if err := gs.AddGroupMembers(ctx, groupID, add...); err != nil { + return nil, nil, err + } + + return add, rm, nil } func (gs *groupService) ListGroupsBySubject(ctx context.Context, subject gidx.PrefixedID, pagination crdbx.Paginator) (types.Groups, error) { paginate := crdbx.Paginate(pagination, crdbx.ContextAsOfSystemTime(ctx, "-1m")) - const ( - membersTable = "group_members" - groupsTable = "groups" - ) - q := fmt.Sprintf( `SELECT %s FROM %s LEFT JOIN %s ON %s %s WHERE %s = $1 %s %s %s`, // SELECT @@ -424,3 +454,20 @@ func (gs *groupService) ListGroupsBySubject(ctx context.Context, subject gidx.Pr return groups, nil } + +func (gs *groupService) GroupMembersCount(ctx context.Context, groupID gidx.PrefixedID) (int, error) { + q := fmt.Sprintf( + "SELECT COUNT(*) FROM %s WHERE %s = $1", + membersTable, groupMemberCols.GroupID, + ) + + row := gs.db.QueryRowContext(ctx, q, groupID) + + var count int + + if err := row.Scan(&count); err != nil { + return -1, err + } + + return count, nil +} diff --git a/internal/storage/helpers.go b/internal/storage/helpers.go new file mode 100644 index 00000000..2371b3f0 --- /dev/null +++ b/internal/storage/helpers.go @@ -0,0 +1,29 @@ +package storage + +// Diff compares the current and incoming values and returns the difference. +func Diff[T any, V comparable](current, incoming []T, val func(T) V) (add, rm []T) { + curr := make(map[V]struct{}, len(current)) + in := make(map[V]struct{}, len(incoming)) + + for _, c := range current { + curr[val(c)] = struct{}{} + } + + for _, i := range incoming { + in[val(i)] = struct{}{} + } + + for _, c := range current { + if _, ok := in[val(c)]; !ok { + rm = append(rm, c) + } + } + + for _, i := range incoming { + if _, ok := curr[val(i)]; !ok { + add = append(add, i) + } + } + + return +} diff --git a/internal/types/groups.go b/internal/types/groups.go index dad65a91..2d9d0ed0 100644 --- a/internal/types/groups.go +++ b/internal/types/groups.go @@ -57,14 +57,16 @@ type GroupService interface { // ListGroupsBySubject retrieves a list of groups that a subject is a member of. ListGroupsBySubject(ctx context.Context, subject gidx.PrefixedID, pagination crdbx.Paginator) (Groups, error) - // AddMembers adds subjects to a group. - AddMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) error - // ListMembers retrieves a list of subjects in a group. - ListMembers(ctx context.Context, groupID gidx.PrefixedID, pagination crdbx.Paginator) ([]gidx.PrefixedID, error) - // RemoveMember removes a subject from a group. - RemoveMember(ctx context.Context, groupID gidx.PrefixedID, subject gidx.PrefixedID) error - // ReplaceMembers replaces the members of a group with a new set of subjects. - ReplaceMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) error + // AddGroupMembers adds subjects to a group. + AddGroupMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) error + // ListGroupMembers retrieves a list of subjects in a group. + ListGroupMembers(ctx context.Context, groupID gidx.PrefixedID, pagination crdbx.Paginator) ([]gidx.PrefixedID, error) + // RemoveGroupMember removes a subject from a group. + RemoveGroupMember(ctx context.Context, groupID gidx.PrefixedID, subject gidx.PrefixedID) error + // ReplaceGroupMembers replaces the members of a group with a new set of subjects. + ReplaceGroupMembers(ctx context.Context, groupID gidx.PrefixedID, subjects ...gidx.PrefixedID) (add, rm []gidx.PrefixedID, err error) + // GroupMembersCount retrieves the number of members in a group. + GroupMembersCount(ctx context.Context, groupID gidx.PrefixedID) (int, error) } // Groups represents a list of groups