feat(webhooks): publish AUTH_INVALIDATED on federated auth failures - #1795
Open
orhanrauf wants to merge 1 commit into
Open
feat(webhooks): publish AUTH_INVALIDATED on federated auth failures#1795orhanrauf wants to merge 1 commit into
orhanrauf wants to merge 1 commit into
Conversation
When federated search detects a credential failure (e.g. Slack returns invalid_auth, an OAuth refresh fails), we now atomically: 1. Flip is_authenticated=False on the source connection so the UI's computed status switches to NEEDS_REAUTH immediately, instead of waiting for the next sync job to record the error. 2. Publish a SourceConnectionLifecycleEvent.auth_invalidated event. Customers subscribed via webhooks can wire this into their re-auth notification flow. Both steps are gated by a per-source-connection Redis lock with a 1 h TTL, so a single broken connection that fails on every search doesn't spam customer webhooks. The notifier fails open (still flips DB + publishes) if Redis is unreachable, and swallows all errors so a search request never 500s because notification had trouble. - SourceConnectionEventType.AUTH_INVALIDATED added to the enum; the webhook EventType auto-derives SOURCE_CONNECTION_AUTH_INVALIDATED. - SourceConnectionLifecycleEvent gets an auth_invalidated() factory and an error_reason field (machine-readable, never contains tokens). - AuthInvalidationNotifier (domains/source_connections/auth_invalidation.py) is the single entry-point for runtime auth failures. - SearchPlanExecutor (v2) calls the notifier for each AUTH_INVALIDATED partial failure, dedup'd per request. - Legacy SearchService.search() replaces the direct is_authenticated flip in _handle_failed_federated_auth with notifier dispatch when one is wired in. - Container/factory wires the notifier; test_container fixture builds a real notifier against fake Redis/sc_repo/event_bus so tests exercise the actual code path.
Contributor
There was a problem hiding this comment.
2 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/airweave/domains/source_connections/auth_invalidation.py">
<violation number="1" location="backend/airweave/domains/source_connections/auth_invalidation.py:81">
P1: Dedupe key set too early and never released on failure. Transient DB/event errors can block retries for 1 hour. Clear dedupe key when notifier exits with failure after winning the lock.</violation>
<violation number="2" location="backend/airweave/domains/source_connections/auth_invalidation.py:82">
P1: DB flip fail still treated as success. Event still publishes and function returns True. This can leave `is_authenticated` unchanged while emitting AUTH_INVALIDATED. Stop and return failure when update fails.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # SET NX EX — only succeeds if the key isn't set. | ||
| # Returns truthy on first detection within the TTL window. | ||
| acquired = await self._redis.set(dedupe_key, "1", ex=DEDUPE_TTL_SECONDS, nx=True) | ||
| except Exception as exc: |
Contributor
There was a problem hiding this comment.
P1: DB flip fail still treated as success. Event still publishes and function returns True. This can leave is_authenticated unchanged while emitting AUTH_INVALIDATED. Stop and return failure when update fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/airweave/domains/source_connections/auth_invalidation.py, line 82:
<comment>DB flip fail still treated as success. Event still publishes and function returns True. This can leave `is_authenticated` unchanged while emitting AUTH_INVALIDATED. Stop and return failure when update fails.</comment>
<file context>
@@ -0,0 +1,146 @@
+ # SET NX EX — only succeeds if the key isn't set.
+ # Returns truthy on first detection within the TTL window.
+ acquired = await self._redis.set(dedupe_key, "1", ex=DEDUPE_TTL_SECONDS, nx=True)
+ except Exception as exc:
+ # Redis is down or misbehaving. Fail open: still flip DB +
+ # publish so we never silently swallow the signal.
</file context>
| try: | ||
| # SET NX EX — only succeeds if the key isn't set. | ||
| # Returns truthy on first detection within the TTL window. | ||
| acquired = await self._redis.set(dedupe_key, "1", ex=DEDUPE_TTL_SECONDS, nx=True) |
Contributor
There was a problem hiding this comment.
P1: Dedupe key set too early and never released on failure. Transient DB/event errors can block retries for 1 hour. Clear dedupe key when notifier exits with failure after winning the lock.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/airweave/domains/source_connections/auth_invalidation.py, line 81:
<comment>Dedupe key set too early and never released on failure. Transient DB/event errors can block retries for 1 hour. Clear dedupe key when notifier exits with failure after winning the lock.</comment>
<file context>
@@ -0,0 +1,146 @@
+ try:
+ # SET NX EX — only succeeds if the key isn't set.
+ # Returns truthy on first detection within the TTL window.
+ acquired = await self._redis.set(dedupe_key, "1", ex=DEDUPE_TTL_SECONDS, nx=True)
+ except Exception as exc:
+ # Redis is down or misbehaving. Fail open: still flip DB +
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When federated search detects a credential failure (e.g. Slack returns `invalid_auth`, an OAuth refresh fails), we now atomically:
Both steps are gated by a per-source-connection Redis lock with a 1 h TTL, so a single broken connection that fails on every search doesn't spam customer webhooks. The notifier fails open (still flips DB + publishes) if Redis is unreachable, and swallows all errors so a search request never 500s because notification had trouble.
What changes
Dedupe semantics (per the design discussion)
Test plan
Follow-ups (deliberately out of scope)
Summary by cubic
Immediately publish AUTH_INVALIDATED webhooks and set
is_authenticated=falsewhen federated auth fails, so customers can trigger re-auth and the UI updates right away. Dedupe notifications per connection via Redis to avoid spam and never block search requests.SourceConnectionEventType.AUTH_INVALIDATEDandSourceConnectionLifecycleEvent.auth_invalidated()with optionalerror_reason.AuthInvalidationNotifierthat dedupes per connection (Redis, 1h TTL), flipsis_authenticated, and publishes the event; fails open and swallows errors.SearchPlanExecutorfor AUTH_INVALIDATED partial failures (deduped within a request).SearchService.search()to use the notifier when provided; otherwise falls back to the direct DB flip.Written for commit 35b62f2. Summary will update on new commits. Review in cubic