From 6b19a73abc5c0320ac483dcfe3aa4b9bf40faaa2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 12 Feb 2024 16:28:16 +0100 Subject: [PATCH] backend-{plugin,app}-api: refactored HttpAuth to separate allowing principals from auth method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Carl-Erik Bergström Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- .../httpAuth/httpAuthServiceFactory.ts | 46 +++++++++---------- .../httpRouter/createCredentialsBarrier.ts | 11 +---- packages/backend-plugin-api/api-report.md | 22 +++------ .../src/services/definitions/AuthService.ts | 2 + .../services/definitions/HttpAuthService.ts | 30 +++--------- .../src/services/definitions/index.ts | 5 +- 6 files changed, 40 insertions(+), 76 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts index d23c35fb43..1723e3b76c 100644 --- a/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts @@ -17,7 +17,7 @@ import { AuthService, BackstageCredentials, - BackstageHttpAccessToPrincipalTypesMapping, + BackstagePrincipalTypes, DiscoveryService, HttpAuthService, coreServices, @@ -96,33 +96,33 @@ class DefaultHttpAuthService implements HttpAuthService { this.#extractCredentialsFromRequest(req)); } - async credentials< - TAllowed extends - | keyof BackstageHttpAccessToPrincipalTypesMapping = 'unknown', - >( + async credentials( req: Request, options?: { - allow: Array; + allow?: Array; + allowedAuthMethods?: Array<'token' | 'cookie'>; }, - ): Promise< - BackstageCredentials - > { + ): Promise> { const credentials = toInternalBackstageCredentials( await this.#getCredentials(req), ); - // TODO break out into more readable function as we always - // want to check cookie regardless if options are set or not - // Probably asserts function that ensures authMethod = 'token' after cookie check - if (credentials.authMethod === 'cookie') { - if (options && !options.allow.includes('user-cookie' as TAllowed)) { - throw new NotAllowedError( - `This endpoint does not allow 'user-cookie' credentials`, - ); - } - } else if ( - options && - !options.allow.includes(credentials.principal.type as TAllowed) + const allowedPrincipalTypes = options?.allow; + const allowedAuthMethods: Array<'token' | 'cookie' | 'none'> = + options?.allowedAuthMethods ?? ['token']; + + if ( + credentials.authMethod !== 'none' && + !allowedAuthMethods.includes(credentials.authMethod) + ) { + throw new NotAllowedError( + `This endpoint does not allow the '${credentials.authMethod}' auth method`, + ); + } + + if ( + allowedPrincipalTypes && + !allowedPrincipalTypes.includes(credentials.principal.type as TAllowed) ) { throw new NotAllowedError( `This endpoint does not allow '${credentials.principal.type}' credentials`, @@ -175,10 +175,8 @@ class DefaultHttpAuthService implements HttpAuthService { export const httpAuthServiceFactory = createServiceFactory({ service: coreServices.httpAuth, deps: { - config: coreServices.rootConfig, - logger: coreServices.rootLogger, - discovery: coreServices.discovery, auth: coreServices.auth, + discovery: coreServices.discovery, plugin: coreServices.pluginMetadata, }, async factory({ auth, discovery, plugin }) { diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts index c502dc25e5..6cdff9d32c 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts @@ -60,18 +60,11 @@ export function createCredentialsBarrier(options: { predicate(req.path), ); - if (allowsCookie) { - // don't we need a user-cookie allow type here? - await httpAuth.credentials(req, { - allow: ['user-cookie', 'user', 'service'], - }); - next(); - return; - } - await httpAuth.credentials(req, { allow: ['user', 'service'], + allowedAuthMethods: allowsCookie ? ['token', 'cookie'] : ['token'], }); + next(); }; diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index a798f1b1f7..27dfeb13b6 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -101,15 +101,6 @@ export type BackstageCredentials = { principal: TPrincipal; }; -// @public (undocumented) -export type BackstageHttpAccessToPrincipalTypesMapping = { - user: BackstageUserPrincipal; - 'user-cookie': BackstageUserPrincipal; - service: BackstageServicePrincipal; - unauthenticated: BackstageNonePrincipal; - unknown: unknown; -}; - // @public (undocumented) export type BackstageNonePrincipal = { type: 'none'; @@ -119,6 +110,8 @@ export type BackstageNonePrincipal = { export type BackstagePrincipalTypes = { user: BackstageUserPrincipal; service: BackstageServicePrincipal; + unauthenticated: BackstageNonePrincipal; + unknown: unknown; }; // @public (undocumented) @@ -293,16 +286,13 @@ export interface ExtensionPointConfig { // @public (undocumented) export interface HttpAuthService { // (undocumented) - credentials< - TAllowed extends keyof BackstageHttpAccessToPrincipalTypesMapping = 'unknown', - >( + credentials( req: Request_2, options?: { - allow: Array; + allow?: Array; + allowedAuthMethods?: Array<'token' | 'cookie'>; }, - ): Promise< - BackstageCredentials - >; + ): Promise>; // (undocumented) issueUserCookie(res: Response_2): Promise; // (undocumented) diff --git a/packages/backend-plugin-api/src/services/definitions/AuthService.ts b/packages/backend-plugin-api/src/services/definitions/AuthService.ts index a35d5a5160..4566d4a3fa 100644 --- a/packages/backend-plugin-api/src/services/definitions/AuthService.ts +++ b/packages/backend-plugin-api/src/services/definitions/AuthService.ts @@ -55,6 +55,8 @@ export type BackstageCredentials = { export type BackstagePrincipalTypes = { user: BackstageUserPrincipal; service: BackstageServicePrincipal; + unauthenticated: BackstageNonePrincipal; + unknown: unknown; }; /** diff --git a/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts b/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts index 937bb1ee82..cc8aace8ba 100644 --- a/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts +++ b/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts @@ -15,33 +15,17 @@ */ import { Request, Response } from 'express'; -import { - BackstageCredentials, - BackstageServicePrincipal, - BackstageNonePrincipal, - BackstageUserPrincipal, -} from './AuthService'; - -/** @public */ -export type BackstageHttpAccessToPrincipalTypesMapping = { - user: BackstageUserPrincipal; - 'user-cookie': BackstageUserPrincipal; - service: BackstageServicePrincipal; - unauthenticated: BackstageNonePrincipal; - unknown: unknown; -}; +import { BackstageCredentials, BackstagePrincipalTypes } from './AuthService'; /** @public */ export interface HttpAuthService { - credentials< - TAllowed extends - | keyof BackstageHttpAccessToPrincipalTypesMapping = 'unknown', - >( + credentials( req: Request, - options?: { allow: Array }, - ): Promise< - BackstageCredentials - >; + options?: { + allow?: Array; + allowedAuthMethods?: Array<'token' | 'cookie'>; + }, + ): Promise>; requestHeaders(options: { forward: BackstageCredentials; diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index 810f1754b7..5955c4cbb1 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -35,10 +35,7 @@ export type { HttpRouterService, HttpRouterServiceAuthPolicy, } from './HttpRouterService'; -export type { - HttpAuthService, - BackstageHttpAccessToPrincipalTypesMapping, -} from './HttpAuthService'; +export type { HttpAuthService } from './HttpAuthService'; export type { LifecycleService, LifecycleServiceStartupHook,