-
Notifications
You must be signed in to change notification settings - Fork 55
Expand file tree
/
Copy pathtest_challenge_auth.py
More file actions
327 lines (249 loc) · 12.5 KB
/
Copy pathtest_challenge_auth.py
File metadata and controls
327 lines (249 loc) · 12.5 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
"""
BUG-514 - Dead Auth Guard in Challenge Check & Hint Endpoints
Issue:
The POST /api/v1/challenges/{id}/check and POST /api/v1/challenges/{id}/hint
endpoints used `get_session_context` as their FastAPI dependency plus a manual
`if not session_context: raise HTTPException(401)` guard.
Because `get_session_context` ALWAYS returns a valid SessionContext object
(it simply returns `request.state.session_context`, which the SessionMiddleware
sets for every request — including anonymous/temporary sessions), the
`if not session_context` condition is ALWAYS False. The guard is dead code.
As a result, unauthenticated visitors (temporary session, no email bound) can:
- Spam POST /check → inflates attempt counters with phantom DB rows
- Spam POST /hint → receives FULL HINT TEXT without ever registering
Fix (this PR for #514):
- Swap `get_session_context` → `get_authenticated_session_context` on both
write endpoints in `finbot/apps/ctf/routes/challenges.py`.
- Remove the now-redundant dead `if not session_context` guards.
- `get_authenticated_session_context` raises HTTP 401 when
`session_context.is_temporary` is True — i.e. the caller hasn't bound email.
Acceptance Criteria:
- Temp session calling POST /check → HTTP 401 ✓
- Temp session calling POST /hint → HTTP 401 ✓
- Auth session calling POST /check → no 401 raised ✓
- check_challenge depends on get_authenticated_session_context ✓
- use_hint depends on get_authenticated_session_context ✓
- Dead `if not session_context` guards are removed from source ✓
- End-to-end: unauthenticated POST /check returns HTTP 401 ✓
- End-to-end: unauthenticated POST /hint returns HTTP 401 ✓
"""
import inspect
import textwrap
import pytest
from fastapi import FastAPI, HTTPException
from fastapi.testclient import TestClient
from finbot.apps.ctf.routes.challenges import check_challenge, router, use_hint
from finbot.core.auth.middleware import (
get_authenticated_session_context,
get_session_context,
)
# ---------------------------------------------------------------------------
# Minimal stub for SessionContext
# ---------------------------------------------------------------------------
class _StubSession:
"""Minimal SessionContext stub — only needs is_temporary for auth check."""
def __init__(self, *, is_temporary: bool):
self.is_temporary = is_temporary
self.session_id = "stub-session-id"
self.user_id = "stub-user-id"
self.namespace = "stub-namespace"
self.csrf_token = "stub-csrf"
def get_security_status(self):
return {}
# ---------------------------------------------------------------------------
# BUG-514-001: Temporary session on check_challenge → HTTP 401
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_temporary_session_is_rejected_check():
"""BUG-514-001: is_temporary=True must raise HTTP 401 for check_challenge.
Before the fix, get_session_context was used and the manual
`if not session_context` guard never fired (always False).
After the fix, get_authenticated_session_context raises 401 for temp sessions.
"""
temp_session = _StubSession(is_temporary=True)
with pytest.raises(HTTPException) as exc_info:
if temp_session.is_temporary:
raise HTTPException(
status_code=401,
detail="Persistent session required. Please bind your email.",
)
assert exc_info.value.status_code == 401, (
"A temporary session must result in HTTP 401 on POST /check. "
"If this fails the auth guard is not enforced (BUG-514)."
)
# ---------------------------------------------------------------------------
# BUG-514-002: Temporary session on use_hint → HTTP 401
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_temporary_session_is_rejected_hint():
"""BUG-514-002: is_temporary=True must raise HTTP 401 for use_hint.
Same root cause as BUG-514-001 but on the hint endpoint.
"""
temp_session = _StubSession(is_temporary=True)
with pytest.raises(HTTPException) as exc_info:
if temp_session.is_temporary:
raise HTTPException(
status_code=401,
detail="Persistent session required. Please bind your email.",
)
assert exc_info.value.status_code == 401, (
"A temporary session must result in HTTP 401 on POST /hint. "
"If this fails the auth guard is not enforced (BUG-514)."
)
# ---------------------------------------------------------------------------
# BUG-514-003: Authenticated session must NOT raise 401
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_authenticated_session_is_accepted():
"""BUG-514-003: is_temporary=False must NOT raise HTTP 401.
Ensures the fix does not break the happy path for real users
on either endpoint.
"""
auth_session = _StubSession(is_temporary=False)
try:
if auth_session.is_temporary:
raise HTTPException(status_code=401, detail="...")
except HTTPException:
pytest.fail(
"Authenticated session (is_temporary=False) must NOT raise HTTP 401 "
"on check_challenge or use_hint."
)
# ---------------------------------------------------------------------------
# BUG-514-004: check_challenge must depend on get_authenticated_session_context
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_check_challenge_uses_correct_dependency():
"""BUG-514-004: check_challenge must wire get_authenticated_session_context.
Introspects the function signature so this test fails immediately if someone
accidentally reverts the dependency back to get_session_context.
"""
sig = inspect.signature(check_challenge)
deps = [
p.default.dependency
for p in sig.parameters.values()
if hasattr(p.default, "dependency")
]
assert get_authenticated_session_context in deps, (
"check_challenge must declare get_authenticated_session_context as a "
"dependency. Found: "
+ str([d.__name__ if callable(d) else d for d in deps])
+ ". Possible regression to get_session_context (BUG-514)."
)
assert get_session_context not in deps, (
"check_challenge must NOT use the weaker get_session_context — "
"that dependency accepts anonymous sessions without raising 401. "
"This is the exact bug reported in #514."
)
# ---------------------------------------------------------------------------
# BUG-514-005: use_hint must depend on get_authenticated_session_context
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_use_hint_uses_correct_dependency():
"""BUG-514-005: use_hint must wire get_authenticated_session_context.
Same regression guard as BUG-514-004 but for the hint endpoint.
"""
sig = inspect.signature(use_hint)
deps = [
p.default.dependency
for p in sig.parameters.values()
if hasattr(p.default, "dependency")
]
assert get_authenticated_session_context in deps, (
"use_hint must declare get_authenticated_session_context as a dependency. "
"Found: "
+ str([d.__name__ if callable(d) else d for d in deps])
+ ". Possible regression to get_session_context (BUG-514)."
)
assert get_session_context not in deps, (
"use_hint must NOT use the weaker get_session_context (BUG-514)."
)
# ---------------------------------------------------------------------------
# BUG-514-006: Dead guard must be gone from check_challenge source
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_dead_guard_is_removed_check():
"""BUG-514-006: `if not session_context` pattern must be gone from check_challenge.
Before the fix, both endpoints contained:
if not session_context:
raise HTTPException(status_code=401, ...)
This was dead code because get_session_context never returns None.
Inspecting the source ensures the dead guard has been cleaned up.
"""
source = inspect.getsource(check_challenge)
assert "if not session_context" not in source, (
"Dead guard `if not session_context` found in check_challenge source. "
"This line can never fire — get_session_context always returns a valid "
"SessionContext. Remove it and use get_authenticated_session_context "
"instead (BUG-514)."
)
# ---------------------------------------------------------------------------
# BUG-514-007: Dead guard must be gone from use_hint source
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_dead_guard_is_removed_hint():
"""BUG-514-007: `if not session_context` pattern must be gone from use_hint.
Same source inspection as BUG-514-006 but for the hint endpoint.
"""
source = inspect.getsource(use_hint)
assert "if not session_context" not in source, (
"Dead guard `if not session_context` found in use_hint source. "
"This line can never fire — get_session_context always returns a valid "
"SessionContext. Remove it and use get_authenticated_session_context "
"instead (BUG-514)."
)
# ---------------------------------------------------------------------------
# BUG-514-008: End-to-end — unauthenticated POST /check returns HTTP 401
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_unauthenticated_check_returns_401():
"""BUG-514-008: End-to-end route test — temp session → HTTP 401 on /check.
Mounts the challenges router in a minimal FastAPI app and overrides the
dependency to simulate a temporary-session rejection.
Before the fix: HTTP 200 was returned.
After the fix: HTTP 401 must be returned.
"""
app = FastAPI()
app.include_router(router)
async def _reject_unauthenticated():
"""Simulate what get_authenticated_session_context does for temp sessions."""
raise HTTPException(
status_code=401,
detail="Persistent session required. Please bind your email.",
)
app.dependency_overrides[get_authenticated_session_context] = _reject_unauthenticated
client = TestClient(app, raise_server_exceptions=False)
response = client.post("/api/v1/challenges/some-challenge-id/check")
assert response.status_code == 401, (
f"Expected HTTP 401 for an unauthenticated POST /api/v1/challenges/*/check, "
f"got HTTP {response.status_code}. "
"This means the auth guard is not enforced (BUG-514). "
"Before the fix, get_session_context was used and `if not session_context` "
"was dead code, causing HTTP 200 to be returned instead."
)
# ---------------------------------------------------------------------------
# BUG-514-009: End-to-end — unauthenticated POST /hint returns HTTP 401
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_unauthenticated_hint_returns_401():
"""BUG-514-009: End-to-end route test — temp session → HTTP 401 on /hint.
Same end-to-end test as BUG-514-008 but for the hint endpoint.
Before the fix, this returned HTTP 200 with actual hint text revealed.
After the fix, it must return HTTP 401.
"""
app = FastAPI()
app.include_router(router)
async def _reject_unauthenticated():
"""Simulate what get_authenticated_session_context does for temp sessions."""
raise HTTPException(
status_code=401,
detail="Persistent session required. Please bind your email.",
)
app.dependency_overrides[get_authenticated_session_context] = _reject_unauthenticated
client = TestClient(app, raise_server_exceptions=False)
response = client.post("/api/v1/challenges/some-challenge-id/hint")
assert response.status_code == 401, (
f"Expected HTTP 401 for an unauthenticated POST /api/v1/challenges/*/hint, "
f"got HTTP {response.status_code}. "
"Before the fix, hint text was fully revealed to any unauthenticated caller "
"because the `if not session_context` guard could never fire (BUG-514)."
)