Skip to content

Conversation

@williamjallen
Copy link
Collaborator

Pages rendered by AngularJS which require authentication currently redirect to the index page after login. This is inconsistent with the behavior elsewhere, where the user is redirected back to the page they attempted to access prior to being redirected to the login page.

This PR is a temporary patch to make the login redirect behavior more consistent. The ultimate goal is to remove AngularJS entirely.

Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

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

@williamjallen, I've got the redirect to test but it seems to be dropping the URL query parameters and I'm not sure that's what is intended:

Steps to reproduce:
I have a private project called test, I can see the testOverview page when signed in at /testOverview.php?project=test
image

If I sign out and attempt to visit that URL, it shows me the login screen without changing the URL in the address bar:

image

and after logging in takes me back to testOverview.php but without any query parameters and I get an error message:

image

@williamjallen williamjallen force-pushed the angular-login-redirect branch from 7e8bbdb to 4c57616 Compare October 30, 2023 23:46
@williamjallen
Copy link
Collaborator Author

Good catch! The same issue also occurred on non-angular pages, so I fixed it in both locations.

Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

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

Much better! Thanks!!

@josephsnyder josephsnyder added this pull request to the merge queue Oct 31, 2023
Merged via the queue into Kitware:master with commit ecfa8b8 Oct 31, 2023
@williamjallen williamjallen deleted the angular-login-redirect branch October 31, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants