-
Notifications
You must be signed in to change notification settings - Fork 95
feat(auth): Implement secure httpOnly cookie authentication for Okta #1920
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| import json | ||
| import logging | ||
| import os | ||
| import urllib.request | ||
| import urllib.parse | ||
| import base64 | ||
| from http.cookies import SimpleCookie | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| logger.setLevel(os.environ.get('LOG_LEVEL', 'INFO')) | ||
|
|
||
|
|
||
| def handler(event, context): | ||
| """Main Lambda handler - routes requests to appropriate function""" | ||
| path = event.get('path', '') | ||
| method = event.get('httpMethod', '') | ||
|
|
||
| if path == '/auth/token-exchange' and method == 'POST': | ||
| return token_exchange_handler(event) | ||
| elif path == '/auth/logout' and method == 'POST': | ||
| return logout_handler(event) | ||
| elif path == '/auth/userinfo' and method == 'GET': | ||
| return userinfo_handler(event) | ||
| else: | ||
| return error_response(404, 'Not Found', event) | ||
|
|
||
|
|
||
| def error_response(status_code, message, event=None): | ||
| """Return error response with CORS headers""" | ||
| response = { | ||
| 'statusCode': status_code, | ||
| 'headers': get_cors_headers(event) if event else {'Content-Type': 'application/json'}, | ||
| 'body': json.dumps({'error': message}), | ||
| } | ||
| return response | ||
|
|
||
|
|
||
| def get_cors_headers(event): | ||
| """Get CORS headers for response""" | ||
| cloudfront_url = os.environ.get('CLOUDFRONT_URL', '') | ||
arjunp99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return { | ||
| 'Content-Type': 'application/json', | ||
| 'Access-Control-Allow-Origin': cloudfront_url, | ||
| 'Access-Control-Allow-Credentials': 'true', | ||
| 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', | ||
| 'Access-Control-Allow-Headers': 'Content-Type', | ||
| } | ||
|
|
||
|
|
||
| def token_exchange_handler(event): | ||
| """Exchange authorization code for tokens and set httpOnly cookies""" | ||
| try: | ||
| body = json.loads(event.get('body', '{}')) | ||
| code = body.get('code') | ||
| code_verifier = body.get('code_verifier') | ||
|
|
||
| if not code or not code_verifier: | ||
| return error_response(400, 'Missing code or code_verifier', event) | ||
|
|
||
| okta_url = os.environ.get('CUSTOM_AUTH_URL', '') | ||
| client_id = os.environ.get('CUSTOM_AUTH_CLIENT_ID', '') | ||
| redirect_uri = os.environ.get('CUSTOM_AUTH_REDIRECT_URL', '') | ||
|
|
||
| if not okta_url or not client_id: | ||
| return error_response(500, 'Missing Okta configuration', event) | ||
|
|
||
| # Call Okta token endpoint | ||
| token_url = f'{okta_url}/v1/token' | ||
| token_data = { | ||
| 'grant_type': 'authorization_code', | ||
| 'code': code, | ||
| 'code_verifier': code_verifier, | ||
| 'client_id': client_id, | ||
| 'redirect_uri': redirect_uri, | ||
| } | ||
|
|
||
| data = urllib.parse.urlencode(token_data).encode('utf-8') | ||
| req = urllib.request.Request( | ||
| token_url, | ||
| data=data, | ||
| headers={'Content-Type': 'application/x-www-form-urlencoded'}, | ||
| ) | ||
|
|
||
| try: | ||
| with urllib.request.urlopen(req, timeout=10) as response: | ||
| tokens = json.loads(response.read().decode('utf-8')) | ||
| except urllib.error.HTTPError as e: | ||
| error_body = e.read().decode('utf-8') | ||
| logger.error(f'Token exchange failed: {error_body}') | ||
| return error_response(401, 'Authentication failed. Please try again.', event) | ||
|
|
||
| cookies = build_cookies(tokens) | ||
|
|
||
| return { | ||
| 'statusCode': 200, | ||
| 'headers': get_cors_headers(event), | ||
| 'multiValueHeaders': {'Set-Cookie': cookies}, | ||
| 'body': json.dumps({'success': True}), | ||
| } | ||
|
|
||
| except Exception as e: | ||
| logger.error(f'Token exchange error: {str(e)}') | ||
| return error_response(500, 'Internal server error', event) | ||
|
|
||
|
|
||
| def build_cookies(tokens): | ||
| """Build httpOnly cookies for tokens""" | ||
| cookies = [] | ||
| secure = True | ||
| httponly = True | ||
| samesite = 'Lax' | ||
| max_age = 3600 # 1 hour | ||
|
|
||
| for token_name in ['access_token', 'id_token']: | ||
| if tokens.get(token_name): | ||
| cookie = SimpleCookie() | ||
| cookie[token_name] = tokens[token_name] | ||
| cookie[token_name]['path'] = '/' | ||
| cookie[token_name]['secure'] = secure | ||
| cookie[token_name]['httponly'] = httponly | ||
| cookie[token_name]['samesite'] = samesite | ||
| cookie[token_name]['max-age'] = max_age | ||
| cookies.append(cookie[token_name].OutputString()) | ||
|
|
||
| return cookies | ||
|
|
||
|
|
||
| def logout_handler(event): | ||
| """Clear all auth cookies""" | ||
| cookies = [] | ||
| for cookie_name in ['access_token', 'id_token', 'refresh_token']: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also logout from okta. Here we are deleting those cookies but that doesn't mean we will be logging out of Okta. Should we make a call to the okta endpoint to let Okta know that we want to logout ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good point! I'll implement Okta logout by redirecting to Okta's /v1/logout endpoint from the frontend after clearing cookies. This fully ends the Okta session so the user must re-authenticate on next login. The flow will be: Frontend calls /auth/logout to clear cookies |
||
| cookie = SimpleCookie() | ||
| cookie[cookie_name] = '' | ||
| cookie[cookie_name]['path'] = '/' | ||
| cookie[cookie_name]['max-age'] = 0 | ||
| cookies.append(cookie[cookie_name].OutputString()) | ||
|
|
||
| return { | ||
| 'statusCode': 200, | ||
| 'headers': get_cors_headers(event), | ||
| 'multiValueHeaders': {'Set-Cookie': cookies}, | ||
| 'body': json.dumps({'success': True}), | ||
| } | ||
|
|
||
|
|
||
| def userinfo_handler(event): | ||
| """Return user info from id_token cookie""" | ||
| try: | ||
| cookie_header = event.get('headers', {}).get('Cookie') or event.get('headers', {}).get('cookie', '') | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does the headers change between browsers. Asking this since you are fetching both Cookie and cookie key
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, HTTP header names can be passed with different casing depending on the proxy/gateway configuration. API Gateway sometimes normalizes headers to lowercase (cookie) while the HTTP spec uses Cookie. Checking both ensures we handle all cases reliably. |
||
| cookies = SimpleCookie() | ||
| cookies.load(cookie_header) | ||
|
|
||
| id_token_cookie = cookies.get('id_token') | ||
| if not id_token_cookie: | ||
| return error_response(401, 'Not authenticated', event) | ||
|
|
||
| id_token = id_token_cookie.value | ||
|
|
||
| # Decode JWT payload | ||
| parts = id_token.split('.') | ||
| if len(parts) != 3: | ||
| return error_response(401, 'Invalid token format', event) | ||
|
|
||
| payload = parts[1] | ||
| padding = 4 - len(payload) % 4 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a comment and document what you are trying to do here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added comments on this logic. |
||
| if padding != 4: | ||
| payload += '=' * padding | ||
|
|
||
| decoded = base64.urlsafe_b64decode(payload) | ||
| claims = json.loads(decoded) | ||
|
|
||
| email_claim = os.environ.get('CLAIMS_MAPPING_EMAIL', 'email') | ||
| user_id_claim = os.environ.get('CLAIMS_MAPPING_USER_ID', 'sub') | ||
|
|
||
| email = claims.get(email_claim, claims.get('email', claims.get('sub', ''))) | ||
| user_id = claims.get(user_id_claim, claims.get('sub', '')) | ||
|
|
||
| return { | ||
| 'statusCode': 200, | ||
| 'headers': get_cors_headers(event), | ||
| 'body': json.dumps( | ||
| { | ||
| 'email': email, | ||
| 'name': claims.get('name', email), | ||
| 'sub': user_id, | ||
| } | ||
| ), | ||
| } | ||
|
|
||
| except Exception as e: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there are any specific exception raised by base64.urlsafe_b64decode or other package you are using . Catch the important ones
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added specific exception handling: All errors are logged with details but return generic messages to clients. |
||
| logger.error(f'Userinfo error: {str(e)}') | ||
| return error_response(500, 'Internal server error', event) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| Duration, | ||
| RemovalPolicy, | ||
| CfnOutput, | ||
| Fn, | ||
| ) | ||
|
|
||
| from .cdk_asset_trail import setup_cdk_asset_trail | ||
|
|
@@ -30,6 +31,7 @@ def __init__( | |
| custom_waf_rules=None, | ||
| tooling_account_id=None, | ||
| backend_region=None, | ||
| custom_auth=None, | ||
| **kwargs, | ||
| ): | ||
| super().__init__(scope, id, **kwargs) | ||
|
|
@@ -166,6 +168,55 @@ def __init__( | |
| log_file_prefix='cloudfront-logs/frontend', | ||
| ) | ||
|
|
||
| # Add API Gateway behaviors for cookie-based authentication (when using custom_auth) | ||
| if custom_auth and backend_region: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the backend region check for ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The backend_region check is redundant - looking at |
||
| # Get API Gateway URL from SSM parameter (set by backend stack) | ||
| api_gateway_url_param = ssm.StringParameter.from_string_parameter_name( | ||
| self, | ||
| 'ApiGatewayUrlParam', | ||
| string_parameter_name=f'/dataall/{envname}/apiGateway/backendUrl', | ||
| ) | ||
|
|
||
| # Extract API Gateway domain from URL using CloudFormation intrinsic functions | ||
| # Input: https://xyz123.execute-api.us-east-1.amazonaws.com/prod/ | ||
| # Split by '/': ['https:', '', 'xyz123.execute-api.us-east-1.amazonaws.com', 'prod', ''] | ||
| # Select index 2: 'xyz123.execute-api.us-east-1.amazonaws.com' | ||
| api_gateway_origin = origins.HttpOrigin( | ||
| domain_name=Fn.select(2, Fn.split('/', api_gateway_url_param.string_value)), | ||
| origin_path='/prod', | ||
| protocol_policy=cloudfront.OriginProtocolPolicy.HTTPS_ONLY, | ||
| ) | ||
|
|
||
| # Add behavior for /auth/* routes (token exchange, userinfo, logout) | ||
| cloudfront_distribution.add_behavior( | ||
| path_pattern='/auth/*', | ||
| origin=api_gateway_origin, | ||
| cache_policy=cloudfront.CachePolicy.CACHING_DISABLED, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the typical behaviour for auth endpoints to not have caching ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is standard practice. Auth endpoints should never be cached - each request is unique (one-time auth codes, session-specific cookies, user-specific data). Caching would return stale data and break login/logout. |
||
| origin_request_policy=cloudfront.OriginRequestPolicy.ALL_VIEWER_EXCEPT_HOST_HEADER, | ||
| allowed_methods=cloudfront.AllowedMethods.ALLOW_ALL, | ||
| viewer_protocol_policy=cloudfront.ViewerProtocolPolicy.HTTPS_ONLY, | ||
| ) | ||
|
|
||
| # Add behavior for /graphql/* routes | ||
| cloudfront_distribution.add_behavior( | ||
| path_pattern='/graphql/*', | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't have any cloudfront dist behaviour for graphql or search. What's the benefit of adding it here ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is required for httpOnly cookies to work. Browsers only send cookies to same-origin requests. Before, tokens were in localStorage and sent via Authorization header (works cross-origin). Now with httpOnly cookies, the browser won't send them to a different origin (API Gateway). Routing through CloudFront makes frontend and API same-origin, so cookies are sent automatically. |
||
| origin=api_gateway_origin, | ||
| cache_policy=cloudfront.CachePolicy.CACHING_DISABLED, | ||
| origin_request_policy=cloudfront.OriginRequestPolicy.ALL_VIEWER_EXCEPT_HOST_HEADER, | ||
| allowed_methods=cloudfront.AllowedMethods.ALLOW_ALL, | ||
| viewer_protocol_policy=cloudfront.ViewerProtocolPolicy.HTTPS_ONLY, | ||
| ) | ||
|
|
||
| # Add behavior for /search/* routes | ||
| cloudfront_distribution.add_behavior( | ||
| path_pattern='/search/*', | ||
| origin=api_gateway_origin, | ||
| cache_policy=cloudfront.CachePolicy.CACHING_DISABLED, | ||
| origin_request_policy=cloudfront.OriginRequestPolicy.ALL_VIEWER_EXCEPT_HOST_HEADER, | ||
| allowed_methods=cloudfront.AllowedMethods.ALLOW_ALL, | ||
| viewer_protocol_policy=cloudfront.ViewerProtocolPolicy.HTTPS_ONLY, | ||
| ) | ||
|
|
||
| ssm_distribution_id = ssm.StringParameter( | ||
| self, | ||
| f'SSMDistribution{envname}', | ||
|
|
@@ -276,16 +327,12 @@ def __init__( | |
|
|
||
| @staticmethod | ||
| def error_responses(): | ||
| # Only intercept 404 for SPA routing (redirect to index.html) | ||
| # Do NOT intercept 403 - let API Gateway errors pass through | ||
| return [ | ||
| cloudfront.ErrorResponse( | ||
| http_status=404, | ||
| response_http_status=404, | ||
| ttl=Duration.seconds(0), | ||
| response_page_path='/index.html', | ||
| ), | ||
| cloudfront.ErrorResponse( | ||
| http_status=403, | ||
| response_http_status=403, | ||
| response_http_status=200, | ||
| ttl=Duration.seconds(0), | ||
| response_page_path='/index.html', | ||
| ), | ||
|
|
||
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.
Instead of 'Not Found' can we say something more descriptive like Incorrect route for authentication etc ?
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.
Changed to: 'Auth endpoint not found. Valid routes: /auth/token-exchange, /auth/logout, /auth/userinfo'