Skip to content

Comments

Fix use correct PATCH permission for /users/{username} endpoint#61462

Merged
vincbeck merged 1 commit intoapache:mainfrom
henry3260:fix-fab-auth-patch-permission-users-endpoint
Feb 5, 2026
Merged

Fix use correct PATCH permission for /users/{username} endpoint#61462
vincbeck merged 1 commit intoapache:mainfrom
henry3260:fix-fab-auth-patch-permission-users-endpoint

Conversation

@henry3260
Copy link
Contributor

@henry3260 henry3260 commented Feb 4, 2026

What

This PR updates the permission check for the PATCH /roles/{user} endpoint in the FAB Auth Manager FastAPI routes.
Specifically, it changes the permission check from PATCH to PUT in the requires_fab_custom_view dependency.

Why

Previously, the endpoint was using requires_fab_custom_view("PATCH", ...) for a PATCH route, which caused a mismatch between the HTTP method and the required permission.
Aligning the permission check with the actual HTTP method ensures correct and predictable access control.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

That's expected, in Airflow auth model we use PUT to any modification operation. Which is the case here. I dont see any benefit of using PATCH

@henry3260
Copy link
Contributor Author

henry3260 commented Feb 4, 2026

That's expected, in Airflow auth model we use PUT to any modification operation. Which is the case here. I dont see any benefit of using PATCH

I noticed that the /roles/{name} endpoint uses PATCH in the permission check, so I want to update the /users/{username} endpoint to also use PATCH for consistency. wdyt?

or update /roles/{name} to use PUT instead?

@vincbeck
Copy link
Contributor

vincbeck commented Feb 4, 2026

That's expected, in Airflow auth model we use PUT to any modification operation. Which is the case here. I dont see any benefit of using PATCH

I noticed that the /roles/{name} endpoint uses PATCH in the permission check, so I want to update the /users/{username} endpoint to also use PATCH for consistency. wdyt?

or update /roles/{name} to use PUT instead?

Good catch, it should be PUT, can you please update it?

@henry3260 henry3260 force-pushed the fix-fab-auth-patch-permission-users-endpoint branch from 531ace8 to b7a880a Compare February 5, 2026 06:51
@henry3260 henry3260 force-pushed the fix-fab-auth-patch-permission-users-endpoint branch from b7a880a to 3f7506a Compare February 5, 2026 07:21
@vincbeck vincbeck merged commit 1327aaa into apache:main Feb 5, 2026
86 checks passed
@henry3260 henry3260 deleted the fix-fab-auth-patch-permission-users-endpoint branch February 5, 2026 15:18
jhgoebbert pushed a commit to jhgoebbert/airflow_Owen-CH-Leung that referenced this pull request Feb 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants