You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Aaryan123456679 (via GitHub)" <gi...@apache.org> on 2026/07/22 15:04:40 UTC
[PR] Add Reset Password button to Edit User view [airflow]
Aaryan123456679 opened a new pull request, #70243:
URL: https://github.com/apache/airflow/pull/70243
The Show User view already surfaces a Reset Password action button (via FAB's show-widget actions block), but the Edit User view has no equivalent, forcing admins to navigate back to Show User just to reset a password.
This wires the same, already-registered `resetpasswords` action into the Edit User page:
- `CustomUserDBModelView` gets a dedicated `edit_widget` (`UserEditFormWidget`) and overrides `edit()` to inject the `resetpasswords` action, `pk`, and `modelview_name` into the widget's template context, mirroring the existing `show()` override pattern in the same file.
- A new template, `appbuilder/general/widgets/user_edit_form.html`, extends FAB's built-in `form_vertical.html` with the same `render_action_links` block already used by `show.html`. No new routes or permissions were introduced — the fix reuses the existing `resetpasswords` action, route, and permission mapping.
closes: #37030
##### Test plan
- [x] Added `test_user_edit_view_shows_reset_password_action_with_access` — asserts the Reset Password link renders on the Edit User page for a user with read/edit access to Users and read access to Passwords.
- [x] Added `test_user_edit_view_hides_reset_password_action_without_access` — asserts the link is absent when the user lacks read access to Users.
- [x] Ran the full existing test file (`test_views_custom_user_views.py`, 28 tests) plus the broader `providers/fab/tests/unit/fab/www/views/` and `providers/fab/tests/unit/fab/auth_manager/views/` suites (46 tests total) — all passed, no regressions.
- [x] `prek run --from-ref upstream/main --stage pre-commit` and `--stage manual` — passed.
- [x] `breeze run mypy providers/fab/src/airflow/providers/fab/auth_manager/views/user.py` — passed.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Sonnet 5)
Generated-by: Claude Code (Sonnet 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "Aaryan123456679 (via GitHub)" <gi...@apache.org>.
Aaryan123456679 commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5697316428
Hi Team, kindly look into this PR. Friendly Ping
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #70243:
URL: https://github.com/apache/airflow/pull/70243
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5151796788
The feature makes sense — having to bounce back to Show User just to reset a password is a genuine annoyance — and I checked the part that matters most here: there is no permission bypass. `lib.render_action_links` applies its own filter as the first thing it does:
```jinja
{% macro render_action_links(actions, pk, modelview_name) %}
{% set actions = actions | get_actions_on_show(modelview_name) %}
```
so injecting the action into `template_args` cannot surface a button the user is not entitled to, and the action endpoint is separately protected. Your tests cover both directions, which is the right shape for this.
I'd like to suggest a different approach to the template, though, because copying `form_vertical.html` brings a problem with it.
`render_action_links` emits its own `<form id="action_form" ...>` (see `appbuilder/general/lib.html`). In the copied template the call sits inside the model `<form class="form-vertical">`, so the rendered page has a form nested inside another form. That is invalid HTML — browsers drop the inner element — so the button may work only incidentally, and could stop working on a browser or FAB version that handles it differently. FAB's own `show.html` does not hit this because the Show view has no surrounding form, so the pattern does not carry over as directly as it looks.
You are right that `form_vertical.html` cannot be extended — it defines no blocks, which I assume is exactly why you copied it. But the *page* template can be: `appbuilder/general/model/edit.html` wraps the widget in `{% block edit_form %}`. Overriding that block puts the actions after the form rather than inside it:
```jinja
{% extends "appbuilder/general/model/edit.html" %}
{% import 'appbuilder/general/lib.html' as lib %}
{% block edit_form %}
{{ super() }}
{% if actions %}
<div class="well well-sm">
{{ lib.render_action_links(actions, pk, modelview_name) }}
</div>
{% endif %}
{% endblock %}
```
and the view can pass the values straight to `render_template` rather than reaching into widget internals:
```python
edit_template = "appbuilder/general/model/user_edit.html"
@expose("/edit/<pk>", methods=["GET", "POST"])
@has_access
def edit(self, pk):
pk = self._deserialize_pk_if_composite(pk)
widgets = self._edit(pk)
if not widgets:
return self.post_edit_redirect()
return self.render_template(
self.edit_template,
title=self.edit_title,
widgets=widgets,
related_views=self._related_views,
actions={"resetpasswords": self.actions.get("resetpasswords")},
pk=pk,
modelview_name=self.__class__.__name__,
)
```
That drops the ~45 lines of duplicated form markup, removes the need for `UserEditFormWidget` altogether, and means any future FAB change to `form_vertical.html` is picked up automatically instead of silently drifting from our copy. The permission filtering is unchanged, since it is the same macro doing the work.
Two smaller notes:
The description says the new template "extends FAB's built-in `form_vertical.html`" — it is a copy rather than an `extends`. Worth correcting before merge, since the body becomes the commit message and the maintenance implications of the two are quite different.
"Mirroring the existing `show()` override pattern" is also a slightly generous reading: `show()` *removes* entries FAB has already populated, whereas this *adds* one FAB never populates. The result is safe because of the macro-level filtering, but they aren't the same manoeuvre, and it's worth knowing the safety comes from the macro rather than from symmetry with `show()`.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5690156255
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #70243:
URL: https://github.com/apache/airflow/pull/70243#discussion_r4064703442
##########
providers/fab/tests/unit/fab/www/views/test_views_custom_user_views.py:
##########
@@ -141,6 +141,49 @@ def test_user_model_view_with_access(self, url, permission, expected_text, app,
response = client.get(url.replace("{user.id}", str(user_with_access.id)), follow_redirects=True)
check_content_in_response(expected_text, response)
+ def test_user_edit_view_shows_reset_password_action_with_access(self, app, client):
+ # Visibility of the action link is gated on "read" access to Users (the same
+ # rule the Show User view relies on); "read" on Passwords is what's required to
+ # actually perform the reset once the link is followed.
+ user_with_access = create_user(
+ app,
+ username="has_access",
+ role_name="role_has_access",
+ permissions=[
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_USER),
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_USER),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_PASSWORD),
Review Comment:
These two tests vary two permissions at once: the positive case has `CAN_READ` on both **Users** and **Passwords**, the negative has neither. So the pair is equally consistent with "gated on Users read" and "gated on Passwords read" — it can't actually pin the distinction your comment on line 145 draws.
For what it's worth, I traced it and **your comment is correct**: `render_action_links` → `get_actions_on_show` → `is_item_visible("resetpasswords", …)`, which resolves `get_action_permission_name("resetpasswords")` through `method_permission_name` to `can_read`, and checks it against `class_permission_name`. On the edit endpoint that property (`user.py:61-70`) falls through `class_permission_name_mapping` to `RESOURCE_USER`; only on the `action` endpoint does it map to `RESOURCE_PASSWORD`. So visibility really is Users-read and execution really is Passwords-read.
The case that would pin it is the one that's missing — `CAN_READ` on Users *without* `CAN_READ` on Passwords. The link renders and the action 403s. That's pre-existing behaviour you've inherited from the Show view rather than anything you've introduced, but on an auth surface it's the seam most worth having a test say out loud.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "Aaryan123456679 (via GitHub)" <gi...@apache.org>.
Aaryan123456679 commented on code in PR #70243:
URL: https://github.com/apache/airflow/pull/70243#discussion_r4064963606
##########
providers/fab/tests/unit/fab/www/views/test_views_custom_user_views.py:
##########
@@ -141,6 +141,49 @@ def test_user_model_view_with_access(self, url, permission, expected_text, app,
response = client.get(url.replace("{user.id}", str(user_with_access.id)), follow_redirects=True)
check_content_in_response(expected_text, response)
+ def test_user_edit_view_shows_reset_password_action_with_access(self, app, client):
+ # Visibility of the action link is gated on "read" access to Users (the same
+ # rule the Show User view relies on); "read" on Passwords is what's required to
+ # actually perform the reset once the link is followed.
+ user_with_access = create_user(
+ app,
+ username="has_access",
+ role_name="role_has_access",
+ permissions=[
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_USER),
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_USER),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_PASSWORD),
Review Comment:
Added test_user_edit_view_shows_reset_password_action_without_passwords_read_access (b2a25d6210). The user has read and edit on Users but no read on Passwords. The test asserts the "Reset Password" link renders, and that POSTing the action, which is what the button submits, does not reach the reset form.
I also extended the existing with-access test so the same POST does redirect to /resetpassword/form. That way the denial check can't pass just because the POST is broken.
While writing it I found that a GET on /users/action/resetpasswords/<id> is refused for CSRF reasons before any permission check runs, so a GET-based check wouldn't have exercised the Passwords-read gate at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "Aaryan123456679 (via GitHub)" <gi...@apache.org>.
Aaryan123456679 commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5765160696
@potiuk the follow-ups are done:
The missing test case is in b2a25d6210: Users read without Passwords read shows the link, and following it is refused. The with-access test also asserts the same POST reaches the reset form.
Before/after screenshots of the Edit User page are in the PR description.
I replied on the inline thread and resolved it.
Ready for another look whenever you have time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "Aaryan123456679 (via GitHub)" <gi...@apache.org>.
Aaryan123456679 commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5153138662
Pushed a follow-up commit that switches the Edit User view to override `edit.html`'s `edit_form` block instead of copying `form_vertical.html`, so the Reset Password action now renders outside the model's `<form>` — no more nested forms. `CustomUserDBModelView.edit()` now passes `actions`/`pk`/`modelview_name` directly to `render_template` instead of reaching into widget internals, and the `UserEditFormWidget` subclass is gone.
Also corrected the PR description: the new template extends FAB's `edit.html` (it's a genuine override now, not a copy), and I reworded the "mirrors `show()`" note — `show()` removes entries FAB already populated, this adds one FAB never populates, and the safety here comes from `render_action_links`' own permission filter rather than symmetry with `show()`.
Existing tests (`test_user_edit_view_shows_reset_password_action_with_access`, `test_user_edit_view_hides_reset_password_action_without_access`) needed no changes since they assert on rendered output, not widget/template internals — reran the full file (28 tests) and it passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5714690129
Any chance you can attached a screenshot to the description so that we can see the final result?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Reset Password button to Edit User view [airflow]
Posted by "Aaryan123456679 (via GitHub)" <gi...@apache.org>.
Aaryan123456679 commented on PR #70243:
URL: https://github.com/apache/airflow/pull/70243#issuecomment-5765143559
<img width="2200" height="1854" alt="pr70243-after-edit-user" src="https://github.com/user-attachments/assets/014f173b-0b96-4bd4-bc5e-60dcfe5463a5" />
After
<img width="2200" height="1800" alt="pr70243-before-edit-user" src="https://github.com/user-attachments/assets/b47f7288-c410-4f39-9edc-fe75af20769d" />
Before
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org