diff --git a/server/auth/OidcAuthStrategy.js b/server/auth/OidcAuthStrategy.js index 10ecff7c4..dce29ae35 100644 --- a/server/auth/OidcAuthStrategy.js +++ b/server/auth/OidcAuthStrategy.js @@ -107,7 +107,6 @@ class OidcAuthStrategy { this.openIdAuthSession.delete(state) sessionData = { state: state, - nonce: mobileSession.nonce, sso_redirect_uri: mobileSession.sso_redirect_uri } isMobileCallback = true @@ -434,7 +433,9 @@ class OidcAuthStrategy { } // Generate nonce to bind id_token to this session (OIDC Core 3.1.2.1) - const nonce = OpenIDClient.generators.nonce() + // Nonce is only used for web flow. Mobile flow relies on PKCE for replay protection, + // and some IdPs don't echo the nonce in the id_token for authorization code flow. + const nonce = isMobileFlow ? undefined : OpenIDClient.generators.nonce() if (isMobileFlow) { // For mobile: store session data in the openIdAuthSession Map (keyed by state) @@ -442,7 +443,6 @@ class OidcAuthStrategy { this.openIdAuthSession.set(state, { mobile_redirect_uri: req.query.redirect_uri, sso_redirect_uri: redirectUri, - nonce: nonce, created_at: Date.now() }) } diff --git a/test/server/auth/OidcAuthStrategy.test.js b/test/server/auth/OidcAuthStrategy.test.js index 990f444e6..fdae419b4 100644 --- a/test/server/auth/OidcAuthStrategy.test.js +++ b/test/server/auth/OidcAuthStrategy.test.js @@ -693,8 +693,8 @@ describe('OidcAuthStrategy', function () { sinon.stub(strategy, 'verifyUser').resolves(mockUser) // Pre-populate Map as if getAuthorizationUrl stored mobile session + // Note: mobile flow does not use nonce (relies on PKCE instead) strategy.openIdAuthSession.set('mobile-state', { - nonce: 'mobile-nonce', sso_redirect_uri: 'http://localhost/auth/openid/mobile-redirect', mobile_redirect_uri: 'audiobookshelf://oauth' }) @@ -711,9 +711,9 @@ describe('OidcAuthStrategy', function () { // Should delete the Map entry after use expect(strategy.openIdAuthSession.has('mobile-state')).to.be.false - // Should use mobile nonce and code_verifier from query + // Should use code_verifier from query; nonce is undefined for mobile flow const [, , checks] = mockClient.callback.firstCall.args - expect(checks.nonce).to.equal('mobile-nonce') + expect(checks.nonce).to.be.undefined expect(checks.code_verifier).to.equal('mobile-verifier') }) @@ -965,7 +965,7 @@ describe('OidcAuthStrategy', function () { expect(strategy.openIdAuthSession.has('mob-state')).to.be.true const stored = strategy.openIdAuthSession.get('mob-state') expect(stored.mobile_redirect_uri).to.equal('audiobookshelf://oauth') - expect(stored.nonce).to.equal('mock-nonce') + expect(stored.nonce).to.be.undefined expect(stored.sso_redirect_uri).to.include('/auth/openid/mobile-redirect') })