diff --git a/cypress/e2e/login.cy.js b/cypress/e2e/login.cy.js index 40ce83a75..418109b5b 100644 --- a/cypress/e2e/login.cy.js +++ b/cypress/e2e/login.cy.js @@ -20,7 +20,7 @@ describe('Login page', () => { }); it('should redirect to repo list on valid login', () => { - cy.intercept('GET', '**/api/auth/me').as('getUser'); + cy.intercept('GET', '**/api/auth/profile').as('getUser'); cy.get('[data-test="username"]').type('admin'); cy.get('[data-test="password"]').type('admin'); diff --git a/cypress/support/commands.js b/cypress/support/commands.js index a0a3f620d..5117d6cfc 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -29,7 +29,7 @@ Cypress.Commands.add('login', (username, password) => { cy.session([username, password], () => { cy.visit('/login'); - cy.intercept('GET', '**/api/auth/me').as('getUser'); + cy.intercept('GET', '**/api/auth/profile').as('getUser'); cy.get('[data-test=username]').type(username); cy.get('[data-test=password]').type(password); diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index f6347eb4f..9835af3c8 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -9,8 +9,7 @@ import * as passportAD from '../passport/activeDirectory'; import { User } from '../../db/types'; import { AuthenticationElement } from '../../config/generated/config'; -import { toPublicUser } from './publicApi'; -import { isAdminUser } from './utils'; +import { isAdminUser, toPublicUser } from './utils'; const router = express.Router(); const passport = getPassport(); @@ -107,7 +106,7 @@ router.get('/openidconnect/callback', (req: Request, res: Response, next: NextFu passport.authenticate(authStrategies['openidconnect'].type, (err: any, user: any, info: any) => { if (err) { console.error('Authentication error:', err); - return res.status(401).end(); + return res.status(500).end(); } if (!user) { console.error('No user found:', info); @@ -116,7 +115,7 @@ router.get('/openidconnect/callback', (req: Request, res: Response, next: NextFu req.logIn(user, (err) => { if (err) { console.error('Login error:', err); - return res.status(401).end(); + return res.status(500).end(); } console.log('Logged in successfully. User:', user); return res.redirect(`${uiHost}:${uiPort}/dashboard/profile`); @@ -133,79 +132,96 @@ router.post('/logout', (req: Request, res: Response, next: NextFunction) => { }); router.get('/profile', async (req: Request, res: Response) => { - if (req.user) { - const userVal = await db.findUser((req.user as User).username); - if (!userVal) { - res.status(400).send('Error: Logged in user not found').end(); - return; - } - res.send(toPublicUser(userVal)); - } else { - res.status(401).end(); + if (!req.user) { + res + .status(401) + .send({ + message: 'Not logged in', + }) + .end(); + return; } + + const userVal = await db.findUser((req.user as User).username); + if (!userVal) { + res.status(404).send('User not found').end(); + return; + } + + res.send(toPublicUser(userVal)); }); router.post('/gitAccount', async (req: Request, res: Response) => { - if (req.user) { - try { - let username = - req.body.username == null || req.body.username === 'undefined' - ? req.body.id - : req.body.username; - username = username?.split('@')[0]; - - if (!username) { - res.status(400).send('Error: Missing username. Git account not updated').end(); - return; - } + if (!req.user) { + res + .status(401) + .send({ + message: 'Not logged in', + }) + .end(); + return; + } - const reqUser = await db.findUser((req.user as User).username); - if (username !== reqUser?.username && !reqUser?.admin) { - res.status(403).send('Error: You must be an admin to update a different account').end(); - return; - } + try { + let username = + req.body.username == null || req.body.username === 'undefined' + ? req.body.id + : req.body.username; + username = username?.split('@')[0]; - const user = await db.findUser(username); - if (!user) { - res.status(400).send('Error: User not found').end(); - return; - } + if (!username) { + res + .status(400) + .send({ + message: 'Missing username. Git account not updated', + }) + .end(); + return; + } - console.log('Adding gitAccount' + req.body.gitAccount); - user.gitAccount = req.body.gitAccount; - db.updateUser(user); - res.status(200).end(); - } catch (e: any) { + const reqUser = await db.findUser((req.user as User).username); + if (username !== reqUser?.username && !reqUser?.admin) { res - .status(500) + .status(403) .send({ - message: `Error updating git account: ${e.message}`, + message: 'Must be an admin to update a different account', }) .end(); + return; } - } else { - res.status(401).end(); - } -}); -router.get('/me', async (req: Request, res: Response) => { - if (req.user) { - const userVal = await db.findUser((req.user as User).username); - if (!userVal) { - res.status(400).send('Error: Logged in user not found').end(); + const user = await db.findUser(username); + if (!user) { + res + .status(404) + .send({ + message: 'User not found', + }) + .end(); return; } - res.send(toPublicUser(userVal)); - } else { - res.status(401).end(); + + user.gitAccount = req.body.gitAccount; + db.updateUser(user); + res.status(200).end(); + } catch (e: any) { + res + .status(500) + .send({ + message: `Failed to update git account: ${e.message}`, + }) + .end(); } }); router.post('/create-user', async (req: Request, res: Response) => { if (!isAdminUser(req.user)) { - res.status(401).send({ - message: 'You are not authorized to perform this action...', - }); + res + .status(403) + .send({ + message: 'Not authorized to create users', + }) + .end(); return; } @@ -213,20 +229,27 @@ router.post('/create-user', async (req: Request, res: Response) => { const { username, password, email, gitAccount, admin: isAdmin = false } = req.body; if (!username || !password || !email || !gitAccount) { - res.status(400).send({ - message: 'Missing required fields: username, password, email, and gitAccount are required', - }); + res + .status(400) + .send({ + message: + 'Missing required fields: username, password, email, and gitAccount are required', + }) + .end(); return; } await db.createUser(username, password, email, gitAccount, isAdmin); - res.status(201).send({ - message: 'User created successfully', - username, - }); + res + .status(201) + .send({ + message: 'User created successfully', + username, + }) + .end(); } catch (error: any) { console.error('Error creating user:', error); - res.status(400).send({ + res.status(500).send({ message: error.message || 'Failed to create user', }); } diff --git a/src/service/routes/publicApi.ts b/src/service/routes/publicApi.ts deleted file mode 100644 index 1b408a562..000000000 --- a/src/service/routes/publicApi.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { PublicUser, User } from '../../db/types'; - -export const toPublicUser = (user: User): PublicUser => { - return { - username: user.username || '', - displayName: user.displayName || '', - email: user.email || '', - title: user.title || '', - gitAccount: user.gitAccount || '', - admin: user.admin || false, - }; -}; diff --git a/src/service/routes/push.ts b/src/service/routes/push.ts index d1c2fae2c..fbce5335e 100644 --- a/src/service/routes/push.ts +++ b/src/service/routes/push.ts @@ -38,7 +38,7 @@ router.get('/:id', async (req: Request, res: Response) => { router.post('/:id/reject', async (req: Request, res: Response) => { if (!req.user) { res.status(401).send({ - message: 'not logged in', + message: 'Not logged in', }); return; } @@ -55,14 +55,14 @@ router.post('/:id/reject', async (req: Request, res: Response) => { const list = await db.getUsers({ email: committerEmail }); if (list.length === 0) { - res.status(401).send({ - message: `There was no registered user with the committer's email address: ${committerEmail}`, + res.status(404).send({ + message: `No user found with the committer's email address: ${committerEmail}`, }); return; } if (list[0].username.toLowerCase() === username.toLowerCase() && !list[0].admin) { - res.status(401).send({ + res.status(403).send({ message: `Cannot reject your own changes`, }); return; @@ -72,16 +72,23 @@ router.post('/:id/reject', async (req: Request, res: Response) => { if (isAllowed) { const result = await db.reject(id, null); - console.log(`user ${username} rejected push request for ${id}`); + console.log(`User ${username} rejected push request for ${id}`); res.send(result); } else { - res.status(401).send({ - message: 'User is not authorised to reject changes', + res.status(403).send({ + message: `User ${username} is not authorised to reject changes on this project`, }); } }); router.post('/:id/authorise', async (req: Request, res: Response) => { + if (!req.user) { + res.status(401).send({ + message: 'Not logged in', + }); + return; + } + const questions = req.body.params?.attestation; // TODO: compare attestation to configuration and ensure all questions are answered @@ -90,72 +97,73 @@ router.post('/:id/authorise', async (req: Request, res: Response) => { (question: { checked: boolean }) => !!question.checked, ); - if (req.user && attestationComplete) { - const id = req.params.id; + if (!attestationComplete) { + res.status(400).send({ + message: 'Attestation is not complete', + }); + return; + } - const { username } = req.user as { username: string }; + const id = req.params.id; - const push = await db.getPush(id); - if (!push) { - res.status(404).send({ - message: 'Push request not found', - }); - return; - } + const { username } = req.user as { username: string }; - // Get the committer of the push via their email address - const committerEmail = push.userEmail; + const push = await db.getPush(id); + if (!push) { + res.status(404).send({ + message: 'Push request not found', + }); + return; + } - const list = await db.getUsers({ email: committerEmail }); + // Get the committer of the push via their email address + const committerEmail = push.userEmail; - if (list.length === 0) { - res.status(401).send({ - message: `There was no registered user with the committer's email address: ${committerEmail}`, - }); - return; - } + const list = await db.getUsers({ email: committerEmail }); - if (list[0].username.toLowerCase() === username.toLowerCase() && !list[0].admin) { - res.status(401).send({ - message: `Cannot approve your own changes`, + if (list.length === 0) { + res.status(404).send({ + message: `No user found with the committer's email address: ${committerEmail}`, + }); + return; + } + + if (list[0].username.toLowerCase() === username.toLowerCase() && !list[0].admin) { + res.status(403).send({ + message: `Cannot approve your own changes`, + }); + return; + } + + // If we are not the author, now check that we are allowed to authorise on this + // repo + const isAllowed = await db.canUserApproveRejectPush(id, username); + if (isAllowed) { + console.log(`User ${username} approved push request for ${id}`); + + const reviewerList = await db.getUsers({ username }); + const reviewerEmail = reviewerList[0].email; + + if (!reviewerEmail) { + res.status(404).send({ + message: `There was no registered email address for the reviewer: ${username}`, }); return; } - // If we are not the author, now check that we are allowed to authorise on this - // repo - const isAllowed = await db.canUserApproveRejectPush(id, username); - if (isAllowed) { - console.log(`user ${username} approved push request for ${id}`); - - const reviewerList = await db.getUsers({ username }); - const reviewerEmail = reviewerList[0].email; - - if (!reviewerEmail) { - res.status(401).send({ - message: `There was no registered email address for the reviewer: ${username}`, - }); - return; - } - - const attestation = { - questions, - timestamp: new Date(), - reviewer: { - username, - reviewerEmail, - }, - }; - const result = await db.authorise(id, attestation); - res.send(result); - } else { - res.status(401).send({ - message: `user ${username} not authorised to approve push's on this project`, - }); - } + const attestation = { + questions, + timestamp: new Date(), + reviewer: { + username, + reviewerEmail, + }, + }; + const result = await db.authorise(id, attestation); + res.send(result); } else { - res.status(401).send({ - message: 'You are unauthorized to perform this action...', + res.status(403).send({ + message: `User ${username} not authorised to approve pushes on this project`, }); } }); @@ -163,7 +171,7 @@ router.post('/:id/authorise', async (req: Request, res: Response) => { router.post('/:id/cancel', async (req: Request, res: Response) => { if (!req.user) { res.status(401).send({ - message: 'not logged in', + message: 'Not logged in', }); return; } @@ -175,12 +183,12 @@ router.post('/:id/cancel', async (req: Request, res: Response) => { if (isAllowed) { const result = await db.cancel(id); - console.log(`user ${username} canceled push request for ${id}`); + console.log(`User ${username} canceled push request for ${id}`); res.send(result); } else { - console.log(`user ${username} not authorised to cancel push request for ${id}`); - res.status(401).send({ - message: 'User ${req.user.username)} not authorised to cancel push requests on this project.', + console.log(`User ${username} not authorised to cancel push request for ${id}`); + res.status(403).send({ + message: `User ${username} not authorised to cancel push requests on this project`, }); } }); diff --git a/src/service/routes/users.ts b/src/service/routes/users.ts index 2e689817e..8701223c5 100644 --- a/src/service/routes/users.ts +++ b/src/service/routes/users.ts @@ -2,7 +2,7 @@ import express, { Request, Response } from 'express'; const router = express.Router(); import * as db from '../../db'; -import { toPublicUser } from './publicApi'; +import { toPublicUser } from './utils'; router.get('/', async (req: Request, res: Response) => { console.log('fetching users'); @@ -15,7 +15,12 @@ router.get('/:id', async (req: Request, res: Response) => { console.log(`Retrieving details for user: ${username}`); const user = await db.findUser(username); if (!user) { - res.status(404).send('Error: User not found').end(); + res + .status(404) + .send({ + message: `User ${username} not found`, + }) + .end(); return; } res.send(toPublicUser(user)); diff --git a/src/service/routes/utils.ts b/src/service/routes/utils.ts index a9c501801..694732a5d 100644 --- a/src/service/routes/utils.ts +++ b/src/service/routes/utils.ts @@ -1,3 +1,5 @@ +import { PublicUser, User as DbUser } from '../../db/types'; + interface User extends Express.User { username: string; admin?: boolean; @@ -6,3 +8,14 @@ interface User extends Express.User { export function isAdminUser(user?: Express.User): user is User & { admin: true } { return user !== null && user !== undefined && (user as User).admin === true; } + +export const toPublicUser = (user: DbUser): PublicUser => { + return { + username: user.username || '', + displayName: user.displayName || '', + email: user.email || '', + title: user.title || '', + gitAccount: user.gitAccount || '', + admin: user.admin || false, + }; +}; diff --git a/src/ui/components/Navbars/DashboardNavbarLinks.tsx b/src/ui/components/Navbars/DashboardNavbarLinks.tsx index 2ed5c3d8f..d23d3b65a 100644 --- a/src/ui/components/Navbars/DashboardNavbarLinks.tsx +++ b/src/ui/components/Navbars/DashboardNavbarLinks.tsx @@ -28,11 +28,11 @@ const DashboardNavbarLinks: React.FC = () => { const [openProfile, setOpenProfile] = useState(null); const [, setAuth] = useState(true); const [, setIsLoading] = useState(true); - const [, setIsError] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); const [user, setUser] = useState(null); useEffect(() => { - getUser(setIsLoading, setUser, setAuth, setIsError); + getUser(setIsLoading, setUser, setAuth, setErrorMessage); }, []); const handleClickProfile = (event: React.MouseEvent) => { @@ -66,6 +66,7 @@ const DashboardNavbarLinks: React.FC = () => { return (
+ {errorMessage &&
{errorMessage}
}