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

Unable to pass {username} with slashes to Airflow REST API #39887

Closed
1 of 2 tasks
wilsonhooi86 opened this issue May 28, 2024 · 11 comments
Closed
1 of 2 tasks

Unable to pass {username} with slashes to Airflow REST API #39887

wilsonhooi86 opened this issue May 28, 2024 · 11 comments
Labels
area:API Airflow's REST/HTTP API area:core kind:bug This is a clearly a bug pending-response

Comments

@wilsonhooi86
Copy link

Apache Airflow version

Other Airflow 2 version (please specify below)

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

2.8.1

What happened?

MWAA has announced that we can now use Airflow REST API. The goal is to use REST API Patch User to add/remove users' roles programmatically.

MWAA create username with a prefix of assumed-role/ for each user login. Example {username}: assumed-role/user1 .

API Template URL: https://airflow.apache.org/auth/fab/v1/users/assumed_role/{username}
Actual URL: https://airflow.apache.org/auth/fab/v1/users/assumed_role/assumed-role/user1

As we call the Airflow REST API https://airflow.apache.org/auth/fab/v1/users/assumed_role/assumed-role/user1 , it will show below error:

{
"detail": "The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.",
"status": 404,
"title": "Not Found",
"type": "about:blank"
}

We believe the REST API render the slash as an URL path.

Question is, can we enhance the Airflow REST API to accept slashes in URL parameter?

What you think should happen instead?

Airflow REST API able to accept slashes in URL parameter. Example:
username: assumed-role/user1
https://airflow.apache.org/auth/fab/v1/users/assumed-role/user1

How to reproduce

Please refer to What Happened? section

Operating System

MWAA

Versions of Apache Airflow Providers

No response

Deployment

Amazon (AWS) MWAA

Deployment details

I am not sure that this issue is specific to the service provider.

Anything else?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@wilsonhooi86 wilsonhooi86 added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels May 28, 2024
Copy link

boring-cyborg bot commented May 28, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@RNHTTR RNHTTR added area:API Airflow's REST/HTTP API and removed needs-triage label for new issues that we didn't triage yet labels May 28, 2024
@Taragolis
Copy link
Contributor

Airflow REST API able to accept slashes in URL parameter. Example:

It shouldn't. / have a special meaning for separate paths in URI.
Instead of that it should work with url-encoded values, did you try to provide assumed-role%2Fuser1 instead?

Copy link

This issue has been automatically marked as stale because it has been open for 14 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jun 15, 2024
@wilsonhooi86
Copy link
Author

Airflow REST API able to accept slashes in URL parameter. Example:

It shouldn't. / have a special meaning for separate paths in URI. Instead of that it should work with url-encoded values, did you try to provide assumed-role%2Fuser1 instead?

Hi @Taragolis , yes, I tried to pass in as assumed-role%2Fuser1 but it still parse as "/" in the url and produce the same error as below:

{
"detail": "The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.",
"status": 404,
"title": "Not Found",
"type": "about:blank"
}

@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Jul 26, 2024
@jaklan
Copy link

jaklan commented Jul 26, 2024

@Taragolis any hints how to resolve that issue? Also encounter that problem on MWAA because of assumed-role/ prefix

@uranusjr
Copy link
Member

uranusjr commented Aug 2, 2024

IIRC this is a restriction in WSGI and shared by many Python web frameworks, including Flask (backing Airflow) pallets/flask#900

There’s probably no good way to resolve this from the webserver level.

@jaklan
Copy link

jaklan commented Aug 2, 2024

@uranusjr if usernames wish slashes are not supported by API (and won't be), they should be simply not supported at all then

@uranusjr
Copy link
Member

uranusjr commented Aug 7, 2024

I just checked and it seems like even ASGI frameworks have this issue too. ASGI has since added raw_path to support this, but FastAPI is not using it when routing requests. (More accurately, Starlette is the layer that actually handles routing.) So I think we’ll have to consider this as impossible at least for the foreseeable future.

I’ll close this and propose a PR to add a note in the documentation.

refs
fastapi/fastapi#7858
encode/uvicorn#261

@uranusjr uranusjr closed this as not planned Won't fix, can't repro, duplicate, stale Aug 7, 2024
@jaklan
Copy link

jaklan commented Aug 7, 2024

@uranusjr what about a proposal to disallow usernames with slashes instead of "note in the documentation"? If it's not compatible with Airflow REST API - I don't really see why it should be supported.

@uranusjr
Copy link
Member

uranusjr commented Aug 7, 2024

The problem is the username can be supplied from anywhere (the auth manager is pluggable), so we can’t block them at creation, only when they are used in Airflow. The username is also only unusable from the REST API. I anticipate many people are already using those usernames “just fine” from the Airflow UI, and disallowing them would break all their setups.

@jaklan
Copy link

jaklan commented Aug 7, 2024

@uranusjr sure, I'm aware of backward-compatibility issues, but maybe it would be rationale to deprecate it in Airflow 3.x?

The username is also only unusable from the REST API

I would say it's a 🚨, not "only" 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:core kind:bug This is a clearly a bug pending-response
Projects
None yet
Development

No branches or pull requests

5 participants