Skip to content

Commit 607767c

Browse files
SEP-952: Unify auth in get_current_user() — accept Bearer token and cookie (#555)
Updated SEP auth to support Bearer-first with cookie fallback during SPA migration, and aligned SEP error/static handling plus tests to preserve correct API vs HTML behavior. - `app/sep/deps.py`: composed `get_current_user` to try `oauth2_scheme` + `app.api.deps.get_current_user first`, then fall back to cookie auth (`get_current_user_from_cookie`) without changing legacy cookie redirect behavior. - `app/sep/main.py`: updated SEP default `HTTPException` handling to return JSON for Bearer-authenticated requests instead of redirecting, so SPA/API clients receive proper 401/403 responses. - `app/sep/utils/static.py`: simplified `AuthenticatedStaticFiles` to use unified `get_current_user(request)` instead of maintaining separate Bearer/cookie logic. --------- Co-authored-by: Yan Orestes <yan.orestes@percona.com>
1 parent 1fc6844 commit 607767c

7 files changed

Lines changed: 271 additions & 74 deletions

File tree

app/sep/deps.py

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
from pydantic import ValidationError
2727
from sqlmodel.ext.asyncio.session import AsyncSession
2828

29+
from app.api.deps import get_current_user as get_current_user_api
30+
from app.api.deps import oauth2_scheme
2931
from app.core.alerts.config import alert_settings
3032
from app.core.auth.exceptions import HTTPForbiddenException
3133
from app.core.auth.utils import get_user_model
@@ -123,17 +125,18 @@ def get_access_token_from_cookie(
123125
AccessTokenCookie = Annotated[str, Depends(get_access_token_from_cookie)]
124126

125127

126-
async def get_current_user(
127-
request: Request,
128-
) -> User:
129-
"""Return the authenticated user from a cookie token.
128+
async def get_current_user_from_cookie(request: Request) -> User:
129+
"""Return the authenticated user from the signed session cookie.
130130
131-
:param request: The HTTP request object from which the base URL is derived.
131+
Loads and verifies the session cookie, decodes the JWT into a user, and
132+
rejects inactive accounts with a login redirect (legacy Jinja2 behavior).
133+
134+
:param request: The incoming HTTP request.
132135
:type request: Request
133136
:return: The authenticated user.
134137
:rtype: User
135-
:raises LoginRedirectException: If the token is invalid or the user is
136-
inactive.
138+
:raises LoginRedirectException: If the cookie or JWT is invalid or the user
139+
is inactive.
137140
"""
138141
token = get_access_token_from_cookie(request)
139142
try:
@@ -149,6 +152,49 @@ async def get_current_user(
149152
return user
150153

151154

155+
def is_bearer_authenticated(request: Request) -> bool:
156+
"""Return whether the request carries an ``Authorization: Bearer`` header.
157+
158+
Inspects only the ``Authorization`` header prefix — the token itself is not
159+
validated. Intended as a routing signal to pick between Bearer and cookie
160+
authentication, and to render API-style error responses for SPA clients.
161+
162+
:param request: The incoming HTTP request.
163+
:type request: Request
164+
:return: ``True`` when the header starts with ``Bearer ``, ``False`` otherwise.
165+
:rtype: bool
166+
"""
167+
return request.headers.get("authorization", "").lower().startswith("bearer ")
168+
169+
170+
async def get_current_user(
171+
request: Request,
172+
) -> User:
173+
"""Return the authenticated user from a Bearer token or session cookie.
174+
175+
The ``Authorization: Bearer`` header is tried first (React SPA) and, when
176+
present, failures from :func:`app.api.deps.get_current_user` are raised as
177+
HTTP API errors (401/403) rather than converted into a login redirect —
178+
including the case of a malformed/empty Bearer token. When the header is
179+
absent, authentication falls back to the signed session cookie (legacy
180+
Jinja2).
181+
182+
:param request: The incoming HTTP request.
183+
:type request: Request
184+
:return: The authenticated user.
185+
:rtype: User
186+
:raises HTTPUnauthorizedException: If a Bearer token is present but invalid.
187+
:raises HTTPForbiddenException: If the user resolved from Bearer is inactive.
188+
:raises LoginRedirectException: If cookie-based auth fails or the cookie user
189+
is inactive.
190+
"""
191+
if is_bearer_authenticated(request):
192+
bearer_token = await oauth2_scheme(request)
193+
return await get_current_user_api(bearer_token)
194+
195+
return await get_current_user_from_cookie(request)
196+
197+
152198
IsAuthenticated = Depends(get_current_user)
153199
CurrentUser = Annotated[User, IsAuthenticated]
154200

app/sep/main.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
get_current_user,
4646
get_default_context,
4747
get_tasks_index_context,
48+
is_bearer_authenticated,
4849
IsAuthenticated,
4950
IsCsrfValidated,
5051
IsNotAuthenticated,
@@ -263,11 +264,11 @@ async def json_exception_handler(
263264

264265

265266
@sep_app.exception_handler(HTTPException)
266-
async def default_exception_handler(
267-
request: Request, exc: HTTPException
268-
) -> RedirectResponse:
267+
async def default_exception_handler(request: Request, exc: HTTPException) -> Response:
269268
"""Define default exception handler."""
270-
if request.url.path.startswith("/checksums/api/"):
269+
if request.url.path.startswith("/checksums/api/") or is_bearer_authenticated(
270+
request
271+
):
271272
return JSONResponse(
272273
{"detail": exc.detail},
273274
status_code=exc.status_code,

app/sep/utils/static.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@
1515

1616
"""Define utilities regarding static files."""
1717

18-
from fastapi import HTTPException, Request
18+
from fastapi import Request
1919
from starlette.staticfiles import StaticFiles
2020
from starlette.types import Receive, Scope, Send
2121

22-
from app.api.deps import get_current_user as get_current_user_api
23-
from app.api.deps import oauth2_scheme
2422
from app.sep.deps import get_current_user
2523

2624

@@ -43,9 +41,5 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
4341
"""
4442
if scope["type"] == "http":
4543
request = Request(scope, receive, send)
46-
try:
47-
token = await oauth2_scheme(request)
48-
await get_current_user_api(token)
49-
except HTTPException:
50-
await get_current_user(request)
44+
await get_current_user(request)
5145
await super().__call__(scope, receive, send)

changelog.d/SEP-952.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
SEP authentication now accepts `Authorization: Bearer` tokens in addition to the signed session cookie. API/SPA consumers receive 401/403 JSON on auth failure instead of a 303 redirect to `/login`; cookie-authenticated Jinja2 requests continue to redirect as before.

tests/app/sep/test_deps.py

Lines changed: 142 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@
2323
from itsdangerous import BadSignature
2424
from pydantic import ValidationError
2525

26-
from app.core.auth.exceptions import HTTPForbiddenException
26+
from app.core.auth.exceptions import (
27+
HTTPForbiddenException,
28+
HTTPUnauthorizedException,
29+
)
2730
from app.core.exceptions import HTTPConflictException, HTTPNotFoundException
2831
from app.models import CasdoorUser
2932
from app.sep.config import sep_settings
@@ -68,11 +71,18 @@
6871
EXPECTED_NODE_COUNT = 5
6972

7073

71-
def _make_request() -> Request:
72-
"""Build a minimal Request with messages state for testing."""
74+
def _make_request(authorization: str | None = None) -> Request:
75+
"""Build a minimal Request with messages state for testing.
76+
77+
:param authorization: Value for the ``Authorization`` header, if any.
78+
:type authorization: str | None
79+
"""
80+
headers = []
81+
if authorization is not None:
82+
headers.append((b"authorization", authorization.encode()))
7383
scope = {
7484
"type": "http",
75-
"headers": [],
85+
"headers": headers,
7686
"client": ("127.0.0.1", "80"),
7787
"path": "/",
7888
"app": MagicMock(),
@@ -98,6 +108,134 @@ def test_returns_setting_when_configured(self) -> None:
98108
class TestGetCurrentUser:
99109
"""Test get_current_user dependency."""
100110

111+
@pytest.mark.asyncio
112+
async def test_valid_bearer_returns_user(self) -> None:
113+
"""Assert a valid Bearer path returns the user from the API dependency."""
114+
request = _make_request(authorization="Bearer bearer-token")
115+
active_user = CasdoorUserFactory.build(is_forbidden=False)
116+
mock_oauth2 = AsyncMock(return_value="bearer-token")
117+
mock_api_user = AsyncMock(return_value=active_user)
118+
with (
119+
patch("app.sep.deps.oauth2_scheme", mock_oauth2),
120+
patch("app.sep.deps.get_current_user_api", mock_api_user),
121+
patch(
122+
"app.sep.deps.get_access_token_from_cookie",
123+
side_effect=AssertionError(
124+
"cookie must not be read when Bearer succeeds"
125+
),
126+
),
127+
):
128+
result = await get_current_user(request)
129+
assert result is active_user
130+
mock_oauth2.assert_awaited_once_with(request)
131+
mock_api_user.assert_awaited_once_with("bearer-token")
132+
133+
@pytest.mark.asyncio
134+
async def test_valid_cookie_returns_user(self) -> None:
135+
"""Assert cookie-only auth still returns an active user."""
136+
request = _make_request()
137+
active_user = CasdoorUserFactory.build(is_forbidden=False)
138+
with (
139+
patch(
140+
"app.sep.deps.oauth2_scheme",
141+
AsyncMock(
142+
side_effect=AssertionError(
143+
"oauth2_scheme must not be called without a Bearer header"
144+
)
145+
),
146+
),
147+
patch(
148+
"app.sep.deps.get_access_token_from_cookie", return_value="cookie-token"
149+
),
150+
patch.object(CasdoorUser, "from_jwt", return_value=active_user),
151+
):
152+
result = await get_current_user(request)
153+
assert result is active_user
154+
155+
@pytest.mark.asyncio
156+
async def test_bearer_and_cookie_present_bearer_wins(self) -> None:
157+
"""Assert Authorization Bearer is preferred over session cookie."""
158+
request = _make_request(authorization="Bearer bearer-token")
159+
bearer_user = CasdoorUserFactory.build(username="bearer-user")
160+
cookie_user = CasdoorUserFactory.build(username="cookie-user")
161+
with (
162+
patch("app.sep.deps.oauth2_scheme", AsyncMock(return_value="bearer-token")),
163+
patch(
164+
"app.sep.deps.get_current_user_api", AsyncMock(return_value=bearer_user)
165+
),
166+
patch(
167+
"app.sep.deps.get_access_token_from_cookie",
168+
return_value="cookie-token",
169+
),
170+
patch.object(
171+
CasdoorUser,
172+
"from_jwt",
173+
return_value=cookie_user,
174+
),
175+
):
176+
result = await get_current_user(request)
177+
assert result.username == "bearer-user"
178+
179+
@pytest.mark.asyncio
180+
async def test_neither_bearer_nor_cookie_raises_redirect(self) -> None:
181+
"""Assert missing Bearer and missing cookie raises LoginRedirectException."""
182+
request = _make_request()
183+
with (
184+
patch(
185+
"app.sep.deps.get_access_token_from_cookie",
186+
side_effect=LoginRedirectException(request),
187+
),
188+
pytest.raises(LoginRedirectException),
189+
):
190+
await get_current_user(request)
191+
192+
@pytest.mark.asyncio
193+
async def test_invalid_bearer_raises_unauthorized(self) -> None:
194+
"""Assert invalid JWT via Bearer raises HTTPUnauthorizedException."""
195+
request = _make_request(authorization="Bearer bad-token")
196+
with (
197+
patch("app.sep.deps.oauth2_scheme", AsyncMock(return_value="bad-token")),
198+
patch(
199+
"app.sep.deps.get_current_user_api",
200+
AsyncMock(side_effect=HTTPUnauthorizedException),
201+
),
202+
pytest.raises(HTTPUnauthorizedException),
203+
):
204+
await get_current_user(request)
205+
206+
@pytest.mark.asyncio
207+
async def test_inactive_user_via_bearer_raises_forbidden(self) -> None:
208+
"""Assert inactive user resolved via Bearer raises HTTPForbiddenException."""
209+
request = _make_request(authorization="Bearer token")
210+
with (
211+
patch("app.sep.deps.oauth2_scheme", AsyncMock(return_value="token")),
212+
patch(
213+
"app.sep.deps.get_current_user_api",
214+
AsyncMock(side_effect=HTTPForbiddenException("User is not active")),
215+
),
216+
pytest.raises(HTTPForbiddenException),
217+
):
218+
await get_current_user(request)
219+
220+
@pytest.mark.asyncio
221+
async def test_malformed_bearer_header_does_not_fall_back_to_cookie(self) -> None:
222+
"""Assert a malformed Bearer header surfaces the 401 without cookie fallback."""
223+
request = _make_request(authorization="Bearer ")
224+
with (
225+
patch(
226+
"app.sep.deps.oauth2_scheme",
227+
AsyncMock(side_effect=HTTPException(status_code=401)),
228+
),
229+
patch(
230+
"app.sep.deps.get_access_token_from_cookie",
231+
side_effect=AssertionError(
232+
"cookie must not be read for a Bearer-authenticated request"
233+
),
234+
),
235+
pytest.raises(HTTPException),
236+
):
237+
await get_current_user(request)
238+
101239
@pytest.mark.asyncio
102240
async def test_bad_signature_raises_redirect(self) -> None:
103241
"""Assert BadSignature during JWT decode raises LoginRedirectException."""

tests/app/sep/test_main.py

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@
2222
from fastapi.responses import HTMLResponse
2323
from fastapi.testclient import TestClient
2424

25-
from app.core.auth.exceptions import BaseAuthProviderException
25+
from app.core.auth.exceptions import (
26+
BaseAuthProviderException,
27+
HTTPForbiddenException,
28+
HTTPUnauthorizedException,
29+
)
2630
from app.core.exceptions import HTTPBadGatewayException, HTTPServiceUnavailableException
2731
from app.sep.config import sep_settings
2832
from app.sep.deps import get_access_token_from_cookie
@@ -279,6 +283,60 @@ def test_default_error_handler(self, mocker, dummy_context, test_client):
279283
assert response.headers["location"] == fake_referer
280284
messages_error_mock.assert_called_once_with(mocker.ANY, error_detail)
281285

286+
@pytest.mark.parametrize(
287+
("exc", "expected_status"),
288+
[
289+
pytest.param(
290+
HTTPUnauthorizedException(),
291+
status.HTTP_401_UNAUTHORIZED,
292+
id="bearer_unauthorized",
293+
),
294+
pytest.param(
295+
HTTPForbiddenException("User is not active"),
296+
status.HTTP_403_FORBIDDEN,
297+
id="bearer_forbidden",
298+
),
299+
],
300+
)
301+
def test_default_error_handler_bearer_returns_json(
302+
self, mocker, dummy_context, test_client, exc, expected_status
303+
):
304+
"""Test returning JSON for Bearer-authenticated HTTP exceptions."""
305+
mocker.patch(
306+
"app.sep.main.templates.TemplateResponse",
307+
side_effect=exc,
308+
)
309+
310+
response = test_client.get(
311+
"/",
312+
headers={"Authorization": "Bearer any-token"},
313+
follow_redirects=False,
314+
)
315+
316+
assert response.status_code == expected_status
317+
assert response.json()["detail"] == exc.detail
318+
319+
def test_default_error_handler_unauthorized_without_bearer_redirects(
320+
self, mocker, dummy_context, test_client
321+
):
322+
"""Test using referer redirect when Authorization Bearer is absent."""
323+
mocker.patch(
324+
"app.sep.main.templates.TemplateResponse",
325+
side_effect=HTTPUnauthorizedException(),
326+
)
327+
messages_error_mock = mocker.patch("app.sep.main.messages.error")
328+
fake_referer = "/some-page"
329+
330+
response = test_client.get(
331+
"/",
332+
headers={"Referer": fake_referer},
333+
follow_redirects=False,
334+
)
335+
336+
assert response.status_code == status.HTTP_303_SEE_OTHER
337+
assert response.headers["location"] == fake_referer
338+
messages_error_mock.assert_called_once()
339+
282340
def test_auth_provider_exception_handler(
283341
self, mocker, dummy_access_token, dummy_context, test_client_with_session_cookie
284342
):

0 commit comments

Comments
 (0)