Skip to content

fix: distinguish authentication from authorization redirects - #2479

Open
aminghadersohi wants to merge 2 commits into
dpgaspar:masterfrom
aminghadersohi:fab-authenticated-access-and-next
Open

fix: distinguish authentication from authorization redirects#2479
aminghadersohi wants to merge 2 commits into
dpgaspar:masterfrom
aminghadersohi:fab-authenticated-access-and-next

Conversation

@aminghadersohi

Copy link
Copy Markdown
Contributor

Description

This changes FAB's browser-auth behavior so authentication and authorization failures are no longer treated as the same condition:

  • an anonymous request denied by @has_access still flashes the existing message and redirects to /login/?next=<requested URL>;
  • an authenticated-but-unauthorized request now returns FAB/Flask's HTML 403 response rather than redirecting to login;
  • because a login URL therefore represents an authentication requirement, already-authenticated requests to login now honor a safe next URL instead of discarding it. This is applied consistently to DB, LDAP, OAuth, SAML, and Remote User auth views. Missing or unsafe destinations continue to fall back to the application index.

This is intentionally a draft and explicitly invites maintainer discussion: returning 403 instead of 302 for authenticated users is a behavior change, but it avoids redirect loops and aligns MVC behavior with the existing distinction made by @has_access_api.

The fix was split out of Apache Superset PR #43606, following Vitor Avila's suggestion to address the coupled behavior in FAB. It relates to Preset Shortcut story 97504.

The separate backslash redirect-safety issue discussed on the Superset PR is deliberately not included here; it is being handled separately.

Tests cover the 403 response and redirect behavior across every FAB browser auth view, including rejection of an external next destination. The changelog is updated because this affects application-visible behavior.

Testing

python -m pytest -q tests/security/test_auth_views.py tests/security/test_mvc_security.py \
  -k 'authenticated_login_uses_safe_next_url or authenticated_db_login_valid_next_url or authenticated_access_denial_returns_forbidden or sec_login'
14 passed, 30 deselected

python -m flake8 flask_appbuilder/security/decorators.py flask_appbuilder/security/views.py \
  tests/security/test_auth_views.py tests/security/test_mvc_security.py
# passed

git diff --check
# passed

ADDITIONAL INFORMATION

  • Has associated issue: Shortcut #97504
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

def login(self):
if g.user is not None and g.user.is_authenticated:
return redirect(self.appbuilder.get_url_for_index)
return redirect(self._get_safe_next_url())
def login(self):
if g.user is not None and g.user.is_authenticated:
return redirect(self.appbuilder.get_url_for_index)
return redirect(self._get_safe_next_url())
if g.user is not None and g.user.is_authenticated:
log.debug("Already authenticated %s", g.user)
return redirect(self.appbuilder.get_url_for_index)
return redirect(self._get_safe_next_url())
def login(self, idp: Optional[str] = None) -> WerkzeugResponse:
if g.user is not None and g.user.is_authenticated:
return redirect(self.appbuilder.get_url_for_index)
return redirect(self._get_safe_next_url())
if g.user is not None and g.user.is_authenticated:
next_url = request.args.get("next", "")
return redirect(get_safe_redirect(next_url))
return redirect(self._get_safe_next_url())
@aminghadersohi
aminghadersohi marked this pull request as ready for review September 4, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants