Add Microsoft Entra ID (Azure AD) OIDC simulator#373
Open
taras wants to merge 3 commits into
Open
Conversation
A drop-in replacement for core Entra authentication user flows, built on @simulacrum/foundation-simulator and modeled on the auth0 simulator. Supports the OpenID discovery document, JWKS, AAD instance discovery, the authorization-code + PKCE flow, refresh-token, client-credentials and ROPC grants, userinfo and logout — all returning Entra v2.0 shaped tokens and metadata, so applications point their authority at it with no source changes.
📝 WalkthroughWalkthroughThis PR adds a new ChangesEntra Simulator Implementation
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Package Changes Through 56105c8There are 1 changes which include @simulacrum/entra-simulator with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
packages/entra/example/index.mts (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
constfor non-reassigned variables.
config,port,authority, andappare never reassigned. Usingconstis idiomatic and prevents accidental reassignment.♻️ Proposed fix
-let config = getConfig(); -let port = config.port ?? 4400; -let authority = `https://localhost:${port}/${config.tenant}`; - -let app = simulation({ +const config = getConfig(); +const port = config.port ?? 4400; +const authority = `https://localhost:${port}/${config.tenant}`; + +const app = simulation({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/entra/example/index.mts` around lines 3 - 7, The example setup currently uses mutable bindings for values that are never reassigned. Update the variable declarations in the entrypoint around getConfig, the derived port and authority values, and the simulation app creation to use const instead of let, keeping the same initialization flow while making the intent explicit and preventing accidental reassignment.packages/entra/tsconfig.json (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
includepattern doesn't cover.mtsfiles.The example file
example/index.mtsuses the.mtsextension, but theincludeglobexample/**/*.tsonly matches.tsfiles. TypeScript treats.mtsas a distinct extension, soexample/index.mtsis excluded fromtsc --noEmittype checking.♻️ Proposed fix
- "include": ["src/**/*.ts", "test/**/*.ts", "example/**/*.ts"] + "include": ["src/**/*.ts", "test/**/*.ts", "example/**/*.mts"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/entra/tsconfig.json` at line 7, The tsconfig include list in this package misses .mts files, so example/index.mts is not type-checked by tsc --noEmit. Update the include globs in the Entra tsconfig so the existing src/test/example patterns also cover .mts alongside .ts, keeping the change aligned with the current include configuration.packages/entra/src/middleware/error-handling.ts (1)
28-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider gating stack trace exposure behind a debug flag.
Returning
error.stackin the 500 response body is helpful for local debugging but could leak internal paths and structure if the simulator is exposed beyond localhost. Consider omitting the stack unless a debug/verbose mode is enabled.♻️ Suggested refactor
} else { console.error(error); res.status(500).json({ error: { name: error.name, message: error.message, - stack: error.stack, + ...(process.env.NODE_ENV !== "production" && { stack: error.stack }), }, }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/entra/src/middleware/error-handling.ts` around lines 28 - 34, The 500 response in error-handling should not always expose the stack trace. Update the middleware in error-handling to conditionally include error.stack only when a debug/verbose flag is enabled, and otherwise return just the safe error fields. Keep the change localized to the error response construction so the existing error-handling flow remains intact.packages/entra/test/entra.test.ts (1)
37-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for
fragmentandform_postresponse modes.The PR claims support for
query,fragment, andform_postresponse modes, but the suite only exercisesquery. Adding tests forfragment(code in URL fragment) andform_post(HTML auto-submit form) would increase confidence in these claimed features.As per PR objectives, the simulator supports "
query,fragment, andform_postresponse modes."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/entra/test/entra.test.ts` around lines 37 - 81, The /authorize test suite only covers the query response mode, so add coverage in the existing describe("/authorize") block for the fragment and form_post paths exercised by the authorize flow. Use the same authorize request setup in entra.test.ts and assert that fragment returns a redirect with the code in the URL fragment, while form_post returns an HTML auto-submit form containing the expected authorization fields. Reference the existing authorize handling and response mode assertions so the new tests validate the simulator’s claimed support for query, fragment, and form_post.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/entra/package.json`:
- Around line 36-57: The package metadata is pointing TypeScript at the CommonJS
declaration file instead of the ESM declaration file. Update the top-level types
entry in package.json to use the ESM declaration output for the package’s main
entry, and add a matching types condition under both exports["."] and
publishConfig.exports["."] so consumers resolve the correct declarations from
the same entrypoint.
In `@packages/entra/README.md`:
- Around line 78-86: Add language tags to the fenced examples in the README so
markdownlint MD040 passes; update the plain code blocks around the tenant URL
examples in the documentation to use a text fence, and apply the same fix to the
other matching fenced example block referenced by the review. Use the existing
README example sections as the targets for the change.
- Around line 185-195: Clarify the endpoint scope list by explicitly noting that
`/oidc/userinfo` is the exception and is mounted globally, not tenant-scoped.
Update the README endpoint section near the tenant-scoped routes to call out
this special case, referencing the `/oidc/userinfo` entry so readers don’t
assume it follows the same `:tenant` routing as the other endpoints.
In `@packages/entra/src/handlers/entra-handlers.ts`:
- Around line 90-101: The autoPostForm helper in entra-handlers.ts is
interpolating action and fields directly into HTML attributes, which allows
reflected HTML injection in form_post responses. Update autoPostForm to escape
the action (redirect_uri) and every field name/value before building the form
markup, using the same HTML-escaping approach already used in login.ts. Keep the
existing behavior of auto-submitting the form, but ensure all interpolated
strings in autoPostForm are safely encoded.
- Around line 73-74: The redirect URL construction in entra-handlers.ts assumes
redirect_uri has no existing query string, so update the redirect logic in both
redirectWithCode and redirectWithError to choose the separator based on whether
query.redirect_uri already contains a query. Keep using "#" for fragment mode,
but for query mode use "&" when redirect_uri already has "?" and "?" otherwise,
so the res.redirect calls build valid URLs in all cases.
In `@packages/entra/src/handlers/openid-handlers.ts`:
- Around line 7-8: baseUrl currently builds URLs directly from req.get("Host"),
which can be undefined and produce invalid discovery URLs. Update baseUrl in
openid-handlers.ts to guard against a missing Host header by either falling back
to a configured/default host or rejecting the request with a 400 before
generating URLs, and make sure the callers that rely on baseUrl handle the
missing-header path cleanly.
- Around line 74-81: The discovery document in openid-handlers is advertising
grant types that the token endpoint does not actually support. Update the
`grant_types_supported` list in the OpenID discovery response to match the
grants handled by `token.ts` and `handleTokenRequest`/the token handler,
removing unsupported entries like `urn:ietf:params:oauth:grant-type:jwt-bearer`
and `implicit` unless corresponding token support is added. Keep the advertised
grants aligned with the token endpoint’s accepted flows (`authorization_code`,
`refresh_token`, `password`, `client_credentials`) so clients don’t discover
unusable options.
In `@packages/entra/src/handlers/token.ts`:
- Around line 204-221: `refreshTokenTokens` is overwriting the original
authentication time by calling `epochTime()` when issuing refreshed ID tokens.
Preserve the original `auth_time` by carrying it through the refresh flow: add
`auth_time` to `RefreshTokenPayload`, ensure it is set when the refresh token is
created, and then pass `decoded.auth_time` into `finishUserTokens` from
`refreshTokenTokens` instead of the current time so refreshed tokens keep the
original authentication timestamp.
In `@packages/entra/src/handlers/utils.ts`:
- Around line 21-22: `decodeAuthorizationCode` and `decodeRefreshToken`
currently let malformed base64url/JSON input surface as raw SyntaxError
exceptions, so update these helpers in utils.ts to catch parse/decode failures
and rethrow a typed OAuth-style error instead. Use the existing decoder
functions (`decodeAuthorizationCode`, `decodeRefreshToken`) as the entry points,
and convert invalid input into the same error shape/status your middleware
expects for `invalid_grant` (or equivalent) so the simulator returns a proper
400 response. Keep the logic localized to these helpers so callers don’t need to
change.
In `@packages/entra/src/middleware/session.ts`:
- Around line 5-13: Update createSession in session.ts to set the cookieSession
option httpOnly to true so the session cookie cannot be read by JavaScript;
silent iframe auth will still work because the browser sends the cookie
automatically. Also remove the weak default secret in createSession by requiring
an explicit secret parameter or otherwise warning loudly when the default is
used, so consumers do not accidentally ship with the fallback "shhh" value.
In `@packages/entra/src/store/index.ts`:
- Around line 88-100: The extendStore helper is dropping the logs setting from
ExtendStoreConfig, so createSimulationStore never receives it and action logging
stays off; update extendStore to forward extended?.logs alongside actions,
selectors, and schema, keeping the returned object shape aligned with the
existing extendActions, extendSelectors, and inputSchema wiring.
---
Nitpick comments:
In `@packages/entra/example/index.mts`:
- Around line 3-7: The example setup currently uses mutable bindings for values
that are never reassigned. Update the variable declarations in the entrypoint
around getConfig, the derived port and authority values, and the simulation app
creation to use const instead of let, keeping the same initialization flow while
making the intent explicit and preventing accidental reassignment.
In `@packages/entra/src/middleware/error-handling.ts`:
- Around line 28-34: The 500 response in error-handling should not always expose
the stack trace. Update the middleware in error-handling to conditionally
include error.stack only when a debug/verbose flag is enabled, and otherwise
return just the safe error fields. Keep the change localized to the error
response construction so the existing error-handling flow remains intact.
In `@packages/entra/test/entra.test.ts`:
- Around line 37-81: The /authorize test suite only covers the query response
mode, so add coverage in the existing describe("/authorize") block for the
fragment and form_post paths exercised by the authorize flow. Use the same
authorize request setup in entra.test.ts and assert that fragment returns a
redirect with the code in the URL fragment, while form_post returns an HTML
auto-submit form containing the expected authorization fields. Reference the
existing authorize handling and response mode assertions so the new tests
validate the simulator’s claimed support for query, fragment, and form_post.
In `@packages/entra/tsconfig.json`:
- Line 7: The tsconfig include list in this package misses .mts files, so
example/index.mts is not type-checked by tsc --noEmit. Update the include globs
in the Entra tsconfig so the existing src/test/example patterns also cover .mts
alongside .ts, keeping the change aligned with the current include
configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00a5add8-b06a-49c2-8972-7ec90bd3c5a1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.changes/config.json.changes/entra-simulator.mdREADME.mdpackages/entra/README.mdpackages/entra/bin/start.mjspackages/entra/example/index.mtspackages/entra/package.jsonpackages/entra/src/auth/constants.tspackages/entra/src/auth/date.tspackages/entra/src/config/get-config.tspackages/entra/src/handlers/entra-handlers.tspackages/entra/src/handlers/index.tspackages/entra/src/handlers/openid-handlers.tspackages/entra/src/handlers/token.tspackages/entra/src/handlers/url.tspackages/entra/src/handlers/utils.tspackages/entra/src/index.tspackages/entra/src/middleware/create-cors.tspackages/entra/src/middleware/error-handling.tspackages/entra/src/middleware/no-cache.tspackages/entra/src/middleware/session.tspackages/entra/src/store/entities.tspackages/entra/src/store/index.tspackages/entra/src/types.tspackages/entra/src/views/login.tspackages/entra/test/entra.test.tspackages/entra/test/helpers.tspackages/entra/test/openid-handlers.test.tspackages/entra/tsconfig.jsonpackages/entra/tsdown.config.ts
This is a local dev tool, so security-only findings are ignored; the focus is developer experience — avoiding opaque failures and confusing behavior. - Return 400 invalid_grant (not an opaque 500 + stack) for malformed authorization codes and refresh tokens, matching real Entra - Escape reflected values in the form_post auto-submit page so a legitimate state/redirect_uri containing " & < survives intact instead of corrupting the posted state (which would make the app reject the callback) - Use & instead of ? when the redirect_uri already carries a query string - Forward extendStore `logs` so `logs: true` actually enables action logging - Point package "types" at the emitted ./dist/index.d.mts (the .d.cts path did not exist, breaking types for classic-resolution consumers) - Drop grant types the token endpoint does not implement (jwt-bearer, implicit) from the discovery document - Preserve auth_time across refresh so refreshed id tokens report the original sign-in time - Clarify in the README that /oidc/userinfo is mounted globally, not tenant-scoped - Add tests for fragment and form_post response modes, HTML escaping, the redirect-separator fix, and malformed-grant 400s Deliberately not changed: session httpOnly and error stack-trace exposure are security-only concerns that do not apply to a localhost dev tool (and the stack trace aids debugging).
Member
Author
|
Thanks @coderabbitai — triaged all 15 findings through a developer-experience lens, since this is a local dev tool that never runs in production (so security-only hardening is out of scope here). Addressed (commit 56105c8):
Intentionally not changed:
Note: the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/entra/test/entra.test.ts`:
- Around line 262-272: Strengthen the form_post escaping test in the entra test
suite by also asserting that the reflected state value escapes double quotes,
not just angle brackets. Update the html-escapes reflected values in the
form_post body case to verify the response body contains the escaped quote
entity (for the injected a"><script>... payload) alongside the existing
raw-script and <script> checks, using loginWith and the response body
assertions to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b406e853-ec15-4af2-aa08-29c3134afcfe
📒 Files selected for processing (9)
packages/entra/README.mdpackages/entra/package.jsonpackages/entra/src/handlers/entra-handlers.tspackages/entra/src/handlers/openid-handlers.tspackages/entra/src/handlers/token.tspackages/entra/src/handlers/utils.tspackages/entra/src/store/index.tspackages/entra/src/types.tspackages/entra/test/entra.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/entra/README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/entra/src/store/index.ts
- packages/entra/src/types.ts
- packages/entra/src/handlers/utils.ts
- packages/entra/src/handlers/openid-handlers.ts
- packages/entra/package.json
- packages/entra/src/handlers/token.ts
- packages/entra/src/handlers/entra-handlers.ts
|
The fixes in 56105c8 look good — the On the intentionally-skipped items, that's a fair call — the DX-first framing makes sense for a simulator that's explicitly scoped to local development and never touches production traffic. I won't push further on For the ✏️ Learnings added
✅ Action performedComments resolved. Approval is disabled; enable |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
We want to run applications that authenticate against Microsoft Entra ID
(formerly Azure AD) locally — for development, previews, and integration tests —
without mock data and without changing the application's authentication source
code. Today we have the auth0 simulator but no Entra equivalent.
This adds
@simulacrum/entra-simulatoras a drop-in replacement for the coreEntra OIDC authentication user flows. An application only needs to change
where its identity provider lives (the authority/issuer URL); every core
sign-in flow then behaves as it would against real Entra.
Approach
New package at
packages/entra, modeled directly onpackages/auth0and builton
@simulacrum/foundation-simulator./:tenant/oauth2/v2.0/{authorize,token,logout},/:tenant/v2.0/.well-known/openid-configuration,/:tenant/discovery/v2.0/keys,/:tenant/discovery/instance(covers MSAL's/common/discovery/instance), and aGraph-style
/oidc/userinfo. The:tenantpath segment is authoritative — itbecomes the
tidclaim and issuer tenant so discoveryissuerand issuedtokens stay internally consistent.
client_credentials (app-only token with
roles), and ROPC/password. Supportsresponse_modeofquery,fragment,form_post, and silent auth(
prompt=none) via the session cookie.ver,iss,sub,aud,oid,tid,preferred_username,email,nonce,scp/roles,azp, andthe id_token header sets
kidmatching the JWKS so real client librariesverify signatures. Reuses the same dev RSA key as the auth0 simulator.
JSON (as auth0 does for refresh tokens), so the token endpoint needs no
server-side session for the exchange. PKCE is verified per RFC 7636.
next-auth.
Verification
vitest, matching the auth0 sibling), covering discovery/JWKS/instance discovery, the full auth-code + PKCE flow, all grant types, userinfo,
logout, and silent auth.
curlas a real client would,including cryptographically verifying the issued id_token against the
published JWKS with issuer + audience validation.
tsc,oxlint,oxfmt, and thetsdownbuild (attw + publint) are all clean.A changeset is included (
minorbump for the new package).Summary by CodeRabbit
New Features
Documentation
Tests