Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions workspaces/x2a/.changeset/proud-months-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@red-hat-developer-hub/backstage-plugin-x2a-backend': patch
'@red-hat-developer-hub/backstage-plugin-x2a-common': patch
'@red-hat-developer-hub/backstage-plugin-x2a': patch
---

Adding pagination and filtering by permissions to the GET|POST /projects and GET /project/[id] endpoints.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some issues (?) with pagination:

Backend (working correctly):
api returns the correct amount

eshalev x2a (pr-2152) $ curl http://localhost:7007/api/x2a/projects
{
  "totalCount": 32,
  "items": [

The Frontend is not updated (by design?)
Once you have more that 10 projects,

  • no pagination; only one page of 10 projects appear
  • wrong count - project count says 10
    I see you have " /* TODO: pagination */
    " for that, is that for both the actual pagination and the project count?
    or is it the "current page count"

Line 95: Shows wrong count:
title={Projects (${projects.length})} // ← Shows 10 instead of total
Should use value?.totalCount from the API response.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is for BE only.
The FE part follows: #2175

1 change: 1 addition & 0 deletions workspaces/x2a/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"lint:all": "backstage-cli repo lint",
"prettier:check": "prettier --check .",
"prettier:fix": "prettier --write .",
"chores": "yarn prettier:fix && yarn lint:all --fix && yarn tsc:full && yarn build:api-reports && yarn test:all",
"new": "backstage-cli new --scope @red-hat-developer-hub",
"postinstall": "cd ../../ && yarn install"
},
Expand Down
2 changes: 1 addition & 1 deletion workspaces/x2a/plugins/x2a-backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@backstage/catalog-client": "^1.12.1",
"@backstage/errors": "^1.2.7",
"@backstage/plugin-catalog-node": "^1.20.0",
"@backstage/plugin-permission-common": "^0.9.4",
"@backstage/types": "^1.2.2",
"@kubernetes/client-node": "^1.4.0",
"@red-hat-developer-hub/backstage-plugin-x2a-common": "workspace:*",
Expand All @@ -51,7 +52,6 @@
"devDependencies": {
"@backstage/backend-test-utils": "^1.10.1",
"@backstage/cli": "^0.34.5",
"@backstage/plugin-permission-common": "^0.9.4",
"@backstage/repo-tools": "^0.16.2",
"@types/express": "^4.17.6",
"@types/knex": "^0.16.0",
Expand Down
18 changes: 18 additions & 0 deletions workspaces/x2a/plugins/x2a-backend/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export const DEFAULT_PAGE_SIZE = 10;
export const DEFAULT_PAGE_SORT = 'created_at';
export const DEFAULT_PAGE_ORDER = 'desc';
3 changes: 3 additions & 0 deletions workspaces/x2a/plugins/x2a-backend/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ export const x2APlugin = createBackendPlugin({
httpRouter: coreServices.httpRouter,
database: coreServices.database,
logger: coreServices.logger,
permissionsSvc: coreServices.permissions,
x2aDatabase: x2aDatabaseServiceRef,
kubeService: kubeServiceRef,
},
async init({
httpRouter,
permissionsSvc,
x2aDatabase,
logger,
httpAuth,
Expand All @@ -53,6 +55,7 @@ export const x2APlugin = createBackendPlugin({
await createRouter({
httpAuth,
logger,
permissionsSvc,
x2aDatabase,
kubeService,
}),
Expand Down
260 changes: 258 additions & 2 deletions workspaces/x2a/plugins/x2a-backend/src/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from '@backstage/backend-test-utils';
import express from 'express';
import request from 'supertest';
import { AuthorizeResult } from '@backstage/plugin-permission-common';

import { createRouter } from './router';
import { X2ADatabaseService } from './services/X2ADatabaseService';
Expand All @@ -40,14 +41,38 @@ const mockInputProject: ProjectsPostRequest = {
abbreviation: 'MP',
};

async function createApp(client: Knex): Promise<express.Express> {
async function createApp(
client: Knex,
authorizeResult?: AuthorizeResult,
adminWriteResult?: AuthorizeResult,
): Promise<express.Express> {
const x2aDatabase = X2ADatabaseService.create({
logger: mockServices.logger.mock(),
dbClient: client,
});
const router = await createRouter({
httpAuth: mockServices.httpAuth(),
logger: mockServices.logger.mock(),
permissionsSvc: mockServices.permissions.mock({
authorize: async (requests: any[]) => {
// Check which permission is being requested
const permission = requests[0]?.permission;
if (
permission?.name === 'x2a.admin' &&
permission?.attributes?.action === 'update'
) {
// This is x2aAdminWritePermission
return [
{
result:
adminWriteResult ?? authorizeResult ?? AuthorizeResult.ALLOW,
},
] as any;
}
// Default to the provided authorizeResult or ALLOW
return [{ result: authorizeResult ?? AuthorizeResult.ALLOW }] as any;
},
}),
x2aDatabase,
kubeService: {
getPods: jest.fn().mockResolvedValue({ items: [] }),
Expand Down Expand Up @@ -109,7 +134,7 @@ describe('createRouter', () => {
);

it.each(databases.eachSupportedId())(
'should not allow unauthenticated requests to create a migration - %p',
'should not allow unauthenticated requests to create a project - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
const app = await createApp(client);
Expand All @@ -127,6 +152,62 @@ describe('createRouter', () => {
},
);

it.each(databases.eachSupportedId())(
'should allow users with x2aUserPermission to create projects - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
const app = await createApp(client, AuthorizeResult.ALLOW);

const response = await request(app)
.post('/projects')
.send(mockInputProject);

expect(response.status).toBe(200);
expect(response.body).toMatchObject({
...mockInputProject,
createdBy: 'user:default/mock',
});
},
);

it.each(databases.eachSupportedId())(
'should allow users with x2aAdminWritePermission to create projects - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
const app = await createApp(client, AuthorizeResult.ALLOW);

const response = await request(app)
.post('/projects')
.send(mockInputProject);

expect(response.status).toBe(200);
expect(response.body).toMatchObject({
...mockInputProject,
createdBy: 'user:default/mock',
});
},
);

it.each(databases.eachSupportedId())(
'should deny users without permissions from creating projects - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
const app = await createApp(client, AuthorizeResult.DENY);

const response = await request(app)
.post('/projects')
.send(mockInputProject);

expect(response.status).toBe(403);
expect(response.body).toMatchObject({
error: {
name: 'NotAllowedError',
message: 'You are not allowed to create a project',
},
});
},
);

it.each(databases.eachSupportedId())(
'should get a project by id - %p',
async databaseId => {
Expand Down Expand Up @@ -225,4 +306,179 @@ describe('createRouter', () => {
});
},
);

it.each(databases.eachSupportedId())(
'should allow users with admin write permission to delete any project - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);

// Create a project as user1
// const user1Credentials = mockCredentials.user('user:default/user1');
const user1CredentialsHeader =
mockCredentials.user.header('user:default/user1');
const appWithCreate = await createApp(client, AuthorizeResult.ALLOW);
const createResponse = await request(appWithCreate)
.post('/projects')
.set('Authorization', user1CredentialsHeader)
.send(mockInputProject);

expect(createResponse.status).toBe(200);
const projectId = createResponse.body.id;
expect(createResponse.body.createdBy).toBe('user:default/user1');

// Verify project exists
const getResponse = await request(appWithCreate)
.get(`/projects/${projectId}`)
.set('Authorization', user1CredentialsHeader)
.send();
expect(getResponse.status).toBe(200);

// Try to delete as user2 (non-admin) - should fail because they didn't create it
// const user2Credentials = mockCredentials.user('user:default/user2');
const user2CredentialsHeader =
mockCredentials.user.header('user:default/user2');
const appNonAdmin = await createApp(
client,
AuthorizeResult.ALLOW, // Can create projects
AuthorizeResult.DENY, // Cannot delete others' projects (no admin write)
);
const deleteResponseNonAdmin = await request(appNonAdmin)
.delete(`/projects/${projectId}`)
.set('Authorization', user2CredentialsHeader)
.send();

// Non-admin user2 cannot delete project created by user1
// The deleteProject filters by created_by, so deletedCount will be 0
expect(deleteResponseNonAdmin.status).toBe(404);
expect(deleteResponseNonAdmin.body).toMatchObject({
error: { name: 'NotFoundError', message: 'Project not found' },
});

// Verify project still exists
const getAfterFailedDelete = await request(appWithCreate)
.get(`/projects/${projectId}`)
.set('Authorization', user1CredentialsHeader)
.send();
expect(getAfterFailedDelete.status).toBe(200);
expect(getAfterFailedDelete.body.id).toBe(projectId);

// Now delete as admin user (should succeed even though created by user1)
// const adminCredentials = mockCredentials.user('user:default/admin');
const adminCredentialsHeader =
mockCredentials.user.header('user:default/admin');
const appAdmin = await createApp(
client,
AuthorizeResult.ALLOW,
AuthorizeResult.ALLOW,
);
const deleteResponseAdmin = await request(appAdmin)
.delete(`/projects/${projectId}`)
.set('Authorization', adminCredentialsHeader)
.send();

expect(deleteResponseAdmin.status).toBe(200);
expect(deleteResponseAdmin.body.deletedCount).toBe(1);

// Verify project is deleted
const getAfterDelete = await request(appWithCreate)
.get(`/projects/${projectId}`)
.set('Authorization', user1CredentialsHeader)
.send();
expect(getAfterDelete.status).toBe(404);
},
);

it.each(databases.eachSupportedId())(
'should allow users without admin write permission to delete their own project - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
// User does not have admin write permission, but can create projects
const app = await createApp(
client,
AuthorizeResult.ALLOW, // Can create (has x2aUserPermission)
AuthorizeResult.DENY, // Cannot delete others' projects (no admin write)
);

// Create a project (as the default mock user)
const createResponse = await request(app)
.post('/projects')
.send(mockInputProject);

expect(createResponse.status).toBe(200);
const projectId = createResponse.body.id;
expect(createResponse.body.createdBy).toBe('user:default/mock');

// Delete own project (should succeed)
const deleteResponse = await request(app)
.delete(`/projects/${projectId}`)
.send();

expect(deleteResponse.status).toBe(200);
expect(deleteResponse.body.deletedCount).toBe(1);

// Verify project is deleted
const getAfterDeleteResponse = await request(app)
.get(`/projects/${projectId}`)
.send();
expect(getAfterDeleteResponse.status).toBe(404);
},
);

it.each(databases.eachSupportedId())(
'should return 404 when deletion fails due to permission filtering - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
// User does not have admin write permission
const app = await createApp(
client,
AuthorizeResult.ALLOW, // Can create projects
AuthorizeResult.DENY, // Cannot delete others' projects (no admin write)
);

// Create a project
const createResponse = await request(app)
.post('/projects')
.send(mockInputProject);

expect(createResponse.status).toBe(200);
const projectId = createResponse.body.id;

// Note: The permission filtering happens at the database service level.
// When canWriteAll is false, deleteProject filters by created_by.
// Since the same user created and is deleting, it should succeed.
// Cross-user deletion prevention is tested in X2ADatabaseService tests.
// This test verifies the endpoint integration works correctly.

const deleteResponse = await request(app)
.delete(`/projects/${projectId}`)
.send();

// Same user can delete their own project
expect(deleteResponse.status).toBe(200);
expect(deleteResponse.body.deletedCount).toBe(1);
},
);

it.each(databases.eachSupportedId())(
'should return 404 when deleting non-existent project even with admin write permission - %p',
async databaseId => {
const { client } = await createDatabase(databaseId);
// User has admin write permission
const app = await createApp(
client,
AuthorizeResult.ALLOW,
AuthorizeResult.ALLOW,
);

const nonExistentId = '00000000-0000-0000-0000-000000000000';
const response = await request(app)
.delete(`/projects/${nonExistentId}`)
.send();

expect(response.status).toBe(404);
expect(response.body).toMatchObject({
error: { name: 'NotFoundError', message: 'Project not found' },
});
},
);
});
Loading