-
Notifications
You must be signed in to change notification settings - Fork 5
Add support for AWS App Load Balancer frontend as auth reverse proxy. #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,12 @@ | |||||||||||
| import httpx | ||||||||||||
| from modules.config.config_manager import config_manager | ||||||||||||
|
|
||||||||||||
| import jwt | ||||||||||||
| import requests | ||||||||||||
| import base64 | ||||||||||||
| import json | ||||||||||||
| import time | ||||||||||||
Check noticeCode scanning / CodeQL Unused import Note
Import of 'time' is not used.
|
||||||||||||
| import time |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect base64 decoding for base64url-encoded JWTs. JWT headers use base64url encoding (URL-safe), not standard base64. Use base64.urlsafe_b64decode() instead of base64.b64decode(), similar to the existing pattern in backend/core/capabilities.py lines 26-28. The current implementation may fail on JWTs containing - or _ characters.
| # Add padding if missing, as Python's b64decode expects it | |
| jwt_headers_decoded = base64.b64decode(jwt_headers_encoded + '===').decode("utf-8") | |
| # Add padding if missing, as Python's urlsafe_b64decode expects it | |
| padded = jwt_headers_encoded + '=' * (-len(jwt_headers_encoded) % 4) | |
| jwt_headers_decoded = base64.urlsafe_b64decode(padded).decode("utf-8") |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential security issue: Manual JWT header parsing is error-prone and unnecessary. Instead of manually decoding the JWT header to extract kid and signer, use PyJWT's built-in jwt.get_unverified_header(encoded_jwt) which safely handles base64url decoding. This reduces the risk of decoding errors and is more maintainable.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logger.error() instead of print() for error messages. The codebase uses structured logging throughout (see lines 48, 51 in the existing code), and error messages should be logged using the logger for consistency and proper log aggregation.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing security: No timeout specified for external HTTP request. The requests.get() call should include a timeout to prevent the authentication from hanging indefinitely if AWS's public key endpoint is slow or unresponsive. Add a timeout parameter, e.g., requests.get(url, timeout=5.0).
| req = requests.get(url) | |
| req = requests.get(url, timeout=5.0) |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use httpx instead of requests for consistency. The codebase uses httpx for HTTP requests (see line 6 and usage in is_user_in_group function). Replace requests.get() with an httpx client to maintain consistency and support async patterns if needed in the future.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for HTTP request failures. The requests.get() call at line 100 should check the response status code before using the text. If the public key fetch fails (e.g., 404, 500), pub_key will contain an error message instead of a valid key, causing JWT validation to fail silently. Add: req.raise_for_status() after line 100 to ensure proper error handling.
| req = requests.get(url) | |
| req = requests.get(url) | |
| req.raise_for_status() |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance concern: Public key is fetched from AWS on every authentication request. Consider implementing a caching mechanism for the public keys (keyed by kid) with an appropriate TTL (e.g., 1 hour). AWS ALB rotates keys infrequently, and caching would significantly reduce latency and external API calls. The cache should handle key rotation by falling back to a fresh fetch if validation fails.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logger.error() instead of print() for error messages. This error logging should be consistent with the existing patterns in this module.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logger.error() instead of print() for error messages. All error logging in this function should use the logger instance defined at line 15.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logger.error() instead of print() for error messages. Consistent with the rest of the module's error handling.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logger.error() instead of print() for error messages. This maintains consistency with the error logging patterns used throughout the codebase.
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logger.error() instead of print() for error messages. All error messages should be logged consistently using the logger.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |||||||||||||||||||||||||||||||||
| from starlette.responses import Response | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from core.auth import get_user_from_header | ||||||||||||||||||||||||||||||||||
| from core.auth import get_user_from_aws_alb_jwt | ||||||||||||||||||||||||||||||||||
| from core.capabilities import verify_file_token | ||||||||||||||||||||||||||||||||||
| from infrastructure.app_factory import app_factory | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -22,6 +23,9 @@ def __init__( | |||||||||||||||||||||||||||||||||
| app, | ||||||||||||||||||||||||||||||||||
| debug_mode: bool = False, | ||||||||||||||||||||||||||||||||||
| auth_header_name: str = "X-User-Email", | ||||||||||||||||||||||||||||||||||
| auth_header_type: str = "email-string", | ||||||||||||||||||||||||||||||||||
| auth_aws_expected_alb_arn: str = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/your-alb-name/...", | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| auth_aws_region: str = "us-east-1", | ||||||||||||||||||||||||||||||||||
| proxy_secret_enabled: bool = False, | ||||||||||||||||||||||||||||||||||
| proxy_secret_header: str = "X-Proxy-Secret", | ||||||||||||||||||||||||||||||||||
| proxy_secret: str = None, | ||||||||||||||||||||||||||||||||||
|
|
@@ -30,6 +34,9 @@ def __init__( | |||||||||||||||||||||||||||||||||
| super().__init__(app) | ||||||||||||||||||||||||||||||||||
| self.debug_mode = debug_mode | ||||||||||||||||||||||||||||||||||
| self.auth_header_name = auth_header_name | ||||||||||||||||||||||||||||||||||
| self.auth_header_type = auth_header_type | ||||||||||||||||||||||||||||||||||
| self.auth_aws_expected_alb_arn = auth_aws_expected_alb_arn | ||||||||||||||||||||||||||||||||||
| self.auth_aws_region = auth_aws_region | ||||||||||||||||||||||||||||||||||
| self.proxy_secret_enabled = proxy_secret_enabled | ||||||||||||||||||||||||||||||||||
| self.proxy_secret_header = proxy_secret_header | ||||||||||||||||||||||||||||||||||
| self.proxy_secret = proxy_secret | ||||||||||||||||||||||||||||||||||
|
|
@@ -83,17 +90,22 @@ async def dispatch(self, request: Request, call_next) -> Response: | |||||||||||||||||||||||||||||||||
| user_email = None | ||||||||||||||||||||||||||||||||||
| if self.debug_mode: | ||||||||||||||||||||||||||||||||||
| # In debug mode, honor auth header if provided, otherwise use config test user | ||||||||||||||||||||||||||||||||||
| x_email_header = request.headers.get(self.auth_header_name) | ||||||||||||||||||||||||||||||||||
| if x_email_header: | ||||||||||||||||||||||||||||||||||
| user_email = get_user_from_header(x_email_header) | ||||||||||||||||||||||||||||||||||
| x_auth_header = request.headers.get(self.auth_header_name) | ||||||||||||||||||||||||||||||||||
| if x_auth_header: | ||||||||||||||||||||||||||||||||||
| user_email = get_user_from_header(x_auth_header) | ||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
| # Get test user from config | ||||||||||||||||||||||||||||||||||
| config_manager = app_factory.get_config_manager() | ||||||||||||||||||||||||||||||||||
| user_email = config_manager.app_settings.test_user | ||||||||||||||||||||||||||||||||||
| # logger.info(f"Debug mode: using user {user_email}") | ||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+95
to
101
|
||||||||||||||||||||||||||||||||||
| user_email = get_user_from_header(x_auth_header) | |
| else: | |
| # Get test user from config | |
| config_manager = app_factory.get_config_manager() | |
| user_email = config_manager.app_settings.test_user | |
| # logger.info(f"Debug mode: using user {user_email}") | |
| else: | |
| if self.auth_header_type == "aws-alb-jwt": | |
| user_email = get_user_from_aws_alb_jwt(x_auth_header, self.auth_aws_expected_alb_arn, self.auth_aws_region) | |
| else: | |
| user_email = get_user_from_header(x_auth_header) | |
| else: | |
| # Get test user from config | |
| config_manager = app_factory.get_config_manager() | |
| user_email = config_manager.app_settings.test_user | |
| # logger.info(f"Debug mode: using user {user_email}") |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -212,6 +212,27 @@ def agent_mode_available(self) -> bool: | |||||||||||||||||||||||||||||||||||||||
| description="HTTP header name to extract authenticated username from reverse proxy", | ||||||||||||||||||||||||||||||||||||||||
| validation_alias="AUTH_USER_HEADER" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| # Authentication header configuration | ||||||||||||||||||||||||||||||||||||||||
| auth_user_header_type: str = Field( | ||||||||||||||||||||||||||||||||||||||||
| default="email-string", | ||||||||||||||||||||||||||||||||||||||||
| description="The datatype stored in AUTH_USER_HEADER", | ||||||||||||||||||||||||||||||||||||||||
| validation_alias="AUTH_USER_HEADER_TYPE" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| # Authentication AWS expected ALB ARN | ||||||||||||||||||||||||||||||||||||||||
| auth_aws_expected_alb_arn: str = Field( | ||||||||||||||||||||||||||||||||||||||||
| default="arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/your-alb-name/...", | ||||||||||||||||||||||||||||||||||||||||
| description="The expected AWS ALB ARN", | ||||||||||||||||||||||||||||||||||||||||
| validation_alias="AUTH_AWS_EXPECTED_ALB_ARN" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+225
to
+229
|
||||||||||||||||||||||||||||||||||||||||
| default="arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/your-alb-name/...", | |
| description="The expected AWS ALB ARN", | |
| validation_alias="AUTH_AWS_EXPECTED_ALB_ARN" | |
| ) | |
| default="", | |
| description="The expected AWS ALB ARN", | |
| validation_alias="AUTH_AWS_EXPECTED_ALB_ARN" | |
| ) | |
| def model_post_init(self, __context): | |
| # Validate that if auth_user_header_type is aws-alb-jwt, ARN must be set and not placeholder | |
| if self.auth_user_header_type == "aws-alb-jwt": | |
| placeholder = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/your-alb-name/..." | |
| if not self.auth_aws_expected_alb_arn or self.auth_aws_expected_alb_arn == placeholder: | |
| raise ValueError( | |
| "auth_aws_expected_alb_arn must be set to a valid AWS ALB ARN when auth_user_header_type is 'aws-alb-jwt'. " | |
| "Current value is empty or a placeholder." | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing dependency
PyJWTin requirements.txt. Thejwtmodule is imported but not listed as a dependency. AddPyJWTto requirements.txt to ensure the application can decode and verify JWT tokens.