Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 12 additions & 1 deletion apps/api/src/saml/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,18 @@
* NameID + attribute statement template that produces the assertion shape
* required by specs/api/saml.md.
*/
import * as samlify from 'samlify';
import * as samlifyNs from 'samlify';

// samlify is CommonJS. Under Node's ESM loader, cjs-module-lexer does not
// detect `SamlLib` (it is re-exported through a getter), so `import * as`
// leaves it undefined and only `default` (the whole module.exports) carries
// it. vitest's interop exposes it as a named export, which is why the tests
// never saw the production 500. Resolve from whichever view has it.
type SamlifyModule = typeof samlifyNs;
const samlify: SamlifyModule =
(samlifyNs as SamlifyModule & { default?: SamlifyModule }).default?.SamlLib !== undefined
? (samlifyNs as SamlifyModule & { default: SamlifyModule }).default
: samlifyNs;

const { IdentityProvider, ServiceProvider, Constants, SamlLib, setSchemaValidator } = samlify;

Expand Down
45 changes: 45 additions & 0 deletions plans/samlify-esm-interop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
status: done
depends: [saml-login-return-path]
specs:
- specs/api/saml.md
issues: []
pr: 183
---

# Plan: samlify ESM/CJS interop in the production build

## Scope

The first signed-in Slack SSO attempt against the live site 500'd:
`Cannot read properties of undefined (reading 'replaceTagsByValue')` in
`dist/saml/config.js`. samlify is CommonJS; under Node's ESM loader,
cjs-module-lexer does not detect `SamlLib` (re-exported through a getter),
so `import * as samlify` leaves it undefined and only `default`
(module.exports) carries it. vitest's interop exposes it as a named export,
which is why all 19 SAML tests passed while production failed.

In: resolve the module from whichever view carries `SamlLib`; verify the
compiled output under plain Node. Out: replacing samlify.

## Implements

- [api/saml.md](../specs/api/saml.md) — no behaviour change; the assertion
pipeline works as specified once the library resolves.

## Approach

`apps/api/src/saml/config.ts`: pick `samlifyNs.default` when it carries
`SamlLib`, else the namespace. Verified with
`node -e "import('./dist/saml/config.js')"` after `npm run build`.

## Validation

- `npm run type-check && npm run lint`; SAML suite 19/19.
- Post-deploy: Slack "Test configuration" completes for a signed-in user.

## Follow-ups

- Tracked as: a build-output smoke test (import the compiled API under plain
Node in CI) would have caught this before release — worth adding to
`ci.yml` after `npm run build`.
Loading