Skip to content
This repository has been archived by the owner on Oct 24, 2024. It is now read-only.

Security Component Bug #921

Open
2 of 8 tasks
abodnar63 opened this issue Sep 27, 2020 · 5 comments
Open
2 of 8 tasks

Security Component Bug #921

abodnar63 opened this issue Sep 27, 2020 · 5 comments

Comments

@abodnar63
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

I've noticed that the Security component creates a new instance of AuthService after every rendering. Potentially it could cause a lot of different concurrency issues but right now I found issue in token renew functionality. If the application renders multiple times the Security component than during token renew it will send multiple requests to the token endpoint:
Screenshot 2020-09-27 at 20 27 34
Screenshot 2020-09-27 at 20 33 13

I see that you tried to prevent such behaviour with useMemo but the problem is that useMemo compares dependencies by referential equality. When application renders the component it gets a new object of the properties. Props could have the same attributes but for every render, it is a new Object. Like {} === {} returns false.

Expected behavior

I would expect to have one instance of AuthService to prevent multiple calls to API. I think there a few ways to fix it. One of them is to use a Singletone pattern for service creation and do a deep check of props before creating the new instance.

Minimal reproduction of the problem with instructions

To reproduce the issue with token auto-renew you can just get this sample https://github.com/okta/samples-js-react/tree/master/okta-hosted-login. You need to render https://github.com/okta/samples-js-react/blob/master/okta-hosted-login/src/App.jsx for multiple times so you can use a timer or add a button which will change the state of the component after every click
Screenshot 2020-09-27 at 21 16 30
In config enable autoRenew:

tokenManager: {
      storage: 'sessionStorage',
      autoRenew: true
 }

If you don't want to wait one hour for session expiration you can change manually value of expiresAt for idToken and accessToken in session storage okta-token-storage and refresh the page to read new values by application. For calculating expiresAt you can use this code: Math.round((new Date().getTime() + 1000 * 60 * 2 )/ 1000) It returns expiresAt value for 2 minutes later.
Render the component multiple times after refreshing by clicking the button and you will see multiple requests for token renew after one minute and a half.

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

Temporary workaround for dealing with this issue is to prevent multiple rendering of Security component.

Environment

  • Package Version: "@okta/okta-react": "3.0.7",
  • Browser: Chrome
  • OS: macOS Catalina
  • Node version (node -v): v12.18.3
  • Other:
@aarongranick-okta
Copy link
Contributor

@abodnar63 Thank you for submitting this issue and helpful information for reproducing the problem. We will investigate this issue and develop a fix. In the meantime, as a workaround, it may work to create an instance of the authService yourself and pass it to the Security component as described here: https://github.com/okta/okta-oidc-js/tree/master/packages/okta-react#alternate-configuration-using-authservice-instance

@aarongranick-okta
Copy link
Contributor

internal ref: OKTA-334529

@shuowu
Copy link
Contributor

shuowu commented Oct 2, 2020

@abodnar63 Generally, we suggest you wrap your <App> component inside the <Security> component, so the <Security> component does not implicitly create AuthService instance with App Comp state change.
Or, you can also try to explicitly provide the authService as suggested in #921 (comment)

@abodnar63
Copy link
Author

@shuowu, yes in this example I provided only a simple way to reproducing this issue.

In my project, it is not possible to wrap <App> component by <Security> because the Okta authentication flow is not on top of the application startup flow and it should first get the correct config for Okta. I fixed this issue by using useMemo hook for preventing multiple rendering of <Security> component.

@maheshwariG
Copy link

Hi all, I have a requirement where we have two CTA one for internal login and one for partner login. For internal user we have different client I'd and issuer and for partner we have different client I'd and login. We having spa react. Please let me know how to handle this.

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

4 participants