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

AIP-84 Authentications and Permissions #42360

Open
23 of 47 tasks
pierrejeambrun opened this issue Sep 20, 2024 · 38 comments
Open
23 of 47 tasks

AIP-84 Authentications and Permissions #42360

pierrejeambrun opened this issue Sep 20, 2024 · 38 comments
Assignees
Labels
AIP-84 Modern Rest API area:auth kind:meta High-level information important to the community

Comments

@pierrejeambrun
Copy link
Member

pierrejeambrun commented Sep 20, 2024

Meta issue for Authentications and Permissions for the API.

I Multiple PRs to add back permissions to all endpoints (the ones from the legacy API that were removed during the migration)

Public API

Private / UI API (<=> views.py) @Lee-W

Execution API

misc

II Additional Tasks

Default to SimpleAuthManager for development time of airflow 3 as long as FABAuthManager does not support JWT auth, this is the only way we can progress on this while #44849 is not completed

Committer

  • I acknowledge that I am a maintainer/committer of the Apache Airflow project.
@pierrejeambrun pierrejeambrun added kind:meta High-level information important to the community AIP-84 Modern Rest API labels Sep 20, 2024
@dosubot dosubot bot added the area:auth label Sep 20, 2024
@jason810496
Copy link
Contributor

Hi @pierrejeambrun, could you please assign this ticket to me? Thanks!

@pierrejeambrun
Copy link
Member Author

Hello @jason810496,

There has been some update on this one, it is not well scoped and depends on the work that Vincent is doing on the FABAuthManager and JWT backend. I do not recommend to take this one at this point. Removing the good first issue tag

@jason810496
Copy link
Contributor

Gotcha, I will take other tickets. Thanks !

@pierrejeambrun
Copy link
Member Author

Related PR #42634

@pierrejeambrun
Copy link
Member Author

cc: @vincbeck

@pierrejeambrun pierrejeambrun removed the kind:meta High-level information important to the community label Dec 12, 2024
@pierrejeambrun pierrejeambrun self-assigned this Feb 6, 2025
@pierrejeambrun
Copy link
Member Author

This is now unblocked since #44884 has been implemented. We can start implementing the permissions for different endpoints.

@pierrejeambrun
Copy link
Member Author

@jason810496 if you still have some interest for this let me know so I can assign you :)

@jason810496
Copy link
Contributor

@jason810496 if you still have some interest for this let me know so I can assign you :)

Sure! I can work on this issue, I will look into the context tomorrow then ask if I not sure about the details.

@pierrejeambrun
Copy link
Member Author

Great. Don't hesitate to do 1 PR per 'resource'. (for instance for DagRun endpoints, for taskinstance endpoints, connection enpoints etc...).

I think @rawwar will provide some help too so you can split the work easily. :)

@jason810496
Copy link
Contributor

jason810496 commented Feb 25, 2025

I will start with the Dag endpoint first and provide an update here before working on the other endpoints. I’ll also list all the necessary endpoints later.

Before proceeding further, I’d like to double-check something. After reviewing #44884, it seems that we might need a general middleware or a Depends function (in FastAPI) as an authentication layer to read the Bearer token from authorization request headers. This authentication layer should leverage the AuthManager initialized in the FastAPI app (accessible via request.app.state.auth_manager).
Somehow like action_logging in airflow.api_fastapi.logging.decorators

Update:
Continue finish airflow.api_fastapi.core_api.security and take airflow.api_connexion.security as reference.

@jason810496
Copy link
Contributor

I’ll also list all the necessary endpoints later.

Endpoints:

  • configuration
  • connection
  • dag
  • asset
  • pool
  • variable
  • view
  • custom_view

@pierrejeambrun
Copy link
Member Author

Thanks @jason810496 meta issue updated

@rawwar rawwar self-assigned this Feb 25, 2025
@rawwar
Copy link
Collaborator

rawwar commented Feb 25, 2025

@jason810496 , I'll pick asset, pool and variable. I'll work on them this week.

@jason810496
Copy link
Contributor

jason810496 commented Feb 25, 2025

@jason810496 , I'll pick asset, pool and variable. I'll work on them this week.

Then I'll take configuration and connection also, if I am fast enough I can take the rest also ! ( Not sure I have enough time currently )

@Lee-W
Copy link
Member

Lee-W commented Mar 7, 2025

I just updated the current status in the descpriont

@Lee-W
Copy link
Member

Lee-W commented Mar 7, 2025

@jason810496 @rawwar @vatsrahul1001 , are there any other known dependencies in public auths? It would be nice if we could list them so that we know what to do first 🙂 I'll take a look at the non-assigned ones later. as for the format, you can take a look at the UI part.

@rawwar
Copy link
Collaborator

rawwar commented Mar 7, 2025

@jason810496 @rawwar @vatsrahul1001 , are there any other known dependencies in public auths? It would be nice if we could list them so that we know what to do first 🙂 I'll take a look at the non-assigned ones later. as for the format, you can take a look at the UI part.

except asset_alias,dag_report my rest of the PR's depend on dags

@jason810496
Copy link
Contributor

jason810496 commented Mar 7, 2025

@jason810496 @rawwar @vatsrahul1001 , are there any other known dependencies in public auths? It would be nice if we could list them so that we know what to do first 🙂 I'll take a look at the non-assigned ones later. as for the format, you can take a look at the UI part.

Still working on fixing the Kubernetes tests. If not resolved today, I’ll continue over the weekend. I have idea of how to fix it but need to test it locally. ( Adapt #47460 idea to k8s system test )

@rawwar
Copy link
Collaborator

rawwar commented Mar 8, 2025

@pierrejeambrun, dag report is a new endpoint in AF3. What permissions should a user need to access it?

Should this be a separate permission or should it be a sub-entity of dags?

@vatsrahul1001
Copy link
Collaborator

@pierrejeambrun, I see that the dag_tags and tasks endpoints are unassigned. I can pick them up.

@Lee-W
Copy link
Member

Lee-W commented Mar 10, 2025

@vatsrahul1001 just assigned and updated the list. Thanks!

@jason810496
Copy link
Contributor

The DAG-related blocker #47433 has just been resolved!
Entities that depend on DAGs can now rebase to the latest main branch and leverage ReadableDagsFilterDep and EditableDagsFilterDep from the airflow.api_fastapi.core_api.security module to obtain the get_permitted_dag_ids filter.

@rawwar
Copy link
Collaborator

rawwar commented Mar 11, 2025

The DAG-related blocker #47433 has just been resolved!

Finally

Great job @jason810496 , @Lee-W

@Lee-W
Copy link
Member

Lee-W commented Mar 12, 2025

@pierrejeambrun do we need auth for execution/health?

@rawwar
Copy link
Collaborator

rawwar commented Mar 12, 2025

@pierrejeambrun do we need auth for execution/health?

I don't think we should have auth for health endpoints, as monitoring tools can use them. Maybe we should have been calling it /monitor to be consistent with others?

@Lee-W
Copy link
Member

Lee-W commented Mar 12, 2025

Yep, I don't think we should have auth either. as for whether to call it /monitor, I think /health is quite commonly used and probably shouldn't be changed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-84 Modern Rest API area:auth kind:meta High-level information important to the community
Projects
Status: In Progress
Development

No branches or pull requests

7 participants