Uh oh!
There was an error while loading. Please reload this page.
Auto-detect broken OAuth tokens on refresh failures - #492
Merged
Conversation
Replaces the old @show_maintenance assign approach with a dynamic layout swap via socket.private[:live_layout] — the underlying LiveView keeps running so form state and scroll position are preserved when maintenance toggles on or off. URL never changes. Core changes: - Layout override in on_mount hook instead of redirect. When maintenance turns on, put_in socket.private[:live_layout] swaps the layout live; when it ends, PubSub triggers restoration of the original layout - New PhoenixKitWeb.Layouts :maintenance template with countdown timer - HTTP plug renders inline 503 HTML (with Retry-After header) for controller routes, with proper Phoenix.HTML escaping to prevent XSS - Scheduled maintenance windows with start/end UTC datetimes, 1-year upper bound, and 60-second tolerance for datetime-local minute precision - cleanup_expired_schedule auto-disables stale state on every page access - Process.send_after timer unblocks users when scheduled end arrives (clamped to Erlang's 32-bit timeout limit) - PubSub broadcasts on every state change so all connected LiveViews react instantly; admin tabs stay put, user tabs swap layouts - Manual toggle clears expired schedule on enable to avoid stale locks - Activity logging for all admin actions (toggle, content, schedule) - All user-facing strings wrapped in gettext Schedule validation rejects: empty, past start/end, end before start, dates >1 year in the future. Datetime inputs use the system time_zone setting for display and convert to UTC for storage. Extracts timezone helpers (offset_to_seconds, shift_to_offset, parse_datetime_local, format_datetime_local) to PhoenixKit.Utils.Date so they can be tested in isolation. Adds 93 new tests: unit tests for validate_schedule and PubSub, integration tests for Maintenance context and the plug (including XSS regression test), and doctested timezone helpers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move phx-change from individual inputs to the form element so the live preview updates on every keystroke (input-level phx-change on text inputs only fires on blur, making the preview appear broken) - Remove the "Preview" link that navigated to /maintenance. The path went through locale-prefixed routes and got caught by the publishing module's /:language/:group catch-all. The settings page already has an inline Live Preview card rendering the same content, so the link was redundant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HIGH (merge blockers): - Register MaintenanceCountdown hook in priv/static/assets/phoenix_kit.js alongside the other phoenix_kit hooks. Parent apps already include this file, so the hook is now reliably available (was previously injected by a plug script that wasn't guaranteed to run before LiveSocket init). Remove the fragile inline injection from the Integration plug. - Plug's 503 HTML now uses inline CSS instead of linking to /assets/css/app.css (which wasn't actually served — the real digested path is /assets/app.css). The page is now self-contained with light + dark mode support via prefers-color-scheme, so it works on any route regardless of the parent app's asset pipeline. - Replace String.contains?/2 with String.starts_with?/2 in the plug's auth_route?/1 and static_asset?/1. A parent-app path like /blog/users/log-in-to-us would have bypassed maintenance mode. Add a regression test covering parent-app look-alike paths. MEDIUM: - disable_system/0 now clears maintenance_scheduled_end in addition to maintenance_scheduled_start so a stale end time doesn't surprise-disable the next re-enable. Update the test to assert both fields are cleared. - Track the Process.send_after timer ref in socket assigns and cancel it on reschedule (via new reschedule_maintenance_end_timer/1) so schedule changes don't leave a stale "auto-off" signal in flight. - Settings LiveView's PubSub handler now re-reads header and subtext so multi-admin editing stays in sync. The save handler broadcasts status change to trigger this sync. - schedule_error_message/1 catch-all now logs a warning with the unknown atom so future validation additions surface instead of being silently swallowed. - Document check_maintenance_mode/1's required call sites (all 6 on_mount hooks listed in the @doc) so new live_sessions don't forget it. - Add a HACK comment near put_in socket.private[:live_layout] noting it relies on Phoenix LiveView internals (same pattern as maybe_apply_plugin_layout) and should be revisited on major LV upgrades. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses the final MEDIUM improvement from PR review: instead of each of the 6 on_mount hooks explicitly calling check_maintenance_mode/1, fold it into mount_phoenix_kit_current_scope/3 which all 6 already use. New live_sessions that use a scope-mounting on_mount hook now inherit maintenance mode enforcement automatically — no way to forget it. Removes 6 redundant call sites and updates the @doc comment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Token refresh failures used to be logged and forgotten — the admin UI continued to show the integration as "connected" until they manually clicked Test Connection. Now: - Integrations.record_validation/3 is the single source of truth for writing status + validation_status + last_validated_at, broadcasting integration_validated PubSub events, and no-oping when nothing changed. - refresh_access_token/1 calls it on both success (auto-recovering from a previously-errored state) and failure. - Activity entries integration.token_refresh_failed and integration.auto_recovered added. - IntegrationForm.save_validation_result/3 deleted; Test Connection now uses the same helper.
ddon pushed a commit
that referenced
this pull request
Apr 15, 2026
Follow-up to #492. Three related issues found in review: - record_validation/3 silently no-oped when called with a settings-row UUID (e.g. from authenticated_request -> refresh_access_token via external modules that hold UUIDs). The automatic failure path then never flipped the admin UI, defeating the PR's stated goal. Resolves the key once via resolve_storage/1 which handles both "provider:name" keys and UUIDs, returning the canonical storage key plus decrypted data. - The actor_uuid parameter was ignored. Dropped to arity 2 — the manual path already attributes activity via validate_connection/2, and the two automatic activity entries are correctly auto-attributed. - Events.broadcast_validated fired even when save_integration failed. Guarded behind {:ok, _} so subscribers don't render stale state. Also adds integration tests for record_validation covering success, error formatting, no-op on unchanged status, missing-row case, and the UUID path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ddon pushed a commit
that referenced
this pull request
Apr 15, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation
When a Google (or other OAuth) refresh token becomes invalid — revoked, expired due to 6-month dormancy, password change, etc. — PhoenixKit silently kept showing the integration as "connected" in the admin UI. Every API call 400'd, sync jobs failed, user-facing actions failed, and logs filled with
{:refresh_failed, 400}— but the admin had no signal until they manually clicked "Test Connection" on that integration's settings page.This was a real incident: a broken Google Docs OAuth integration went unnoticed for hours. The only fix was to click Test Connection and then reconnect.
What changes
PhoenixKit.Integrations.record_validation/3is now the single source of truth for writing an integration's health fields (status,validation_status,last_validated_at) and broadcasting theintegration_validatedPubSub event. It's a no-op when nothing changed, so it's safe to call from hot paths without causing DB churn.refresh_access_token/1now wraps the OAuth exchange and callsrecord_validationon both ends:status: "error"with a human-readable reason like"Token refresh failed (HTTP 400)". Admin UI flips to the error badge immediately, no manual test needed.status: "connected"and clears the error message.Two new activity entries for the audit trail:
integration.token_refresh_failedintegration.auto_recoveredThe existing manual "Test Connection" flow in
IntegrationFormhad its ownsave_validation_result/3helper — deleted; it now delegates toIntegrations.record_validation/3. Single code path, same behaviour.