-
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
Allow to login into UI with token from api/v1/auth/login #2234
Conversation
Hi, @tdowgiel |
Thanks for asking, I’m in middle of testing it. Probably it won’t change a lot. I will give you sign when it will be fully tested.
|
Okay, nevertheless, let me comment some code-style issues and ideas about this PR. |
Also could you please increase minor version of |
# Conflicts: # CHANGELOG.md # cvat-ui/src/components/cvat-app.tsx
|
||
if ( cookies['sessionid'] && cookies['csrftoken']) { | ||
window.location.reload(); | ||
<Redirect to="/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.
I suppose redirect after reload is useless. Moreover it should be wrapped into return ();
Maybe something like window.location = window.location.origin;
would be more suitable
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.
For some reason. I need to reload window to apply this changes. However, i have fixed issue with endless reloading when wrong token, by moving this to useEffect
Hi, @tdowgiel, I hope you are doing well. I tried this PR and it doesn't seem to be a working solution. Also POST request to |
…on with wrong tokens
I have tested it in many ways. Response used in cvat-core is not only returning key but also setting cookie because of headers. Please look on example response and set-cookie header:
This is scenario:
|
import { Redirect, useParams } from 'react-router'; | ||
import { useCookies } from 'react-cookie'; | ||
|
||
export default function LoginWithTokenComponent(){ |
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.
Please, add return type for the function JSX.Element
cvat-ui/src/components/cvat-app.tsx
Outdated
@@ -10,6 +10,7 @@ import notification from 'antd/lib/notification'; | |||
import Spin from 'antd/lib/spin'; | |||
import Text from 'antd/lib/typography/Text'; | |||
import GlobalErrorBoundary from 'components/global-error-boundary/global-error-boundary'; | |||
import LoginWithTokenComponent from './login-with-token/login-with-token'; |
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.
Please, use absolute import: components/login-with-token/login-with-token
, or put the line after all absolute (linter says that relative import should occur after absolute)
# Conflicts: # CHANGELOG.md # cvat-ui/package-lock.json # cvat-ui/package.json
That's amazing, thanks for your patience and for the contribution! |
Motivation and context
Allow to automatically log into CVAT-UI by 3rd part apps on the same cluster
How has this been tested?
Checklist
develop
branchcvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.