panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Discovery issues with v6 and Azure AD B2C #718

Closed Anton-Plagemann closed 3 days ago

Anton-Plagemann commented 3 days ago

What happened?

I've had some issues with the discovery function when migrating from v5 to v6. The discovery failed with the error discovered metadata issuer does not match the expected issuer.

Example urls that fail (worked previously with v5):

Workaround Adding /.well-known/openid-configuration to the end makes the discovery work:

Version

v6.1.1

Runtime

Node.js

Runtime Details

Node v22.8.0

Code to reproduce

const { discovery } = await import("openid-client")

async function testDiscoveryEndpoints() {
  const tenantName = 'openidclientdemo.onmicrosoft.com'
  const tenantId = '0e96f835-6e34-470c-800b-2e2c5908c54c'
  const policy = 'B2C_1_signupsignin'

  const urls = [
    `https://openidclientdemo.b2clogin.com/${tenantName}/${policy}/v2.0`,
    `https://openidclientdemo.b2clogin.com/${tenantId}/${policy}/v2.0`
  ]

  for (const url of urls) {
    try {
      console.log(`Testing URL: ${url}`)
      const result = await discovery(new URL(url), 'dummy_client_id')
      console.log('Success:', result)
    } catch (error) {
      console.error('Failed:', error.message)
    }
  }
}

// Execute test
await testDiscoveryEndpoints()

// Output:
// Testing URL: https://openidclientdemo.b2clogin.com/openidclientdemo.onmicrosoft.com/B2C_1_signupsignin/v2.0
// Failed: discovered metadata issuer does not match the expected issuer
// Testing URL: https://openidclientdemo.b2clogin.com/0e96f835-6e34-470c-800b-2e2c5908c54c/B2C_1_signupsignin/v2.0
// Failed: discovered metadata issuer does not match the expected issuer

Required

panva commented 3 days ago

Thanks. I'll take a look. I expect the same shenanigans as with the multitenant common endpoints on Entra ID. I lacked reproduction cases, hopefully yours will help

panva commented 3 days ago

Sigh...

Can you apply this patch (minus the TS bits) locally and let me know what comes next? I assume nothing but I'd like to wait with its release before knowing it is the full extent of necessary spec behaviour bending needed.

diff --git a/src/index.ts b/src/index.ts
index 24d1852..89ab459 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1038,6 +1038,17 @@ function handleEntraId(
   return false
 }

+function handleB2Clogin(server: URL, options?: DiscoveryRequestOptions) {
+  if (
+    server.hostname.endsWith('.b2clogin.com') &&
+    (!options?.algorithm || options.algorithm === 'oidc')
+  ) {
+    return true
+  }
+
+  return false
+}
+
 /**
  * Performs Authorization Server Metadata discovery and returns a
  * {@link Configuration} with the discovered
@@ -1117,6 +1128,7 @@ export async function discovery(

   if (resolve && new URL(as.issuer).href !== server.href) {
     handleEntraId(server, as, options) ||
+      handleB2Clogin(server, options) ||
       (() => {
         throw new ClientError(
           'discovered metadata issuer does not match the expected issuer',
Anton-Plagemann commented 3 days ago

Works perfectly 👍

I tested all variations known to me:

Testing URL: https://openidclientdemo.b2clogin.com/openidclientdemo.onmicrosoft.com/B2C_1_signupsignin/v2.0
Success: Configuration {}
Testing URL: https://openidclientdemo.b2clogin.com/0e96f835-6e34-470c-800b-2e2c5908c54c/B2C_1_signupsignin/v2.0
Success: Configuration {}
Testing URL: https://openidclientdemo.b2clogin.com/openidclientdemo.onmicrosoft.com/v2.0/.well-known/openid-configuration?p=B2C_1_signupsignin
Success: Configuration {}
Testing URL: https://openidclientdemo.b2clogin.com/0e96f835-6e34-470c-800b-2e2c5908c54c/v2.0/.well-known/openid-configuration?p=B2C_1_signupsignin
Success: Configuration {}
Testing URL: https://openidclientdemo.b2clogin.com/openidclientdemo.onmicrosoft.com/B2C_1_signupsignin/v2.0/.well-known/openid-configuration
Success: Configuration {}
Testing URL: https://openidclientdemo.b2clogin.com/0e96f835-6e34-470c-800b-2e2c5908c54c/B2C_1_signupsignin/v2.0/.well-known/openid-configuration
Success: Configuration {}

btw: The test urls are real b2c urls. You can also use them for testing (I've quickly created an Azure AD B2C tenant for this issue).

panva commented 3 days ago

I see, there's even more variation with the policy being in a query string... sigh.

Works perfectly 👍

Even subsequent interactions with the server like Authorization Grant, Refresh Token Grant, etc?

btw: The test urls are real b2c urls. You can also use them for testing (I've quickly created an Azure AD B2C tenant for this issue).

Thank you for that, if at all possible please share the client credentials privately (email, twitter DM).

Anton-Plagemann commented 3 days ago

Even subsequent interactions with the server like Authorization Grant, Refresh Token Grant, etc?

Yes, I've tested login (auth code grant), refresh and logout (buildEndSessionUrl).

Thank you for that, if at all possible please share the client credentials privately (email, twitter DM).

Sure! Shared them via email 👍

panva commented 3 days ago

https://github.com/panva/openid-client/releases/tag/v6.1.3