-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Redirect query param #2527
Redirect query param #2527
Conversation
@@ -24,7 +26,7 @@ export default function LoginWithTokenComponent(): JSX.Element { | |||
); | |||
|
|||
if (cookies.sessionid && cookies.csrftoken) { | |||
return <Redirect to='/tasks' />; | |||
return <Redirect to={redirectParam || '/tasks'} />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redirection works fine at first time.
Unfortunately when user is already loged in {cookies are set} It does not redirect to requested page any more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @tdowgiel
Please, try another implementation according to updated PR description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, Thanks for the update.
Tested on Chrome 87 - macOS (Big Sur) and Windows 10
Unfortunately issue mentioned before still persists:
Steps to reproduce:
1.
Action:
Login to CVAT using ?nexto fo example http://auth/login-with-token/aj52qlbiux7manf8xwkz9etz1ivg8atk/nR9CeVzrL0AZMh8ZuAEzBr6swCeWUVbWT6KfScBbNuCMUW4dU1tU50tKb7ofUp9B?next=/tasks/1
Expecteted and Actual result:
CVAT is logged and page is redirected to /tasks/1
2.
Action:
Open new browser tab and past link once again:
Expected result:
CVAT is logged and page is redirected to /tasks/1
Actual result:
CVAT is logged but page is redirected to /tasks
Regards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tdowgiel
I thought you told about login with UI elements (twice or more) with the redirection query parameter. Anyway, looks like we fixed also login with a token.
16a0293
to
6a779a7
Compare
Motivation and context
This PR provide redirect parameter for auth page for redirecting after sucessfull login.
How has this been tested?
Manually
Test scenarios
/tasks/<task_id>
it will redirect to/auth/login?next=/tasks/<task_id>
Using token login add parameter next manually:
/login-with-token/:sessionId/:csrfToken?next=/tasks/<task_id>
Checklist
develop
branch- [ ] I have updated the documentation accordingly- [ ] I have added tests to cover my changes- [ ] I have linked related issues (read github docs)cvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.