Merge pull request #8 from PADAS/refactors-to-auth-handler

GUNDI-3355: Refactor to auth handler

app/actions/client.py

@@ -1,4 +1,5 @@
 from movebank_client import MovebankClient
+from movebank_client.errors import MBClientError, MBForbiddenError
 
 from app.actions.configurations import AuthenticateConfig
 from app.services.errors import ConfigurationNotFound

app/actions/handlers.py

@@ -28,11 +28,18 @@ async def action_auth(integration, action_config: AuthenticateConfig):
         password=action_config.password.get_secret_value(),
     )
 
-    token = await mb_client.get_token()
-
-    if token:
-        logger.info(f"Auth successful for integration '{integration.name}'. Token: '{token['api-token']}'")
-        return {"valid_credentials": True}
+    try:
+        token = await mb_client.get_token()
+    except client.MBForbiddenError:
+        logger.exception(f"Auth unsuccessful for integration {str(integration.id)}. MB returned 403 (wrong credentials)")
+        return {"valid_credentials": False, "message": "Invalid credentials"}
+    except client.MBClientError as e:
+        logger.exception(f"Auth action failed for integration {str(integration.id)}. Exception: {e}")
+        return {"error": "An internal error occurred while trying to test credentials. Please try again later."}
     else:
-        logger.error(f"Auth unsuccessful for integration {integration}.")
-        return {"valid_credentials": False}
+        if token:
+            logger.info(f"Auth successful for integration '{integration.name}'. Token: '{token['api-token']}'")
+            return {"valid_credentials": True}
+        else:
+            logger.error(f"Auth unsuccessful for integration {integration}.")
+            return {"valid_credentials": False}

app/api_schemas.py

@@ -6,4 +6,3 @@ class ActionRequest(BaseModel):
     action_id: str
     run_in_background: bool = False
     config_overrides: dict = None
-    config: dict = None

app/routers/actions.py

@@ -33,13 +33,11 @@ async def execute(
             integration_id=request.integration_id,
             action_id=request.action_id,
             config_overrides=request.config_overrides,
-            config_data=request.config
         )
         return {"message": "Action execution started in background"}
     else:
         return await execute_action(
             integration_id=request.integration_id,
             action_id=request.action_id,
             config_overrides=request.config_overrides,
-            config_data=request.config
         )

app/services/action_runner.py

@@ -16,13 +16,12 @@
 logger = logging.getLogger(__name__)
 
 
-async def execute_action(integration_id: str, action_id: str, config_overrides: dict = None, config_data: dict = None):
+async def execute_action(integration_id: str, action_id: str, config_overrides: dict = None):
     """
     Interface for executing actions.
     :param integration_id: The UUID of the integration
     :param action_id: "test_auth", "pull_observations", "pull_events"
     :param config_overrides: Optional dictionary with configuration overrides
-    :param config_data: Optional dictionary with an already-set configuration (to be passed to action handler as it is)
     :return: action result if any, or raise an exception
     """
     logger.info(f"Executing action '{action_id}' for integration '{integration_id}'...")
@@ -47,35 +46,31 @@ async def execute_action(integration_id: str, action_id: str, config_overrides:
             content=jsonable_encoder({"detail": message}),
         )
 
-    # If "config" variable present in the request, we bypass the integration/config retrieval from the portal
-    if config_data:
-        config = config_data
-    else:
-        # Look for the configuration of the action being executed
-        action_config = find_config_for_action(
-            configurations=integration.configurations,
-            action_id=action_id
+    # Look for the configuration of the action being executed
+    action_config = find_config_for_action(
+        configurations=integration.configurations,
+        action_id=action_id
+    )
+    if not action_config and not config_overrides:
+        message = f"Configuration for action '{action_id}' for integration {str(integration.id)} " \
+                  f"is missing. Please fix the integration setup in the portal or include a config to override."
+        logger.error(message)
+        await publish_event(
+            event=IntegrationActionFailed(
+                payload=ActionExecutionFailed(
+                    integration_id=integration_id,
+                    action_id=action_id,
+                    error=f"Configuration missing for action '{action_id}'",
+                    config_data={"configurations": [i.dict() for i in integration.configurations]},
+                )
+            ),
+            topic_name=settings.INTEGRATION_EVENTS_TOPIC,
+        )
+        return JSONResponse(
+            status_code=status.HTTP_404_NOT_FOUND,
+            content=jsonable_encoder({"detail": message}),
         )
-        if not action_config:
-            message = f"Configuration for action '{action_id}' for integration {str(integration.id)} " \
-                      f"is missing. Please fix the integration setup in the portal."
-            logger.error(message)
-            await publish_event(
-                event=IntegrationActionFailed(
-                    payload=ActionExecutionFailed(
-                        integration_id=integration_id,
-                        action_id=action_id,
-                        error=f"Configuration missing for action '{action_id}'",
-                        config_data={"configurations": [i.dict() for i in integration.configurations]},
-                    )
-                ),
-                topic_name=settings.INTEGRATION_EVENTS_TOPIC,
-            )
-            return JSONResponse(
-                status_code=status.HTTP_404_NOT_FOUND,
-                content=jsonable_encoder({"detail": message}),
-            )
-        config = action_config.data
+    config = action_config.data if action_config else {}
     try:  # Execute the action
         handler, config_model = action_handlers[action_id]
         if config_overrides:

requirements.in

@@ -1 +1 @@
-movebank-client~=1.1.0
+movebank-client~=1.1.1

requirements.txt

@@ -85,7 +85,7 @@ idna==3.7
     #   yarl
 marshmallow==3.21.3
     # via environs
-movebank-client==1.1.0
+movebank-client==1.1.1
     # via -r requirements.in
 multidict==6.0.5
     # via