Skip to content

Commit 17227e4

Browse files
alukachalukach
authored
fix(auth)!: correct HTTP status codes for authentication and authorization failures (#108)
BREAKING CHANGE: Authentication failures now return 401 instead of 403 Changes the EnforceAuthMiddleware to follow HTTP RFC 7235 semantics: - Unauthenticated requests (no/invalid token) now return 401 Unauthorized instead of 403 Forbidden - Authorization failures (valid token, insufficient scopes) now return 403 Forbidden instead of 401 Unauthorized This is a breaking change for clients that expect 403 for unauthenticated requests. The correct behavior is: - 401: "Who are you?" - Authentication required/failed - 403: "I know who you are, but you lack permission" - Authorization failed Additional improvements: - Remove sensitive token data from error logs (security fix) - Handle missing JWT scope claim gracefully (prevent KeyError) - Improve error messages for better debugging - Add detailed logging for authorization failures
1 parent f63154c commit 17227e4

File tree

4 files changed

+76
-52
lines changed

4 files changed

+76
-52
lines changed

‎docs/user-guide/route-level-auth.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ PRIVATE_ENDPOINTS='{
107107
- Users must be authenticated AND have the required scope(s)
108108
- Different HTTP methods can require different scopes
109109
- Scopes are checked against the user's JWT scope claim
110-
- Unauthorized requests receive a 403 Forbidden response
110+
- Unauthorized requests receive a 401 Unauthorized response
111111

112112
> [!TIP]
113113
>

‎src/stac_auth_proxy/middleware/EnforceAuthMiddleware.py‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,19 @@ def validate_token(
127127
if not auth_header:
128128
if auto_error:
129129
raise HTTPException(
130-
status_code=status.HTTP_403_FORBIDDEN,
130+
status_code=status.HTTP_401_UNAUTHORIZED,
131131
detail="Not authenticated",
132+
headers={"WWW-Authenticate": "Bearer"},
132133
)
133134
return None
134135

135136
# Extract token from header
136137
token_parts = auth_header.split(" ")
137138
if len(token_parts) != 2 or token_parts[0].lower() != "bearer":
138-
logger.error("Invalid token: %r", auth_header)
139+
logger.error("Invalid Authorization header format")
139140
raise HTTPException(
140141
status_code=status.HTTP_401_UNAUTHORIZED,
141-
detail="Could not validate credentials",
142+
detail="Invalid Authorization header format",
142143
headers={"WWW-Authenticate": "Bearer"},
143144
)
144145
[_, token] = token_parts
@@ -154,32 +155,43 @@ def validate_token(
154155
audience=self.allowed_jwt_audiences,
155156
)
156157
except jwt.InvalidAudienceError as e:
157-
logger.error("InvalidAudienceError: %r", e)
158+
logger.error("Token audience validation failed: %s", str(e))
158159
raise HTTPException(
159160
status_code=status.HTTP_401_UNAUTHORIZED,
160-
detail="Could not validate Audience",
161+
detail="Invalid token audience",
161162
headers={"WWW-Authenticate": "Bearer"},
162163
)
163164
except (
164165
jwt.exceptions.InvalidTokenError,
165166
jwt.exceptions.DecodeError,
166167
jwt.exceptions.PyJWKClientError,
167168
) as e:
168-
logger.error("InvalidTokenError: %r", e)
169+
logger.error("Token validation failed: %s", type(e).__name__)
169170
raise HTTPException(
170171
status_code=status.HTTP_401_UNAUTHORIZED,
171-
detail="Could not validate credentials",
172+
detail="Invalid or expired token",
172173
headers={"WWW-Authenticate": "Bearer"},
173174
) from e
174175

176+
# Check authorization (scopes)
175177
if required_scopes:
176-
for scope in required_scopes:
177-
if scope not in payload["scope"].split(" "):
178-
raise HTTPException(
179-
status_code=status.HTTP_401_UNAUTHORIZED,
180-
detail="Not enough permissions",
181-
headers={"WWW-Authenticate": f'Bearer scope="{scope}"'},
182-
)
178+
token_scopes = set(payload.get("scope", "").split())
179+
missing_scopes = set(required_scopes) - token_scopes
180+
if missing_scopes:
181+
logger.warning(
182+
"Insufficient scopes for user %s. Required: %s, Has: %s",
183+
payload.get("sub"),
184+
required_scopes,
185+
token_scopes,
186+
)
187+
raise HTTPException(
188+
status_code=status.HTTP_403_FORBIDDEN,
189+
detail=f"Insufficient permissions. Required scopes: {', '.join(missing_scopes)}",
190+
headers={
191+
"WWW-Authenticate": f'Bearer scope="{" ".join(required_scopes)}"'
192+
},
193+
)
194+
183195
return payload
184196

185197
@property

‎tests/test_authn.py‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@
3636
],
3737
)
3838
def test_default_public_false(source_api_server, path, method, token_builder):
39-
"""Private endpoints permit access with a valid token."""
39+
"""Private endpoints require authentication and return 401 when not authenticated."""
4040
test_app = app_factory(upstream_url=source_api_server)
4141
valid_auth_token = token_builder({})
4242

4343
client = TestClient(test_app)
4444
response = client.request(method=method, url=path, headers={})
45-
assert response.status_code == 403
45+
assert response.status_code == 401 # Not authenticated -> 401
4646

4747
response = client.request(
4848
method=method, url=path, headers={"Authorization": f"Bearer {valid_auth_token}"}
@@ -88,7 +88,7 @@ def test_default_public_false(source_api_server, path, method, token_builder):
8888
def test_default_public_false_with_scopes(
8989
source_api_server, rules, token, permitted, token_builder
9090
):
91-
"""Private endpoints permit access with a valid token."""
91+
"""Private endpoints permit access with valid token AND required scopes."""
9292
test_app = app_factory(
9393
upstream_url=source_api_server,
9494
default_public=False,
@@ -102,7 +102,8 @@ def test_default_public_false_with_scopes(
102102
url="/collections",
103103
headers={"Authorization": f"Bearer {valid_auth_token}"},
104104
)
105-
assert response.status_code == (200 if permitted else 401)
105+
# Authenticated but lacking scopes -> 403, not 401
106+
assert response.status_code == (200 if permitted else 403)
106107

107108

108109
@pytest.mark.parametrize(
@@ -151,7 +152,7 @@ def test_scopes(
151152
method,
152153
expected_permitted,
153154
):
154-
"""Private endpoints permit access with a valid token."""
155+
"""Private endpoints require valid token AND required scopes."""
155156
test_app = app_factory(
156157
upstream_url=source_api_server,
157158
default_public=True,
@@ -165,7 +166,8 @@ def test_scopes(
165166
url=path,
166167
headers={"Authorization": f"Bearer {valid_auth_token}"},
167168
)
168-
expected_status_code = 200 if expected_permitted else 401
169+
# User is authenticated, so insufficient scopes -> 403, not 401
170+
expected_status_code = 200 if expected_permitted else 403
169171
assert response.status_code == expected_status_code
170172

171173

@@ -201,10 +203,10 @@ def test_options_bypass_auth(
201203
@pytest.mark.parametrize(
202204
"path,method,default_public,private_endpoints,expected_status",
203205
[
204-
# Test that non-OPTIONS requests still require auth when endpoints are private
205-
("/collections", "GET", False, {}, 403),
206-
("/collections", "POST", False, {}, 403),
207-
("/search", "GET", False, {}, 403),
206+
# Test that non-OPTIONS requests return 401 when not authenticated
207+
("/collections", "GET", False, {}, 401),
208+
("/collections", "POST", False, {}, 401),
209+
("/search", "GET", False, {}, 401),
208210
# Test that OPTIONS requests bypass auth even when endpoints are private
209211
("/collections", "OPTIONS", False, {}, 200),
210212
("/search", "OPTIONS", False, {}, 200),
@@ -214,7 +216,7 @@ def test_options_bypass_auth(
214216
"POST",
215217
True,
216218
{r"^/collections$": [("POST", "collection:create")]},
217-
403,
219+
401,
218220
),
219221
(
220222
"/collections",
@@ -300,7 +302,7 @@ def test_options_vs_other_methods_with_valid_auth(
300302
),
301303
("InvalidFormat", 401),
302304
("Bearer", 401),
303-
("", 403), # No auth header returns 403, not 401
305+
("", 401), # No auth header returns 401 (not authenticated)
304306
],
305307
)
306308
def test_with_invalid_tokens_fails(invalid_token, expected_status, source_api_server):
@@ -401,9 +403,19 @@ def test_jwt_audience_validation(
401403
@pytest.mark.parametrize(
402404
"aud_value,scope,expected_status,description",
403405
[
404-
(["stac-api"], "openid", 401, "Valid audience but missing scope"),
406+
(
407+
["stac-api"],
408+
"openid",
409+
403,
410+
"Valid audience but missing scope (authenticated but not authorized)",
411+
),
405412
(["stac-api"], "collection:create", 200, "Valid audience and valid scope"),
406-
(["wrong-api"], "collection:create", 401, "Invalid audience but valid scope"),
413+
(
414+
["wrong-api"],
415+
"collection:create",
416+
401,
417+
"Invalid audience but valid scope (authentication failed)",
418+
),
407419
],
408420
)
409421
def test_audience_validation_with_scopes(

‎tests/test_defaults.py‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@
1818
("/search", "GET", 200),
1919
("/search", "POST", 200),
2020
("/collections", "GET", 200),
21-
("/collections", "POST", 403),
21+
("/collections", "POST", 401),
2222
("/collections/example-collection", "GET", 200),
23-
("/collections/example-collection", "PUT", 403),
24-
("/collections/example-collection", "DELETE", 403),
23+
("/collections/example-collection", "PUT", 401),
24+
("/collections/example-collection", "DELETE", 401),
2525
("/collections/example-collection/items", "GET", 200),
26-
("/collections/example-collection/items", "POST", 403),
26+
("/collections/example-collection/items", "POST", 401),
2727
("/collections/example-collection/items/example-item", "GET", 200),
28-
("/collections/example-collection/items/example-item", "PUT", 403),
29-
("/collections/example-collection/items/example-item", "DELETE", 403),
30-
("/collections/example-collection/bulk_items", "POST", 403),
28+
("/collections/example-collection/items/example-item", "PUT", 401),
29+
("/collections/example-collection/items/example-item", "DELETE", 401),
30+
("/collections/example-collection/bulk_items", "POST", 401),
3131
("/api.html", "GET", 200),
3232
("/api", "GET", 200),
3333
],
@@ -57,22 +57,22 @@ def test_default_public_true(source_api_server, path, method, expected_status):
5757
@pytest.mark.parametrize(
5858
"path,method,expected_status",
5959
[
60-
("/", "GET", 403),
61-
("/conformance", "GET", 403),
62-
("/queryables", "GET", 403),
63-
("/search", "GET", 403),
64-
("/search", "POST", 403),
65-
("/collections", "GET", 403),
66-
("/collections", "POST", 403),
67-
("/collections/example-collection", "GET", 403),
68-
("/collections/example-collection", "PUT", 403),
69-
("/collections/example-collection", "DELETE", 403),
70-
("/collections/example-collection/items", "GET", 403),
71-
("/collections/example-collection/items", "POST", 403),
72-
("/collections/example-collection/items/example-item", "GET", 403),
73-
("/collections/example-collection/items/example-item", "PUT", 403),
74-
("/collections/example-collection/items/example-item", "DELETE", 403),
75-
("/collections/example-collection/bulk_items", "POST", 403),
60+
("/", "GET", 401),
61+
("/conformance", "GET", 401),
62+
("/queryables", "GET", 401),
63+
("/search", "GET", 401),
64+
("/search", "POST", 401),
65+
("/collections", "GET", 401),
66+
("/collections", "POST", 401),
67+
("/collections/example-collection", "GET", 401),
68+
("/collections/example-collection", "PUT", 401),
69+
("/collections/example-collection", "DELETE", 401),
70+
("/collections/example-collection/items", "GET", 401),
71+
("/collections/example-collection/items", "POST", 401),
72+
("/collections/example-collection/items/example-item", "GET", 401),
73+
("/collections/example-collection/items/example-item", "PUT", 401),
74+
("/collections/example-collection/items/example-item", "DELETE", 401),
75+
("/collections/example-collection/bulk_items", "POST", 401),
7676
("/api.html", "GET", 200),
7777
("/api", "GET", 200),
7878
],

0 commit comments

Comments
 (0)