Skip to content

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
@npalaska npalaska force-pushed the login_workflow_fixes branch 2 times, most recently from c42fbda to e12844c Compare March 18, 2021 23:24
webbnh
webbnh previously approved these changes Mar 19, 2021
dbutenhof
dbutenhof previously approved these changes Mar 22, 2021
@portante portante added this to the v0.71 milestone Mar 23, 2021
dbutenhof
dbutenhof previously approved these changes Mar 23, 2021
portante
portante previously approved these changes Mar 24, 2021
@npalaska npalaska requested a review from dbutenhof March 24, 2021 12:38
@npalaska npalaska dismissed stale reviews from webbnh and dbutenhof via 38dea5f March 24, 2021 21:32
@npalaska npalaska force-pushed the login_workflow_fixes branch from c79c54c to 38dea5f Compare March 24, 2021 21:32
@npalaska npalaska requested review from dbutenhof and webbnh March 24, 2021 21:33
webbnh
webbnh previously approved these changes Mar 25, 2021
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.

GitHub didn't note anything changed in this latest round from the previous round, and neither did I. So, I've no reason not to approve.

However, I am left wondering whether maybe something you intended to commit didn't make it in (like a change to users_api.py line 397, for instance?).

dbutenhof
dbutenhof previously approved these changes Mar 25, 2021
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
@npalaska npalaska force-pushed the login_workflow_fixes branch from 38dea5f to 37cc5fe Compare March 26, 2021 17:30
dbutenhof
dbutenhof previously approved these changes Mar 26, 2021
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
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

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
@npalaska npalaska force-pushed the login_workflow_fixes branch from 37cc5fe to 29861a2 Compare March 26, 2021 22:22
@portante portante requested review from dbutenhof and webbnh March 26, 2021 22:29
@portante portante added the bug label Mar 26, 2021
@portante portante merged commit cf66c53 into distributed-system-analysis:main Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Of and relating to application programming interfaces to services and functions bug Server

Projects

None yet

4 participants