Skip to content

Commit 7a0f89a

Browse files
refactor(group): remove AddGroupUsers handler + authz entry
The AddGroupUsers RPC was removed from proton in raystack/proton#488. With the SDK fully off this RPC since #1609 and the handler now just a thin loop over SetGroupMemberRole, drop the handler, its test, and the authorization-interceptor entry. Regenerated proto stubs reflect the removal. PROTON_COMMIT bumped to the proton branch SHA; will be re-pinned to the merge commit once that PR lands. RemoveGroupUser stays — no replacement RPC for group member removal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 74c44fa commit 7a0f89a

8 files changed

Lines changed: 4994 additions & 5364 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ TAG := $(shell git rev-list --tags --max-count=1)
44
VERSION := $(shell git describe --tags ${TAG})
55
.PHONY: build check fmt lint test test-race vet test-cover-html help install proto admin-app compose-up-dev
66
.DEFAULT_GOAL := build
7-
PROTON_COMMIT := "859ba765e6cfd44736ddcf42664b742fe7fd916e"
7+
PROTON_COMMIT := "e423740e7d1a964937fcbb8f9b085261b34b7c3e"
88

99
admin-app:
1010
@echo " > generating admin build"

internal/api/v1beta1connect/group.go

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -392,55 +392,6 @@ func (h *ConnectHandler) ListGroupUsers(ctx context.Context, request *connect.Re
392392
}), nil
393393
}
394394

395-
func (h *ConnectHandler) AddGroupUsers(ctx context.Context, request *connect.Request[frontierv1beta1.AddGroupUsersRequest]) (*connect.Response[frontierv1beta1.AddGroupUsersResponse], error) {
396-
errorLogger := NewErrorLogger()
397-
398-
_, err := h.orgService.Get(ctx, request.Msg.GetOrgId())
399-
if err != nil {
400-
switch {
401-
case errors.Is(err, organization.ErrDisabled):
402-
return nil, connect.NewError(connect.CodeNotFound, ErrOrgDisabled)
403-
case errors.Is(err, organization.ErrNotExist):
404-
return nil, connect.NewError(connect.CodeNotFound, ErrOrgNotFound)
405-
default:
406-
errorLogger.LogServiceError(ctx, request, "AddGroupUsers.Get", err,
407-
"org_id", request.Msg.GetOrgId())
408-
return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError)
409-
}
410-
}
411-
412-
var joinedErr error
413-
for _, userID := range request.Msg.GetUserIds() {
414-
if err := h.membershipService.SetGroupMemberRole(ctx, request.Msg.GetId(), userID, schema.UserPrincipal, schema.GroupMemberRole); err != nil {
415-
joinedErr = errors.Join(joinedErr, err)
416-
}
417-
}
418-
if joinedErr != nil {
419-
errorLogger.LogServiceError(ctx, request, "AddGroupUsers.SetGroupMemberRole", joinedErr,
420-
"group_id", request.Msg.GetId(),
421-
"user_ids", request.Msg.GetUserIds())
422-
423-
// errors.Is walks the joined error tree; the first matching case wins.
424-
// If multiple users failed with different errors, callers see the most
425-
// specific code we recognize rather than a generic Internal.
426-
switch {
427-
case errors.Is(joinedErr, group.ErrNotExist), errors.Is(joinedErr, group.ErrInvalidID), errors.Is(joinedErr, group.ErrInvalidUUID):
428-
return nil, connect.NewError(connect.CodeNotFound, ErrGroupNotFound)
429-
case errors.Is(joinedErr, user.ErrNotExist):
430-
return nil, connect.NewError(connect.CodeNotFound, ErrUserNotExist)
431-
case errors.Is(joinedErr, user.ErrDisabled):
432-
return nil, connect.NewError(connect.CodeFailedPrecondition, user.ErrDisabled)
433-
case errors.Is(joinedErr, membership.ErrNotOrgMember):
434-
return nil, connect.NewError(connect.CodeFailedPrecondition, ErrNotOrgMember)
435-
case errors.Is(joinedErr, membership.ErrLastGroupOwnerRole):
436-
return nil, connect.NewError(connect.CodeFailedPrecondition, ErrLastGroupOwnerRole)
437-
default:
438-
return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError)
439-
}
440-
}
441-
return connect.NewResponse(&frontierv1beta1.AddGroupUsersResponse{}), nil
442-
}
443-
444395
func (h *ConnectHandler) RemoveGroupUser(ctx context.Context, request *connect.Request[frontierv1beta1.RemoveGroupUserRequest]) (*connect.Response[frontierv1beta1.RemoveGroupUserResponse], error) {
445396
errorLogger := NewErrorLogger()
446397

internal/api/v1beta1connect/group_test.go

Lines changed: 0 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,149 +1236,6 @@ func TestConnectHandler_ListGroupUsers(t *testing.T) {
12361236
}
12371237
}
12381238

1239-
func TestConnectHandler_AddGroupUsers(t *testing.T) {
1240-
someGroupID := utils.NewString()
1241-
someUserID := utils.NewString()
1242-
tests := []struct {
1243-
name string
1244-
setup func(ms *mocks.MembershipService, os *mocks.OrganizationService)
1245-
request *connect.Request[frontierv1beta1.AddGroupUsersRequest]
1246-
want *connect.Response[frontierv1beta1.AddGroupUsersResponse]
1247-
wantErr error
1248-
}{
1249-
{
1250-
name: "should return error if org does not exist",
1251-
setup: func(_ *mocks.MembershipService, os *mocks.OrganizationService) {
1252-
os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist)
1253-
},
1254-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1255-
Id: someGroupID,
1256-
OrgId: testOrgID,
1257-
}),
1258-
want: nil,
1259-
wantErr: connect.NewError(connect.CodeNotFound, ErrOrgNotFound),
1260-
},
1261-
{
1262-
name: "should return error if org is disabled",
1263-
setup: func(_ *mocks.MembershipService, os *mocks.OrganizationService) {
1264-
os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled)
1265-
},
1266-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1267-
Id: someGroupID,
1268-
OrgId: testOrgID,
1269-
}),
1270-
want: nil,
1271-
wantErr: connect.NewError(connect.CodeNotFound, ErrOrgDisabled),
1272-
},
1273-
{
1274-
name: "should return internal server error if SetGroupMemberRole fails for any user",
1275-
setup: func(ms *mocks.MembershipService, os *mocks.OrganizationService) {
1276-
os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil)
1277-
ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, someUserID, schema.UserPrincipal, schema.GroupMemberRole).Return(errors.New("some error"))
1278-
},
1279-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1280-
Id: someGroupID,
1281-
OrgId: testOrgID,
1282-
UserIds: []string{someUserID},
1283-
}),
1284-
want: nil,
1285-
wantErr: connect.NewError(connect.CodeInternal, ErrInternalServerError),
1286-
},
1287-
{
1288-
name: "should return success if SetGroupMemberRole succeeds for each user",
1289-
setup: func(ms *mocks.MembershipService, os *mocks.OrganizationService) {
1290-
os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil)
1291-
ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, someUserID, schema.UserPrincipal, schema.GroupMemberRole).Return(nil)
1292-
},
1293-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1294-
Id: someGroupID,
1295-
OrgId: testOrgID,
1296-
UserIds: []string{someUserID},
1297-
}),
1298-
want: connect.NewResponse(&frontierv1beta1.AddGroupUsersResponse{}),
1299-
wantErr: nil,
1300-
},
1301-
{
1302-
name: "should return not found if group does not exist",
1303-
setup: func(ms *mocks.MembershipService, os *mocks.OrganizationService) {
1304-
os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil)
1305-
ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, someUserID, schema.UserPrincipal, schema.GroupMemberRole).Return(group.ErrNotExist)
1306-
},
1307-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1308-
Id: someGroupID,
1309-
OrgId: testOrgID,
1310-
UserIds: []string{someUserID},
1311-
}),
1312-
want: nil,
1313-
wantErr: connect.NewError(connect.CodeNotFound, ErrGroupNotFound),
1314-
},
1315-
{
1316-
name: "should return not found if user does not exist",
1317-
setup: func(ms *mocks.MembershipService, os *mocks.OrganizationService) {
1318-
os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil)
1319-
ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, someUserID, schema.UserPrincipal, schema.GroupMemberRole).Return(user.ErrNotExist)
1320-
},
1321-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1322-
Id: someGroupID,
1323-
OrgId: testOrgID,
1324-
UserIds: []string{someUserID},
1325-
}),
1326-
want: nil,
1327-
wantErr: connect.NewError(connect.CodeNotFound, ErrUserNotExist),
1328-
},
1329-
{
1330-
name: "should return failed precondition if user is not a member of the org",
1331-
setup: func(ms *mocks.MembershipService, os *mocks.OrganizationService) {
1332-
os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil)
1333-
ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, someUserID, schema.UserPrincipal, schema.GroupMemberRole).Return(membership.ErrNotOrgMember)
1334-
},
1335-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1336-
Id: someGroupID,
1337-
OrgId: testOrgID,
1338-
UserIds: []string{someUserID},
1339-
}),
1340-
want: nil,
1341-
wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrNotOrgMember),
1342-
},
1343-
{
1344-
name: "should return failed precondition if demoting last group owner",
1345-
setup: func(ms *mocks.MembershipService, os *mocks.OrganizationService) {
1346-
os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil)
1347-
ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, someUserID, schema.UserPrincipal, schema.GroupMemberRole).Return(membership.ErrLastGroupOwnerRole)
1348-
},
1349-
request: connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{
1350-
Id: someGroupID,
1351-
OrgId: testOrgID,
1352-
UserIds: []string{someUserID},
1353-
}),
1354-
want: nil,
1355-
wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrLastGroupOwnerRole),
1356-
},
1357-
}
1358-
for _, tt := range tests {
1359-
t.Run(tt.name, func(t *testing.T) {
1360-
mockMembershipSvc := new(mocks.MembershipService)
1361-
mockOrgSvc := new(mocks.OrganizationService)
1362-
if tt.setup != nil {
1363-
tt.setup(mockMembershipSvc, mockOrgSvc)
1364-
}
1365-
h := ConnectHandler{
1366-
membershipService: mockMembershipSvc,
1367-
orgService: mockOrgSvc,
1368-
}
1369-
got, err := h.AddGroupUsers(context.Background(), tt.request)
1370-
if tt.wantErr != nil {
1371-
assert.Error(t, err)
1372-
assert.Equal(t, tt.wantErr.(*connect.Error).Code(), err.(*connect.Error).Code())
1373-
assert.Equal(t, tt.wantErr.(*connect.Error).Message(), err.(*connect.Error).Message())
1374-
} else {
1375-
assert.NoError(t, err)
1376-
assert.EqualValues(t, tt.want, got)
1377-
}
1378-
})
1379-
}
1380-
}
1381-
13821239
func TestConnectHandler_RemoveGroupUser(t *testing.T) {
13831240
randomID := utils.NewString()
13841241

pkg/server/connect_interceptors/authorization.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,6 @@ var authorizationValidationMap = map[string]func(ctx context.Context, handler *v
466466
pbreq := req.(*connect.Request[frontierv1beta1.ListGroupUsersRequest])
467467
return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.GroupNamespace, ID: pbreq.Msg.GetId()}, schema.GetPermission, req)
468468
},
469-
"/raystack.frontier.v1beta1.FrontierService/AddGroupUsers": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error {
470-
pbreq := req.(*connect.Request[frontierv1beta1.AddGroupUsersRequest])
471-
return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.GroupNamespace, ID: pbreq.Msg.GetId()}, schema.UpdatePermission, req)
472-
},
473469
"/raystack.frontier.v1beta1.FrontierService/RemoveGroupUser": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error {
474470
pbreq := req.(*connect.Request[frontierv1beta1.RemoveGroupUserRequest])
475471
return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.GroupNamespace, ID: pbreq.Msg.GetId()}, schema.UpdatePermission, req)

0 commit comments

Comments
 (0)