Skip to content

Commit c353975

Browse files
committed
fix(auth): enforce surface isolation and repair dashboard logout
1 parent 8bb6e23 commit c353975

9 files changed

Lines changed: 61 additions & 21 deletions

File tree

accounts/mixins.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ def user_has_surface_access(user, role: str) -> bool:
4040
if not getattr(user, "is_authenticated", False):
4141
return False
4242

43-
if is_admin_user(user):
44-
return True
45-
4643
surface_group_lookup = {
4744
ROLE_OPS: GROUP_NAMES[ROLE_OPS],
4845
ROLE_CUSTOMER: GROUP_NAMES[ROLE_CUSTOMER],
@@ -52,6 +49,9 @@ def user_has_surface_access(user, role: str) -> bool:
5249
if role not in surface_group_lookup:
5350
return False
5451

52+
if role == ROLE_ADMIN:
53+
return is_admin_user(user)
54+
5555
return user_in_group(user, surface_group_lookup[role])
5656

5757

accounts/services/surface_redirects.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ def get_primary_surface(user):
2727

2828
def user_can_access_path(user, path):
2929
"""Return True when the user may land on the requested relative path."""
30-
if is_admin_user(user):
31-
return True
32-
3330
for role, prefixes in SURFACE_ALLOWED_PREFIXES.items():
3431
if path.startswith(prefixes):
3532
return user_has_surface_access(user, role)

templates/console/admin_dashboard.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ <h1>Operate the platform shell with one clear management entry point.</h1>
1616
</p>
1717
<div class="rh-hero-actions">
1818
<a class="btn btn-primary rh-btn-primary" href="/admin/">Open Django admin</a>
19-
<a class="btn btn-outline-secondary rh-btn-secondary" href="{% url 'landing' %}">Return to landing page</a>
2019
</div>
2120
</div>
2221
<aside class="rh-hero-visual" aria-label="Admin console summary">

templates/partials/_app_nav.html

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,12 @@
3838
<div class="rh-topbar-meta" aria-label="Signed-in user">
3939
<span class="rh-header-note">{{ request.user.get_username }}</span>
4040
</div>
41-
<a class="btn btn-outline-secondary btn-sm rh-topbar-secondary" href="{% url 'accounts:logout' %}">
42-
Sign out
43-
</a>
41+
<form method="post" action="{% url 'accounts:logout' %}" class="m-0">
42+
{% csrf_token %}
43+
<button type="submit" class="btn btn-outline-secondary btn-sm rh-topbar-secondary">
44+
Sign out
45+
</button>
46+
</form>
4447
{% endif %}
4548
</div>
4649
</div>

tests/test_accounts_auth.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,13 @@ def test_user_has_surface_access_returns_false_for_unknown_role() -> None:
105105
assert user_has_surface_access(ops_user, "unknown") is False
106106

107107

108-
def test_user_has_surface_access_returns_true_for_superuser() -> None:
109-
"""Superusers should be allowed onto every surface."""
108+
def test_user_has_surface_access_limits_superuser_to_admin_surface() -> None:
109+
"""Superusers should not automatically bypass non-admin surface boundaries."""
110110

111111
admin_user = UserFactory(is_superuser=True, is_staff=True)
112112

113-
assert user_has_surface_access(admin_user, "ops") is True
113+
assert user_has_surface_access(admin_user, "admin") is True
114+
assert user_has_surface_access(admin_user, "ops") is False
114115

115116

116117
def test_user_has_surface_access_returns_false_for_anonymous_user() -> None:

tests/test_console_access.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,14 @@ def test_ops_can_access_ops_console(client):
6666

6767
assert response.status_code == 200
6868
assert b"Work the live return queue inside the same shared ReturnHub shell." in response.content
69+
70+
71+
def test_logout_view_accepts_post_and_redirects(client):
72+
"""Logout should work through a POST request from the shared dashboard nav."""
73+
user = create_user_for_role("ops.logout", ROLE_OPS)
74+
client.force_login(user)
75+
76+
response = client.post(reverse("accounts:logout"))
77+
78+
assert response.status_code == 302
79+
assert response.headers["Location"] == reverse("landing")

tests/test_console_shell.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,33 @@ def test_merchant_console_renders_for_merchant_user(client) -> None:
157157
assert response.status_code == 200
158158
assert "Merchant Console" in body
159159
assert "Review linked cases" in body
160+
161+
162+
@pytest.mark.django_db
163+
def test_authenticated_console_nav_uses_post_logout_form(client) -> None:
164+
"""The shared console nav should submit logout via POST instead of a GET link."""
165+
ops_user = UserFactory(email="console-logout@example.com")
166+
add_group(ops_user, "Ops")
167+
168+
client.force_login(ops_user)
169+
response = client.get("/console/ops/")
170+
171+
body = response.content.decode()
172+
assert response.status_code == 200
173+
assert 'method="post"' in body
174+
assert 'action="/logout/"' in body
175+
assert "Sign out" in body
176+
177+
178+
@pytest.mark.django_db
179+
def test_admin_console_does_not_render_return_to_landing_button(client) -> None:
180+
"""The admin dashboard should not show a landing-page return action."""
181+
admin_user = UserFactory(email="console-admin@example.com", is_superuser=True, is_staff=True)
182+
add_group(admin_user, "Admin")
183+
184+
client.force_login(admin_user)
185+
response = client.get("/console/admin/")
186+
187+
body = response.content.decode()
188+
assert response.status_code == 200
189+
assert "Return to landing page" not in body

tests/test_console_views.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,14 @@ def test_console_routes_forbid_wrong_role(client) -> None:
9191

9292

9393
@pytest.mark.django_db
94-
def test_superuser_can_access_console_routes_without_group_membership(client) -> None:
95-
"""Superusers should bypass group checks for authenticated console routes."""
94+
def test_superuser_without_admin_group_gets_403_on_ops_console(client) -> None:
95+
"""Superusers without the admin role should not bypass non-admin console boundaries."""
9696
admin_user = UserFactory(is_superuser=True, is_staff=True)
9797

9898
client.force_login(admin_user)
9999
response = client.get(reverse("console:ops-dashboard"))
100100

101-
assert response.status_code == 200
102-
assert "Ops Console" in response.content.decode()
101+
assert response.status_code == 403
103102

104103

105104
@pytest.mark.django_db

tests/test_surface_smoke.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,17 @@ def test_admin_can_open_admin_console(client, seeded_data):
3232
assert console_response.status_code == 200
3333

3434

35-
def test_admin_can_open_customer_portal_pages(client, seeded_data):
36-
"""Admin should be able to inspect customer portal page 1 and page 2."""
35+
def test_admin_gets_403_on_customer_portal_pages(client, seeded_data):
36+
"""Admin should not be able to open customer-only portal pages."""
3737
login_response = login(client, "/login/admin/", "admin.demo")
3838
assert login_response.status_code == 302
3939
assert login_response.headers["Location"] == "/console/admin/"
4040

4141
customer_page_1 = client.get("/customer/?page=1")
4242
customer_page_2 = client.get("/customer/?page=2")
4343

44-
assert customer_page_1.status_code == 200
45-
assert customer_page_2.status_code == 200
44+
assert customer_page_1.status_code == 403
45+
assert customer_page_2.status_code == 403
4646

4747

4848
def test_ops_can_open_console_and_ops_page_one_and_two(client, seeded_data):

0 commit comments

Comments
 (0)