fix(l4d2-web): harden auth redirect targets
This commit is contained in:
parent
deca2c9153
commit
58fb8b2b63
4 changed files with 32 additions and 0 deletions
|
|
@ -73,6 +73,19 @@ def test_login_ignores_unsafe_next(client) -> None:
|
|||
|
||||
assert response.status_code == 302
|
||||
assert response.headers["Location"].endswith("/dashboard")
|
||||
|
||||
|
||||
def test_login_ignores_backslash_next(client) -> None:
|
||||
with session_scope() as session:
|
||||
session.add(User(username="alice", password_digest=hash_password("secret"), admin=False))
|
||||
|
||||
response = client.post(
|
||||
"/login",
|
||||
data={"username": "alice", "password": "secret", "next": "/\\evil.com"},
|
||||
)
|
||||
|
||||
assert response.status_code == 302
|
||||
assert response.headers["Location"].endswith("/dashboard")
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add protected-page and root redirect tests**
|
||||
|
|
@ -151,6 +164,8 @@ def is_safe_next(target: str | None) -> bool:
|
|||
return False
|
||||
if "://" in target:
|
||||
return False
|
||||
if "\\" in target:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ A `next` target is safe only when it is a local absolute path:
|
|||
- It must start with `/`.
|
||||
- It must not start with `//`.
|
||||
- It must not contain a URL scheme such as `https://`.
|
||||
- It must not contain backslashes, which browsers may normalize into path separators.
|
||||
|
||||
Unsafe values are ignored and replaced with `/dashboard`. This avoids open redirects while keeping the implementation simple.
|
||||
|
||||
|
|
@ -85,6 +86,7 @@ Tests should cover:
|
|||
- Anonymous protected pages redirect to `/login?next=<path>`.
|
||||
- A successful login with a safe `next` redirects to that target.
|
||||
- A successful login with an unsafe `next` redirects to `/dashboard`.
|
||||
- A successful login with a backslash-containing `next` redirects to `/dashboard`.
|
||||
- Anonymous `/` redirects to `/login`.
|
||||
- Logged-in `/` redirects to `/dashboard`.
|
||||
- Non-admin users still receive `403` for admin pages.
|
||||
|
|
|
|||
|
|
@ -51,6 +51,8 @@ def is_safe_next(target: str | None) -> bool:
|
|||
return False
|
||||
if "://" in target:
|
||||
return False
|
||||
if "\\" in target:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -72,6 +72,19 @@ def test_login_ignores_unsafe_next(client) -> None:
|
|||
assert response.headers["Location"].endswith("/dashboard")
|
||||
|
||||
|
||||
def test_login_ignores_backslash_next(client) -> None:
|
||||
with session_scope() as session:
|
||||
session.add(User(username="alice", password_digest=hash_password("secret"), admin=False))
|
||||
|
||||
response = client.post(
|
||||
"/login",
|
||||
data={"username": "alice", "password": "secret", "next": "/\\evil.com"},
|
||||
)
|
||||
|
||||
assert response.status_code == 302
|
||||
assert response.headers["Location"].endswith("/dashboard")
|
||||
|
||||
|
||||
def test_login_sets_session(client) -> None:
|
||||
with session_scope() as session:
|
||||
session.add(User(username="alice", password_digest=hash_password("secret"), admin=False))
|
||||
|
|
|
|||
Loading…
Reference in a new issue