Address self-review findings on AWS IAM auth strategy

- Add raise after _handle_credential_error to fix implicit None return
- Add logger.warning on silent us-east-1 region fallback
- Add logger.debug for partial cached credential detection
- Add comment explaining lazy boto3 import
- Fix em-dash in NoCredentialsError user message
- Fix fragile test assertion to match current error message wording
- Remove dead required_fields variable from test_missing_fields
- Add test_optional_param_not_required for optional ConnectorParam path
- Remove unused ExternalDatasetReference import from test file

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: jjdaurora

src/fides/api/schemas/connection_configuration/connection_secrets_saas.py

@@ -162,7 +162,7 @@ def get_saas_schema(self) -> Type[SaaSSchema]:
                         json_schema_extra=extra,
                     ),
                 )
-                if connector_param.default_value
+                if connector_param.default_value or connector_param.optional
                 else (
                     param_type,
                     FieldInfo(

src/fides/api/schemas/saas/saas_config.py

@@ -334,6 +334,7 @@ class ConnectorParam(BaseModel):
     multiselect: Optional[bool] = False
     description: Optional[str] = None
     sensitive: Optional[bool] = False
+    optional: bool = False
     type: Optional[str] = None
     allowed_values: Optional[List[str]] = None
     # type="endpoint" marks this param as a URL endpoint/domain param.

src/fides/api/service/authentication/authentication_strategy_aws_iam.py

@@ -99,6 +99,11 @@ def _get_assumed_role_credentials(
         if cached_key and cached_secret and cached_token and cached_expiry:
             if not self._is_close_to_expiration(cached_expiry):
                 return Credentials(cached_key, cached_secret, cached_token)
+        elif any([cached_key, cached_secret, cached_token, cached_expiry]):
+            logger.debug(
+                "Partial cached credentials found for {}; refreshing.",
+                connection_config.key,
+            )
 
         return self._refresh_assumed_role_credentials(secrets, connection_config)
 
@@ -107,6 +112,7 @@ def _refresh_assumed_role_credentials(
         secrets: Dict[str, Any],
         connection_config: ConnectionConfig,
     ) -> Credentials:
+        # Imported lazily to avoid a hard dependency on boto3 at module load time.
         import boto3
 
         assume_role_arn = secrets["aws_assume_role_arn"]
@@ -155,6 +161,7 @@ def _refresh_assumed_role_credentials(
 
         except (ClientError, NoCredentialsError) as exc:
             self._handle_credential_error(exc, connection_config)
+            raise  # unreachable; _handle_credential_error is NoReturn
 
     def _resolve_region(
         self, url: Optional[str], connection_config: ConnectionConfig
@@ -174,6 +181,12 @@ def _resolve_region(
             if len(parts) >= 4 and parts[-2] == "amazonaws" and parts[-1] == "com":
                 return parts[-3]
 
+        logger.warning(
+            "Could not infer AWS region from URL or secrets for connector {}; "
+            "defaulting to us-east-1. Set aws_region in the connector secrets or "
+            "authentication configuration to avoid this.",
+            connection_config.key,
+        )
         return "us-east-1"
 
     def _is_close_to_expiration(self, expires_at: int) -> bool:
@@ -224,9 +237,12 @@ def _handle_credential_error(
 
         if isinstance(exc, NoCredentialsError):
             user_message = (
-                "No AWS credentials found. Provide either aws_access_key_id "
-                "and aws_secret_access_key, or ensure the Fides environment "
-                "has AWS credentials configured (e.g. via instance profile)."
+                "No base AWS credentials found to authenticate the STS AssumeRole call. "
+                "Providing an IAM Role ARN alone is not sufficient. AWS requires credentials "
+                "to call sts:AssumeRole. Either provide aws_access_key_id and "
+                "aws_secret_access_key alongside the ARN, or ensure the Fides environment "
+                "has ambient AWS credentials configured (e.g. instance profile, environment "
+                "variables, or ~/.aws/credentials)."
             )
         elif isinstance(exc, ClientError):
             error_code = exc.response.get("Error", {}).get("Code", "")

tests/fides/ops/schemas/connection_configuration/test_connection_secrets_saas.py

@@ -10,7 +10,6 @@
 )
 from fides.api.schemas.saas.saas_config import (
     ConnectorParam,
-    ExternalDatasetReference,
     SaaSConfig,
 )
 from fides.config.security_settings import DomainValidationMode
@@ -44,23 +43,24 @@ def test_missing_fields(self, saas_config: SaaSConfig):
         with pytest.raises(ValidationError) as exc:
             schema.model_validate(config)
 
-        required_fields = [
-            connector_param.name
-            for connector_param in (
-                saas_config.connector_params + saas_config.external_references
-            )
-            if isinstance(
-                connector_param, ExternalDatasetReference
-            )  # external refs are required
-            or not connector_param.default_value
-        ]
-
         errors = exc._excinfo[1].errors()
         assert (
             errors[0]["msg"]
             == "Value error, custom_schema must be supplied all of: [username, api_key, api_version, page_size, account_types, customer_id]."
         )
 
+    def test_optional_param_not_required(self, saas_config: SaaSConfig):
+        saas_config.connector_params = [
+            ConnectorParam(name="required_key"),
+            ConnectorParam(name="optional_key", optional=True),
+        ]
+        saas_config.external_references = []
+        schema = SaaSSchemaFactory(saas_config).get_saas_schema()
+        # optional_key absent — should not raise
+        schema.model_validate({"required_key": "value"})
+        # optional_key present — should also work
+        schema.model_validate({"required_key": "value", "optional_key": "val"})
+
     def test_extra_fields(
         self, saas_config: SaaSConfig, saas_example_secrets: Dict[str, Any]
     ):

tests/fides/ops/service/authentication/test_authentication_strategy_aws_iam.py

@@ -59,9 +59,7 @@ def test_default_service(self):
         assert strategy.service == "execute-api"
 
     def test_custom_service(self):
-        strategy = AuthenticationStrategy.get_strategy(
-            "aws_iam", {"service": "lambda"}
-        )
+        strategy = AuthenticationStrategy.get_strategy("aws_iam", {"service": "lambda"})
         assert strategy.service == "lambda"
 
     def test_custom_region(self):
@@ -142,9 +140,7 @@ def test_static_keys_with_session_token(self):
 
 class TestAssumeRole:
     @patch("fides.api.service.authentication.authentication_strategy_aws_iam.boto3")
-    def test_assume_role_signs_request(
-        self, mock_boto3, assume_role_connection_config
-    ):
+    def test_assume_role_signs_request(self, mock_boto3, assume_role_connection_config):
         future_expiry = datetime.now(timezone.utc) + timedelta(hours=1)
         mock_sts = MagicMock()
         mock_sts.assume_role.return_value = {
@@ -253,7 +249,7 @@ def test_assume_role_no_credentials(
 
         with pytest.raises(FidesopsException) as exc:
             strategy.add_authentication(req, assume_role_connection_config)
-        assert "No AWS credentials found" in str(exc.value)
+        assert "No base AWS credentials found" in str(exc.value)
 
 
 class TestCredentialCaching:
@@ -291,9 +287,7 @@ def test_refreshes_credentials_when_close_to_expiration(
     ):
         from botocore.credentials import Credentials
 
-        mock_refresh.return_value = Credentials(
-            "ASIA_NEW", "new_secret", "new_token"
-        )
+        mock_refresh.return_value = Credentials("ASIA_NEW", "new_secret", "new_token")
 
         close_expiry = int(
             (
@@ -325,9 +319,7 @@ def test_refreshes_credentials_when_close_to_expiration(
 
 class TestRegionResolution:
     def test_region_from_configuration(self, static_key_secrets):
-        connection_config = ConnectionConfig(
-            key="test", secrets=static_key_secrets
-        )
+        connection_config = ConnectionConfig(key="test", secrets=static_key_secrets)
         req = Request(
             method="GET",
             url="https://abc123.execute-api.us-east-1.amazonaws.com/prod/users",
@@ -342,9 +334,7 @@ def test_region_from_configuration(self, static_key_secrets):
         assert "ap-southeast-1" in auth_header
 
     def test_region_from_secrets(self, static_key_secrets):
-        connection_config = ConnectionConfig(
-            key="test", secrets=static_key_secrets
-        )
+        connection_config = ConnectionConfig(key="test", secrets=static_key_secrets)
         req = Request(
             method="GET",
             url="https://custom-domain.example.com/users",
@@ -447,9 +437,7 @@ def test_custom_service_name(self, static_key_connection_config):
             url="https://abc123.execute-api.us-west-2.amazonaws.com/prod/users",
         ).prepare()
 
-        strategy = AuthenticationStrategy.get_strategy(
-            "aws_iam", {"service": "lambda"}
-        )
+        strategy = AuthenticationStrategy.get_strategy("aws_iam", {"service": "lambda"})
         authenticated_request = strategy.add_authentication(
             req, static_key_connection_config
         )