From 29f59be4ec386d46d75c49e2e835a758bfce6bdb Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 26 May 2026 10:04:20 -0700 Subject: [PATCH 1/2] Add integration test: OAuth Resource path-segment routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression guard for the symptom Dawson reported on 2026-05-22 (CM/Studio Okta SSO): "providerName is 'oauth', not the {configId} described, that isn't passed". The fixture configures two static providers — one literally named "oauth" (a decoy) and one named "oac-tenant-acme" with a distinctive client_id. The test fires /oauth/oac-tenant-acme/login and asserts the redirect goes to the tenant provider, not the decoy. If parseRoute ever regresses into extracting the literal "oauth" path prefix as providerName (the reported symptom), the decoy provider would incorrectly handle the request and the assertion would fail. This test PASSES on main + Harper 5.0.7, which means the bug as diagnosed is not reproducible in this code path on this stack. The v1.x / Harper v4 stack is not covered by this test; CM consumes @harperfast/oauth ^1.4.0 against harperdb ^4.7.28, and that combination may or may not exhibit the same routing behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../path-segment-routing-app/config.yaml | 32 ++++++++ .../path-segment-routing-app/package.json | 6 ++ integrationTests/path-segment-routing.test.ts | 76 +++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 integrationTests/fixtures/path-segment-routing-app/config.yaml create mode 100644 integrationTests/fixtures/path-segment-routing-app/package.json create mode 100644 integrationTests/path-segment-routing.test.ts diff --git a/integrationTests/fixtures/path-segment-routing-app/config.yaml b/integrationTests/fixtures/path-segment-routing-app/config.yaml new file mode 100644 index 0000000..4c5573d --- /dev/null +++ b/integrationTests/fixtures/path-segment-routing-app/config.yaml @@ -0,0 +1,32 @@ +# Integration test fixture: verifies the OAuth Resource correctly extracts +# the providerName from the URL path segment, NOT the literal "oauth" prefix. +# +# Two providers are configured with distinctive client_ids. A request to +# /oauth//login should redirect with that provider's client_id. If +# parseRoute were to extract the literal "oauth" path segment (the symptom +# Dawson reported on 2026-05-22 from CM/Studio), the request would route +# to the provider literally named "oauth" and we'd see the wrong client_id. + +rest: true + +'@harperfast/oauth': + package: '@harperfast/oauth' + providers: + # Provider literally named "oauth" — present specifically as a decoy. + # If parseRoute is broken and treats "oauth" as the providerName, this + # is what would (incorrectly) handle requests for other tenants. + oauth: + provider: generic + clientId: ${OAUTH_DECOY_CLIENT_ID} + clientSecret: decoy-secret + authorizationUrl: 'http://decoy.test/authorize' + tokenUrl: 'http://decoy.test/token' + userInfoUrl: 'http://decoy.test/userinfo' + # Tenant-style provider with a distinctive client_id we can assert on. + oac-tenant-acme: + provider: generic + clientId: ${OAUTH_TENANT_CLIENT_ID} + clientSecret: tenant-secret + authorizationUrl: 'http://tenant.test/authorize' + tokenUrl: 'http://tenant.test/token' + userInfoUrl: 'http://tenant.test/userinfo' diff --git a/integrationTests/fixtures/path-segment-routing-app/package.json b/integrationTests/fixtures/path-segment-routing-app/package.json new file mode 100644 index 0000000..46a896d --- /dev/null +++ b/integrationTests/fixtures/path-segment-routing-app/package.json @@ -0,0 +1,6 @@ +{ + "name": "path-segment-routing-app-fixture", + "private": true, + "type": "module", + "description": "Integration-test fixture for OAuth Resource path-segment routing. The @harperfast/oauth dep is installed at test setup time via scripts/install-fixtures.js (npm pack + install); harper is resolved from the repo root." +} diff --git a/integrationTests/path-segment-routing.test.ts b/integrationTests/path-segment-routing.test.ts new file mode 100644 index 0000000..0dee878 --- /dev/null +++ b/integrationTests/path-segment-routing.test.ts @@ -0,0 +1,76 @@ +/** + * Regression guard for the symptom Dawson reported on 2026-05-22 (CM/Studio + * Okta SSO): + * + * "providerName is 'oauth', not the {configId} described, that isn't passed" + * + * Diagnosis would be that the plugin extracts the literal "oauth" path + * prefix as providerName instead of the segment after it. If true, requests + * to /oauth//login would dispatch to the provider literally named + * "oauth" regardless of what tenantId the caller asked for. + * + * This test configures TWO providers — one literally named "oauth" (a + * decoy) and one named "oac-tenant-acme" — with distinctive client_ids, + * then asserts a request to /oauth/oac-tenant-acme/login redirects with + * the TENANT's client_id, not the decoy's. If parseRoute ever regresses + * into returning the literal "oauth" segment, the assertion will fail + * because the decoy provider's authorization URL would be used. + */ +import { suite, test, before, after } from 'node:test'; +import { strictEqual } from 'node:assert/strict'; +import { join, dirname } from 'node:path'; +import { createRequire } from 'node:module'; +import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; + +const require = createRequire(import.meta.url); + +function getHarperBinPath(): string { + return join(dirname(require.resolve('harper')), 'bin', 'harper.js'); +} + +const fixturePath = join(import.meta.dirname, 'fixtures', 'path-segment-routing-app'); + +const TENANT_CLIENT_ID = 'tenant-acme-client-id'; +const DECOY_CLIENT_ID = 'decoy-oauth-client-id'; + +suite('OAuth Resource routes by URL path segment (not literal "oauth")', (ctx: ContextWithHarper) => { + before(async () => { + await setupHarperWithFixture(ctx, fixturePath, { + harperBinPath: getHarperBinPath(), + env: { + OAUTH_TENANT_CLIENT_ID: TENANT_CLIENT_ID, + OAUTH_DECOY_CLIENT_ID: DECOY_CLIENT_ID, + }, + config: { logging: { stdStreams: true } }, + }); + }); + + after(async () => { + await teardownHarper(ctx); + }); + + test('/oauth/oac-tenant-acme/login dispatches to the tenant provider', async () => { + const response = await fetch(`${ctx.harper.httpURL}/oauth/oac-tenant-acme/login`, { + redirect: 'manual', + }); + + strictEqual(response.status, 302, `expected 302 redirect, got ${response.status}`); + const location = response.headers.get('location'); + strictEqual(typeof location, 'string', 'Location header missing'); + const url = new URL(location!); + + // The tenant provider's authorizationUrl is http://tenant.test/authorize; + // the decoy's is http://decoy.test/authorize. If parseRoute returned + // "oauth" instead of "oac-tenant-acme", we'd land at decoy.test. + strictEqual( + url.origin + url.pathname, + 'http://tenant.test/authorize', + `request was routed to ${url.origin} — expected tenant.test (decoy is decoy.test)` + ); + strictEqual( + url.searchParams.get('client_id'), + TENANT_CLIENT_ID, + `client_id was ${url.searchParams.get('client_id')} — expected tenant's (${TENANT_CLIENT_ID}), decoy's is ${DECOY_CLIENT_ID}` + ); + }); +}); From 7a5e7b4830eeed551d3380f34bcf7e6e475de05a Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 26 May 2026 14:40:50 -0700 Subject: [PATCH 2/2] Address agy review: tighten failure-mode wording, oauth-named tenant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review-driven adjustments to the path-segment-routing fixture: 1. Tighten the docstring/comment wording about what a parseRoute regression would look like in this test. Codex caught that "the decoy would handle the request" overstates the failure mode — with the bug, action would be the tenant URL segment (an unknown action → 404), so the test would fail on `expected 302` rather than redirecting to decoy.test. Reworded both files to describe the failure as "would NOT produce a tenant-shaped 302" and lean on the assertions to surface the specific mismatch. 2. Rename the tenant provider from oac-tenant-acme to oac-oauth-tenant (agy's suggestion). The substring "oauth" inside the providerName now also guards against an over-aggressive stripping regression — e.g., a future parseRoute that mangles /oauth/ matches occurring inside the segment, not just at the prefix. Renamed the env var value and the URL accordingly. Also split the url.origin/pathname assertion to avoid string-concat false-failure modes (agy suggestion). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../path-segment-routing-app/config.yaml | 22 ++++++++--- integrationTests/path-segment-routing.test.ts | 39 +++++++++---------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/integrationTests/fixtures/path-segment-routing-app/config.yaml b/integrationTests/fixtures/path-segment-routing-app/config.yaml index 4c5573d..a547e1c 100644 --- a/integrationTests/fixtures/path-segment-routing-app/config.yaml +++ b/integrationTests/fixtures/path-segment-routing-app/config.yaml @@ -3,9 +3,19 @@ # # Two providers are configured with distinctive client_ids. A request to # /oauth//login should redirect with that provider's client_id. If -# parseRoute were to extract the literal "oauth" path segment (the symptom -# Dawson reported on 2026-05-22 from CM/Studio), the request would route -# to the provider literally named "oauth" and we'd see the wrong client_id. +# parseRoute were to extract the literal "oauth" path segment as providerName +# (the symptom Dawson reported on 2026-05-22 from CM/Studio), the request +# would NOT produce a tenant-shaped 302 — it would either route to the +# oauth-named decoy provider with action='oac-oauth-tenant' (an unknown +# action → 404) or route to some other unintended state. The exact failure +# shape depends on parseRoute's specific regression; the assertion only +# requires that the tenant.test authorizationUrl + tenant client_id +# appear in the redirect. +# +# The tenant provider name deliberately contains the substring "oauth" so +# this fixture also guards against an over-aggressive stripping regression +# (e.g., parseRoute mangling /oauth/ matches that occur inside the segment +# rather than only at the prefix). rest: true @@ -14,7 +24,7 @@ rest: true providers: # Provider literally named "oauth" — present specifically as a decoy. # If parseRoute is broken and treats "oauth" as the providerName, this - # is what would (incorrectly) handle requests for other tenants. + # is what would (incorrectly) be selected as the provider. oauth: provider: generic clientId: ${OAUTH_DECOY_CLIENT_ID} @@ -23,7 +33,9 @@ rest: true tokenUrl: 'http://decoy.test/token' userInfoUrl: 'http://decoy.test/userinfo' # Tenant-style provider with a distinctive client_id we can assert on. - oac-tenant-acme: + # The name contains "oauth" as a substring to catch over-aggressive + # path-stripping regressions in addition to the prefix-extraction case. + oac-oauth-tenant: provider: generic clientId: ${OAUTH_TENANT_CLIENT_ID} clientSecret: tenant-secret diff --git a/integrationTests/path-segment-routing.test.ts b/integrationTests/path-segment-routing.test.ts index 0dee878..7d3ef45 100644 --- a/integrationTests/path-segment-routing.test.ts +++ b/integrationTests/path-segment-routing.test.ts @@ -4,17 +4,18 @@ * * "providerName is 'oauth', not the {configId} described, that isn't passed" * - * Diagnosis would be that the plugin extracts the literal "oauth" path - * prefix as providerName instead of the segment after it. If true, requests - * to /oauth//login would dispatch to the provider literally named - * "oauth" regardless of what tenantId the caller asked for. + * If parseRoute extracted the literal "oauth" path prefix as providerName + * (rather than the segment after it), requests to /oauth//login + * would resolve to whatever provider is registered under the name "oauth" — + * with the rest of the URL becoming the action — and would NOT produce the + * expected tenant-shaped 302. * - * This test configures TWO providers — one literally named "oauth" (a - * decoy) and one named "oac-tenant-acme" — with distinctive client_ids, - * then asserts a request to /oauth/oac-tenant-acme/login redirects with - * the TENANT's client_id, not the decoy's. If parseRoute ever regresses - * into returning the literal "oauth" segment, the assertion will fail - * because the decoy provider's authorization URL would be used. + * This test configures two providers — one literally named "oauth" (a decoy) + * and one named "oac-oauth-tenant" (deliberately containing the substring + * "oauth" to also catch over-aggressive stripping) — with distinctive + * client_ids, then asserts that /oauth/oac-oauth-tenant/login redirects to + * the tenant provider's authorizationUrl with the tenant's client_id. Any + * regression that breaks path-segment routing will fail this assertion. */ import { suite, test, before, after } from 'node:test'; import { strictEqual } from 'node:assert/strict'; @@ -30,7 +31,7 @@ function getHarperBinPath(): string { const fixturePath = join(import.meta.dirname, 'fixtures', 'path-segment-routing-app'); -const TENANT_CLIENT_ID = 'tenant-acme-client-id'; +const TENANT_CLIENT_ID = 'oauth-tenant-client-id'; const DECOY_CLIENT_ID = 'decoy-oauth-client-id'; suite('OAuth Resource routes by URL path segment (not literal "oauth")', (ctx: ContextWithHarper) => { @@ -49,8 +50,8 @@ suite('OAuth Resource routes by URL path segment (not literal "oauth")', (ctx: C await teardownHarper(ctx); }); - test('/oauth/oac-tenant-acme/login dispatches to the tenant provider', async () => { - const response = await fetch(`${ctx.harper.httpURL}/oauth/oac-tenant-acme/login`, { + test('/oauth/oac-oauth-tenant/login dispatches to the tenant provider', async () => { + const response = await fetch(`${ctx.harper.httpURL}/oauth/oac-oauth-tenant/login`, { redirect: 'manual', }); @@ -60,13 +61,11 @@ suite('OAuth Resource routes by URL path segment (not literal "oauth")', (ctx: C const url = new URL(location!); // The tenant provider's authorizationUrl is http://tenant.test/authorize; - // the decoy's is http://decoy.test/authorize. If parseRoute returned - // "oauth" instead of "oac-tenant-acme", we'd land at decoy.test. - strictEqual( - url.origin + url.pathname, - 'http://tenant.test/authorize', - `request was routed to ${url.origin} — expected tenant.test (decoy is decoy.test)` - ); + // the decoy's is http://decoy.test/authorize. A parseRoute that returned + // "oauth" as providerName would either 404 (action mismatch) or pull + // the decoy's client_id — both fail these assertions. + strictEqual(url.origin, 'http://tenant.test'); + strictEqual(url.pathname, '/authorize'); strictEqual( url.searchParams.get('client_id'), TENANT_CLIENT_ID,