From ab0dd846ab067a2439e3da9f02cf3dd438b1f07d Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:17:41 +0200 Subject: [PATCH] fix(ios): store the OAuth redirect exactly as Entra returns it iOS sign-in has never worked. It fails silently, and the trailing slash in the redirect URI is why. Entra normalises a custom-scheme redirect that carries no path, so a request sent as `org.ganesha.elebook://oauthredirect` comes back as `org.ganesha.elebook://oauthredirect/`. AppAuth-iOS compares the callback against the configured redirect component by component, path included, so '' != '/' and shouldHandleURL: rejects it. OIDExternalUserAgentIOS then discards the BOOL from resumeExternalUserAgentFlowWithURL:, so the rejection is never reported and the authorization session waits forever. The person sees a spinner that never stops, with no error and nothing to retry. Android never noticed. Its intent filter matches on the scheme alone (appAuthRedirectScheme in android/app/build.gradle), so the extra slash is irrelevant there. Same config, same Entra registration, opposite outcomes. Captured on an iPhone 15 Pro by routing the flow through the external browser so the callback surfaced in application(_:open:): scheme: org.ganesha.elebook host: oauthredirect path: '/' resumed: REJECTED (url mismatch) full: org.ganesha.elebook://oauthredirect/?code=1.AQkA3Rhcn71... Storing the redirect the way Entra returns it makes the comparison succeed on iOS and changes nothing on Android, which never inspected the path. The pre-fix spelling is accepted and normalised so existing configs self-heal rather than failing validation. One migration consequence: getTokenStorageService digests redirectUrl, so the Keychain/Keystore service name moves and anyone already signed in is signed out once and has to sign in again. Observations, packs and the local database are untouched. iOS loses nothing, never having been able to sign in. The field contributes no isolation in any case -- it is validated to a single constant, so every deployment shares it. Co-Authored-By: Claude Opus 5 --- __tests__/unit/configureApp.test.ts | 25 ++++++++++++++++++++++- __tests__/unit/deploymentConfig.test.ts | 10 +++++++-- deployment.example.json | 4 ++-- docs/setup.md | 4 ++-- scripts/configure-app.js | 27 ++++++++++++++++++++++++- 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/__tests__/unit/configureApp.test.ts b/__tests__/unit/configureApp.test.ts index 811245b00..83a6f4e4a 100644 --- a/__tests__/unit/configureApp.test.ts +++ b/__tests__/unit/configureApp.test.ts @@ -122,4 +122,27 @@ describe('deployment configuration', () => { configureApp(options); expect(fs.statSync(generated).mtimeMs).toBe(0); }); -}); \ No newline at end of file +}); + +describe('configureApp redirect normalisation', () => { + // Entra returns a custom-scheme redirect with a trailing slash appended. + // AppAuth-iOS compares the callback path against the configured redirect, so + // storing it without the slash makes iOS reject its own callback and hang. + // Android matches on scheme alone and is unaffected either way. + it('stores the redirect exactly as Entra returns it, with the trailing slash', () => { + const config = validateConfig({ ...example, redirectUrl: 'org.ganesha.elebook://oauthredirect/' }); + + expect(config.redirectUrl).toBe('org.ganesha.elebook://oauthredirect/'); + }); + + it('normalises the pre-fix spelling so existing configs self-heal', () => { + const config = validateConfig({ ...example, redirectUrl: 'org.ganesha.elebook://oauthredirect' }); + + expect(config.redirectUrl).toBe('org.ganesha.elebook://oauthredirect/'); + }); + + it('still rejects a redirect belonging to a different app', () => { + expect(() => validateConfig({ ...example, redirectUrl: 'different.app://oauthredirect/' })) + .toThrow(/redirectUrl/); + }); +}); diff --git a/__tests__/unit/deploymentConfig.test.ts b/__tests__/unit/deploymentConfig.test.ts index 215fc887c..5aea88b9d 100644 --- a/__tests__/unit/deploymentConfig.test.ts +++ b/__tests__/unit/deploymentConfig.test.ts @@ -22,7 +22,7 @@ const LONG_DEPLOYMENT = { tenantId: '77777777-7777-4777-8777-777777777777', mobileClientId: '88888888-8888-4888-8888-888888888888', apiClientId: '99999999-9999-4999-8999-999999999999', - redirectUrl: 'org.ganesha.elebook://oauthredirect', + redirectUrl: 'org.ganesha.elebook://oauthredirect/', }; describe('deployment token storage', () => { @@ -40,8 +40,14 @@ describe('deployment token storage', () => { it('derives the documented service name for the example deployment', () => { // Pinned on purpose: changing the derivation orphans tokens stored by earlier builds. + // + // This value moved once, when redirectUrl gained the trailing slash that + // iOS needs to accept its own OAuth callback. Anyone already signed in is + // signed out by that change and has to sign in again; observations, packs + // and the local database are untouched. iOS loses nothing, never having + // been able to sign in at all. expect(getTokenStorageService(example)).toBe( - 'org.ganesha.elebook.entra.92eda79296364b345b9cb882a37983eb15de4e4ac4d7c6aa511b6d2d5ed543ed', + 'org.ganesha.elebook.entra.df2facb18aac60dd0a72437d48a060371a0843065ae7fa0383d7a47f1d1a355b', ); }); diff --git a/deployment.example.json b/deployment.example.json index c1055ecd1..21abc816b 100644 --- a/deployment.example.json +++ b/deployment.example.json @@ -4,5 +4,5 @@ "tenantId": "11111111-1111-4111-8111-111111111111", "mobileClientId": "22222222-2222-4222-8222-222222222222", "apiClientId": "33333333-3333-4333-8333-333333333333", - "redirectUrl": "org.ganesha.elebook://oauthredirect" -} \ No newline at end of file + "redirectUrl": "org.ganesha.elebook://oauthredirect/" +} diff --git a/docs/setup.md b/docs/setup.md index 98bde0925..8a1dab347 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -13,7 +13,7 @@ The app needs exactly six public strings. Start with [deployment.example.json](. "tenantId": "11111111-1111-4111-8111-111111111111", "mobileClientId": "22222222-2222-4222-8222-222222222222", "apiClientId": "33333333-3333-4333-8333-333333333333", - "redirectUrl": "org.ganesha.elebook://oauthredirect" + "redirectUrl": "org.ganesha.elebook://oauthredirect/" } ``` @@ -24,7 +24,7 @@ The app needs exactly six public strings. Start with [deployment.example.json](. | `tenantId` | Tenant UUID. | | `mobileClientId` | Native public-client registration UUID. | | `apiClientId` | API registration UUID. | -| `redirectUrl` | Exactly `org.ganesha.elebook://oauthredirect`, matching the native registrations. | +| `redirectUrl` | Exactly `org.ganesha.elebook://oauthredirect/`, matching the native registrations. The trailing slash is required: Entra appends it to a custom-scheme redirect that has no path, and AppAuth-iOS compares the callback path against this value, so omitting it makes iOS reject its own callback and hang. The pre-fix spelling without the slash is accepted and normalised. | Pack acquisition has a narrower limit: project IDs and pack version strings must fit within 40 UTF-16 code units. For live setup, keep `projectId` within that limit even though configuration accepts up to 128 characters; see [pack candidate paths](../src/services/packDownloadService/candidate.ts). Identities are digested into short directory names, so their length does not affect on-device paths. diff --git a/scripts/configure-app.js b/scripts/configure-app.js index 1066cfc9f..7183a6e22 100644 --- a/scripts/configure-app.js +++ b/scripts/configure-app.js @@ -10,7 +10,29 @@ const CONFIG_FIELDS = [ 'redirectUrl', ]; const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; -const NATIVE_REDIRECT = 'org.ganesha.elebook://oauthredirect'; +/** + * The trailing slash is load-bearing on iOS, and is not cosmetic. + * + * Entra normalises a custom-scheme redirect that has no path, so a request + * sent as `org.ganesha.elebook://oauthredirect` comes back as + * `org.ganesha.elebook://oauthredirect/`. AppAuth-iOS compares the callback + * against the configured redirect component by component, including path, so + * '' != '/' and it rejects the callback. It then discards the rejection -- + * OIDExternalUserAgentIOS ignores the BOOL from + * resumeExternalUserAgentFlowWithURL: -- and the authorization session waits + * forever. Sign-in spins with no error and no way to retry. + * + * Android never noticed: its intent filter matches on the scheme alone + * (appAuthRedirectScheme in android/app/build.gradle), so the extra slash is + * irrelevant there. Storing the redirect exactly as Entra returns it makes the + * comparison succeed on iOS and changes nothing on Android. + * + * Confirmed on an iPhone 15 Pro: the callback arrives as + * `org.ganesha.elebook://oauthredirect/?code=...` with path '/'. + */ +const NATIVE_REDIRECT = 'org.ganesha.elebook://oauthredirect/'; +/** Pre-fix spelling. Accepted and normalised so existing configs self-heal. */ +const NATIVE_REDIRECT_LEGACY = 'org.ganesha.elebook://oauthredirect'; function validateApiUrl(value) { let url; @@ -49,6 +71,9 @@ function validateConfig(value) { if (!/^[a-z0-9][a-z0-9_-]{0,127}$/i.test(config.projectId)) { throw new Error('projectId must contain only letters, digits, underscores, and hyphens.'); } + if (config.redirectUrl === NATIVE_REDIRECT_LEGACY) { + config.redirectUrl = NATIVE_REDIRECT; + } if (config.redirectUrl !== NATIVE_REDIRECT) { throw new Error('redirectUrl must match the native redirect registered by this app.'); }