Merge pull request #250 from Parsifal-M/feature/use-perm-identity

Use PolicyQueryUser over BackstageIdentity
Parsifal-M · Dec 1, 2024 · b31f837 · b31f837
2 parents 22b00b7 + 97d8c9e
commit b31f837
Show file tree

Hide file tree

Showing 7 changed files with 463 additions and 477 deletions.
diff --git a/.changeset/lazy-rules-count.md b/.changeset/lazy-rules-count.md
@@ -0,0 +1,5 @@
+---
+'@parsifal-m/plugin-permission-backend-module-opa-wrapper': patch
+---
+
+Change to using PolicyUser instead of BackstageIdentity when passing the user information to the OPA policy to be more inline with how it was intended to be used.
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -40,10 +40,10 @@ jobs:
         run: yarn tsc:full
 
       - name: Run Tests
-        run: yarn test:all --coverage
+        run: yarn test:silent
 
       - name: Upload results to Codecov
         uses: codecov/codecov-action@v4
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
-          slug: Parsifal-M/backstage-opa-plugins
+          slug: Parsifal-M/backstage-opa-plugins
diff --git a/package.json b/package.json
@@ -3,7 +3,7 @@
   "version": "1.0.0",
   "private": true,
   "engines": {
-    "node": "16 || 18"
+    "node": "20 || 22"
   },
   "scripts": {
     "dev": "concurrently \"yarn start\" \"yarn start-backend\"",
@@ -19,6 +19,7 @@
     "test": "backstage-cli repo test",
     "test:all": "backstage-cli repo test --coverage",
     "test:changed": "backstage-cli repo test --onlyChanged --coverage",
+    "test:silent": "backstage-cli repo test --silent --coverage",
     "lint": "backstage-cli repo lint --since origin/main",
     "lint:all": "backstage-cli repo lint",
     "prettier:write": "prettier --write .",
@@ -51,9 +52,7 @@
   },
   "resolutions": {
     "@types/react": "^18",
-    "@types/react-dom": "^18",
-    "@yarnpkg/parsers": "3.0.0-rc.4",
-    "swagger-ui-react": "5.10.5"
+    "@types/react-dom": "^18"
   },
   "prettier": "@spotify/prettier-config",
   "lint-staged": {

diff --git a/plugins/permission-backend-module-opa-wrapper/src/permission-evaluator/opaEvaluator.test.ts b/plugins/permission-backend-module-opa-wrapper/src/permission-evaluator/opaEvaluator.test.ts
@@ -1,13 +1,14 @@
 import { OpaClient } from '../opa-client/opaClient';
 import { LoggerService } from '@backstage/backend-plugin-api';
-import { PolicyQuery } from '@backstage/plugin-permission-node';
-import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
+import {
+  PolicyQuery,
+  PolicyQueryUser,
+} from '@backstage/plugin-permission-node';
 import { AuthorizeResult } from '@backstage/plugin-permission-common';
 import { Config } from '@backstage/config';
 import { policyEvaluator } from './opaEvaluator';
 
 jest.mock('../opa-client/opaClient');
-jest.mock('winston');
 jest.mock('@backstage/config');
 
 describe('policyEvaluator', () => {
@@ -46,13 +47,21 @@ describe('policyEvaluator', () => {
         resourceType: 'someResourceType',
       },
     };
-    const mockUser: BackstageIdentityResponse = {
+    const mockUser: PolicyQueryUser = {
       identity: {
-        userEntityRef: 'parsifal-m',
+        userEntityRef: 'user:default/parsifal-m',
         ownershipEntityRefs: ['user:default/parsifal-m', 'group:default/users'],
         type: 'user',
       },
       token: 'mockToken',
+      credentials: {
+        $$type: '@backstage/BackstageCredentials',
+        principal: 'user:default/parsifal-m',
+      },
+      info: {
+        userEntityRef: 'user:default/parsifal-m',
+        ownershipEntityRefs: ['user:default/parsifal-m', 'group:default/users'],
+      },
     };
     const mockopaEntryPoint = 'some/package/admin';
 
@@ -79,13 +88,21 @@ describe('policyEvaluator', () => {
         resourceType: 'someResourceType',
       },
     };
-    const mockUser: BackstageIdentityResponse = {
+    const mockUser: PolicyQueryUser = {
       identity: {
-        userEntityRef: 'parsifal-m',
+        userEntityRef: 'user:default/parsifal-m',
         ownershipEntityRefs: ['user:default/parsifal-m', 'group:default/users'],
         type: 'user',
       },
       token: 'mockToken',
+      credentials: {
+        $$type: '@backstage/BackstageCredentials',
+        principal: 'user:default/parsifal-m',
+      },
+      info: {
+        userEntityRef: 'user:default/parsifal-m',
+        ownershipEntityRefs: ['user:default/parsifal-m', 'group:default/users'],
+      },
     };
     const mockopaEntryPoint = 'some/package/admin';
 

diff --git a/plugins/permission-backend-module-opa-wrapper/src/permission-evaluator/opaEvaluator.ts b/plugins/permission-backend-module-opa-wrapper/src/permission-evaluator/opaEvaluator.ts
@@ -1,4 +1,3 @@
-import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
 import {
   PolicyDecision,
   AuthorizeResult,
@@ -7,7 +6,10 @@ import {
   PermissionRuleParams,
 } from '@backstage/plugin-permission-common';
 import { OpaClient } from '../opa-client/opaClient';
-import { PolicyQuery } from '@backstage/plugin-permission-node';
+import {
+  PolicyQuery,
+  PolicyQueryUser,
+} from '@backstage/plugin-permission-node';
 import { PolicyEvaluationInput } from '../types';
 import { LoggerService } from '@backstage/backend-plugin-api';
 
@@ -18,15 +20,15 @@ export const policyEvaluator = (
 ) => {
   return async (
     request: PolicyQuery,
-    user?: BackstageIdentityResponse,
+    user: PolicyQueryUser,
   ): Promise<PolicyDecision> => {
     const input: PolicyEvaluationInput = {
       permission: {
         name: request.permission.name,
       },
       identity: {
-        user: user?.identity.userEntityRef,
-        claims: user?.identity.ownershipEntityRefs ?? [],
+        user: user.info.userEntityRef,
+        claims: user.info.ownershipEntityRefs ?? [],
       },
     };
 

diff --git a/plugins/permission-backend-module-opa-wrapper/src/policy.ts b/plugins/permission-backend-module-opa-wrapper/src/policy.ts
@@ -1,8 +1,8 @@
-import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
 import { PolicyDecision } from '@backstage/plugin-permission-common';
 import {
   PermissionPolicy,
   PolicyQuery,
+  PolicyQueryUser,
 } from '@backstage/plugin-permission-node';
 import { OpaClient } from './opa-client';
 import { LoggerService } from '@backstage/backend-plugin-api';
@@ -19,7 +19,7 @@ export class OpaPermissionPolicy implements PermissionPolicy {
 
   async handle(
     request: PolicyQuery,
-    user?: BackstageIdentityResponse,
+    user: PolicyQueryUser,
   ): Promise<PolicyDecision> {
     const opaRbacPolicy = policyEvaluator(this.opaClient, this.logger);
     return await opaRbacPolicy(request, user);