Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discovery issues with v6 and Azure AD B2C #718

Closed
2 tasks done
Anton-Plagemann opened this issue Oct 23, 2024 · 6 comments
Closed
2 tasks done

Discovery issues with v6 and Azure AD B2C #718

Anton-Plagemann opened this issue Oct 23, 2024 · 6 comments
Labels
IdP issue problem is on the IdP side

Comments

@Anton-Plagemann
Copy link

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

  • I have searched the issues tracker and discussions for similar topics and couldn't find anything related.
  • I agree to follow this project's Code of Conduct
@panva
Copy link
Owner

panva commented Oct 23, 2024

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
Copy link
Owner

panva commented Oct 23, 2024

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
Copy link
Author

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
Copy link
Owner

panva commented Oct 23, 2024

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
Copy link
Author

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 panva closed this as completed in 8f0ed4c Oct 23, 2024
panva added a commit that referenced this issue Oct 23, 2024
@panva panva added IdP issue problem is on the IdP side and removed triage labels Oct 23, 2024
@panva
Copy link
Owner

panva commented Oct 23, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IdP issue problem is on the IdP side
Projects
None yet
Development

No branches or pull requests

2 participants