fix(marketing): call marketing.landing view directly (avoid redirect loop)

recordings.index previously redirected anonymous users to
url_for('marketing.landing'), but both endpoints are mounted at '/'.
Since recordings_bp registers first, Flask's URL map routed back to
recordings.index -> infinite redirect loop. Now we invoke the marketing
landing view function directly for anonymous requests, preserving the
URL map and avoiding the loop.
This commit is contained in:
Allison
2026-04-27 16:31:31 -04:00
parent 1071e56173
commit af2953995c
2 changed files with 62 additions and 35 deletions

View File

@@ -39,6 +39,7 @@ from src.file_exporter import export_recording, mark_export_as_deleted
from src.utils.ffprobe import get_codec_info, get_creation_date, FFProbeError from src.utils.ffprobe import get_codec_info, get_creation_date, FFProbeError
from src.utils.audio_conversion import convert_if_needed from src.utils.audio_conversion import convert_if_needed
from src.utils.file_hash import compute_file_sha256 from src.utils.file_hash import compute_file_sha256
from src.marketing.routes import landing as _marketing_landing
# Create blueprint # Create blueprint
recordings_bp = Blueprint('recordings', __name__) recordings_bp = Blueprint('recordings', __name__)
@@ -1360,16 +1361,22 @@ def reset_status(recording_id):
def index(): def index():
"""Root route handler. """Root route handler.
Anonymous users are redirected to the marketing landing page so the Anonymous users see the marketing landing page so the public site is
public site is reachable at "/". Authenticated users continue to see reachable at "/". Authenticated users continue to see the recordings
the recordings dashboard (legacy Speakr UI). dashboard (legacy Speakr UI).
Phase 1 of marketing redesign 2026 (B-1.3) replaced the previous Phase 1 of marketing redesign 2026 (B-1.3) replaced the previous
@login_required decorator with this inline check to resolve the route @login_required decorator with this inline check to resolve the route
collision between recordings_bp.index and marketing_bp.landing. collision between recordings_bp.index and marketing_bp.landing.
NOTE: We invoke the marketing.landing view function directly (rather
than redirecting via url_for('marketing.landing')) because both
endpoints are mounted at "/". Since recordings_bp is registered first,
Flask's URL map resolves "/" to recordings.index, so a redirect would
loop back into this same handler indefinitely.
""" """
if not current_user.is_authenticated: if not current_user.is_authenticated:
return redirect(url_for('marketing.landing')) return _marketing_landing()
# Check if user is a group admin # Check if user is a group admin
is_team_admin = GroupMembership.query.filter_by( is_team_admin = GroupMembership.query.filter_by(

View File

@@ -6,6 +6,13 @@ redirected anonymous users to /login. After B-1.3, anonymous users land
on the public marketing site; only authenticated users see the legacy on the public marketing site; only authenticated users see the legacy
Speakr dashboard. Speakr dashboard.
Implementation note: recordings.index calls marketing.routes.landing()
directly (not via redirect) because both endpoints are mounted at "/".
A redirect to url_for('marketing.landing') would resolve back to
recordings.index (registered first in the URL map) and cause an infinite
redirect loop. The expected behavior is therefore a direct 200 render,
NOT a redirect.
Pattern: no conftest.py, env vars set at module load time, then import Pattern: no conftest.py, env vars set at module load time, then import
src.app.app directly. Mirrors tests/test_blueprint_registration.py. src.app.app directly. Mirrors tests/test_blueprint_registration.py.
""" """
@@ -21,43 +28,55 @@ os.environ.setdefault('SECRET_KEY', 'test-secret-key-for-marketing-root-redirect
from src.app import app # noqa: E402 from src.app import app # noqa: E402
def test_anonymous_user_at_root_does_not_go_to_login(): def test_anonymous_user_no_login_redirect():
"""Anonymous user GET / must NOT be redirected to /login.""" """Anonymous user GET / must NOT redirect to /login."""
client = app.test_client() client = app.test_client()
response = client.get('/', follow_redirects=False) response = client.get('/', follow_redirects=False)
if response.status_code in (301, 302, 303, 307, 308): if response.status_code in (301, 302, 303, 307, 308):
location = response.headers.get('Location', '') location = response.headers.get('Location', '')
assert '/login' not in location, ( assert '/login' not in location, (
f"Expected anonymous user to see marketing, but redirected to: {location}" f"Expected no /login redirect, got: {location}"
) )
def test_anonymous_user_at_root_sees_marketing_or_redirects_to_marketing(): def test_anonymous_user_at_root_sees_marketing():
"""Anonymous user GET / must see marketing landing (200) OR redirect to marketing.landing.""" """Anonymous user GET / receives marketing landing directly (no redirect).
The handler must invoke the marketing.landing view function directly
rather than redirecting, because url_for('marketing.landing') resolves
back to '/' and would loop into recordings.index forever.
"""
client = app.test_client() client = app.test_client()
response = client.get('/', follow_redirects=False) response = client.get('/', follow_redirects=False)
# Direct render -- must be 200, NOT a redirect
if response.status_code in (301, 302, 303, 307, 308):
# Acceptable: marketing landing is served via redirect from
# recordings_bp.index when the URL map prefers the recordings rule.
location = response.headers.get('Location', '')
assert '/login' not in location, (
f"Anonymous user redirected to login instead of marketing: {location}"
)
# The redirect target should be the marketing landing or root itself.
# Because marketing.landing is mounted at '/', url_for('marketing.landing')
# produces '/' — the redirect Location should not point back at any
# private route.
assert ('/admin' not in location and '/account' not in location), (
f"Unexpected redirect from / : {location}"
)
else:
assert response.status_code == 200, ( assert response.status_code == 200, (
f"Expected 200 at / for anonymous, got {response.status_code}" f"Expected 200 at / for anonymous (direct render, no redirect "
f"to avoid loop), got {response.status_code}"
) )
# Verify it's the marketing placeholder content
body_lower = response.data.lower() body_lower = response.data.lower()
assert (b'marketing' in body_lower) or (b'dictia' in body_lower), ( assert (b'dictia' in body_lower) or (b'marketing' in body_lower), (
"Expected marketing landing content at / for anonymous user" f"Expected marketing content at /, got: {response.data[:200]!r}"
)
def test_root_route_no_redirect_loop():
"""GET / must not bounce through redirects.
Following redirects should reach a final 200 in <=1 hops; if Flask
returns >1 redirect chain at /, it indicates the recordings.index
handler is redirecting back to itself via url_for('marketing.landing').
"""
client = app.test_client()
response = client.get('/', follow_redirects=False)
# Either an immediate 200 (preferred) or at most one redirect.
assert response.status_code not in (301, 302, 303, 307, 308) or (
'/login' not in response.headers.get('Location', '')
and response.headers.get('Location', '') != '/'
), (
f"Detected potential redirect loop at /: status="
f"{response.status_code}, Location="
f"{response.headers.get('Location', '')}"
) )
@@ -66,15 +85,16 @@ def test_root_route_resolves_to_recordings_index():
The collision between recordings.index and marketing.landing is The collision between recordings.index and marketing.landing is
deliberate: recordings.index is registered first and intercepts the deliberate: recordings.index is registered first and intercepts the
request, then redirects anonymous users to marketing.landing request, then dispatches anonymous users to the marketing.landing
(which itself maps to '/'). This test pins that contract so a view function directly (no URL-level redirect). This test pins that
future refactor can't silently swap the registration order. contract so a future refactor can't silently swap the registration
order.
""" """
rules_for_root = [ rules_for_root = [
r for r in app.url_map.iter_rules() if str(r) == '/' r for r in app.url_map.iter_rules() if str(r) == '/'
] ]
endpoints = {r.endpoint for r in rules_for_root} endpoints = {r.endpoint for r in rules_for_root}
# Both endpoints must exist for the redirect chain to work. # Both endpoints must exist; recordings.index is the active one.
assert 'recordings.index' in endpoints, ( assert 'recordings.index' in endpoints, (
f"Expected recordings.index to own '/', got endpoints: {endpoints}" f"Expected recordings.index to own '/', got endpoints: {endpoints}"
) )