From 701dccef94f2071c173e0a06e4e9098086858402 Mon Sep 17 00:00:00 2001 From: Jacky Jiang Date: Tue, 15 Aug 2023 21:15:02 +1000 Subject: [PATCH] #3477: decision endpoint should make decision based on anonymous user's data when the user doesn't exist --- CHANGES.md | 1 + .../src/createOpaRouter.ts | 14 +++- .../src/test/createOpaRouter.spec.ts | 81 ++++++++++++++++++- .../src/test/mockDatabase.ts | 18 ++++- 4 files changed, 105 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 75354f9c3c..bbaa29eafe 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,6 +7,7 @@ - Move format `ESRI FEATURESERVER` under `csv-geo-au` in the default map preview format perference list - Fixed: registry records editor UI doesn't encode long record ID correctly - Provides HTTP header name information on user api key creation screen +- #3477: decision endpoint should make decision based on anonymous user's data when the user doesn't exist ## v2.2.5 diff --git a/magda-authorization-api/src/createOpaRouter.ts b/magda-authorization-api/src/createOpaRouter.ts index 73d1b48441..5316214619 100644 --- a/magda-authorization-api/src/createOpaRouter.ts +++ b/magda-authorization-api/src/createOpaRouter.ts @@ -120,10 +120,16 @@ export default function createOpaRouter(options: OpaRouterOptions): Router { } async function appendUserInfoToInput(req: express.Request) { - const userInfo: User = await database.getCurrentUserInfo( - req, - jwtSecret - ); + let userInfo: User; + + try { + userInfo = await database.getCurrentUserInfo(req, jwtSecret); + } catch (e) { + console.error( + `Failed to get current user info for decision making: ${e}` + ); + userInfo = await database.getDefaultAnonymousUserInfo(); + } let reqData: any = {}; const contentType = req.get("content-type"); diff --git a/magda-authorization-api/src/test/createOpaRouter.spec.ts b/magda-authorization-api/src/test/createOpaRouter.spec.ts index 60760abf17..972646d84f 100644 --- a/magda-authorization-api/src/test/createOpaRouter.spec.ts +++ b/magda-authorization-api/src/test/createOpaRouter.spec.ts @@ -17,6 +17,7 @@ import Database from "../Database"; import mockApiKeyStore from "./mockApiKeyStore"; import { ANONYMOUS_USERS_ROLE_ID } from "magda-typescript-common/src/authorization-api/constants"; import testDataSimple from "magda-typescript-common/src/test/sampleOpaResponses/simple.json"; +import buildJwt from "magda-typescript-common/src/session/buildJwt"; describe("Auth api router", function (this: Mocha.ISuiteCallbackContext) { this.timeout(10000); @@ -102,10 +103,6 @@ describe("Auth api router", function (this: Mocha.ISuiteCallbackContext) { return app; } - /*function setMockRequestSession(req: Request, userId: string) { - return req.set("X-Magda-Session", buildJwt(argv.jwtSecret, userId)); - }*/ - describe("Test `/decision`", () => { it("should return 400 status code if not specify operation uri", async () => { const scope = createOpaNockScope(); @@ -487,6 +484,82 @@ describe("Auth api router", function (this: Mocha.ISuiteCallbackContext) { expect(query.pretty).to.be.equal("true"); expect(scope.isDone()).to.be.equal(true); }); + + it("should making decisions using the correct users info based on singed JWT token carried in request header", async () => { + let data: any; + + mockUserDataStore.reset(); + + const user = mockUserDataStore.createRecord({ + displayName: "test user", + photoURL: "", + isAdmin: false, + email: "xxx@email123.com", + source: "source_1", + sourceId: "source_id_1" + }); + + const scope = createOpaNockScope((queryParams, requestData) => { + data = requestData; + }); + const req = request(app) + .get(`/decision/object/any-object/any-operation`) + .set("X-Magda-Session", buildJwt(argv.jwtSecret, user.id)); + + const res = await req; + // should be no error and return 200 status code + expect(res.ok).to.be.equal(true); + expect(res.body.hasResidualRules).to.be.equal(false); + expect(res.body.result).to.be.equal(false); + expect(scope.isDone()).to.be.equal(true); + expect(data.input.user.id).to.equal(user.id); + expect(data.input.user.email).to.equal(user.email); + expect(data.input.operationUri).to.equal( + "object/any-object/any-operation" + ); + expect(data.input.resourceUri).to.equal("object/any-object"); + expect(data.input.timestamp).to.be.within( + Date.now() - 20000, + Date.now() + 20000 + ); + }); + + it("should making decisions using anonymous users info when failed to retrieve the user info (user doesn't exist)", async () => { + let data: any; + + mockUserDataStore.reset(); + + const scope = createOpaNockScope((queryParams, requestData) => { + data = requestData; + }); + const req = request(app) + .get(`/decision/object/any-object/any-operation`) + .set( + "X-Magda-Session", + buildJwt( + argv.jwtSecret, + "ddd4a2ca-c536-45a9-8bee-eea21c630e4b" + ) + ); + + const res = await req; + // should be no error and return 200 status code + expect(res.ok).to.be.equal(true); + expect(res.body.hasResidualRules).to.be.equal(false); + expect(res.body.result).to.be.equal(false); + expect(scope.isDone()).to.be.equal(true); + expect( + data.input.user.roles?.map((item: any) => item.id) + ).to.have.members([ANONYMOUS_USERS_ROLE_ID]); + expect(data.input.operationUri).to.equal( + "object/any-object/any-operation" + ); + expect(data.input.resourceUri).to.equal("object/any-object"); + expect(data.input.timestamp).to.be.within( + Date.now() - 20000, + Date.now() + 20000 + ); + }); }); describe("Test `/decision` endpoint decision evlautation & encoding: ", () => { diff --git a/magda-authorization-api/src/test/mockDatabase.ts b/magda-authorization-api/src/test/mockDatabase.ts index 07ef7bc9ed..b2423ba4ff 100644 --- a/magda-authorization-api/src/test/mockDatabase.ts +++ b/magda-authorization-api/src/test/mockDatabase.ts @@ -12,6 +12,7 @@ import Database from "../Database"; import NestedSetModelQueryer, { NodeRecord } from "../NestedSetModelQueryer"; import pg from "pg"; import mockApiKeyStore from "./mockApiKeyStore"; +import { defaultAnonymousUserInfo } from "../Database"; export default class MockDatabase { getUser(id: string): Promise> { @@ -77,6 +78,19 @@ export default class MockDatabase { check() {} + async getDefaultAnonymousUserInfo(): Promise { + const user = { ...defaultAnonymousUserInfo }; + try { + user.permissions = await this.getRolePermissions(user.roles[0].id); + user.roles[0].permissionIds = user.permissions.map( + (item) => item.id + ); + return user; + } catch (e) { + return user; + } + } + async getCurrentUserInfo(req: any, jwtSecret: string): Promise { const db = sinon.createStubInstance(Database); db.getUserPermissions.callsFake(this.getUserPermissions); @@ -84,7 +98,9 @@ export default class MockDatabase { db.getUserRoles.callsFake(this.getUserRoles); db.getUser.callsFake(this.getUser); db.getCurrentUserInfo.callThrough(); - db.getDefaultAnonymousUserInfo.callThrough(); + db.getDefaultAnonymousUserInfo.callsFake( + this.getDefaultAnonymousUserInfo + ); return await db.getCurrentUserInfo(req, jwtSecret); }