okta / okta-auth-js

The official js wrapper around Okta's auth API
Other
453 stars 265 forks source link

LoginCallback redirects to same '/login/callback' without any parameters, not everytime and not for all users. #610

Open ATud opened 3 years ago

ATud commented 3 years ago

After investigation of this problem (very similar to https://github.com/okta/okta-auth-js/issues/601), the fact that the timing of redirection count, which leads me thinking there is an async issue somewhere.

I try to explain the context below, and also put a video: https://www.screencast.com/t/TJcCZZq25Ueq

I don't know why the application redirect to the 'login/callback', since no configuration is present and from documentation i understood it should redirect to '/' if nothing is present.

The configuration for the 'login/callback' was simple and straightforward:

import { Security, LoginCallback } from '@okta/okta-react';

....
<Security oktaAuth={this.oktaAuth} onAuthRequired={this.onAuthRequired}>
        <Route exact path="/login" component={OktaLoginPage} />
        <Route exact path="/login/callback" component={LoginCallback} />

in package.json,

    "@okta/okta-auth-js": "^4.5.1",
    "@okta/okta-react": "^4.1.0",

Since this behavior happens inside LoginCallback I have copied that in order to be able to debug it (see the video above). I tried to inspect authState and I saw that authState is never updated with the token received from /token call, I presume because the redirect from 'login/callback?code=.....' to 'login/callback' without params was done before ?

So I thought, ok, let's try to make manual redirection after the last time the useEfectCallback is done:

React.useEffect(
    () => {
      try {
        oktaAuth.handleLoginRedirect().then(() => {  
        debugger;
        if (!authState.isPending) {
            console.log(authState);
            window.location.replace(window.location.origin);
          }
        });
      } catch (error) {
        debugger;
        console.error(error);//eslint-disable-line
        window.location.replace(window.location.origin);
      }
    },
    [oktaAuth, authState],
  );

The ABOVE code does not work, because when the page reloads, the authState is STILL not filled up with data received from '/token'.

IF i set a timeout inside the function to 1 second, it works, after the redirection I get inside my react app the correct authState, so it MUST be a timing issue.

React.useEffect(
    () => {
      try {
        oktaAuth.handleLoginRedirect().then(() => {
          debugger;
          if (!authState.isPending) {
            console.log(authState);
            setTimeout(() => {
                 window.location.replace(window.location.origin);
            }, 1000);
          }
        });
      } catch (error) {
        debugger;
        console.error(error);//eslint-disable-line
        window.location.replace(window.location.origin);
      }
    },
    [oktaAuth, authState],
  );

The ABOVE code Works!. But of course this is not reliable it cannot be considered a solution.

I have tried numerous options in the options object, nothing worked:

restoreOriginalUri: async (oktaAuth, originalUri) => {
    // redirect with custom router
    this.props.history.push('/');
  }

OR: .authStateManager.subscribe(() => { //custom function here}); OR even setting the original uri with setOriginalUri

On preproduction environment and local i get this warning, i think it has something to do with this: vendors~main.0c154034606762343482.chunk.js:971 [okta-auth-sdk] WARN: updateAuthState is an asynchronous method with no return, please subscribe to the latest authState update with authStateManager.subscribe(handler) method before calling updateAuthState.

Can you guys please help?

oleksandrpravosudko-okta commented 3 years ago

Thanks for detailed report and investigation @ATud.

It does look like a race issue. Linking this issue to internal story used to track#601: internal ref: OKTA-361608

I will try to find a workaround before we have a permanent fix.

shuowu commented 3 years ago

@ATud Some suggestions: 1) Can you try version 4.6.0 which should have a fix for error handling in handleLoginRedirect? 2) By using LoginCallback component in your routes, you don't need to call handleLoginCallback explicitly. It's handled here 3) Same thing for restoreOriginalUri, it is also handled in okta-react SDK (https://github.com/okta/okta-react/blob/master/src/Security.tsx#L47) 4) Try the react custom login sample 5) You can also reach out to our support team, they can help you dive deep with your issues.

ATud commented 3 years ago

Hello @shuowu , thank you for the reply: 1: Version 4.6.0 did not help: https://www.screencast.com/t/G8HjWEL9ue. I didn't expect to help since there were no errors, so any fix for error handling was out of my scope. 2 + 3 + 4: I am sure you did not understood the detailed explanation above. I used a local implementation of LoginCallback ONLY for you guys to see my debugger, and see that there may be an async issue somewhere in the okta sdk. 5: I will take this advice and reach to the support team.

Thank you.

andyrichardson commented 3 years ago

@ATud the following lines look a little sus

  if (!authState.isPending) {
    console.log(authState);
    setTimeout(() => {
      window.location.replace(window.location.origin);
    }, 1000);
  }

Here's a useEffect block that might cover your use case

  // Handle token retrieval
  useEffect(() => {
    // Ignore if pending or authenticated
    if (authState.isPending || authState.isAuthenticated) {
      return;
    }

    // Store tokens if sourced from redirect
    if (authClient.token.isLoginRedirect()) {
      authClient.storeTokensFromRedirect();
      return;
    }

    authClient.token.getWithRedirect();
  }, [authState, authClient]);

I would guess that your reproduction isn't working because you aren't calling storeTokensFromRedirect() immediately following a redirect

ATud commented 3 years ago

Hello @andyrichardson, thank you for the reply :) Not working unfortunately, I feel that I am unable to explain the problem :) :) The above efect it's placed inside LoginCallback component, because right now the original LoginCallback component behaves like in the video: ->takes the route login/callback?code...... and redirects not to '/' but to 'login/callback;'. The original code from okta-react is:

import * as React from 'react';
import { useOktaAuth } from './OktaContext';
import OktaError from './OktaError';

const LoginCallback: React.FC<{ 
  errorComponent?: React.ComponentType<{ error: Error }>
}> = ({ errorComponent }) => { 
  const { oktaAuth, authState } = useOktaAuth();
  const authStateReady = !authState.isPending;

  const ErrorReporter = errorComponent || OktaError;

  React.useEffect(() => {
    oktaAuth.handleLoginRedirect();
  }, [oktaAuth]);

  if(authStateReady && authState.error) { 
    return <ErrorReporter error={authState.error}/>;
  }

  return null;
};

export default LoginCallback;

The problem here is as i said above, it get's stuck. If I refresh the page, all is ok, authState is always updated.

If I change the original LoginCallback component to using a timeout of 1 second, it gives time to the sdk to complete any 'async' operation is hanging.

import React from 'react';
import PropTypes from 'prop-types';
import { useOktaAuth } from '@okta/okta-react';

const OktaError = ({ error }) => {
  // this is a copy/paste from this library's ./OktaError.js
  if (error.name && error.message) {
    return (
      <p>
        {error.name}: {error.message}
      </p>
    );
  }
  return <p>Error: {error.toString()}</p>;
};
OktaError.propTypes = {
  error: PropTypes.object,
};

const LoginCallback = ({ errorComponent }) => {
  // this is a modified copy from okta this library's ./LoginCallback.js
  const { oktaAuth, authState } = useOktaAuth();
  const authStateReady = !authState.isPending;

  const ErrorReporter = errorComponent || OktaError;
  React.useEffect(
    () => {
      const params = new URLSearchParams(window.location.search);
      const code = params.get('code');
      try {
        oktaAuth.handleLoginRedirect().then(() => {
          if (!authState.isPending && !code) {
            setTimeout(() => {
              window.location.replace(window.location.origin);
            }, 1000);
          }
        });
      } catch (error) {
        console.error(error);//eslint-disable-line
        window.location.replace(window.location.origin);
      }
    },
    [oktaAuth, authState],
  );

  if (authStateReady && authState.error) {
    return <ErrorReporter error={authState.error} />;
  }

  return null;
};

LoginCallback.propTypes = {
  errorComponent: PropTypes.object,
};

export default LoginCallback;

The only difference between the 2 codes is the actual check for 'code' because it redirects me to '/login/callback' ... (don't know why, it is) and the timeout. Actually the 'authState' is never updated even after the okta /token API is called internally.

Once again, with setTimeout work perfectly, or when I manually refresh the page after original LoginCallback redirects to simple '/login/callback' without any params.

aarongranick-okta commented 3 years ago

@ATud handleLoginRedirect may throw/reject. Try adding a .catch() block:

handleLoginRedirect().catch(e => { console.error(e); });

If there is a real error (such as user not assigned to the application), you should be able to see it here.

However these errors should be consistent for a single user. If it is every inconsistent, I suspect a race condition.

Make sure the login callback is only being rendered once, only on the callback route, and that the callback route is unprotected.

ATud commented 3 years ago

@aaronbrodersen-okta i added catch after then, in catch it never enters, but still the app redirects to 'login/callback'. that's why i am 99% sure it's an async issue somewhere. The routing is very straight forward just like in any other demo, AND it works in 90% of cases, if the code was bad it wouldn't have worked at all.

<Security oktaAuth={this.oktaAuth} onAuthRequired={this.onAuthRequired}>
        <Route exact path="/login" component={OktaLoginPage} />
        <Route exact path="/login/callback" component={LoginCallback} /> 

The flow is simple 1: I am logging it in application either with IDP either with normal user

 this.props.oktaAuth.signInWithRedirect({
      idp,
      responseType: 'code',
      scopes: ['openid', 'profile'],
    });

2: Okta SDK automatically logs the user in and redirects the url to :

   http://localhost:3000/login/callback?code=jpQCrneWO_mCV6xyCyMXXXXXXXXX&state=VD7YsMM167G5WdLVpD1YQl6cQJ2uNfHYYYYYYYYYYYY

This is a valid code, so all good.

  1. The LoginCallback from okta parses this and calls internaly the '/token' API AND (HERE IS THE ISSUE) - redirects to 'login/callback/' simple without parameters NOT to '/'. Because of this I have copied the LoginCallback locally to see what happens to authState after handleLoginRedirect triggers the 'then'. And the problem is that when it enters in 'then'... the response from /token API isn't parsed yet or Even it's not received. => authState.isAuthenticated is still false. So the page is stuck in '/login/callback' without parameters. If I refresh the page in 2 seconds, when the page loads 'authState.isAuthenticated: true' and all the response from '/token' is found inside 'authState'
stevenmckinnon commented 3 years ago

+1 on this issue, currently experiencing the same.

    "@okta/okta-auth-js": "^4.7.1",
    "@okta/okta-react": "^4.1.0",

If I hit login again or refresh it fixes itself

aarongranick-okta commented 3 years ago

@atud @andyrichardson @stevenmckinnon We have released a fix for this issue. After upgrading to @okta/okta-react version 5.1.1 you should see OAuth callback errors caught and displayed in the LoginCallback component. Please let us know if this has resolved your issue, or if you are still seeing a problem.

g-viet commented 3 years ago

@aarongranick-okta I got the same issue with using only "@okta/okta-auth-js": "^4.5.1" (not use React) It happened when my token.parseFromUrl() function is called.

g-viet commented 3 years ago

@aarongranick-okta @oleksandrpravosudko-okta Any update here ? or should I create another issue for the problem ?

oleksandrpravosudko-okta commented 3 years ago

@g-viet no updates available yet, unfortunately.

I created a new internal ticket to indicate this still requires attention: Internal Ref: OKTA-390413

Are you able to provide repro steps or describe a scenario which leads to stalled /login/callback handler in your setup?

g-viet commented 3 years ago

@oleksandrpravosudko-okta Thank you so much for your reply. I could't access the internal ref above unfortunately.

Here is the steps how we repro the issue in our project:

In package.json: "@okta/okta-auth-js": "^4.9.0"

And in callback controller, just simple like this:

oktaAuth.token.parseFromUrl().then (res) ->
  return tokenSvc.create {
    provider: "okta"
    token: res.tokens.accessToken.accessToken
    email: res.tokens.accessToken.claims.sub
  }
, (err) ->
  console.log("ERROR:", err)

When the application got the okta callback like these, it redirects to /okta/callback without parameter. Normal case: http://localhost:3000/oktalogin/callback?code=XXXXXXXX&state=YYYYYY

or error case: http://localhost:3000/oktalogin/callback?state=XXXXXXXX&error=access_denied&error_description=User+is+not+assigned+to+the+client+application.

aarongranick-okta commented 3 years ago

@g-viet parseFromUrl does remove the code, state and other OAuth parameters from the URL after processing. Generally, the next step after this method is complete is to redirect, either to your app's authenticated home page or to whichever protected route began the login flow. The method handleLoginRedirect can handle this logic for you

g-viet commented 3 years ago

Thanks @aarongranick-okta , I will try with handleLoginRedirect

BenPiggot commented 2 years ago

Is there any further information about how to solve this issue? I've encountered this same problem and I'm at a loss about how to solve it.

I'm currently using okta-react v5.1.2 and okta-auth-js v4.9.2. I have tried using okta-react 5.x.x and okta-auth-js 6.x.x, but for some reason the minified bundle I need for production causes an error that crashes the app (I've also opened an issue about this as well), so I'm stuck for now trying to understand how to get this to work using the older versions.

aarongranick-okta commented 2 years ago

@BenPiggot The latest version of okta-auth-js should be compatible with the latest version of okta-react. We also have a react sample that may help: https://github.com/okta/samples-js-react