Skip to content

chore: migrate-old-mock-api #431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

chore: migrate-old-mock-api #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52fae60
chore: migrate-old-mock-api
ryanbas21 Oct 21, 2025
5cbafa7
chore: review-comments
ryanbas21 Oct 22, 2025
f200156
chore: setup-tests-normalize-config
ryanbas21 Nov 4, 2025
5ff6bd8
chore: setup-tests-normalize-config
ryanbas21 Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions e2e/am-mock-api/.babelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"presets": [["@nx/js/babel", { "useBuiltIns": "entry" }]]
}
30 changes: 30 additions & 0 deletions e2e/am-mock-api/.eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"extends": ["../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"ignorePatterns": [
"node_modules",
"*.md",
"LICENSE",
".babelrc",
".env*",
".bin",
"dist",
".eslintignore"
]
},
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
19 changes: 19 additions & 0 deletions e2e/am-mock-api/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "am-mock-api",
"version": "0.0.0",
"private": true,
"description": "",
"keywords": [],
"license": "ISC",
"author": "",
"main": "./index.js",
"dependencies": {
"@types/express": "^4.17.17",
"body-parser": "^2.2.0",
"cookie-parser": "^1.4.7",
"cors": "^2.8.5",
"express": "^4.21.2",
"superagent": "^10.2.3",
"uuid": "^13.0.0"
}
}
50 changes: 50 additions & 0 deletions e2e/am-mock-api/project.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{
"name": "am-mock-api",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "e2e/am-mock-api/src",
"projectType": "application",
"tags": ["scope:e2e"],
"targets": {
"build": {
"executor": "@nx/js:tsc",
"outputs": ["{projectRoot}/dist"],
"options": {
"outputPath": "e2e/am-mock-api/dist",
"main": "e2e/am-mock-api/src/index.js",
"clean": true,
"tsConfig": "e2e/am-mock-api/tsconfig.app.json",
"assets": ["e2e/am-mock-api/src/assets"]
},
"configurations": {
"development": {
"watch": true
},
"production": {
"optimization": true,
"extractLicenses": true,
"inspect": false,
"fileReplacements": [
{
"replace": "e2e/am-mock-api/src/environments/environment.ts",
"with": "e2e/am-mock-api/src/environments/environment.prod.ts"
}
]
}
}
},
"serve": {
"executor": "@nx/js:node",
"outputs": ["{projectRoot}/dist"],
"options": {
"buildTarget": "am-mock-api:build"
}
},
"lint": {
"options": {
"fix": true,
"ignore-path": ".eslintignore",
"args": ["**/*.ts"]
}
}
}
}
Empty file added e2e/am-mock-api/src/app/.gitkeep
View file
Open in desktop
Empty file.
36 changes: 36 additions & 0 deletions e2e/am-mock-api/src/app/app.auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* @forgerock/javascript-sdk
*
* app.auth.js
*
* Copyright (c) 2020 - 2025 Ping Identity Corporation. All rights reserved.
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/

import request from 'superagent';
import { AM_URL, REALM_PATH } from './env.config.js';

export let session;
Copy link

@coderabbitai coderabbitai bot Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid mutable module-level state.

Exporting a mutable session variable that gets modified in authorizeApp can lead to race conditions if multiple authorization attempts occur, and makes the module harder to test. Consider returning the session from the function and letting the caller manage state.

Refactor to avoid shared mutable state:

-export let session;
-
 export async function authorizeApp({ un, pw }) {
   try {
     const response = await request
       .post(`${AM_URL}/json/realms/${REALM_PATH}/authenticate`)
       .set('Content-Type', 'application/json')
       .set('Accept-API-Version', 'resource=2.0, protocol=1.0')
       .set('X-OpenAM-Username', un)
       .set('X-OpenAM-Password', pw)
       .send({});
 
-    session = response.body;
+    const session = response.body;
 
     console.log(`REST app identity token: ${session.tokenId}`);
 
     return session;
   } catch (error) {

Then update callers to store the returned session locally.

🧰 Tools
🪛 Biome (2.1.2)

[error] 13-14: Illegal use of an export declaration outside of a module

not allowed inside scripts

(parse)

🤖 Prompt for AI Agents 
In e2e/am-mock-api/src/app/app.auth.js around line 14, the module currently
exports a mutable top-level variable "session" which is set by authorizeApp;
refactor authorizeApp to return the session object instead of mutating exported
state and remove the exported mutable variable. Update all callers to accept and
store the returned session locally (or pass it through callers) so state is
owned by the caller and not shared across the module, and remove any direct
imports/assignments to the old exported "session". Ensure tests and call sites
are updated accordingly.

Copy link
Collaborator Author

@ryanbas21 ryanbas21 Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this exported session used that moving into the module won't break it?

Copy link

@coderabbitai coderabbitai bot Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Search for imports of 'session' from app.auth.js
echo "=== Searching for imports of 'session' from app.auth ==="
rg -n -C3 "import.*session.*from.*app\.auth" e2e/am-mock-api/

echo ""
echo "=== Searching for any imports from app.auth.js ==="
rg -n -C3 "from ['\"].*app\.auth" e2e/am-mock-api/

echo ""
echo "=== Searching for direct references to 'session' in am-mock-api ==="
rg -n -C3 "\bsession\b" e2e/am-mock-api/src/ --type js

echo ""
echo "=== Checking index.js for potential usage ==="
cat e2e/am-mock-api/src/index.js

Length of output: 7601

@ryanbas21, the exported session variable is imported and used in e2e/am-mock-api/src/app/routes.resource.js at line 13:

import { session } from './app.auth.js';

It's then accessed in three places within that file to set authentication headers when making requests to the AM service:

  1. Line 54 - in the authorization middleware: .set('iPlanetDirectoryPro', session.tokenId)
  2. Line 77 - for OAuth introspect validation: .set('iPlanetDirectoryPro', session.tokenId)
  3. Line 85 - for session validation: .set('iPlanetDirectoryPro', session.tokenId)

The current flow works like this:

  1. index.js calls authorizeApp() (without awaiting it) when NODE_ENV === 'LIVE'
  2. authorizeApp() sets the module-level session variable
  3. Later, when routes in routes.resource.js are hit, they access session.tokenId from the shared module state

To refactor this without breaking it, you would need to:

  1. Make index.js await authorizeApp() and store the result
  2. Pass the session to resourceRoutes(app, session) (and potentially authRoutes if needed)
  3. Update the default export in routes.resource.js to accept a session parameter: export default function (app, session) { ... }
  4. Remove the import of session from app.auth.js in routes.resource.js

Would you like me to provide a concrete refactoring example?


export async function authorizeApp({ un, pw }) {
try {
const response = await request
.post(`${AM_URL}/json/realms/${REALM_PATH}/authenticate`)
.set('Content-Type', 'application/json')
.set('Accept-API-Version', 'resource=2.0, protocol=1.0')
.set('X-OpenAM-Username', un)
.set('X-OpenAM-Password', pw)
.send({});

session = response.body;

console.log(`REST app identity token: ${session.tokenId}`);

return session;
} catch (error) {
console.warn('\n###################################################');
console.warn('WARNING: REST app user for Step Up/Txn Auth missing');
console.warn('###################################################\n');
}
Comment on lines +31 to +35
Copy link

@coderabbitai coderabbitai bot Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Propagate authentication failures to caller.

The catch block only logs a warning without throwing or returning an error indicator. Callers cannot determine whether authentication succeeded or failed, potentially leading to undefined behavior when accessing the session.

Apply this diff to signal failure:

   } catch (error) {
     console.warn('\n###################################################');
     console.warn('WARNING: REST app user for Step Up/Txn Auth missing');
     console.warn('###################################################\n');
+    throw error; // or return null/undefined to signal failure
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (error) {
console.warn('\n###################################################');
console.warn('WARNING: REST app user for Step Up/Txn Auth missing');
console.warn('###################################################\n');
}
} catch (error) {
console.warn('\n###################################################');
console.warn('WARNING: REST app user for Step Up/Txn Auth missing');
console.warn('###################################################\n');
throw error; // or return null/undefined to signal failure
}
🤖 Prompt for AI Agents 
In e2e/am-mock-api/src/app/app.auth.js around lines 31 to 35, the catch block
only logs a warning and does not propagate the authentication failure to
callers; update the catch block to propagate the error by either rethrowing the
caught error (throw error) after logging or return a clear failure indicator
(eg. return null/false or an object with success:false) so callers can detect
and handle authentication failure; ensure the chosen approach is consistent with
surrounding code (promise rejection vs return value) and include the original
error details when propagating.

}
66 changes: 66 additions & 0 deletions e2e/am-mock-api/src/app/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* @forgerock/javascript-sdk
*
* constants.js
*
* Copyright (c) 2020 - 2025 Ping Identity Corporation. All rights reserved.
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/

export const authPaths = {
tokenExchange: [
'/am/auth/tokenExchange',
'/am/oauth2/realms/root/access_token',
'/am/oauth2/realms/root/realms/middleware/access_token',
'/am/oauth2/realms/root/realms/middleware-modern/access_token',
'/am/oauth2/realms/root/realms/tokens-expiring-soon/access_token',
'/am/oauth2/realms/root/realms/tokens-expired/access_token',
],
authenticate: [
'/am/auth/authenticate',
'/am/json/realms/root/authenticate',
'/am/json/realms/root/realms/middleware/authenticate',
'/am/json/realms/root/realms/tokens-expiring-soon/authenticate',
'/am/json/realms/root/realms/tokens-expired/authenticate',
],
htmlAuthenticate: ['/am/'],
authorize: [
'/am/auth/authorize',
'/am/oauth2/realms/root/authorize',
'/am/oauth2/realms/root/realms/middleware/authorize',
'/am/oauth2/realms/root/realms/middleware-modern/authorize',
'/am/oauth2/realms/root/realms/tokens-expiring-soon/authorize',
'/am/oauth2/realms/root/realms/tokens-expired/authorize',
],
endSession: [
'/am/auth/endSession',
'/am/oauth2/realms/root/connect/endSession',
'/am/oauth2/realms/root/connect/idpEndSession',
'/am/oauth2/realms/root/realms/middleware/connect/endSession',
'/am/oauth2/realms/root/realms/tokens-expiring-soon/connect/endSession',
'/am/oauth2/realms/root/realms/tokens-expired/connect/endSession',
],
userInfo: [
'/am/auth/userInfo',
'/am/oauth2/realms/root/userinfo',
'/am/oauth2/realms/root/realms/middleware/userinfo',
'/am/oauth2/realms/root/realms/tokens-expiring-soon/userinfo',
'/am/oauth2/realms/root/realms/tokens-expired/userinfo',
],
revoke: [
'/am/auth/revoke',
'/am/oauth2/realms/root/token/revoke',
'/am/oauth2/realms/root/realms/middleware/token/revoke',
'/am/oauth2/realms/root/realms/tokens-expiring-soon/token/revoke',
'/am/oauth2/realms/root/realms/tokens-expired/token/revoke',
],
sessions: [
'/am/auth/sessions',
'/am/json/realms/root/sessions',
'/am/json/realms/root/realms/middleware/sessions',
'/am/json/realms/root/realms/tokens-expiring-soon/sessions',
'/am/json/realms/root/realms/tokens-expired/sessions',
],
accounts: ['/o/oauth2/v2/auth', '/SAMLFailure', '/SAMLTest'],
};
71 changes: 71 additions & 0 deletions e2e/am-mock-api/src/app/env.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* @forgerock/javascript-sdk
*
* env.config.js
*
* Copyright (c) 2020 - 2025 Ping Identity Corporation. All rights reserved.
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/

import { env } from 'process';
Copy link

@coderabbitai coderabbitai bot Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

ESM vs. CommonJS: configure modules so Biome and Node agree.

Biome parse errors suggest these files are treated as scripts. Either mark this package as ESM or switch to require/module.exports.

Add a package.json in e2e/am-mock-api/:

+// e2e/am-mock-api/package.json
+{
+  "name": "am-mock-api",
+  "private": true,
+  "type": "module"
+}

Alternatively, rename files to .mjs or configure Biome to parse ESM (sourceType: module). Based on learnings.

Also applies to: 60-71

🏁 Script executed:

#!/bin/bash
# Confirm Biome is parsing these files as scripts (look for the errors)
rg -n "Illegal use of an import declaration outside of a module" -g "!.git"

Length of output: 0

I need to gather more information about the actual state of the codebase to verify if the Biome errors mentioned in the review actually exist.

🏁 Script executed:

#!/bin/bash
# Check if e2e/am-mock-api/package.json exists
if [ -f "e2e/am-mock-api/package.json" ]; then
  echo "=== e2e/am-mock-api/package.json EXISTS ==="
  cat "e2e/am-mock-api/package.json"
else
  echo "=== e2e/am-mock-api/package.json DOES NOT EXIST ==="
fi

echo ""
echo "=== Directory structure of e2e/am-mock-api ==="
find "e2e/am-mock-api" -type f -name "*.json" -o -name "*.js" -o -name "*.mjs" | head -20

echo ""
echo "=== Content of env.config.js ==="
if [ -f "e2e/am-mock-api/src/app/env.config.js" ]; then
  cat "e2e/am-mock-api/src/app/env.config.js"
else
  echo "File not found"
fi

echo ""
echo "=== Check for Biome config ==="
if [ -f "biome.json" ] || [ -f "biome.jsonc" ]; then
  echo "Found Biome config"
  [ -f "biome.json" ] && cat "biome.json" || cat "biome.jsonc"
else
  echo "No Biome config found at root"
fi

Length of output: 4594

Fix module type mismatch: ESM syntax requires "type": "module".

The package.json specifies "type": "commonjs", but env.config.js uses ESM syntax (import/export statements). This will fail at runtime.

Note: The referenced Biome parse errors do not appear to be reported—the actual issue is the configuration mismatch.

Update e2e/am-mock-api/package.json:

{
  "name": "am-mock-api",
  "version": "0.0.0",
  "private": true,
  "description": "",
  "keywords": [],
  "license": "ISC",
  "author": "",
-  "type": "commonjs",
+  "type": "module",
  "main": "./index.js",
  ...
}

Alternatively, convert all files to CommonJS syntax (require/module.exports).

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (2.1.2)

[error] 10-11: Illegal use of an import declaration outside of a module

not allowed inside scripts

(parse)

🤖 Prompt for AI Agents 
In e2e/am-mock-api/src/app/env.config.js around line 11, the file uses ESM
import/export syntax while e2e/am-mock-api/package.json is set to "type":
"commonjs"; fix by either updating package.json to "type": "module" (so ESM
imports are valid) or convert this file to CommonJS (replace `import { env }
from 'process'` with a require and switch any exports to module.exports) and
ensure all other files in the package follow the same module style.


/**
* Configure your environment defaults below.
*/
const oauth = {
client: 'WebOAuthClient',
scope: 'openid profile me.read',
};
const origins = {
// Ensure all domains are added to the security cert creation
app: process.env.NODE_ENV === 'LIVE' ? 'https://sdkapp.petrov.ca' : 'http://localhost',
forgeops: 'https://default.forgeops.petrov.ca',
mock: 'http://localhost',
resource: 'http://localhost',
};
Comment on lines +21 to +26
Copy link

@coderabbitai coderabbitai bot Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Unify LIVE detection and avoid mixed env checks.

You use three forms: env.LIVE, process.env.NODE_ENV === 'LIVE', and 'live' elsewhere. This will cause mismatched behavior. Centralize to a single boolean and reuse across files.

Apply this diff:

@@
-const origins = {
-  // Ensure all domains are added to the security cert creation
-  app: process.env.NODE_ENV === 'LIVE' ? 'https://sdkapp.petrov.ca' : 'http://localhost',
+const isLive = String(env.LIVE || env.NODE_ENV).toLowerCase() === 'live';
+const origins = {
+  // Ensure all domains are added to the security cert creation
+  app: isLive ? 'https://sdkapp.petrov.ca' : 'http://localhost',
   forgeops: 'https://default.forgeops.petrov.ca',
   mock: 'http://localhost',
   resource: 'http://localhost',
 };
@@
-let amUrl;
-let amPort;
+let amUrl;
+let amPort;
@@
-if (env.LIVE) {
+if (isLive) {
   amUrl = origins.forgeops;
   amPort = ports.forgeops;
 } else {
   amUrl = origins.mock;
   amPort = ports.mock;
 }
@@
 export const FORGEOPS = origins.forgeops;
+export const FORGEOPS_HOST = new URL(FORGEOPS).hostname;
 export const REALM_PATH = realm;

Also applies to: 52-58, 64-71

const paths = {
am: '/am',
};
const ports = {
app: '8443',
forgeops: '443',
mock: '9443',
resource: '9443',
};
const realm = 'root';
const testUsers = [
{
// Already exists in forgeops...
pw: 'password',
un: 'sdkuser',
},
];

/**
* The below will be composed of the above values.
* Do not edit unless you know what you're doing.
*/
let amUrl;
let amPort;

if (env.LIVE) {
amUrl = origins.forgeops;
amPort = ports.forgeops;
} else {
amUrl = origins.mock;
amPort = ports.mock;
}

export const APP_PORT = ports.app;
export const AM_PORT = amPort;
export const MOCK_PORT = process.env.PORT || ports.mock;

export const AM_URL = `${amUrl}:${amPort}${paths.am}`;
export const BASE_URL = `${origins.app}:${ports.app}`;
export const CLIENT_ID = oauth.client;
export const FORGEOPS = origins.forgeops;
export const REALM_PATH = realm;
export const RESOURCE_URL = `${origins.resource}:${ports.resource}`;
export const SCOPE = oauth.scope;
export const USERS = testUsers;
Loading
Loading