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

feat(ui): persist oidc auth details #10389

Merged
merged 10 commits into from
Mar 13, 2023

Conversation

jayeshchoudhary
Copy link
Contributor

@jayeshchoudhary jayeshchoudhary commented Mar 7, 2023

This PR is ui feature

Issue

  • Users need to log in every time the app is refreshed because the authentication token is stored locally in the app, making it inaccessible after a refresh.
  • The app currently redirects users to the home page after login, rather than to the specific page they intended to visit before authentication.

Solution

  • UI should persist your authentication details in the browser session. This means that even if you refresh the app, you will still be logged in until the authentication session expires.
  • Redirect to the appropriate location after login, so you will be taken directly to the page you intended to visit before authentication.

Note

The solution is only implemented for OIDC authentication. Other auth methods will work as before.

Reviewers - @apucher @joshigaurava @mayankshriv

@jayeshchoudhary jayeshchoudhary marked this pull request as draft March 7, 2023 15:18
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2023

Codecov Report

Merging #10389 (427ea99) into master (d475712) will decrease coverage by 50.39%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master   #10389       +/-   ##
=============================================
- Coverage     64.25%   13.87%   -50.39%     
+ Complexity     6059      259     -5800     
=============================================
  Files          1997     1997               
  Lines        108768   108768               
  Branches      16620    16620               
=============================================
- Hits          69888    15090    -54798     
- Misses        33816    92452    +58636     
+ Partials       5064     1226     -3838     
Flag Coverage Δ
unittests1 ?
unittests2 13.87% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1355 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jayeshchoudhary jayeshchoudhary marked this pull request as ready for review March 9, 2023 14:46
@jayeshchoudhary jayeshchoudhary changed the title feat(ui): persist auth details feat(ui): persist oidc auth details Mar 9, 2023
@Jackie-Jiang Jackie-Jiang added ui UI related issue feature labels Mar 9, 2023
@Jackie-Jiang
Copy link
Contributor

cc @joshigaurava @apucher

const { authenticated, authWorkflow } = useAuthProvider();
const history = useHistory();

React.useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor and not a blocker: can you pleease fix the import so that you can use just useEffect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@joshigaurava
Copy link
Contributor

Looks good.

@Jackie-Jiang Jackie-Jiang merged commit 3bad67d into apache:master Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ui UI related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants