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

Notifier never calls the authorization listener #195

Open
isgj opened this issue Oct 2, 2021 · 9 comments
Open

Notifier never calls the authorization listener #195

isgj opened this issue Oct 2, 2021 · 9 comments

Comments

@isgj
Copy link

isgj commented Oct 2, 2021

Expected Behavior

Clear documentation that shows how to handle the callback with code

[REQUIRED] Describe expected behavior

It will be nice to show what happens when the (web)app is redirected to the callback URL with the code

Describe the problem

I've configured and started the SSO flow. My app gets redirected to the callback URL /auth?code=jsDWmM6u8ebY1wtDM.... From this point it is not clear what should happen. The library does nothing, it doesn't exchange the code for tokens, the authorization listener gets never called. Can I make a request to exchange the code? How do I get the verifier? From the electron sample the verifier is taken from the request in the listener.

 AuthorizationServiceConfiguration.fetchFromIssuer('/oauth')
            .then(response => {
                this.configuration = response;
                this.showMessage('Completed fetching configuration');
            })
            .catch(error => {
                console.log('Something bad happened', error);
                this.showMessage(`Something bad happened ${error}`);
            });
        this.authorizationHandler.setAuthorizationNotifier(this.notifier);
        this.notifier.setAuthorizationListener((request, response, error) => {
            log('Authorization request complete ', request, response, error);
            if (response) {
                this.code = response.code;
                this.showMessage(`Authorization Code ${response.code}`);
            }
        });

[REQUIRED] Environment

  • AppAuth-JS version: 1.3.1
  • AppAuth-JS Environment (Node, Browser (UserAgent), ...): Browser (Firefox)
  • Source code snippts (inline or JSBin)
@bobber205
Copy link

@isgj It's because the port is hardcoded to 8000 so if your callback url isn't localhost:8000 the notifier won't capture/see the event and do the token exchange. I'm struggling with the same issue atm

@bobber205
Copy link

@isgj Here's what we're doing

this.notifier.setAuthorizationListener(async (request, response, error) => {

    if (response) {
      const codeVerifier = request.internal && request.internal.code_verifier

      let code_verifier = this.originalExpressRequest.session.auth_request.internal.code_verifier

      let code_verifier_internal = this.originalExpressRequest.session.code_verifier



      this.makeTokenRequest(response.code, codeVerifier)
        .then(response => {
          this.authStateEmitter.emit(AuthStateEmitter.ON_TOKEN_RESPONSE, response)
        })
    }
  })

@isgj
Copy link
Author

isgj commented Oct 22, 2021

@bobber205 do you have an electron app or it's hosted?

@tracplus-hpaterson
Copy link

This issue could depend on what framework you're suing, and how it generates query strings. We found AppAuth's default query string handler assumes a # is present in the URL for routing, Angular style. Other frameworks, such as React, may omit this, confusing the parser so it won't pick up the OAuth response from the query string.

You can work around this by extending the BasicQueryStringUtils from AppAuth to assume a hash is never present:

/**
 * @class NoHashQueryStringUtils
 *
 * `NoHashQueryStringUtils` extends AppAuth.js' default query string parser
 * (designed for Angular) to never assume `#`s are used for internal routing.
 *
 * This works around a bug where React URLs feature no hash, and so the parser
 * never detects the query string and OAuth parameters.
 */
class NoHashQueryStringUtils extends BasicQueryStringUtils {
  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  parse(input, useHash) {
    return super.parse(input, false);
  }
}

const ssoAuthHandler = new RedirectRequestHandler(new LocalStorageBackend(), new NoHashQueryStringUtils(), window.location, new DefaultCrypto());

@flieks
Copy link

flieks commented Nov 5, 2021

I am also having this problem in React. I tried the NoHashQueryStringUtils. No errors but the callback never gets called.

@srmxlt
Copy link

srmxlt commented Apr 21, 2022

I'm facing this problem in a react app as well. In the redirected page, the async notifier is never invoked
e.g. notifier.setAuthorizationListener(async (request, response, error) => { })
anyone know when this authorization listener should be invoked? localstorage update event?

@jordanchang
Copy link

Does your app call await this.authorizationHandler.completeAuthorizationRequestIfPossible() on load?

@thardyman
Copy link

This issue could depend on what framework you're suing, and how it generates query strings. We found AppAuth's default query string handler assumes a # is present in the URL for routing, Angular style. Other frameworks, such as React, may omit this, confusing the parser so it won't pick up the OAuth response from the query string.

You can work around this by extending the BasicQueryStringUtils from AppAuth to assume a hash is never present:

/**
 * @class NoHashQueryStringUtils
 *
 * `NoHashQueryStringUtils` extends AppAuth.js' default query string parser
 * (designed for Angular) to never assume `#`s are used for internal routing.
 *
 * This works around a bug where React URLs feature no hash, and so the parser
 * never detects the query string and OAuth parameters.
 */
class NoHashQueryStringUtils extends BasicQueryStringUtils {
  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  parse(input, useHash) {
    return super.parse(input, false);
  }
}

const ssoAuthHandler = new RedirectRequestHandler(new LocalStorageBackend(), new NoHashQueryStringUtils(), window.location, new DefaultCrypto());

I feel like this should be in the documentation, or even better, add the NoHashQueryStringUtils to the core library.

@jv18creator
Copy link

I am having same issue with my react app, any solution found for this?

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

8 participants