From 0502d826a5f59e4c510a18d81de1bde53c532be8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 15 Feb 2024 17:32:04 +0100 Subject: [PATCH] permissions: migrate to new auth system and accept credentials 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 Co-authored-by: Camila Belo Signed-off-by: Patrik Oldsberg --- .changeset/late-turkeys-remember.md | 5 + .changeset/neat-owls-pump.md | 5 + .changeset/polite-tips-begin.md | 5 + .changeset/soft-grapes-cough.md | 5 + .../permissions/permissionsServiceFactory.ts | 4 +- packages/backend-plugin-api/api-report.md | 26 ++++- .../definitions/PermissionsService.ts | 35 ++++++- .../src/services/definitions/index.ts | 5 +- plugins/permission-common/src/types/api.ts | 9 +- plugins/permission-node/api-report.md | 16 ++-- .../src/ServerPermissionClient.ts | 96 +++++++++++++------ 11 files changed, 168 insertions(+), 43 deletions(-) create mode 100644 .changeset/late-turkeys-remember.md create mode 100644 .changeset/neat-owls-pump.md create mode 100644 .changeset/polite-tips-begin.md create mode 100644 .changeset/soft-grapes-cough.md diff --git a/.changeset/late-turkeys-remember.md b/.changeset/late-turkeys-remember.md new file mode 100644 index 0000000000..0640797b9f --- /dev/null +++ b/.changeset/late-turkeys-remember.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-common': patch +--- + +The `token` option of the `PermissionEvaluator` methods is now deprecated. The options that only apply to backend implementations have been moved to `PermissionsService` from `@backstage/backend-plugin-api` instead. diff --git a/.changeset/neat-owls-pump.md b/.changeset/neat-owls-pump.md new file mode 100644 index 0000000000..ca34d12756 --- /dev/null +++ b/.changeset/neat-owls-pump.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Updated the `permissionsServiceFactory` to forward the `AuthService` to the implementation. diff --git a/.changeset/polite-tips-begin.md b/.changeset/polite-tips-begin.md new file mode 100644 index 0000000000..a1211c91a6 --- /dev/null +++ b/.changeset/polite-tips-begin.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': patch +--- + +Updated the `PermissionsService` methods to accept `BackstageCredentials` through options. diff --git a/.changeset/soft-grapes-cough.md b/.changeset/soft-grapes-cough.md new file mode 100644 index 0000000000..0e43c01a08 --- /dev/null +++ b/.changeset/soft-grapes-cough.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': patch +--- + +The `ServerPermissionClient` has been migrated to implement the `PermissionsService` interface, now accepting the new `BackstageCredentials` object in addition to the `token` option, which is now deprecated. It now also optionally depends on the new `AuthService`. diff --git a/packages/backend-app-api/src/services/implementations/permissions/permissionsServiceFactory.ts b/packages/backend-app-api/src/services/implementations/permissions/permissionsServiceFactory.ts index 3e5b7da9dd..9824eb2145 100644 --- a/packages/backend-app-api/src/services/implementations/permissions/permissionsServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/permissions/permissionsServiceFactory.ts @@ -24,12 +24,14 @@ import { ServerPermissionClient } from '@backstage/plugin-permission-node'; export const permissionsServiceFactory = createServiceFactory({ service: coreServices.permissions, deps: { + auth: coreServices.auth, config: coreServices.rootConfig, discovery: coreServices.discovery, tokenManager: coreServices.tokenManager, }, - async factory({ config, discovery, tokenManager }) { + async factory({ auth, config, discovery, tokenManager }) { return ServerPermissionClient.fromConfig(config, { + auth, discovery, tokenManager, }); diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index d676de2fce..cb44ba158d 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -5,6 +5,8 @@ ```ts /// +import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common'; +import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; import { Handler } from 'express'; import { IdentityApi } from '@backstage/plugin-auth-node'; @@ -13,6 +15,8 @@ import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; +import { QueryPermissionRequest } from '@backstage/plugin-permission-common'; +import { QueryPermissionResponse } from '@backstage/plugin-permission-common'; import { Readable } from 'stream'; import { Request as Request_2 } from 'express'; import { Response as Response_2 } from 'express'; @@ -364,7 +368,27 @@ export interface LoggerService { } // @public (undocumented) -export interface PermissionsService extends PermissionEvaluator {} +export interface PermissionsService extends PermissionEvaluator { + // (undocumented) + authorize( + requests: AuthorizePermissionRequest[], + options?: PermissionsServiceRequestOptions, + ): Promise; + // (undocumented) + authorizeConditional( + requests: QueryPermissionRequest[], + options?: PermissionsServiceRequestOptions, + ): Promise; +} + +// @public +export type PermissionsServiceRequestOptions = + | { + token?: string; + } + | { + credentials: BackstageCredentials; + }; // @public (undocumented) export interface PluginMetadataService { diff --git a/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts b/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts index 5847f55ee6..1db15ce5e5 100644 --- a/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts +++ b/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts @@ -14,7 +14,38 @@ * limitations under the License. */ -import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { + AuthorizePermissionRequest, + AuthorizePermissionResponse, + PermissionEvaluator, + QueryPermissionRequest, + QueryPermissionResponse, +} from '@backstage/plugin-permission-common'; +import { BackstageCredentials } from './AuthService'; + +/** + * Options for {@link @backstage/plugin-permission-common#PermissionEvaluator} requests. + * + * @public + */ +export type PermissionsServiceRequestOptions = + | { + /** @deprecated use the `credentials` option instead. */ + token?: string; + } + | { + credentials: BackstageCredentials; + }; /** @public */ -export interface PermissionsService extends PermissionEvaluator {} +export interface PermissionsService extends PermissionEvaluator { + authorize( + requests: AuthorizePermissionRequest[], + options?: PermissionsServiceRequestOptions, + ): Promise; + + authorizeConditional( + requests: QueryPermissionRequest[], + options?: PermissionsServiceRequestOptions, + ): Promise; +} diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index 5955c4cbb1..8a5176379f 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -44,7 +44,10 @@ export type { LifecycleServiceShutdownOptions, } from './LifecycleService'; export type { LoggerService } from './LoggerService'; -export type { PermissionsService } from './PermissionsService'; +export type { + PermissionsService, + PermissionsServiceRequestOptions, +} from './PermissionsService'; export type { PluginMetadataService } from './PluginMetadataService'; export type { RootHttpRouterService } from './RootHttpRouterService'; export type { RootLifecycleService } from './RootLifecycleService'; diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index 322195a63f..e597384a56 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -263,9 +263,16 @@ export interface PermissionEvaluator { /** * Options for {@link PermissionEvaluator} requests. - * The Backstage identity token should be defined if available. + * * @public */ export type EvaluatorRequestOptions = { + /** + * @deprecated Backend plugins should no longer depend on the + * `PermissionEvaluator`, but instead use the `PermissionService` from + * `@backstage/backend-plugin-api`. Frontend plugins should not need to inject + * this token at all, but instead implicitly rely on underlying fetchApi to do + * it for them. + */ token?: string; }; diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index ea99d22fc5..7d2910d391 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -7,20 +7,21 @@ import { AllOfCriteria } from '@backstage/plugin-permission-common'; import { AnyOfCriteria } from '@backstage/plugin-permission-common'; import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common'; import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; +import { AuthService } from '@backstage/backend-plugin-api'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; import { DefinitivePolicyDecision } from '@backstage/plugin-permission-common'; -import { EvaluatorRequestOptions } from '@backstage/plugin-permission-common'; +import { DiscoveryService } from '@backstage/backend-plugin-api'; import express from 'express'; import { IdentifiedPermissionMessage } from '@backstage/plugin-permission-common'; import { NotCriteria } from '@backstage/plugin-permission-common'; import { Permission } from '@backstage/plugin-permission-common'; import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; -import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { PermissionRuleParams } from '@backstage/plugin-permission-common'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { PermissionsService } from '@backstage/backend-plugin-api'; +import { PermissionsServiceRequestOptions } from '@backstage/backend-plugin-api'; import { PolicyDecision } from '@backstage/plugin-permission-common'; import { QueryPermissionRequest } from '@backstage/plugin-permission-common'; import { ResourcePermission } from '@backstage/plugin-permission-common'; @@ -272,23 +273,24 @@ export type PolicyQuery = { }; // @public -export class ServerPermissionClient implements PermissionEvaluator { +export class ServerPermissionClient implements PermissionsService { // (undocumented) authorize( requests: AuthorizePermissionRequest[], - options?: EvaluatorRequestOptions, + options?: PermissionsServiceRequestOptions, ): Promise; // (undocumented) authorizeConditional( queries: QueryPermissionRequest[], - options?: EvaluatorRequestOptions, + options?: PermissionsServiceRequestOptions, ): Promise; // (undocumented) static fromConfig( config: Config, options: { - discovery: PluginEndpointDiscovery; + discovery: DiscoveryService; tokenManager: TokenManager; + auth?: AuthService; }, ): ServerPermissionClient; } diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index 6275fb835e..87e0eb98d1 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -16,15 +16,20 @@ import { TokenManager, - PluginEndpointDiscovery, + createLegacyAuthAdapters, } from '@backstage/backend-common'; +import { + AuthService, + BackstageCredentials, + DiscoveryService, + PermissionsService, + PermissionsServiceRequestOptions, +} from '@backstage/backend-plugin-api'; import { Config } from '@backstage/config'; import { AuthorizeResult, PermissionClient, - PermissionEvaluator, AuthorizePermissionRequest, - EvaluatorRequestOptions, AuthorizePermissionResponse, PolicyDecision, QueryPermissionRequest, @@ -36,16 +41,17 @@ import { * service-to-service requests. * @public */ -export class ServerPermissionClient implements PermissionEvaluator { +export class ServerPermissionClient implements PermissionsService { + private readonly auth: AuthService; private readonly permissionClient: PermissionClient; - private readonly tokenManager: TokenManager; private readonly permissionEnabled: boolean; static fromConfig( config: Config, options: { - discovery: PluginEndpointDiscovery; + discovery: DiscoveryService; tokenManager: TokenManager; + auth?: AuthService; }, ) { const { discovery, tokenManager } = options; @@ -62,58 +68,88 @@ export class ServerPermissionClient implements PermissionEvaluator { ); } + const { auth } = createLegacyAuthAdapters(options); + return new ServerPermissionClient({ + auth, permissionClient, - tokenManager, permissionEnabled, }); } private constructor(options: { + auth: AuthService; permissionClient: PermissionClient; - tokenManager: TokenManager; permissionEnabled: boolean; }) { + this.auth = options.auth; this.permissionClient = options.permissionClient; - this.tokenManager = options.tokenManager; this.permissionEnabled = options.permissionEnabled; } async authorizeConditional( queries: QueryPermissionRequest[], - options?: EvaluatorRequestOptions, + options?: PermissionsServiceRequestOptions, ): Promise { - return (await this.isEnabled(options?.token)) - ? this.permissionClient.authorizeConditional(queries, options) + return (await this.shouldPermissionsBeApplied(options)) + ? this.permissionClient.authorizeConditional( + queries, + await this.getRequestOptions(options), + ) : queries.map(_ => ({ result: AuthorizeResult.ALLOW })); } async authorize( requests: AuthorizePermissionRequest[], - options?: EvaluatorRequestOptions, + options?: PermissionsServiceRequestOptions, ): Promise { - return (await this.isEnabled(options?.token)) - ? this.permissionClient.authorize(requests, options) + return (await this.shouldPermissionsBeApplied(options)) + ? this.permissionClient.authorize( + requests, + await this.getRequestOptions(options), + ) : requests.map(_ => ({ result: AuthorizeResult.ALLOW })); } - private async isValidServerToken( - token: string | undefined, - ): Promise { - if (!token) { - return false; + private async getRequestOptions(options?: PermissionsServiceRequestOptions) { + if (options && 'credentials' in options) { + if (this.auth.isPrincipal(options.credentials, 'none')) { + return {}; + } + + return this.auth.getPluginRequestToken({ + onBehalfOf: options.credentials, + targetPluginId: 'permissions', + }); } - return this.tokenManager - .authenticate(token) - .then(() => true) - .catch(() => false); + + return options; } - private async isEnabled(token?: string) { - // Check if permissions are enabled before validating the server token. That - // way when permissions are disabled, the noop token manager can be used - // without fouling up the logic inside the ServerPermissionClient, because - // the code path won't be reached. - return this.permissionEnabled && !(await this.isValidServerToken(token)); + private async shouldPermissionsBeApplied( + options?: PermissionsServiceRequestOptions, + ) { + if (!this.permissionEnabled) { + return false; + } + + let credentials: BackstageCredentials; + if (options && 'credentials' in options) { + credentials = options.credentials; + } else { + if (!options?.token) { + return true; + } + try { + credentials = await this.auth.authenticate(options.token); + } catch { + return true; + } + } + + if (this.auth.isPrincipal(credentials, 'service')) { + return false; + } + return true; } }