diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index a0acc57cec..60c0e42f92 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -165,15 +165,14 @@ export class OidcRouter { redirectUri: session.redirectUri, }); } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; this.logger.error( - `Failed to get authorization session: ${ - isError(error) ? error.message : 'Unknown error' - }`, + `Failed to get authorization session: ${description}`, error, ); return res.status(404).json({ error: 'not_found', - error_description: 'Authorization session not found or expired', + error_description: description, }); } }); @@ -211,15 +210,14 @@ export class OidcRouter { redirectUrl: result.redirectUrl, }); } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; this.logger.error( - `Failed to approve authorization session: ${ - isError(error) ? error.message : 'Unknown error' - }`, + `Failed to approve authorization session: ${description}`, error, ); return res.status(400).json({ error: 'invalid_request', - error_description: isError(error) ? error.message : 'Unknown error', + error_description: description, }); } }); diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 4820559b01..017d3ab481 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -378,6 +378,59 @@ describe('OidcService', () => { }), ).rejects.toThrow('Invalid authorization session'); }); + + it('should throw error when trying to approve an already approved session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + await expect( + service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); + + it('should throw error when trying to approve an already rejected session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.rejectAuthorizationSession({ + sessionId: authSession.id, + }); + + await expect( + service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); }); describe('getAuthorizationSession', () => { @@ -413,10 +466,34 @@ describe('OidcService', () => { }), ); }); - }); - describe('rejectAuthorizationSession', () => { - it('should delete a authorization session', async () => { + it('should throw error when trying to get an already approved session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + await expect( + service.getAuthorizationSession({ + sessionId: authSession.id, + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); + + it('should throw error when trying to get an already rejected session', async () => { const { service } = await createOidcService(databaseId); const client = await service.registerClient({ @@ -438,11 +515,34 @@ describe('OidcService', () => { service.getAuthorizationSession({ sessionId: authSession.id, }), - ).resolves.toEqual( - expect.objectContaining({ - status: 'rejected', + ).rejects.toThrow('Authorization session not found or expired'); + }); + }); + + describe('rejectAuthorizationSession', () => { + it('should reject a authorization session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.rejectAuthorizationSession({ + sessionId: authSession.id, + }); + + await expect( + service.getAuthorizationSession({ + sessionId: authSession.id, }), - ); + ).rejects.toThrow('Authorization session not found or expired'); }); it('should throw error for invalid authorization session', async () => { @@ -454,6 +554,57 @@ describe('OidcService', () => { }), ).rejects.toThrow('Invalid authorization session'); }); + + it('should throw error when trying to reject an already approved session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + await expect( + service.rejectAuthorizationSession({ + sessionId: authSession.id, + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); + + it('should throw error when trying to reject an already rejected session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.rejectAuthorizationSession({ + sessionId: authSession.id, + }); + + await expect( + service.rejectAuthorizationSession({ + sessionId: authSession.id, + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); }); describe('authorize', () => { diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index e89dfeac6f..7809d2fd00 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -16,7 +16,11 @@ import { AuthService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; -import { InputError, AuthenticationError } from '@backstage/errors'; +import { + InputError, + AuthenticationError, + NotFoundError, +} from '@backstage/errors'; import { decodeJwt } from 'jose'; import crypto from 'crypto'; import { OidcDatabase } from '../database/OidcDatabase'; @@ -204,13 +208,17 @@ export class OidcService { }); if (!session) { - throw new InputError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } + if (session.status !== 'pending') { + throw new NotFoundError('Authorization session not found or expired'); + } + await this.oidc.updateAuthorizationSession({ id: session.id, userEntityRef, @@ -244,13 +252,17 @@ export class OidcService { }); if (!session) { - throw new InputError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } + if (session.status !== 'pending') { + throw new NotFoundError('Authorization session not found or expired'); + } + const client = await this.oidc.getClient({ clientId: session.clientId }); if (!client) { throw new InputError('Invalid client_id'); @@ -278,13 +290,17 @@ export class OidcService { }); if (!session) { - throw new InputError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } + if (session.status !== 'pending') { + throw new NotFoundError('Authorization session not found or expired'); + } + await this.oidc.updateAuthorizationSession({ id: session.id, status: 'rejected', @@ -424,8 +440,9 @@ export class OidcService { const session = await this.oidc.getAuthorizationSession({ id: authCode.sessionId, }); + if (!session) { - throw new AuthenticationError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (session.clientId !== clientId) { throw new AuthenticationError('Client ID mismatch');