Skip to content

Escape user.username in flash banners for admin password-reset via UI #52419

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 28, 2025
Merged

Escape user.username in flash banners for admin password-reset via UI #52419

merged 1 commit into from
Jun 28, 2025
+5 −3

Conversation

SCH227
Copy link
Contributor

@SCH227 SCH227 commented Jun 28, 2025

In FabAirflowSecurityManagerOverride.reset_user_sessions() two warning strings are built with an f-string that embeds user.username. Each string is passed to _cli_safe_flash(), which does flash(Markup(text), level). Markup() tells Flask not to escape anything.

airflow/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py

Lines 549 to 552 in 262cd81

if num_sessions > MAX_NUM_DATABASE_USER_SESSIONS:
self._cli_safe_flash(
f"The old sessions for user {user.username} have <b>NOT</b> been deleted!<br>"
f"You have a lot ({num_sessions}) of user sessions in the 'SESSIONS' table in "

airflow/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py

Lines 2384 to 2387 in 262cd81

def _cli_safe_flash(text: str, level: str) -> None:
"""Show a flash in a web context or prints a message if not."""
if has_request_context():
flash(Markup(text), level)

The code changes aim to prevent a theoretical XSS attack vector, even though, as pointed out by Airflow Security team, Airflow users have no influence on the username

@SCH227
Sanitize Username
3714c18 
Escape user.username in flash banners to prevent potential HTML injection
@SCH227 SCH227 requested a review from vincbeck as a code owner June 28, 2025 18:12
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Jun 28, 2025

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: [email protected]
    Slack: https://s.apache.org/airflow-slack

@potiuk
Copy link
Member

potiuk commented Jun 28, 2025

NICE ! Very cool

@potiuk potiuk added this to the Airflow 3.0.3 milestone Jun 28, 2025
@potiuk potiuk added the backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch label Jun 28, 2025
@potiuk potiuk merged commit fb94109 into apache:main Jun 28, 2025
67 checks passed
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Jun 28, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

github-actions bot pushed a commit that referenced this pull request Jun 28, 2025
@SCH227
[v3-0-test] Sanitize Username (#52419)
b3dfd1c 
Escape user.username in flash banners to prevent potential HTML injection
(cherry picked from commit fb94109)

Co-authored-by: bu <[email protected]>
@github-actions GitHub Actions
Copy link

Backport successfully created: v3-0-test

Status Branch Result
✅ v3-0-test PR Link

potiuk pushed a commit that referenced this pull request Jun 28, 2025
@github-actions @SCH227
[v3-0-test] Sanitize Username (#52419) (#52421)
012bfb5 
Escape user.username in flash banners to prevent potential HTML injection
(cherry picked from commit fb94109)

Co-authored-by: bu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk approved these changes

@vincbeck vincbeck Awaiting requested review from vincbeck vincbeck is a code owner

Assignees
No one assigned
Labels
area:providers backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch provider:fab
Projects
None yet
Milestone
Airflow 3.0.3
Development

Successfully merging this pull request may close these issues.
2 participants
@SCH227 @potiuk