Skip to content

Commit

Permalink
Add auto_error parameter and fix authorization bug (#63)
Browse files Browse the repository at this point in the history
- Updated `HttpBearerExtractor` to use the `auto_error=False`.
- Fixed bug where bypassable requests sent `None` to the `authorize` function, causing authorization failures.
- Made `authenticate` method more explicit about `id_token_extractor` and removed unnecessary line that returned `None`.
- fix mypy issue
- bump version to 2.0.2

---------

Co-authored-by: Mohammad Torkashvand <[email protected]>
  • Loading branch information
torkashvand and torkashvandmt authored Jun 7, 2024
1 parent 79097ec commit 7b8020a
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .bumpversion.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 2.0.1
current_version = 2.0.2
commit = False
tag = False
parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)(\-(?P<release>[a-z]+)(?P<build>\d+))?
Expand Down
2 changes: 1 addition & 1 deletion oauth2_lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@

"""This is the SURF Oauth2 module that interfaces with the oauth2 setup."""

__version__ = "2.0.1"
__version__ = "2.0.2"
12 changes: 8 additions & 4 deletions oauth2_lib/fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from httpx import AsyncClient, NetworkError
from pydantic import BaseModel
from starlette.requests import ClientDisconnect, HTTPConnection
from starlette.status import HTTP_403_FORBIDDEN
from starlette.websockets import WebSocket
from structlog import get_logger

Expand Down Expand Up @@ -148,7 +149,7 @@ class HttpBearerExtractor(IdTokenExtractor):
"""

async def extract(self, request: Request) -> Optional[str]:
http_bearer = HTTPBearer(auto_error=True)
http_bearer = HTTPBearer(auto_error=False)
credential = await http_bearer(request)

return credential.credentials if credential else None
Expand Down Expand Up @@ -209,12 +210,15 @@ async def authenticate(self, request: HTTPConnection, token: Optional[str] = Non
token_or_extracted_id_token = token
else:
request = cast(Request, request)

if await self.is_bypassable_request(request):
return None

if token is None:
extracted_id_token = await self.id_token_extractor.extract(request)
if not extracted_id_token:
return None
raise HTTPException(status_code=HTTP_403_FORBIDDEN, detail="Not authenticated")

token_or_extracted_id_token = extracted_id_token
else:
token_or_extracted_id_token = token
Expand Down Expand Up @@ -346,7 +350,7 @@ async def authorize(self, request: HTTPConnection, user_info: OIDCUserModel) ->
opa_input = {
"input": {
**(self.opa_kwargs or {}),
**user_info,
**(user_info or {}),
"resource": request.url.path,
"method": request_method,
"arguments": {"path": request.path_params, "query": {**request.query_params}, "json": json},
Expand Down Expand Up @@ -383,7 +387,7 @@ async def authorize(self, request: RequestPath, user_info: OIDCUserModel) -> Opt
opa_input = {
"input": {
**(self.opa_kwargs or {}),
**user_info,
**(user_info or {}),
"resource": request,
"method": "POST",
}
Expand Down
6 changes: 2 additions & 4 deletions tests/test_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,11 @@ async def test_extract_token_success():


@pytest.mark.asyncio
async def test_extract_token_failure():
async def test_extract_token_returns_none():
request = mock.MagicMock()
request.headers = {}
extractor = HttpBearerExtractor()
with pytest.raises(HTTPException) as exc_info:
await extractor.extract(request)
assert exc_info.value.status_code == 403, "Expected HTTP 403 error for missing token"
assert await extractor.extract(request) is None


@pytest.mark.asyncio
Expand Down

0 comments on commit 7b8020a

Please sign in to comment.