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

Infinite UI redirection loop after deactivating an active user #35486

Merged
merged 10 commits into from
Nov 7, 2023

Conversation

ivan-afonichkin
Copy link
Contributor

This PR fixes an infinite redirection loop to /logout when deactivating logged in user.

closes: #35335

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Nov 6, 2023
@potiuk potiuk added this to the Airflow 2.7.4 milestone Nov 6, 2023
@potiuk
Copy link
Member

potiuk commented Nov 6, 2023

Would be great to get a unit test for that one - to avoid regression

@ivan-afonichkin
Copy link
Contributor Author

@potiuk I was just working on it :)

@potiuk
Copy link
Member

potiuk commented Nov 6, 2023

Some static checks (looks like space). I do recommend to install pre-commit (as usual :)) - then it will automatically fix things for you at commit time.

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

The goal of the unit tests is:

  1. testing if our code works as expected
  2. avoiding breaking the functionality of the code in the future

So, when you want to write a new test for a bug fix, the test should fail without the fix and pass after implementing it, which is not the case here.

tests/www/views/test_session.py Outdated Show resolved Hide resolved
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Nov 7, 2023
Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

LGTM

@potiuk potiuk merged commit e512a72 into apache:main Nov 7, 2023
46 checks passed
@eladkal
Copy link
Contributor

eladkal commented Nov 8, 2023

BTW same issue happens if user try to login without permission on airflow homepage.
It is resulted in endless redirect from homepage to login and then from login to home.

We probably need to handle this case as well.

@potiuk
Copy link
Member

potiuk commented Nov 9, 2023

BTW same issue happens if user try to login without permission on airflow homepage. It is resulted in endless redirect from homepage to login and then from login to home.

We probably need to handle this case as well.

No permissions? You mean a real users that has no roles assigned at all ?

romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
…e#35486)

* Infinite UI redirection loop after deactivating an active user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite UI redirection loop when user is changed to "inactive" while having a session opened
5 participants