Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.docker
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ SYNCMASTER__AUTH__KEYCLOAK__SERVER_URL=http://keycloak:8080
SYNCMASTER__AUTH__KEYCLOAK__REALM_NAME=manually_created
SYNCMASTER__AUTH__KEYCLOAK__CLIENT_ID=manually_created
SYNCMASTER__AUTH__KEYCLOAK__CLIENT_SECRET=generated_by_keycloak
SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:8000/auth/callback
SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:3000/auth/callback
SYNCMASTER__AUTH__KEYCLOAK__SCOPE=email
SYNCMASTER__AUTH__KEYCLOAK__VERIFY_SSL=False

Expand Down
2 changes: 1 addition & 1 deletion .env.local
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export SYNCMASTER__AUTH__KEYCLOAK__SERVER_URL=http://localhost:8080
export SYNCMASTER__AUTH__KEYCLOAK__REALM_NAME=manually_created
export SYNCMASTER__AUTH__KEYCLOAK__CLIENT_ID=manually_created
export SYNCMASTER__AUTH__KEYCLOAK__CLIENT_SECRET=generated_by_keycloak
export SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:8000/auth/callback
export SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:3000/auth/callback
export SYNCMASTER__AUTH__KEYCLOAK__SCOPE=email
export SYNCMASTER__AUTH__KEYCLOAK__VERIFY_SSL=False

Expand Down
1 change: 1 addition & 0 deletions docs/changelog/next_release/274.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace 307 redirect to Keycloak auth page with 401 response, due to browser restrictions for redirect + CORS + localhost.
6 changes: 3 additions & 3 deletions docs/reference/server/auth/keycloak/local_installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ Set ``client_authentication`` **ON** to receive client_secret
Configure Redirect URI
~~~~~~~~~~~~~~~~~~~~~~

To configure the redirect URI where the browser will redirect to exchange the code provided from Keycloak for an access token, set the `SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI` environment variable. The default value for local development is `http://localhost:8000/auth/callback`.
To configure the redirect URI where the browser will redirect to exchange the code provided from Keycloak for an access token, set the `SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI` environment variable. The default value for local development is `http://localhost:3000/auth/callback`.

.. code-block:: console

$ export SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:8000/auth/callback
$ export SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:3000/auth/callback

Configure the client redirect URI
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -112,7 +112,7 @@ After this you can user `KeycloakAuthProvider` in your application with provided
.. code-block:: console

$ export SYNCMASTER__AUTH__KEYCLOAK__SERVER_URL=http://keycloak:8080
$ export SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:8000/auth/callback
$ export SYNCMASTER__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:3000/auth/callback
$ export SYNCMASTER__AUTH__KEYCLOAK__REALM_NAME=fastapi_realm
$ export SYNCMASTER__AUTH__KEYCLOAK__CLIENT_ID=fastapi_client
$ export SYNCMASTER__AUTH__KEYCLOAK__CLIENT_SECRET=6x6gn8uJdWSBmP8FqbNRSoGdvaoaFeez
Expand Down
82 changes: 52 additions & 30 deletions poetry.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ faker = "^37.4.0"
coverage = "^7.9.1"
gevent = ">=24.11.1,<26.0.0"
responses = "^0.25.7"
dirty-equals = "^0.9.0"

[tool.poetry.group.dev.dependencies]
mypy = "^1.15.0"
Expand Down
14 changes: 4 additions & 10 deletions syncmaster/server/api/v1/auth.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# SPDX-FileCopyrightText: 2023-2024 MTS PJSC
# SPDX-License-Identifier: Apache-2.0
from http.client import NOT_FOUND
from http.client import NO_CONTENT
from typing import Annotated

from fastapi import APIRouter, Depends, HTTPException, Request
from fastapi.responses import RedirectResponse
from fastapi import APIRouter, Depends, Request, Response
from fastapi.security import OAuth2PasswordRequestForm

from syncmaster.errors.registration import get_error_responses
Expand All @@ -17,7 +16,6 @@
DummyAuthProvider,
KeycloakAuthProvider,
)
from syncmaster.server.utils.state import validate_state

router = APIRouter(
prefix="/auth",
Expand All @@ -42,21 +40,17 @@ async def token(
return AuthTokenSchema.model_validate(token)


@router.get("/callback")
@router.get("/callback", status_code=NO_CONTENT)
async def auth_callback(
request: Request,
code: str,
state: str,
auth_provider: Annotated[KeycloakAuthProvider, Depends(Stub(AuthProvider))],
):
original_redirect_url = validate_state(state)
if not original_redirect_url:
raise HTTPException(status_code=NOT_FOUND, detail="Invalid state parameter")
token = await auth_provider.get_token_authorization_code_grant(
code=code,
redirect_uri=auth_provider.settings.keycloak.redirect_uri,
)
request.session["access_token"] = token["access_token"]
request.session["refresh_token"] = token["refresh_token"]

return RedirectResponse(url=original_redirect_url)
return Response(status_code=NO_CONTENT)
8 changes: 5 additions & 3 deletions syncmaster/server/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

from fastapi import HTTPException, Request, Response, status
from fastapi.exceptions import RequestValidationError
from fastapi.responses import RedirectResponse
from pydantic import ValidationError

from syncmaster.errors.base import APIErrorSchema, BaseErrorSchema
Expand Down Expand Up @@ -113,7 +112,7 @@ async def syncmsater_exception_handler(request: Request, exc: SyncmasterError):
return unknown_exception_handler(request, exc)

content = response.schema( # type: ignore[call-arg]
message=exc.message if hasattr(exc, "message") else "",
message=getattr(exc, "message", ""),
)
if isinstance(exc, AuthDataNotFoundError):
content.code = "not_found"
Expand All @@ -124,7 +123,10 @@ async def syncmsater_exception_handler(request: Request, exc: SyncmasterError):
)

if isinstance(exc, RedirectException):
return RedirectResponse(url=exc.redirect_url)
content.code = "unauthorized"
content.message = "Please authorize using provided URL"
content.details = exc.redirect_url
return exception_json_response(status=status.HTTP_401_UNAUTHORIZED, content=content)

if isinstance(exc, AuthorizationError):
content.code = "unauthorized"
Expand Down
3 changes: 0 additions & 3 deletions syncmaster/server/providers/auth/keycloak_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from syncmaster.server.providers.auth.base_provider import AuthProvider
from syncmaster.server.services.unit_of_work import UnitOfWork
from syncmaster.server.settings.auth.keycloak import KeycloakAuthProviderSettings
from syncmaster.server.utils.state import generate_state

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -137,10 +136,8 @@ async def refresh_access_token(self, refresh_token: str) -> dict[str, Any]:
return new_tokens

def redirect_to_auth(self, path: str) -> None:
state = generate_state(path)
auth_url = self.keycloak_openid.auth_url(
redirect_uri=self.settings.keycloak.redirect_uri,
scope=self.settings.keycloak.scope,
state=state,
)
raise RedirectException(redirect_url=auth_url)
15 changes: 0 additions & 15 deletions syncmaster/server/utils/state.py

This file was deleted.

51 changes: 43 additions & 8 deletions tests/test_unit/test_auth/test_auth_keycloak.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import pytest
import responses
from dirty_equals import IsStr
from httpx import AsyncClient

from syncmaster.server.settings import ServerAppSettings as Settings
Expand All @@ -23,14 +24,18 @@
],
indirect=True,
)
async def test_get_keycloak_user_unauthorized(client: AsyncClient, mock_keycloak_well_known):
async def test_keycloak_get_user_unauthorized(client: AsyncClient, mock_keycloak_well_known):
response = await client.get("/v1/users/some_user_id")

# redirect unauthorized user to Keycloak
assert response.status_code == 307, response.text
assert "protocol/openid-connect/auth?" in str(
response.next_request.url,
)
assert response.status_code == 401, response.text
assert response.json() == {
"error": {
"code": "unauthorized",
"message": "Please authorize using provided URL",
"details": IsStr(regex=r".*protocol/openid-connect/auth\?.*"),
},
}


@responses.activate
Expand All @@ -46,7 +51,7 @@ async def test_get_keycloak_user_unauthorized(client: AsyncClient, mock_keycloak
],
indirect=True,
)
async def test_get_keycloak_user_authorized(
async def test_keycloak_get_user_authorized(
client: AsyncClient,
simple_user: MockUser,
settings: Settings,
Expand Down Expand Up @@ -84,7 +89,7 @@ async def test_get_keycloak_user_authorized(
],
indirect=True,
)
async def test_get_keycloak_user_expired_access_token(
async def test_keycloak_get_user_expired_access_token(
caplog,
client: AsyncClient,
simple_user: MockUser,
Expand Down Expand Up @@ -129,7 +134,7 @@ async def test_get_keycloak_user_expired_access_token(
],
indirect=True,
)
async def test_get_keycloak_user_inactive(
async def test_keycloak_get_user_inactive(
client: AsyncClient,
simple_user: MockUser,
inactive_user: MockUser,
Expand All @@ -155,3 +160,33 @@ async def test_get_keycloak_user_inactive(
"details": None,
},
}


@responses.activate
@pytest.mark.parametrize(
"settings",
[
{
"auth": {
"provider": KEYCLOAK_PROVIDER,
},
},
],
indirect=True,
)
async def test_keycloak_auth_callback(
client: AsyncClient,
settings: Settings,
mock_keycloak_well_known,
mock_keycloak_realm,
mock_keycloak_token_refresh,
caplog,
):
with caplog.at_level(logging.DEBUG):
response = await client.get(
"/v1/auth/callback",
params={"code": "testcode"},
)

assert response.cookies.get("session"), caplog.text # cookie is set
assert response.status_code == 204, response.json()
Loading