Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Query parameter that contains code= will cause SecureRoute to render nothing #924

Open
2 of 8 tasks
veswill3 opened this issue Sep 29, 2020 · 8 comments
Open
2 of 8 tasks

Comments

@veswill3
Copy link

I'm submitting this issue for the package(s):

  • jwt-verifier
  • okta-angular
  • oidc-middleware
  • okta-react
  • okta-react-native

I'm submitting a:

  • Bug report
  • Feature request
  • Other (Describe below)

Current behavior

hard linking (deep link, using an anchor element) into the app with a query parameter that ends with code= seems to break the app and nothing inside of a SecureRoute will render, even after navigating to new routes. Soft linking around with react router usually works, but will also break sometimes.

Expected behavior

query parameters should not cause SecureRoutes to render nothing.

Minimal reproduction of the problem with instructions

install okta-react 3.0.7, then use this as your react application

import React from 'react';
import ReactDOM from 'react-dom';
import { BrowserRouter, Route, Switch, Link } from 'react-router-dom';
import { Security, SecureRoute, LoginCallback } from '@okta/okta-react';

const RootArea = () => <div>root</div>;
const InsecureArea = () => <div>Not secure here</div>;
const First = () => <div>first</div>;
const Second = () => <div>second</div>;
const Third = () => <div>third</div>;
const Fourth = () => <div>fourth</div>;
const SecureArea = () => (
  <>
    <Route path="/secure/first" component={First} />
    <Route path="/secure/second" component={Second} />
    <Route path="/secure/third" component={Third} />
    <Route path="/secure/fourth" component={Fourth} />
  </>
);
const LinkLi = ({ to }) => <li>soft link: <Link to={to}>{to}</Link> --- hard link: <a href={to}>{to}</a></li>;

const App = () => (
  <BrowserRouter>
    <ul>
      <LinkLi to="/" />
      <LinkLi to="/not-secure" />
      <LinkLi to="/secure/first" />
      <LinkLi to="/secure/first?test=pass" />
      <LinkLi to="/secure/second" />
      <LinkLi to="/secure/second?response_code=123" />
      <LinkLi to="/secure/third" />
      <LinkLi to="/secure/third?code=1" />
      <LinkLi to="/secure/fourth" />
      <LinkLi to="/secure/fourth/code=1/hello" />
    </ul>
    <Security
      issuer="XXXXX"
      clientId="XXXXX"
      redirectUri="XXXXX"
      pkce={true}
      tokenManager={{ secure: true }}
    >
      <Switch>
        <Route path="/implicit/callback" exact component={LoginCallback} />
        <Route path="/not-secure" component={InsecureArea} />
        <SecureRoute path="/secure" component={SecureArea} />
        <Route component={RootArea} />
      </Switch>
    </Security>
  </BrowserRouter>
);

ReactDOM.render(<App />, global.document.getElementById('root'));

First click around the soft links and notice that it (probably) seems to work as expected, where the bottom will show first, second, third etc. You should be able to click around the insecure routes, the secure routes, and it all looks good.

Now click one of the hard links that has code= somewhere in the query parameters. Now nothing will show for the secure routes. the insecure routes will still show as you navigate to them, but the secure route will never render anything again until you refresh the page or click another "hard" link.
Note: I have seen even the soft links trigger this issue occasionally with some auth sdk error, but it is hard to reproduce.

Extra information about the use case/user story you are trying to implement

Our current use case is processing payments, where we navigate away to enter payment information, and they send the user back to our application with a bunch of query params that indicate success and failure, and why. One of those query parameters is request_code=xxx which causes our application to fail.

Environment

  • Package Version: 3.0.7
  • Browser: Chrome
  • OS: Mac catalina 10.15.7
  • Node version (node -v): 12.16.3
  • Other: This does not seem to be an issue on okta-react 1.1.4 but I did not do an exhaustive search
@aarongranick-okta
Copy link
Contributor

Thanks for the submission. We are aware of this issue and are working on a fix. See comments here for a possible workaround: okta/okta-auth-js#474 (comment) (the authClient is exposed as a property "_oktaAuth" in the okta-react sdk)

@veswill3
Copy link
Author

@aarongranick-okta Either I applied the workaround incorrectly, or the workaround does not seem to help. Any other ideas?

I had to get to the authClient with the hook. Here is the basic idea of how I tried to apply your suggestion:

<Security
  issuer="XXXXX"
  clientId="XXXXX"
  redirectUri="XXXXX"
  pkce={true}
  tokenManager={{ secure: true }}
>
  <AppWithWorkaround />
</Security>

and the new component:

const AppWithWorkaround = () => {
  const { authService } = useOktaAuth();
  authService._oktaAuth.token.isLoginRedirect = () => false;
  return (
    <Switch>
      <Route path="/implicit/callback" exact component={LoginCallback} />
      <Route path="/not-secure" component={InsecureArea} />
      <SecureRoute path="/secure" component={SecureArea} />
      <Route component={RootArea} />
    </Switch>
  );
};

also, If you would prefer, we can close this issue and continue the discussion in okta/okta-auth-js#474, just let me know.

@aarongranick-okta
Copy link
Contributor

@veswill3 The workaround looks correct. It's possible it may be running into a separate issue: #921 which may cause multiple instances of the "authService" to be created. Try creating the AuthService yourself up-front (and modifying authService._oktaAuth.token.isLoginRedirect as you are doing here), then pass this authService instance to the Security component, as described here: https://github.com/okta/okta-oidc-js/tree/master/packages/okta-react#alternate-configuration-using-authservice-instance

We are working on fixing both of these issues, but I think this workaround should do the trick in the short-term.

@veswill3
Copy link
Author

@aarongranick-okta Unfortunately going that route does not seem to help either. I do get an error immediately however:

"AuthSdkError"
"The app should not attempt to call getToken on callback. Authorize flow is already in process. Use parseFromUrl() to receive tokens."

Could it be related to my specific setup? Does it work for you if you try to implement the workaround in the example above?

@aarongranick-okta
Copy link
Contributor

@veswill3 The error is being thrown here: https://github.com/okta/okta-auth-js/blob/3.2/packages/okta-auth-js/lib/token.js#L550 I see that patching token.isLoginRedirect will not fix it because the method is being called on oauthUtil, which is not exposed in a way that is easy to modify. There are a couple other ways to get around this. Looking at the implementation of isLoginRedirect: https://github.com/okta/okta-auth-js/blob/3.2/packages/okta-auth-js/lib/oauthUtil.js#L246 you could set responseMode to "fragment". this would cause the okta library to look for the code in the hash fragment instead of the query. https://github.com/okta/okta-auth-js#responsemode-1

@veswill3
Copy link
Author

like this?

<Security
  issuer="XXXXX"
  clientId="XXXXX"
  redirectUri="XXXXX"
  pkce={true}
  tokenManager={{ secure: true }}
  responseMode="fragment" // <-- added this
>
  ...
</Security>

This does seem to resolve our issue. I will experiment with our full application to make sure, and track the issue for a future resolution.
Thanks @aarongranick-okta!

@aarongranick-okta
Copy link
Contributor

@veswill3 Yes, exactly. I'm glad it is working for you! For a SPA application, there should be no negative effect of using the hash instead of the query.

@aarongranick-okta
Copy link
Contributor

internal ref: OKTA-331632

related issue in okta-auth-js: okta/okta-auth-js#474

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

No branches or pull requests

2 participants