okta / okta-oidc-js

okta-oidc-js
https://github.com/okta/okta-oidc-js
Other
395 stars 232 forks source link

[okta-react] unnecessary reloads after landing on /implicit/callback #739

Open sleepwalky opened 4 years ago

sleepwalky commented 4 years ago

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

I'm submitting a:

Current behavior

Application is reloaded during redirection after landing on /implicit/callback url.

Expected behavior

Redirection via react-router and history API, so okta-react won't trigger full application reload.

In previous version 1.4.1 I was creating Auth service manually and I could pass history object in params, so redirection in Auth service was handled via history API. But now there is no such possibility.

Environment

sleepwalky commented 4 years ago

What was the motivation for uncoupling history API from AuthService?

swiftone commented 4 years ago

@sleepwalky - Thanks for the report.

The motivation was that we now support multiple routers, not just react-router, so we couldn't make assumptions about the API for any given router object.

For clarity, are you using a HashRouter or BrowserRouter (so we understand the impact of different options).

swiftone commented 4 years ago

Internal ref: OKTA-289846

sleepwalky commented 4 years ago

The motivation was that we now support multiple routers, not just react-router, so we couldn't make assumptions about the API for any given router object.

Totally makes sense.

I'm using https://reacttraining.com/react-router/web/api/Router and I just pass same history object that I was passing to auth service (version 1.4.1).

Quick solution that came into my mind is extending https://github.com/okta/okta-oidc-js/tree/master/packages/okta-react#configuration-options by adding an optional prop something like 'redirectCallback' which will default to window.location.assign or whatever is used in current implementation. React-router users can pass history.push into it. Don't know about other routers.

swiftone commented 4 years ago

Quick solution that came into my mind is extending /packages/okta-react@master#configuration-options by adding an optional prop something like 'redirectCallback' which will default to window.location.assign or whatever

Great minds! That's the same basic idea we're considering. Any router should be able to pass a function that calls their history controller as appropriate.

asharron commented 4 years ago

Hey! Is there any known work around for this in the meantime? Working on a new application and the app will successfully authenticate, but I believe this is causing a troublesome redirect for me and would like to use the history API for react router to handle it.

Let me know if I can help in any way!

license2e commented 4 years ago

@asharron I'll submit a PR to fix this issue, but it would be nice to get this merged first: https://github.com/okta/okta-oidc-js/pull/772 (although the fix doesn't technically doesn't depend on it)

In the meantime, just create a SecureRoute component, with the following code:

import React from 'react'
import { Route } from 'react-router-dom'
import { useOktaAuth } from '@okta/okta-react'

const SecureRoute = (props) => {
  const { authService, authState } = useOktaAuth()

  if (!authState.isAuthenticated) {
    if (!authState.isPending) {
      let fromUri = authService.getFromUri() // <--- this is the fix
      // optionally, you could add extra checks to change the redirect post-auth here
      authService.login(fromUri)
    }
    return null
  }

  return <Route {...props} />
}

export default SecureRoute
license2e commented 4 years ago

The bug: https://github.com/okta/okta-oidc-js/blob/master/packages/okta-react/src/SecureRoute.js#L23

Current code:

const fromUri = history.createHref(history.location);

Should be:

const fromUri = authService.getFromUri();
swiftone commented 4 years ago

@license2e - thanks for the report. I'm pushing to raise the priority on the rendering issue, so I'll try to look at this one as well.

Have signed our CLA per https://github.com/okta/okta-oidc-js/blob/master/CONTRIBUTING.md ?