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

Logging out from Web UI still raises Airflow 405 error #45116

Open
2 tasks done
punx120 opened this issue Dec 20, 2024 · 6 comments
Open
2 tasks done

Logging out from Web UI still raises Airflow 405 error #45116

punx120 opened this issue Dec 20, 2024 · 6 comments
Assignees
Labels
area:core area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet

Comments

@punx120
Copy link
Contributor

punx120 commented Dec 20, 2024

Apache Airflow version

2.10.4

If "Other Airflow 2 version" selected, which one?

No response

What happened?

I still have the same error as described in #40470 even though i'm running 2.10.4 with apache-airflow-providers-fab 1.4.0. It's probably linked to some package - but i did ran
pip install "apache-airflow==2.10.4" --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-2.10.4/constraints-3.8.txt"

What you think should happen instead?

logout process needs to complete correctly

How to reproduce

connected as admin and try to logout

Operating System

Fedora 8.3

Versions of Apache Airflow Providers

apache-airflow-providers-fab==1.4.0

Deployment

Official Apache Airflow Helm Chart

Deployment details

No response

Anything else?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@punx120 punx120 added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Dec 20, 2024
@dosubot dosubot bot added the area:UI Related to UI/UX. For Frontend Developers. label Dec 20, 2024
@Spaarsh
Copy link

Spaarsh commented Dec 25, 2024

Hey! I would like to work on this issue!

@Spaarsh
Copy link

Spaarsh commented Dec 26, 2024

(I have already posted a comment under the referenced issue and am posting the same comment here as well.)

It took me a while to triage this issue. I went through the main-branch code as well as the last released source code of airflow (2.10.4). I think I may have a way of solving this. Please correct me if I am wrong!

Triaging

I thoroughly went through the entirety of the auth-related code (to the best of my ability). There is no wrong method being called. But since the FAB requires us to not allow any GET requests at the /logout API endpoint, the HTML can't be rendered when a user manually enters the URL in the browser point.

Before moving further though, I wanted to ensure that the /logout endpoint does work for POST requests. There is one instance of a JS code sending a POST request to the /logout endpoint via the no_roles_permissions.html, where there is a logout button. In order to test this, I created a user with no roles using the command:

airflow users create \
    --username no_roles_user \
    --firstname No \
    --lastname Roles \
    --email [email protected] \
    --password your_password \
    --role Public

When I then went to the /home page, the expected page showed up:
image

When I clicked on the "logout" button, I was successfully able to log out, indicating no fault at the endpoint itself.

Solution

Hence my solution is as follows:
We can create a new endpoint such as /logout_page which renders an HTML which has a logout button. The logout button has a JS event handler that sends the POST request to the /logout endpoint, resulting in successful logout using GUI.

This way, the GET request doesn't happen on our /logout endpoint itself (thus not violating any FAB requirements) while also enabling a GUI-based logout action.

If this is the correct approach, I am willing to open a PR.

@shahar1
Copy link
Contributor

shahar1 commented Dec 26, 2024

@Spaarsh well analyzed, solution seems solid.
Feel free to tag me in your PR for review

@Spaarsh
Copy link

Spaarsh commented Dec 26, 2024

@shahar1 Sure! I had a question though. Should I add the logout button on some existing html page? Or provide a hyperlink on some existing page?

@shahar1
Copy link
Contributor

shahar1 commented Dec 26, 2024

@shahar1 Sure! I had a question though. Should I add the logout button on some existing html page? Or provide a hyperlink on some existing page?

Just try to make it as much as trivial to the user, as logging out should be a simple action. If you encounter any limitations, we could discuss them in the PR.

Spaarsh added a commit to Spaarsh/airflow that referenced this issue Dec 27, 2024
@Spaarsh
Copy link

Spaarsh commented Dec 28, 2024

@punx120 could you please tell us the steps to recreate this error? That will greatly help in fixing it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants