diff --git a/cmd/seed.go b/cmd/seed.go index f63b8373f7..2e01c9791a 100644 --- a/cmd/seed.go +++ b/cmd/seed.go @@ -11,6 +11,7 @@ import ( "connectrpc.com/connect" "github.com/MakeNowJust/heredoc" "github.com/raystack/frontier/config" + "github.com/raystack/frontier/internal/bootstrap/schema" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" frontierv1beta1connect "github.com/raystack/frontier/proto/v1beta1/frontierv1beta1connect" "github.com/raystack/salt/cli/printer" @@ -112,10 +113,10 @@ func createCustomRolesAndPermissions(ctx context.Context, client frontierv1beta1 } str := "created custom permissions : " - //nolint:staticcheck for _, v := range permissionBodies { - str = fmt.Sprintf("%s %s:%s", str, v.GetNamespace(), v.GetName()) - resourceNamespaces = append(resourceNamespaces, v.GetNamespace()) + permNamespace, permName := schema.PermissionNamespaceAndNameFromKey(v.GetKey()) + str = fmt.Sprintf("%s %s:%s", str, permNamespace, permName) + resourceNamespaces = append(resourceNamespaces, permNamespace) } fmt.Println(str) diff --git a/cmd/seed/permissions.json b/cmd/seed/permissions.json index 53de504843..16b4af625c 100644 --- a/cmd/seed/permissions.json +++ b/cmd/seed/permissions.json @@ -1,50 +1,42 @@ [ { - "name": "create", + "key": "compute.order.create", "title": "Create Order", - "namespace": "compute/order", "metadata": {} }, { - "name": "get", + "key": "compute.order.get", "title": "Read Order", - "namespace": "compute/order", "metadata": {} }, { - "name": "update", + "key": "compute.order.update", "title": "Update Order", - "namespace": "compute/order", "metadata": {} }, { - "name": "delete", + "key": "compute.order.delete", "title": "Delete Order", - "namespace": "compute/order", "metadata": {} }, { - "name": "list", + "key": "compute.order.list", "title": "List Orders", - "namespace": "compute/order", "metadata": {} }, { - "name": "get", + "key": "database.order.get", "title": "List Database Order", - "namespace": "database/order", - "metadata":{} + "metadata": {} }, { - "name": "update", + "key": "database.order.update", "title": "Update Database Order", - "namespace": "database/order", - "metadata":{} + "metadata": {} }, { - "name": "delete", + "key": "database.order.delete", "title": "Delete Database Order", - "namespace": "database/order", - "metadata":{} + "metadata": {} } ] diff --git a/internal/api/v1beta1connect/errors.go b/internal/api/v1beta1connect/errors.go index effd135b42..f8eddc17aa 100644 --- a/internal/api/v1beta1connect/errors.go +++ b/internal/api/v1beta1connect/errors.go @@ -56,6 +56,7 @@ var ( ErrAlreadyApplied = errors.New("credits already applied") ErrInvalidRoleID = errors.New("role id is invalid") ErrNamespaceSplitNotation = errors.New("subject/object should be provided as 'namespace:uuid'") + ErrPermissionKeyNotation = errors.New("permission key should be provided as 'service.resource.verb'") ErrPolicyNotFound = errors.New("policy doesn't exist") ErrProjectNotFound = errors.New("project doesn't exist") ErrGroupNotFound = errors.New("group doesn't exist") diff --git a/internal/api/v1beta1connect/permission.go b/internal/api/v1beta1connect/permission.go index 5cf79e3187..140e32639b 100644 --- a/internal/api/v1beta1connect/permission.go +++ b/internal/api/v1beta1connect/permission.go @@ -23,18 +23,14 @@ func (h *ConnectHandler) CreatePermission(ctx context.Context, request *connect. var permissionSlugs []string for _, permBody := range request.Msg.GetBodies() { permNamespace, permName := schema.PermissionNamespaceAndNameFromKey(permBody.GetKey()) - if permNamespace == "" || permName == "" { - permNamespace, permName = permBody.GetNamespace(), permBody.GetName() //nolint:staticcheck - } if permName == "" || permNamespace == "" { - return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) + return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation) } if !schema.IsValidPermissionName(permName) { return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("permission name cannot contain special characters")) } - - if permNamespace == schema.DefaultNamespace { - return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("permission namespace cannot be "+schema.DefaultNamespace)) + if !schema.IsValidPermissionNamespace(permNamespace) { + return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation) } permissionSlugs = append(permissionSlugs, schema.FQPermissionNameFromNamespace(permNamespace, permName)) @@ -116,7 +112,13 @@ func (h *ConnectHandler) UpdatePermission(ctx context.Context, request *connect. permNamespace, permName := schema.PermissionNamespaceAndNameFromKey(request.Msg.GetBody().GetKey()) if permNamespace == "" || permName == "" { - permNamespace, permName = request.Msg.GetBody().GetNamespace(), request.Msg.GetBody().GetName() //nolint:staticcheck + return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation) + } + if !schema.IsValidPermissionName(permName) { + return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("permission name cannot contain special characters")) + } + if !schema.IsValidPermissionNamespace(permNamespace) { + return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation) } updatedPermission, err := h.permissionService.Update(ctx, permission.Permission{ ID: request.Msg.GetId(), diff --git a/internal/api/v1beta1connect/permission_test.go b/internal/api/v1beta1connect/permission_test.go index 411aa40aef..8670ac3950 100644 --- a/internal/api/v1beta1connect/permission_test.go +++ b/internal/api/v1beta1connect/permission_test.go @@ -71,8 +71,7 @@ func TestHandler_CreatePermission(t *testing.T) { request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ { - Name: testPermissions[testPermissionIdx].Name, - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name), }, }, }), @@ -82,30 +81,43 @@ func TestHandler_CreatePermission(t *testing.T) { errors.New("test error"))), }, { - name: "should return bad request error if namespace id is empty", + name: "should return bad request error if key is missing", setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {}, request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ { - Name: testPermissions[testPermissionIdx].Name, + Title: "no key sent", }, }, }), want: nil, - wantErr: connect.NewError(connect.CodeInvalidArgument, ErrBadRequest), + wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation), }, { - name: "should return bad request error if name is empty", + name: "should return bad request error if key is malformed", setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {}, request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ { - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: "app.resource", }, }, }), want: nil, - wantErr: connect.NewError(connect.CodeInvalidArgument, ErrBadRequest), + wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation), + }, + { + name: "should return bad request error if key namespace is invalid", + setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {}, + request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ + Bodies: []*frontierv1beta1.PermissionRequestBody{ + { + Key: "app..get", + }, + }, + }), + want: nil, + wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation), }, { name: "should return success if permission service return nil error", @@ -143,12 +155,10 @@ func TestHandler_CreatePermission(t *testing.T) { request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ { - Name: testPermissions[testPermissionIdx].Name + "0", - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"), }, { - Name: testPermissions[testPermissionIdx].Name + "1", - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"1"), }, }, }), @@ -174,50 +184,6 @@ func TestHandler_CreatePermission(t *testing.T) { }), wantErr: nil, }, - { - name: "should return success if permission service return nil error with permission key", - setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) { - bs.EXPECT().AppendSchema(mock.AnythingOfType("context.backgroundCtx"), schema.ServiceDefinition{ - Permissions: []schema.ResourcePermission{ - { - Name: testPermissions[testPermissionIdx].Name + "0", - Namespace: testPermissions[testPermissionIdx].NamespaceID, - }, - }, - }).Return(nil) - as.EXPECT().List(mock.Anything, permission.Filter{ - Slugs: []string{ - schema.FQPermissionNameFromNamespace(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"), - }, - }).Return([]permission.Permission{ - { - ID: testPermissions[testPermissionIdx].ID, - Name: testPermissions[testPermissionIdx].Name + "0", - NamespaceID: testPermissions[testPermissionIdx].NamespaceID, - }, - }, nil) - }, - request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ - Bodies: []*frontierv1beta1.PermissionRequestBody{ - { - Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"), - }, - }, - }), - want: connect.NewResponse(&frontierv1beta1.CreatePermissionResponse{ - Permissions: []*frontierv1beta1.Permission{ - { - Id: testPermissions[testPermissionIdx].ID, - Name: testPermissions[testPermissionIdx].Name + "0", - Namespace: testPermissions[testPermissionIdx].NamespaceID, - Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"), - CreatedAt: timestamppb.New(testPermissions[testPermissionIdx].CreatedAt), - UpdatedAt: timestamppb.New(testPermissions[testPermissionIdx].UpdatedAt), - }, - }, - }), - wantErr: nil, - }, } for _, tt := range tests { @@ -255,8 +221,7 @@ func TestHandler_UpdatePermission(t *testing.T) { request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ Id: testPermissions[testPermissionIdx].ID, Body: &frontierv1beta1.PermissionRequestBody{ - Name: testPermissions[testPermissionIdx].Name, - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name), }, }), want: nil, @@ -275,8 +240,7 @@ func TestHandler_UpdatePermission(t *testing.T) { request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ Id: testPermissions[testPermissionIdx].ID, Body: &frontierv1beta1.PermissionRequestBody{ - Name: testPermissions[testPermissionIdx].Name, - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name), }, }), want: nil, @@ -291,8 +255,7 @@ func TestHandler_UpdatePermission(t *testing.T) { }, request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ Body: &frontierv1beta1.PermissionRequestBody{ - Name: testPermissions[testPermissionIdx].Name, - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name), }, }), want: nil, @@ -309,28 +272,47 @@ func TestHandler_UpdatePermission(t *testing.T) { request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ Id: testPermissions[testPermissionIdx].ID, Body: &frontierv1beta1.PermissionRequestBody{ - Name: testPermissions[testPermissionIdx].Name, - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name), }, }), want: nil, wantErr: connect.NewError(connect.CodeInvalidArgument, ErrBadRequest), }, { - name: "should return bad request error if name is empty", - setup: func(as *mocks.PermissionService) { - as.EXPECT().Update(mock.AnythingOfType("context.backgroundCtx"), permission.Permission{ - ID: testPermissions[testPermissionIdx].ID, - NamespaceID: testPermissions[testPermissionIdx].NamespaceID}).Return(permission.Permission{}, permission.ErrInvalidDetail) - }, + name: "should return bad request error if key is missing", + setup: func(as *mocks.PermissionService) {}, request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ Id: testPermissions[testPermissionIdx].ID, Body: &frontierv1beta1.PermissionRequestBody{ - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Title: "no key sent", }, }), want: nil, - wantErr: connect.NewError(connect.CodeInvalidArgument, ErrBadRequest), + wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation), + }, + { + name: "should return bad request error if key is malformed", + setup: func(as *mocks.PermissionService) {}, + request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ + Id: testPermissions[testPermissionIdx].ID, + Body: &frontierv1beta1.PermissionRequestBody{ + Key: "app.resource", + }, + }), + want: nil, + wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation), + }, + { + name: "should return bad request error if key name has special characters", + setup: func(as *mocks.PermissionService) {}, + request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ + Id: testPermissions[testPermissionIdx].ID, + Body: &frontierv1beta1.PermissionRequestBody{ + Key: "app.resource.we$rd", + }, + }), + want: nil, + wantErr: connect.NewError(connect.CodeInvalidArgument, errors.New("permission name cannot contain special characters")), }, { name: "should return success if permission service return nil error", @@ -344,8 +326,7 @@ func TestHandler_UpdatePermission(t *testing.T) { request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{ Id: testPermissions[testPermissionIdx].ID, Body: &frontierv1beta1.PermissionRequestBody{ - Name: testPermissions[testPermissionIdx].Name, - Namespace: testPermissions[testPermissionIdx].NamespaceID, + Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name), }, }), want: connect.NewResponse(&frontierv1beta1.UpdatePermissionResponse{ diff --git a/test/e2e/regression/service_registration_test.go b/test/e2e/regression/service_registration_test.go index 2c7c1c1032..5c36c74d22 100644 --- a/test/e2e/regression/service_registration_test.go +++ b/test/e2e/regression/service_registration_test.go @@ -80,18 +80,15 @@ func (s *ServiceRegistrationRegressionTestSuite) TestServiceRegistration() { createPermResp, err := s.testBench.AdminClient.CreatePermission(ctx, connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ { - Name: "get", - Namespace: "database/instance", - Title: "", + Key: "database.instance.get", + Title: "", }, { - Name: "update", - Namespace: "database/instance", - Title: "update db instance", + Key: "database.instance.update", + Title: "update db instance", }, { - Name: "delete", - Namespace: "database/instance", + Key: "database.instance.delete", Metadata: &structpb.Struct{ Fields: map[string]*structpb.Value{ "description": structpb.NewStringValue("bar"), @@ -117,13 +114,11 @@ func (s *ServiceRegistrationRegressionTestSuite) TestServiceRegistration() { createPermResp, err := s.testBench.AdminClient.CreatePermission(ctx, connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ { - Name: "update", - Namespace: "database/alert", - Title: "update db alert", + Key: "database.alert.update", + Title: "update db alert", }, { - Name: "delete", - Namespace: "database/alert", + Key: "database.alert.delete", Metadata: &structpb.Struct{ Fields: map[string]*structpb.Value{ "description": structpb.NewStringValue("bar"), @@ -166,7 +161,7 @@ func (s *ServiceRegistrationRegressionTestSuite) TestPermissionDeleteCascade() { createPermResp, err := s.testBench.AdminClient.CreatePermission(ctx, connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ - {Name: "act", Namespace: "permcascade/res"}, + {Key: "permcascade.res.act"}, }, })) s.Require().NoError(err) @@ -256,7 +251,7 @@ func (s *ServiceRegistrationRegressionTestSuite) TestPermissionDeleteBlockedByRe // authorizes the "delete" permission on the resource's namespace) createPermResp, err := s.testBench.AdminClient.CreatePermission(ctx, connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{ - {Name: "delete", Namespace: "orphanguard/widget"}, + {Key: "orphanguard.widget.delete"}, }, })) s.Require().NoError(err)