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.

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

Add check to gracefully handle failed login attempts #1535

Merged
merged 6 commits into from
Sep 20, 2019

Conversation

jlowin
Copy link
Member

@jlowin jlowin commented Sep 19, 2019

Thanks for contributing to Prefect!

Please describe your work and make sure your PR:

  • adds new tests (if appropriate)
  • updates CHANGELOG.md (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

Note that your PR will not be reviewed unless all three boxes are checked.

What does this PR change?

If users attempt to instantiate a Client with an invalid token and active tenant stored, the Client will (correctly) raise an error. However, because the error is raised during initialization it will not be possible to load the client object fully. This PR intercepts AuthorizationErrors when the Client is instantiated and clears the saved tenant id (but not the token, in case it is still valid).

The use case:

  • User logs in to a tenant with a token and saves those settings
  • User is removed from the tenant, but the token remains valid
  • User loads a new Client object; it should gracefully handle the invalid login attempt

Why is this PR important?

@codecov
Copy link

codecov bot commented Sep 19, 2019

Codecov Report

Merging #1535 into master will increase coverage by <.01%.
The diff coverage is 100%.

joshmeek
joshmeek previously approved these changes Sep 20, 2019
Copy link

@joshmeek joshmeek left a comment

Choose a reason for hiding this comment

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

The changelog needs to be updated to move this into the latest release, but we could do that in a later PR if desired

@joshmeek joshmeek dismissed their stale review September 20, 2019 19:35

didn't realize there were conflicts

@jlowin
Copy link
Member Author

jlowin commented Sep 20, 2019

I've got a merge conflict anyway

@joshmeek joshmeek merged commit 50fb513 into master Sep 20, 2019
@joshmeek joshmeek deleted the handle-failed-login branch September 20, 2019 20:00
zanieb pushed a commit that referenced this pull request Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants