diff --git a/build_stream/api/auth/jwt_handler.py b/build_stream/api/auth/jwt_handler.py index 6ee35ddfba..63dc0978d2 100644 --- a/build_stream/api/auth/jwt_handler.py +++ b/build_stream/api/auth/jwt_handler.py @@ -134,7 +134,7 @@ def _load_private_key(self) -> str: raise JWTCreationError( f"JWT private key not found: {self.config.private_key_path}" ) from None - except IOError as e: + except IOError: logger.error("Failed to read JWT private key") raise JWTCreationError("Failed to read JWT private key") from None return self._private_key @@ -157,7 +157,7 @@ def _load_public_key(self) -> str: raise JWTValidationError( f"JWT public key not found: {self.config.public_key_path}" ) from None - except IOError as e: + except IOError: logger.error("Failed to read JWT public key") raise JWTValidationError("Failed to read JWT public key") from None return self._public_key @@ -214,7 +214,7 @@ def create_access_token( ) logger.info("Access token created for client: %s", client_id[:8] + "...") return token, int(expires_delta.total_seconds()) - except Exception as e: + except Exception: logger.error("Failed to create access token") raise JWTCreationError("Failed to create access token") from None @@ -253,15 +253,15 @@ def validate_token(self, token: str) -> TokenData: except ExpiredSignatureError: logger.warning("Token has expired") raise JWTExpiredError("Token has expired") from None - except (InvalidAudienceError, InvalidIssuerError) as e: + except (InvalidAudienceError, InvalidIssuerError): logger.warning("Invalid token claims") raise JWTValidationError("Invalid token claims") from None except InvalidSignatureError: logger.warning("Invalid token signature") raise JWTInvalidSignatureError("Invalid token signature") from None - except DecodeError as e: - logger.warning("Invalid token format") - raise JWTValidationError("Invalid token format") from None - except Exception as e: - logger.error("Unexpected error validating token") - raise JWTValidationError("Token validation failed") from None + except DecodeError: + logger.warning("Invalid token format") + raise JWTValidationError("Invalid token format") from None + except Exception: + logger.error("Unexpected error validating token") + raise JWTValidationError("Token validation failed") from None diff --git a/build_stream/api/auth/routes.py b/build_stream/api/auth/routes.py index 1e891e3cd5..6587e253e5 100644 --- a/build_stream/api/auth/routes.py +++ b/build_stream/api/auth/routes.py @@ -20,6 +20,8 @@ from fastapi import APIRouter, Depends, HTTPException, status from fastapi.security import HTTPBasic, HTTPBasicCredentials +from api.vault_client import VaultError + from .schemas import ( AuthErrorResponse, ClientRegistrationRequest, @@ -38,7 +40,6 @@ RegistrationDisabledError, TokenCreationError, ) -from .vault_client import VaultError logger = logging.getLogger(__name__) diff --git a/build_stream/api/auth/service.py b/build_stream/api/auth/service.py index fe423f24dd..3e0587c32d 100644 --- a/build_stream/api/auth/service.py +++ b/build_stream/api/auth/service.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Authentication service for OAuth2 client registration and token generation.""" +"""Authentication service for OAuth2 client management.""" import logging import os @@ -20,9 +20,16 @@ from datetime import datetime, timezone from typing import List, Optional -from .jwt_handler import JWTCreationError, JWTHandler -from .password_handler import generate_credentials, verify_password -from .vault_client import VaultClient, VaultDecryptError, VaultError, VaultNotFoundError +from api.auth.jwt_handler import JWTHandler, JWTCreationError +from api.auth.password_handler import generate_credentials, verify_password +from api.logging_utils import log_secure_info +from api.vault_client import VaultClient, VaultDecryptError, VaultNotFoundError +from core.exceptions import ( + ClientDisabledError, + InvalidClientError, + InvalidScopeError, + TokenCreationError, +) logger = logging.getLogger(__name__) @@ -45,22 +52,6 @@ class RegistrationDisabledError(Exception): """Exception raised when registration is disabled or misconfigured.""" -class InvalidClientError(Exception): - """Exception raised when client credentials are invalid.""" - - -class ClientDisabledError(Exception): - """Exception raised when client account is disabled.""" - - -class InvalidScopeError(Exception): - """Exception raised when requested scope is not allowed.""" - - -class TokenCreationError(Exception): - """Exception raised when token creation fails.""" - - @dataclass class RegisteredClient: """Data class representing a registered OAuth client.""" @@ -187,10 +178,7 @@ def register_client( "is_active": True, } - try: - self.vault_client.save_oauth_client(client_id, client_data) - except VaultError: - raise + self.vault_client.save_oauth_client(client_id, client_data) return RegisteredClient( client_id=client_id, @@ -222,25 +210,26 @@ def verify_client_credentials( try: oauth_clients = self.vault_client.get_oauth_clients() except (VaultNotFoundError, VaultDecryptError): - logger.error("Failed to load OAuth clients from vault") + log_secure_info("error", "Failed to load OAuth clients from vault") + # Ensure no exception details are exposed raise InvalidClientError("Client authentication failed") from None if client_id not in oauth_clients: - logger.warning("Unknown client_id attempted: %s", client_id[:8] + "...") + log_secure_info("warning", "Unknown client_id attempted authentication", client_id) raise InvalidClientError("Client authentication failed") client_data = oauth_clients[client_id] if not client_data.get("is_active", False): - logger.warning("Disabled client attempted token request: %s", client_id[:8] + "...") + log_secure_info("warning", "Disabled client attempted token request", client_id) raise ClientDisabledError("Client account is disabled") stored_hash = client_data.get("client_secret_hash") if not stored_hash or not verify_password(client_secret, stored_hash): - logger.warning("Invalid client secret for: %s", client_id[:8] + "...") + log_secure_info("warning", "Invalid client secret provided", client_id) raise InvalidClientError("Client authentication failed") - logger.info("Client credentials verified: %s", client_id[:8] + "...") + log_secure_info("info", "Client credentials verified successfully", client_id) return client_data def generate_token( @@ -274,10 +263,10 @@ def generate_token( requested_scopes = requested_scope.split() for scope in requested_scopes: if scope not in allowed_scopes: - logger.warning( - "Client %s requested unauthorized scope: %s", - client_id[:8] + "...", - scope, + log_secure_info( + "warning", + f"Client requested unauthorized scope: {scope}", + client_id ) raise InvalidScopeError(f"Scope '{scope}' is not allowed for this client") granted_scopes = requested_scopes @@ -290,11 +279,11 @@ def generate_token( client_name=client_name, scopes=granted_scopes, ) - except JWTCreationError as e: - logger.error("Failed to create access token") + except JWTCreationError: + log_secure_info("error", "Failed to create access token", client_id) raise TokenCreationError("Failed to create access token") from None - logger.info("Token generated for client: %s", client_id[:8] + "...") + log_secure_info("info", "Access token generated successfully", client_id) return TokenResult( access_token=access_token, diff --git a/build_stream/api/dependencies.py b/build_stream/api/dependencies.py index fbac38bcf7..e35b048c5c 100644 --- a/build_stream/api/dependencies.py +++ b/build_stream/api/dependencies.py @@ -26,6 +26,7 @@ JWTInvalidSignatureError, JWTValidationError, ) +from api.logging_utils import log_secure_info logger = logging.getLogger(__name__) @@ -73,7 +74,7 @@ def verify_token( try: token_data = jwt_handler.validate_token(credentials.credentials) - logger.info("Token validated successfully for client: %s", token_data.client_id[:8] + "...") + log_secure_info("info", "Token validated successfully", token_data.client_id) return { "client_id": token_data.client_id, @@ -104,8 +105,8 @@ def verify_token( headers={"WWW-Authenticate": "Bearer"}, ) from None - except JWTValidationError as e: - logger.warning("Token validation failed: %s", str(e)) + except JWTValidationError: + logger.warning("Token validation failed: Invalid token format or content") raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail={ diff --git a/build_stream/api/logging_utils.py b/build_stream/api/logging_utils.py new file mode 100644 index 0000000000..08b6a4c27d --- /dev/null +++ b/build_stream/api/logging_utils.py @@ -0,0 +1,43 @@ +# Copyright 2026 Dell Inc. or its subsidiaries. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Generic secure logging utilities for Build Stream API.""" + +import logging +from typing import Optional + + +def log_secure_info(level: str, message: str, identifier: Optional[str] = None) -> None: + """Log information securely with optional identifier truncation. + + This function provides consistent secure logging across all modules. + When an identifier is provided, only the first 8 characters are logged + to prevent exposure of sensitive data while maintaining debugging capability. + + Args: + level: Log level ('info', 'warning', 'error', 'debug', 'critical') + message: Log message template + identifier: Optional identifier (client_id, token_id, etc.) - first 8 chars logged + """ + logger = logging.getLogger(__name__) + + if identifier: + # Always log first 8 characters for identification + log_message = f"{message}: {identifier[:8]}..." + else: + # Generic message when no identifier context + log_message = message + + log_func = getattr(logger, level) + log_func(log_message) diff --git a/build_stream/api/auth/vault_client.py b/build_stream/api/vault_client.py similarity index 98% rename from build_stream/api/auth/vault_client.py rename to build_stream/api/vault_client.py index fc980d73ba..14cb3049ee 100644 --- a/build_stream/api/auth/vault_client.py +++ b/build_stream/api/vault_client.py @@ -117,7 +117,7 @@ def _run_vault_command( timeout=30, ) return result.stdout - except subprocess.CalledProcessError as e: + except subprocess.CalledProcessError: logger.error("Vault command failed: %s", command) if command == "view": raise VaultDecryptError("Failed to decrypt vault") from None @@ -143,7 +143,7 @@ def read_vault(self, vault_path: str) -> Dict[str, Any]: output = self._run_vault_command("view", vault_path) try: return yaml.safe_load(output) or {} - except yaml.YAMLError as e: + except yaml.YAMLError: logger.error("Failed to parse vault YAML") raise VaultDecryptError("Invalid vault content format") from None @@ -187,7 +187,7 @@ def write_vault(self, vault_path: str, data: Dict[str, Any]) -> None: "--encrypt-vault-id", "default", ] - result = subprocess.run( + subprocess.run( encrypt_cmd, check=True, capture_output=True, diff --git a/build_stream/core/exceptions.py b/build_stream/core/exceptions.py new file mode 100644 index 0000000000..1f53e7a66c --- /dev/null +++ b/build_stream/core/exceptions.py @@ -0,0 +1,31 @@ +# Copyright 2026 Dell Inc. or its subsidiaries. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Core exceptions for the Build Stream API.""" + + +class ClientDisabledError(Exception): + """Exception raised when client account is disabled.""" + + +class InvalidClientError(Exception): + """Exception raised when client credentials are invalid.""" + + +class InvalidScopeError(Exception): + """Exception raised when requested scope is not allowed.""" + + +class TokenCreationError(Exception): + """Exception raised when token creation fails.""" diff --git a/build_stream/tests/conftest.py b/build_stream/tests/conftest.py index e2745b67bc..2339881f5c 100644 --- a/build_stream/tests/conftest.py +++ b/build_stream/tests/conftest.py @@ -22,8 +22,6 @@ """ import base64 -import secrets -import string import sys from pathlib import Path from typing import Dict, Generator @@ -35,65 +33,6 @@ if str(PROJECT_ROOT) not in sys.path: sys.path.insert(0, str(PROJECT_ROOT)) - -def generate_test_client_secret(length: int = 32) -> str: - """Generate a test client secret with proper bld_s_ prefix. - - Args: - length: Total length of the secret including prefix (default: 32) - - Returns: - Test client secret with bld_s_ prefix - """ - if length < 8: - raise ValueError("Client secret length must be at least 8 characters") - - # Generate random part (subtract 6 for "bld_s_" prefix) - random_part_length = max(8, length - 6) - - # Use secure random generation - lowercase = string.ascii_lowercase - uppercase = string.ascii_uppercase - digits = string.digits - special = "!@#$%^&*()_+-=[]{}|;:,.<>?" - - # Start with one of each required character type - password = [ - secrets.choice(lowercase), - secrets.choice(uppercase), - secrets.choice(digits), - secrets.choice(special), - ] - - # Fill remaining length - all_chars = lowercase + uppercase + digits + special - for _ in range(random_part_length - 4): - password.append(secrets.choice(all_chars)) - - # Shuffle to avoid predictable pattern - secrets.SystemRandom().shuffle(password) - - random_part = ''.join(password) - return f"bld_s_{random_part}" - - -def generate_invalid_client_id() -> str: - """Generate an invalid client ID for testing (missing bld_ prefix). - - Returns: - Invalid client ID without proper prefix - """ - return "invalid_client_id_" + ''.join(secrets.choice(string.ascii_lowercase + string.digits) for _ in range(8)) - - -def generate_invalid_client_secret() -> str: - """Generate an invalid client secret for testing (missing bld_s_ prefix). - - Returns: - Invalid client secret without proper prefix - """ - return "invalid_secret_" + ''.join(secrets.choice(string.ascii_lowercase + string.digits) for _ in range(8)) - # Lazy imports to avoid triggering FastAPI route registration # when running E2E tests that don't need these fixtures _APP = None @@ -331,3 +270,30 @@ def valid_token_request() -> Dict: "client_id": None, "client_secret": None, } + + +def generate_test_client_secret() -> str: + """Generate a test client secret that is different from the valid one. + + Returns: + Invalid client secret string for testing (valid format, wrong value). + """ + return "bld_s_invalid_test_secret_12345" + + +def generate_invalid_client_id() -> str: + """Generate an invalid client ID for testing. + + Returns: + Invalid client ID string (contains invalid characters). + """ + return "invalid@client#id" + + +def generate_invalid_client_secret() -> str: + """Generate an invalid client secret for testing. + + Returns: + Invalid client secret string (too short). + """ + return "short" diff --git a/build_stream/tests/integration/test_api_flow_e2e.py b/build_stream/tests/integration/test_api_flow_e2e.py index 4499fab079..619e647e25 100644 --- a/build_stream/tests/integration/test_api_flow_e2e.py +++ b/build_stream/tests/integration/test_api_flow_e2e.py @@ -14,9 +14,9 @@ """End-to-end integration tests for complete API workflow. -These tests validate the complete API workflow from client registration -through token generation and subsequent API calls. This test suite is -designed to be extended as new APIs are added. +These tests validate the complete OAuth2 authentication workflow from client registration +through token generation and validation. This test suite focuses on authentication +and authorization mechanisms, providing comprehensive coverage of the auth API. Usage: pytest tests/integration/test_api_flow_e2e.py -v -m e2e @@ -28,9 +28,28 @@ Test Flow: 1. Health check - Verify server is running - 2. Client Registration - Register a new OAuth client + 2. Client Registration - Register a new OAuth client with proper scopes 3. Token Generation - Obtain access token using client credentials - 4. (Future) Protected API calls using the access token + 4. Token Validation - Verify JWT structure, uniqueness, and scope enforcement + 5. Error Handling - Test various failure scenarios and security validations + 6. Security Validation - Verify proper security measures are enforced + +Test Classes: + - TestCompleteAPIFlow: Main workflow tests (happy path scenarios) + - TestAPIFlowErrorHandling: Error scenario testing + - TestAPIFlowSecurityValidation: Security measure validation + +Key Features Tested: + - OAuth2 client registration with Basic Auth + - JWT token generation with client_credentials grant + - Scope-based authorization (catalog:read, catalog:write) + - Token uniqueness and validation + - Error handling and security measures + - Client credential format validation + - Maximum client limits enforcement + +Note: This test suite focuses specifically on authentication and authorization. +Protected API endpoints (like parse_catalog) are tested separately when implemented. """ # pylint: disable=redefined-outer-name @@ -102,15 +121,21 @@ def api_flow_context(): @pytest.mark.e2e @pytest.mark.integration class TestCompleteAPIFlow: - """End-to-end test suite for complete API workflow. + """End-to-end test suite for complete OAuth2 authentication workflow. - Tests are ordered to follow the natural API flow: - 1. Health check - 2. Client registration - 3. Token generation - 4. (Future) Protected API calls + Tests are ordered to follow the natural authentication flow: + 1. Health check - Verify server is running + 2. Client registration - Register OAuth client with scopes + 3. Token generation - Obtain JWT access token + 4. Token validation - Verify token structure and scopes + 5. Scope enforcement - Test subset and unauthorized scope requests + 6. Security validation - Test invalid credentials and token uniqueness Each test builds on the previous, storing state in the shared context. + This covers the complete authentication and authorization workflow. + + Note: Protected API endpoints are not tested here - they are implemented + separately when the actual endpoints are available. """ def test_01_health_check( @@ -351,9 +376,16 @@ def test_08_multiple_tokens_are_unique( @pytest.mark.e2e @pytest.mark.integration class TestAPIFlowErrorHandling: - """Test error handling across the API flow. + """Test error handling across the OAuth2 authentication flow. - These tests verify proper error responses for various failure scenarios. + These tests verify proper error responses for various failure scenarios: + - Registration without/with invalid authentication + - Token requests for unregistered clients + - Invalid grant types and credentials + - Format validation for client credentials + + Each test ensures that error responses are appropriate and secure, + without exposing sensitive information. """ def test_register_without_auth_fails( @@ -443,9 +475,16 @@ def test_token_with_invalid_grant_type_fails( @pytest.mark.e2e @pytest.mark.integration class TestAPIFlowSecurityValidation: - """Security validation tests for the API flow. + """Security validation tests for the OAuth2 authentication flow. + + These tests verify that security measures are properly enforced: + - Client credential format validation + - Maximum client limits enforcement + - Proper error handling without information disclosure + - Token security and uniqueness validation - These tests verify security measures are properly enforced. + These tests ensure the authentication system follows security best practices + and does not expose sensitive information in error responses. """ def test_client_credentials_format_validation( @@ -454,8 +493,10 @@ def test_client_credentials_format_validation( reset_vault, # noqa: W0613 pylint: disable=unused-argument ): """Verify client credential format validation.""" - from tests.integration.conftest import generate_test_client_secret, generate_invalid_client_id - + from tests.integration.conftest import ( + generate_test_client_secret, + generate_invalid_client_id + ) with httpx.Client(base_url=base_url, timeout=30.0) as client: # Invalid client_id format response = client.post( @@ -476,7 +517,6 @@ def test_client_secret_format_validation( ): """Verify client secret format validation.""" from tests.integration.conftest import generate_invalid_client_secret - with httpx.Client(base_url=base_url, timeout=30.0) as client: response = client.post( "/api/v1/auth/token",