test: OidcFederation strategy coverage (@cipherstash/auth 0.39.0)#101
Conversation
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. 📝 WalkthroughWalkthroughThis PR upgrades ChangesAuth library upgrade and integration tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Add integration-tests/tests/oidc-federation.test.ts covering the new
stack-auth OidcFederation strategy (surfaced via @cipherstash/auth).
The FFI boundary duck-types on the AuthStrategy { getToken } contract and
never branches on strategy type, so OIDC coverage is consumer-side wiring
mirroring js-strategy.test.ts and wasm-round-trip.test.ts.
The hermetic end-to-end block is describe.skip-ped pending two upstream
additions in @cipherstash/auth (tracked in cipherstash-suite):
- OidcFederation is not exported yet (0.38.0 exposes only Auto/AccessKey/
OAuth strategies)
- MockAuthServer needs an OIDC token-exchange mock + base-URL override
A live contract test runs today (no creds/network): it asserts a rejected
getToken propagates as a client error through the exact opts.strategy path
the real OidcFederation will travel.
The OidcFederationStrategy export already shipped in @cipherstash/auth
0.39.0 (npm latest); the prior ^0.38.0 pin predated it. Bump the
integration-tests dep and replace the pending scaffold with real coverage:
- Contract tier (runs in CI, no creds): the published
OidcFederationStrategy constructs via .create and satisfies the FFI's
structural AuthStrategy contract; a rejected getToken propagates through
newClient.
- End-to-end tier (describe.skipIf on CS_OIDC_JWT + dataset creds): a real
federation round-trip — third-party JWT -> CTS token -> encrypt/decrypt.
A hermetic MockAuthServer round-trip isn't reachable through the published
consumer API (napi-only mock; no base-URL override on the public .create),
so the live exchange is env-gated rather than mocked.
0.39.0 also makes AccessKeyStrategy.create take the full workspace CRN
(was the <region>.<provider> segment). Update the existing wasm-round-trip
and js-strategy suites and the event-loop-exit fixture to pass the full CRN,
dropping the now-needless CRN-splitting.
e2cdaa0 to
6e891ab
Compare
- E2E: wrap the wasm OidcFederationStrategy in a plain { getToken } object
before handing it to the Neon newClient. The JsBacked path is proven
against this shape (js-strategy.test.ts); passing a wasm-bindgen class
instance straight through was untested and the block is skip-gated, so a
failure there would have shipped green.
- Contract: drop the vacuous expect(asStrategy).toBeDefined() (always true)
in favour of asserting getToken is callable on the contract-typed handle.
There was a problem hiding this comment.
Pull request overview
This PR updates the integration-tests package to consume @cipherstash/auth@^0.39.0 and adds integration coverage for the newly available OidcFederationStrategy (via the wasm-inline entry), along with updating existing AccessKeyStrategy wiring to match the 0.39.0 API change.
Changes:
- Bump
integration-testsdependency@cipherstash/authfrom^0.38.0to^0.39.0. - Add
tests/oidc-federation.test.tswith a CI-safe contract suite plus an env-gated end-to-end federation + encrypt/decrypt round-trip. - Update existing tests/fixtures to pass the full workspace CRN into
AccessKeyStrategy.create(0.39.0 behavior).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| integration-tests/tests/wasm-round-trip.test.ts | Update AccessKeyStrategy.create call site to pass full workspace CRN (0.39.0 API). |
| integration-tests/tests/oidc-federation.test.ts | Add OIDC Federation strategy contract + env-gated E2E test coverage. |
| integration-tests/tests/js-strategy.test.ts | Update AccessKeyStrategy.create call site to pass full workspace CRN (0.39.0 API). |
| integration-tests/tests/fixtures/event-loop-exit-jsbacked.cjs | Update fixture strategy construction to pass full workspace CRN (0.39.0 API). |
| integration-tests/package.json | Bump @cipherstash/auth dependency to ^0.39.0. |
| integration-tests/package-lock.json | Lockfile updates for @cipherstash/auth@0.39.0 and peer package versions. |
Files not reviewed (1)
- integration-tests/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The skip-gated end-to-end round-trip relied on a static CS_OIDC_JWT, which can't work: OIDC JWTs expire in minutes, and CI sets no such secret, so the block was skipped on every run — green but inert, masking the absence of coverage. It also duplicated existing coverage: the FFI <-> wasm-strategy <-> ZeroKMS encrypt/decrypt path is already proven via AccessKeyStrategy (wasm-round-trip/js-strategy), and the OIDC-specific getJwt -> /api/authorise exchange is tested hermetically inside stack-auth (MockAuthServer + a base-URL override not exposed on the published consumer API). Keep the two contract tests — they run in CI unconditionally with no credentials and nothing skipped — and drop the now-unused E2E block, splitCrn helper, and encrypt/decrypt/isEncrypted imports.
Summary
Adds contract coverage for the new
OidcFederationStrategyauth strategy (introduced instack-auth, surfaced to JS via@cipherstash/auth), and bumps the integration-tests dep to the version that ships it.Key finding: the export was never missing in
cipherstash-suite—OidcFederationStrategyshipped in@cipherstash/auth@0.39.0(current npmlatest). This repo'sintegration-testsjust pinned^0.38.0, the version published immediately before it. So the fix is here, not upstream.The FFI boundary duck-types on the
AuthStrategy = { getToken: () => Promise<{ token: string }> }contract (src/index.cts) and never branches on strategy type, so OIDC coverage is consumer-side wiring mirroringjs-strategy.test.ts(Neon) andwasm-round-trip.test.ts(wasm).Changes
@cipherstash/auth^0.38.0→^0.39.0inintegration-tests.tests/oidc-federation.test.ts— two contract tests that run in CI unconditionally (no credentials, no network):OidcFederationStrategyconstructs via.create(region, workspaceId, getJwt)and is structurally assignable to the FFI'sAuthStrategy;newClientwith a rejectedgetTokenpropagating as a client error.AccessKeyStrategy.createnow takes the full workspace CRN (was the<region>.<provider>segment). Updatedwasm-round-trip.test.ts,js-strategy.test.ts, and theevent-loop-exit-jsbacked.cjsfixture, dropping the now-needless CRN-splitting.Why no live end-to-end round-trip
A federation round-trip (third-party JWT → CTS token → encrypt/decrypt) is deliberately not tested here:
AccessKeyStrategy— the strategy type is irrelevant to the FFI, which only callsgetToken.getJwt→/api/authorise→ CTS token) lives entirely insidestack-auth, tested hermetically there withMockAuthServer+ a base-URL override that the published consumer API doesn't expose.Testing
npx vitest run tests/oidc-federation.test.ts→ 2 passed, 0 skippedkeyset.test.tsworkspace-permission issue (grant_keyset … Forbidden), unrelated to this change.Stacked on #100 (Rust deps → 0.37.0); merge that first and GitHub will retarget this to
main.