Skip to content

Conversation

oiadebayo
Copy link
Contributor

@oiadebayo oiadebayo commented Sep 25, 2025

User description

Description

What - Added live events for okta integration to cover both user and group kinds

Why -

How - Created webhook processors to process relevant events

Type of change

Please leave one option from the following and delete the rest:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New Integration (non-breaking change which adds a new integration)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Non-breaking change (fix of existing functionality that will not change current behavior)
  • Documentation (added/updated documentation)

All tests should be run against the port production environment(using a testing org).

Core testing checklist

  • Integration able to create all default resources from scratch
  • Resync finishes successfully
  • Resync able to create entities
  • Resync able to update entities
  • Resync able to detect and delete entities
  • Scheduled resync able to abort existing resync and start a new one
  • Tested with at least 2 integrations from scratch
  • Tested with Kafka and Polling event listeners
  • Tested deletion of entities that don't pass the selector

Integration testing checklist

  • Integration able to create all default resources from scratch
  • Completed a full resync from a freshly installed integration and it completed successfully
  • Resync able to create entities
  • Resync able to update entities
  • Resync able to detect and delete entities
  • Resync finishes successfully
  • If new resource kind is added or updated in the integration, add example raw data, mapping and expected result to the examples folder in the integration directory.
  • If resource kind is updated, run the integration with the example data and check if the expected result is achieved
  • If new resource kind is added or updated, validate that live-events for that resource are working as expected
  • Docs PR link here

Preflight checklist

  • Handled rate limiting
  • Handled pagination
  • Implemented the code in async
  • Support Multi account

Screenshots

Include screenshots from your environment showing how the resources of the integration will look.

API Documentation

Provide links to the API documentation used for this integration.


PR Type

Enhancement


Description

  • Added live events support for Okta integration

  • Implemented webhook processors for user and group events

  • Created custom webhook manager with Okta verification challenge

  • Added automatic event hook creation and management


Diagram Walkthrough

flowchart LR
  A["Okta Events"] --> B["Custom Webhook Manager"]
  B --> C["User Webhook Processor"]
  B --> D["Group Webhook Processor"]
  C --> E["Port Ocean User Entities"]
  D --> F["Port Ocean Group Entities"]
  G["Event Hook Client"] --> H["Okta Event Hook Creation"]
Loading

File Walkthrough

Relevant files
Enhancement
8 files
integration.py
Added custom webhook manager and handler mixins                   
+53/-4   
main.py
Integrated webhook processors and event hook setup             
+30/-0   
utils.py
Added Okta event types and subscription definitions           
+39/-0   
__init__.py
Created webhook processors package initialization               
+3/-0     
base_webhook_processor.py
Implemented base webhook processor with authentication     
+64/-0   
group_webhook_processor.py
Created group-specific webhook event processor                     
+43/-0   
user_webhook_processor.py
Created user-specific webhook event processor                       
+71/-0   
webhook_client.py
Added Okta event hook management client                                   
+79/-0   
Configuration changes
1 files
spec.yaml
Added webhook secret configuration option                               
+6/-1     

oiadebayo and others added 24 commits September 16, 2025 12:13
Copy link
Contributor

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
Logging full user/group objects at warning level in webhook processors may expose PII in logs. Replace with minimal identifiers (e.g., user_id/group_id) and use debug level.
Authentication robustness: Webhook authentication compares the Authorization header to a shared secret string without timing-safe comparison. Consider using a constant-time compare and clarifying config naming consistency (spec lists oktaWebhookSecret, code reads webhook_secret) to avoid misconfiguration that disables auth.
No other obvious injection or CSRF risks detected.

⚡ Recommended focus areas for review

HTTP Response Consistency

The GET verification branch returns a plain status code integer when the header is missing, while other branches return JSON dicts; this may lead to FastAPI response serialization issues. Consider returning a proper JSON response with an explicit status code and content type.

if request.method == "GET":
    challenge = request.headers.get("x-okta-verification-challenge")
    if challenge:
        logger.info("Responding to Okta verification challenge", webhook_path=path)
        return {"verification": challenge}
    else:
        return status.HTTP_405_METHOD_NOT_ALLOWED

try:
    webhook_event = await WebhookEvent.from_request(request)
    webhook_event.set_timestamp(LiveEventTimestamp.AddedToQueue)
    await self._event_queues[path].put(webhook_event)
    return {"status": "ok"}
except Exception as e:
    logger.exception(f"Error processing webhook: {str(e)}")
    return {"status": "error", "message": str(e)}
Excessive Logging

Using logger.warning to log full user payloads can leak sensitive PII and flood logs. Downgrade to debug and avoid logging full objects; log IDs and minimal context instead.

user = await exporter.get_resource(
    GetUserOptions(
        user_id=user_id,
        include_groups=include_groups,
        include_applications=include_applications,
        fields=fields,
    )
)
logger.warning(f"User data retrieved: {user}")
updated_results.append(user)
Excessive Logging

Logging full group data at warning level can expose sensitive data and increase log volume. Prefer debug level and redact or avoid full payloads.

            if event_type == OktaEventType.GROUP_LIFECYCLE_DELETE.value:
                deleted.append({"id": group_id})
            else:
                group = await exporter.get_resource(group_id)
                logger.warning(f"Group data retrieved: {group}")
                updated.append(group)

return WebhookEventRawResults(updated_raw_results=updated, deleted_raw_results=deleted)

Copy link
Contributor

qodo-merge-pro bot commented Sep 25, 2025

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Avoid replacing core framework components

Avoid replacing the core webhook_manager to handle Okta's GET verification.
Instead, extend the Port Ocean framework to allow registering custom handlers
for different HTTP methods on a webhook path.

Examples:

integrations/okta/integration.py [82-93]
class OktaIntegration(BaseIntegration, OktaHandlerMixin):
    """Okta integration class with custom webhook manager for verification support."""

    def __init__(self, context: PortOceanContext):
        super().__init__(context)
        # Replace the Ocean's webhook manager with our custom one (integration-local change)
        self.context.app.webhook_manager = OktaLiveEventsProcessorManager(
            self.context.app.integration_router,
            signal_handler,
            self.context.config.max_event_processing_seconds,

 ... (clipped 2 lines)

Solution Walkthrough:

Before:

class OktaLiveEventsProcessorManager(LiveEventsProcessorManager):
    """Custom manager to handle Okta verification challenge."""
    def _register_route(self, path: str) -> None:
        async def handle_webhook(request: Request):
            if request.method == "GET":
                # Handle Okta verification challenge
                ...
            else: # POST
                # Process normal webhook event
                ...
        self._router.add_api_route(path, handle_webhook, methods=["POST", "GET"])

class OktaIntegration(BaseIntegration):
    def __init__(self, context: PortOceanContext):
        super().__init__(context)
        # Replace the Ocean's webhook manager with a custom one
        self.context.app.webhook_manager = OktaLiveEventsProcessorManager(...)

After:

# Hypothetical framework extension
# ocean.add_webhook_verification_handler(path, handler_func, method="GET")

# In integration code:
async def handle_okta_verification(request: Request):
    challenge = request.headers.get("x-okta-verification-challenge")
    if challenge:
        return {"verification": challenge}
    # ... handle error

@ocean.on_start()
async def on_start():
    # Register the specific GET handler using the new framework feature
    ocean.add_webhook_verification_handler("/webhook", handle_okta_verification, method="GET")

# The OktaIntegration class no longer needs to replace the webhook_manager.
# The OktaLiveEventsProcessorManager class is removed.
class OktaIntegration(BaseIntegration):
    pass
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a significant architectural flaw—monkey-patching a core framework component—which creates future maintenance risks, and proposes a robust, framework-level solution.

High
Possible issue
Use correct key for webhook secret

Correct the configuration key from webhook_secret to oktaWebhookSecret when
retrieving the webhook secret to ensure the authentication scheme is properly
configured.

integrations/okta/okta/webhook_processors/webhook_client.py [46-52]

-secret = ocean.integration_config.get("webhook_secret")
+secret = ocean.integration_config.get("oktaWebhookSecret")
 if secret:
     channel_config["config"]["authScheme"] = {
         "type": "HEADER",
         "key": "Authorization",
         "value": secret,
     }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical bug where an incorrect configuration key (webhook_secret instead of oktaWebhookSecret) is used, which would cause the webhook authentication to fail silently.

High
Improve webhook handler error handling

Refactor the webhook handler to use HTTPException for errors, return a 400 Bad
Request for missing headers, and avoid leaking exception details in responses.

integrations/okta/integration.py [60-76]

 async def handle_webhook(request: Request) -> dict[str, str]:
+    from fastapi import HTTPException
+
     if request.method == "GET":
         challenge = request.headers.get("x-okta-verification-challenge")
         if challenge:
             logger.info("Responding to Okta verification challenge", webhook_path=path)
             return {"verification": challenge}
         else:
-            return status.HTTP_405_METHOD_NOT_ALLOWED
+            logger.warning("Missing x-okta-verification-challenge header in GET request")
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Missing x-okta-verification-challenge header",
+            )
 
     try:
         webhook_event = await WebhookEvent.from_request(request)
         webhook_event.set_timestamp(LiveEventTimestamp.AddedToQueue)
         await self._event_queues[path].put(webhook_event)
         return {"status": "ok"}
     except Exception as e:
-        logger.exception(f"Error processing webhook: {str(e)}")
-        return {"status": "error", "message": str(e)}
+        logger.exception(f"Error processing webhook: {e}")
+        # Return a generic error to avoid leaking implementation details
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="An error occurred while processing the webhook.",
+        )
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a type hint violation, an incorrect HTTP status code, and a potential information leak, offering improvements that enhance code correctness, robustness, and security.

Medium
General
Move authentication logic to authenticate
Suggestion Impact:The commit relocated authentication logic out of should_process_event into a dedicated verification flow and added header-based validation, aligning with the design intent. However, it implemented a helper _verify_webhook_signature used by should_process_event rather than moving it to authenticate, and it kept using the "webhook_secret" key instead of "oktaWebhookSecret".

code diff:

+    async def authenticate(self, payload: EventPayload, headers: EventHeaders) -> bool:
+        return True
 
-        Okta Event Hooks support an authScheme that forwards a static header (e.g., Authorization)
-        with each call. If a webhook_secret is configured, we expect the Authorization header
-        to match it exactly. If no secret configured, accept events.
+    async def _verify_webhook_signature(self, request: Request) -> bool:
         """
-        if event._original_request is None:
-            return False
+        Validate the authenticity of the webhook payload using the Authorization header.
+        If no secret is configured, validation is bypassed.
+        """
+        secret = ocean.integration_config.get("webhook_secret")
 
-        webhook_secret = ocean.integration_config.get("webhook_secret")
-        if not webhook_secret:
+        if not secret:
             logger.info(
                 "No secret configured for Okta incoming webhooks; accepting event without signature validation."
             )
             return True
 
-        provided = event.headers.get("authorization", "")
-        return provided == webhook_secret
+        provided = request.headers.get("authorization", "")
+        if not provided:
+            logger.error(
+                "Missing 'authorization' header. Webhook authentication failed."
+            )
+            return False
 
-    async def authenticate(self, payload: EventPayload, headers: dict[str, Any]) -> bool:
-        return True
+        return provided == secret
+
+    @abstractmethod
+    async def _should_process_event(self, event: WebhookEvent) -> bool: ...
+
+    async def should_process_event(self, event: WebhookEvent) -> bool:
+        if not (event._original_request and await self._should_process_event(event)):
+            return False
+        return await self._verify_webhook_signature(event._original_request)
 

Relocate the webhook authentication logic from should_process_event to the
authenticate method to align with the AbstractWebhookProcessor base class design
and fix a bug with the secret key.

integrations/okta/okta/webhook_processors/base_webhook_processor.py [35-56]

 async def should_process_event(self, event: WebhookEvent) -> bool:
+    """Checks if the event has an original request attached."""
+    return event._original_request is not None
+
+async def authenticate(self, payload: EventPayload, headers: dict[str, Any]) -> bool:
     """Authenticate webhook using optional Authorization header if provided.
 
     Okta Event Hooks support an authScheme that forwards a static header (e.g., Authorization)
     with each call. If a webhook_secret is configured, we expect the Authorization header
     to match it exactly. If no secret configured, accept events.
     """
-    if event._original_request is None:
-        return False
-
-    webhook_secret = ocean.integration_config.get("webhook_secret")
+    webhook_secret = ocean.integration_config.get("oktaWebhookSecret")
     if not webhook_secret:
         logger.info(
             "No secret configured for Okta incoming webhooks; accepting event without signature validation."
         )
         return True
 
-    provided = event.headers.get("authorization", "")
-    return provided == webhook_secret
+    provided = headers.get("authorization", "")
+    if not provided:
+        logger.warning("Missing Authorization header in webhook request.")
+        return False
 
-async def authenticate(self, payload: EventPayload, headers: dict[str, Any]) -> bool:
-    return True
+    is_valid = provided == webhook_secret
+    if not is_valid:
+        logger.warning("Invalid webhook secret provided.")
 
+    return is_valid
+

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies that authentication logic is misplaced, violating the base class design. Moving it to the authenticate method improves code structure, and the proposed code also fixes a bug by using the correct configuration key oktaWebhookSecret.

Medium
  • Update

Base automatically changed from PORT-16361-add-support-for-okta-resource-resync-new to main October 6, 2025 14:36
@Copilot Copilot AI review requested due to automatic review settings October 7, 2025 12:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds live events support for the Okta integration by implementing webhook processors for both user and group events. The integration can now receive real-time event notifications from Okta and automatically sync changes to Port Ocean entities.

  • Added webhook processors for user and group lifecycle events
  • Implemented custom webhook manager with Okta verification challenge support
  • Created automatic event hook setup and management in Okta

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
main.py Added webhook processor registration and event hook creation on startup
integration.py Implemented custom webhook manager and handler mixins for Okta verification
utils.py Added Okta event types and subscription definitions
webhook_processors/ Created complete webhook processing infrastructure for users and groups
tests/ Added comprehensive test coverage for webhook processors
spec.yaml Added optional webhook secret configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 35 to 53
async def should_process_event(self, event: WebhookEvent) -> bool:
"""Authenticate webhook using optional Authorization header if provided.

Okta Event Hooks support an authScheme that forwards a static header (e.g., Authorization)
with each call. If a webhook_secret is configured, we expect the Authorization header
to match it exactly. If no secret configured, accept events.
"""
if event._original_request is None:
return False

webhook_secret = ocean.integration_config.get("webhook_secret")
if not webhook_secret:
logger.info(
"No secret configured for Okta incoming webhooks; accepting event without signature validation."
)
return True

provided = event.headers.get("authorization", "")
return provided == webhook_secret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use this to check for the event types, whether they matter to the handler

Comment on lines 26 to 32
for event_object in events:
event_type = event_object.get("eventType")
if not isinstance(event_type, str) or event_type not in allowed:
continue
for target in event_object.get("target", []):
if target.get("type") == target_type and target.get("id"):
return True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They send different event types for a single webhook ?

allowed_event_types: Iterable[str],
target_type: str,
) -> bool:
events = payload.get("data", {}).get("events")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the event payload can be empty ?

updated: list[dict[str, Any]] = []
deleted: list[dict[str, Any]] = []

for event_object in payload.get("data", {}).get("events", []):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at this point, we should be 100% sure data and event are not empty, this is check in validate_payload

Comment on lines 29 to 30
event_type = event_object.get("eventType", "")
for target in event_object.get("target", []):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be sure here, use indexing

@github-actions github-actions bot added size/XL and removed size/L labels Oct 13, 2025
Copy link
Member

@mk-armah mk-armah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create utils for the repeated logics if possible

Comment on lines 52 to 54
async def validate_payload(self, payload: EventPayload) -> bool:
"""Basic validation: ensure payload has data.events list."""
return isinstance(payload, dict) and isinstance(payload["data"]["events"], list)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the handlers I see you confidently expect eventType and target to be present,
validate their existence here to eliminate the possibility of a Keyerror in the handlers

Comment on lines 40 to 46
for event_object in payload["data"]["events"]:
event_type = event_object["eventType"]
for target in event_object["target"]:
if target["type"] == "UserGroup" and target["id"]:
group_id = target["id"]
if event_type == OktaEventType.GROUP_LIFECYCLE_DELETE.value:
deleted.append({"id": group_id})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeated logic

targets = event_object["target"]
for target in targets:
if target["type"] == "UserGroup" and target["id"]:
return True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeated logic ?

targets = event_object["target"]
for target in targets:
if target["type"] == "User" and target["id"]:
return True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeated logic ?

for target in targets:
is_user_target = target["type"] == "User" and target["id"]
if not is_user_target:
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this appears to be a repeated logic, lets a util for it

Copy link
Member

@mk-armah mk-armah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 65 to 66
event_type = event_object["eventType"]
targets = event_object["target"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.get

@oiadebayo oiadebayo merged commit e0c3800 into main Oct 13, 2025
23 checks passed
@oiadebayo oiadebayo deleted the PORT-16361-add-support-for-okta-resource-new-live-event branch October 13, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants