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

Issues with handling the azure B2C user flows #188

Open
coicoronado opened this issue Jul 29, 2021 · 6 comments
Open

Issues with handling the azure B2C user flows #188

coicoronado opened this issue Jul 29, 2021 · 6 comments

Comments

@coicoronado
Copy link

coicoronado commented Jul 29, 2021

Expected Behavior

Let the user handle the query params that are inside the Url for configuration discovery with attached query params

[REQUIRED] Describe expected behavior

Given an URL with a query params (ex. https://login.microsofonline.com/{tenant}/v2.0?p=B2C_1_nfwSignIn) make the URL formulated to construct a valid URL (ex. https://login.microsofonline.com/{tenant}/v2.0/.well-known/openid-configuration?p=B2C_1_nfwSignIn)

Describe the problem

The Issue comes when you pass the URL as stated above the formulated URL is not a valid one given the concatenation that occurs on the file authorization_service_configuration.ts

Also see this reference from Azure documentation

[REQUIRED] Actual Behavior

On function fetchFromIssuer
Input https://login.microsofonline.com/{tenant}/v2.0?p=B2C_1_nfwSignIn
Returns https://login.microsofonline.com/{tenant}/v2.0?p=B2C_1_nfwSignIn/.well-known/openid-configuration

[REQUIRED] Steps to reproduce the behavior

Just place the URL https://login.microsofonline.com/{tenant}/v2.0?p=B2C_1_nfwSignIn in the fetchFromIssuer function, I'm using a 3rd party plugin that has a dependency on this repo ionic-appauth

[REQUIRED] Environment

  • AppAuth-JS version: 1.3.1
  • AppAuth-JS Environment (Node, Browser (UserAgent), ...): Ionic which I assume would be browser____
  • Source code snippts (inline or JSBin)

This code resolves my issue but I'm not sure how to make a pull request

static fetchFromIssuer(openIdIssuerUrl: string, requestor?: Requestor):
      Promise<AuthorizationServiceConfiguration> {
    const searchForQueryParams = function(url: string) {
      let result;
      let queryOr: any = url.split('/');
      let query = queryOr[queryOr.length - 1].split('?');
      if (query.length > 1) {
        queryOr.splice(queryOr.length - 1, 1);
        queryOr = queryOr.join('/');
        result = [queryOr, `?${query[query.lenght - 1]}`];
      } else {
        result = [url, ''];
      }

      return result;
    };
    const newUrl = searchForQueryParams(openIdIssuerUrl);
    const fullUrl = `${newUrl[0]}/${WELL_KNOWN_PATH}/${OPENID_CONFIGURATION}${newUrl[1]}`;

    const requestorToUse = requestor || new JQueryRequestor();

    return requestorToUse
        .xhr<AuthorizationServiceConfigurationJson>({url: fullUrl, dataType: 'json', method: 'GET'})
        .then(json => new AuthorizationServiceConfiguration(json));
  }
@tikurahul
Copy link
Collaborator

That is a bug. Will look into it.

@coicoronado
Copy link
Author

The pull request is still pending approval does it need anything else to be worked on? so this can take the next step

@vanpyrzericj
Copy link

@tikurahul curious if you will have time soon to review this PR as I also have a project that is dependent on this being merged and resolved. Thanks!

@TechnoChimp
Copy link

Since this isn't merged in yet, as a workaround I was able to get past this by creating a custom Requestor which just adds the needed query param:

export default class MyRequestor extends Requestor {
  xhr<T>(settings: JQueryAjaxSettings): Promise<T> {
    return new Promise<T>((resolve, reject) => {
      // implementing a subset that is required.
      const url = Url.parse(settings.url!);
      const data = settings.data;
      console.log(url)
      const options = {
        hostname: url.hostname,
        port: url.port,
        path: url.path,
        query: { client_id: config.auth.clientId },
        method: settings.method,
        headers: settings.headers || {}
      };
...

@mvisosky
Copy link

Outside of the underlying issue involving the interpretation of querystring parameters, I wanted to point out that the Azure AD B2C policy name does not have to be included as a querystring parameter for the OpenID Connect discovery document. The B2C policy name can be included as a path segment in the url which negates the need for a querystring parameter.

Example:

https://fabrikamb2c.b2clogin.com/fabrikamb2c.onmicrosoft.com/b2c_1_sign_in/v2.0/.well-known/openid-configuration

@coicoronado
Copy link
Author

coicoronado commented Apr 19, 2022

Sure, but some configurations inside enterprises flow cant be changed for some reason (bureaucracy or legacy) that are available inside azure b2c, therefore the need for this change.

Is not like maintaining an old way of doing this, but unless Azure removes completely this option it should be able to handle them as mentioned in the request and MR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants