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.

Sign up for GitHub

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

Jump to bottom

Fix minor bug in the login workflow #2144

Merged
merged 1 commit into from
Mar 27, 2021
Merged

Fix minor bug in the login workflow #2144

merged 1 commit into from
Mar 27, 2021

Conversation

npalaska
Copy link
Member

Right now, if the authorization header is present during the login request (An attempt at creating another login token) we check the generated auth token with the one in the header. This check is redundant since we already check for the integrityError on authorization tokens.
This PR:

  • Removes the redundant auth token check when Authorization header is present during login.
  • Unit test for an attempt at updating the auth_token externally and confirmed that auth_token can not be updated externally.

@npalaska npalaska added API Of and relating to application programming interfaces to services and functions Server labels Mar 18, 2021
This was linked to issues Mar 18, 2021
We need to restrict the external user from updating auth_tokens #2127
Closed
If the login creates a similar token we should just return the same token without an abort operation #2129
Closed
@npalaska npalaska force-pushed the login_workflow_fixes branch 2 times, most recently from c42fbda to e12844c Compare March 18, 2021 23:24
portante
portante requested changes Mar 19, 2021
View reviewed changes
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
lib/pbench/test/unit/server/test_user_auth.py Show resolved Hide resolved
webbnh
webbnh previously approved these changes Mar 19, 2021
View reviewed changes
dbutenhof
dbutenhof previously approved these changes Mar 22, 2021
View reviewed changes
lib/pbench/server/api/resources/users_api.py Show resolved Hide resolved
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
@portante portante added this to the v0.71 milestone Mar 23, 2021
portante
portante requested changes Mar 23, 2021
View reviewed changes
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
lib/pbench/server/api/resources/users_api.py Outdated Show resolved Hide resolved
@npalaska npalaska mentioned this pull request Mar 23, 2021
Closed
dbutenhof
dbutenhof previously approved these changes Mar 23, 2021
View reviewed changes
portante
portante requested changes Mar 25, 2021
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

Looks good, just needs a rebase.

@npalaska npalaska dismissed stale reviews from dbutenhof and webbnh via 37cc5fe March 26, 2021 17:30
dbutenhof
dbutenhof previously approved these changes Mar 26, 2021
View reviewed changes
webbnh
webbnh approved these changes Mar 26, 2021
View reviewed changes
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

I don't see any changes...was this just a rebase?

webbnh
webbnh previously approved these changes Mar 26, 2021
View reviewed changes
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

I don't see any changes...was this just a rebase?

@npalaska
Copy link
Member Author

yeah just a rebase

portante
portante requested changes Mar 26, 2021
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

One more rebase needed ...

@npalaska npalaska dismissed stale reviews from webbnh and dbutenhof via 29861a2 March 26, 2021 22:22
@portante portante added the bug label Mar 26, 2021
@portante portante merged commit cf66c53 into distributed-system-analysis:main Mar 27, 2021
@npalaska npalaska mentioned this pull request Apr 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@portante portante portante approved these changes

@webbnh webbnh webbnh approved these changes

@dbutenhof dbutenhof Awaiting requested review from dbutenhof

Assignees

@npalaska npalaska

Labels
API Of and relating to application programming interfaces to services and functions bug Server
Projects
None yet
Milestone
v0.71
4 participants
@npalaska @portante @webbnh @dbutenhof