From b7a3053e7ec4cb57046b920b5b340b42e48b8e48 Mon Sep 17 00:00:00 2001 From: timujeen Date: Tue, 25 Nov 2025 16:35:39 +0000 Subject: [PATCH 1/3] Fix OAuth buttons generating incorrect URLs with locale prefix OAuth routes are defined without locale prefix, but Routes.path() was adding locale by default, causing URLs like /phoenix_kit/en/users/auth/google instead of /phoenix_kit/users/auth/google. This made blog catch-all routes intercept OAuth requests, returning 404. Added locale: :none option to all OAuth button links. --- lib/phoenix_kit_web/components/oauth_buttons.ex | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/phoenix_kit_web/components/oauth_buttons.ex b/lib/phoenix_kit_web/components/oauth_buttons.ex index f7efc8b94..b35b8c38f 100644 --- a/lib/phoenix_kit_web/components/oauth_buttons.ex +++ b/lib/phoenix_kit_web/components/oauth_buttons.ex @@ -54,9 +54,10 @@ defmodule PhoenixKitWeb.Components.OAuthButtons do
<%!-- Google Sign-In Button --%> + <%!-- OAuth routes are non-localized, so we use locale: :none --%> <%= if @google_enabled do %> <.link - href={Routes.path("/users/auth/google")} + href={Routes.path("/users/auth/google", locale: :none)} class="btn btn-outline w-full flex items-center justify-center gap-2 hover:bg-base-200 transition-all hover:scale-[1.02] active:scale-[0.98]" > @@ -66,7 +67,7 @@ defmodule PhoenixKitWeb.Components.OAuthButtons do <%!-- Apple Sign-In Button --%> <%= if @apple_enabled do %> <.link - href={Routes.path("/users/auth/apple")} + href={Routes.path("/users/auth/apple", locale: :none)} class="btn btn-outline w-full flex items-center justify-center gap-2 hover:bg-base-200 transition-all hover:scale-[1.02] active:scale-[0.98]" > @@ -76,7 +77,7 @@ defmodule PhoenixKitWeb.Components.OAuthButtons do <%!-- GitHub Sign-In Button --%> <%= if @github_enabled do %> <.link - href={Routes.path("/users/auth/github")} + href={Routes.path("/users/auth/github", locale: :none)} class="btn btn-outline w-full flex items-center justify-center gap-2 hover:bg-base-200 transition-all hover:scale-[1.02] active:scale-[0.98]" > @@ -86,7 +87,7 @@ defmodule PhoenixKitWeb.Components.OAuthButtons do <%!-- Facebook Sign-In Button --%> <%= if @facebook_enabled do %> <.link - href={Routes.path("/users/auth/facebook")} + href={Routes.path("/users/auth/facebook", locale: :none)} class="btn btn-outline w-full flex items-center justify-center gap-2 hover:bg-base-200 transition-all hover:scale-[1.02] active:scale-[0.98]" > From fe8d0333902839413d838b6c4507a6343e06becb Mon Sep 17 00:00:00 2001 From: timujeen Date: Tue, 25 Nov 2025 17:18:21 +0000 Subject: [PATCH 2/3] Add direct database reads for OAuth credentials to avoid cache race conditions When OAuth settings are updated in the admin panel and providers are reconfigured, the cache may not be invalidated yet. This caused OAuth configuration to use stale credentials from cache. Added get_oauth_credentials_direct/1, has_oauth_credentials_direct?/1, and get_settings_direct/1 functions that bypass cache for security-critical operations. OAuthConfig now uses these direct reads when configuring providers after settings updates. --- lib/phoenix_kit/settings/settings.ex | 128 ++++++++++++++++++++++++++ lib/phoenix_kit/users/oauth_config.ex | 32 ++++--- 2 files changed, 147 insertions(+), 13 deletions(-) diff --git a/lib/phoenix_kit/settings/settings.ex b/lib/phoenix_kit/settings/settings.ex index 01a6bf820..9fbc05ada 100644 --- a/lib/phoenix_kit/settings/settings.ex +++ b/lib/phoenix_kit/settings/settings.ex @@ -464,6 +464,7 @@ defmodule PhoenixKit.Settings do Gets OAuth credentials for a specific provider. Returns a map with all credentials for the given provider. + Uses cache for performance - suitable for non-critical reads. ## Examples @@ -487,6 +488,30 @@ defmodule PhoenixKit.Settings do end end + @doc """ + Gets OAuth credentials directly from database, bypassing cache. + + Use this for security-critical operations where fresh data is required, + such as configuring OAuth providers after settings update. + + This prevents race conditions where cache invalidation hasn't completed + before the credentials are read. + + ## Examples + + iex> PhoenixKit.Settings.get_oauth_credentials_direct(:google) + %{client_id: "google-client-id", client_secret: "google-client-secret"} + """ + def get_oauth_credentials_direct(provider) + when provider in [:google, :apple, :github, :facebook] do + case provider do + :google -> get_google_oauth_credentials_direct() + :apple -> get_apple_oauth_credentials_direct() + :github -> get_github_oauth_credentials_direct() + :facebook -> get_facebook_oauth_credentials_direct() + end + end + defp get_google_oauth_credentials do keys = ["oauth_google_client_id", "oauth_google_client_secret"] defaults = %{"oauth_google_client_id" => "", "oauth_google_client_secret" => ""} @@ -545,9 +570,89 @@ defmodule PhoenixKit.Settings do } end + # Direct database reads for OAuth credentials (bypassing cache) + # Used by OAuthConfig.configure_providers() to avoid race conditions + + defp get_google_oauth_credentials_direct do + keys = ["oauth_google_client_id", "oauth_google_client_secret"] + settings = get_settings_direct(keys) + + %{ + client_id: Map.get(settings, "oauth_google_client_id", ""), + client_secret: Map.get(settings, "oauth_google_client_secret", "") + } + end + + defp get_apple_oauth_credentials_direct do + keys = [ + "oauth_apple_client_id", + "oauth_apple_team_id", + "oauth_apple_key_id", + "oauth_apple_private_key" + ] + + settings = get_settings_direct(keys) + + %{ + client_id: Map.get(settings, "oauth_apple_client_id", ""), + team_id: Map.get(settings, "oauth_apple_team_id", ""), + key_id: Map.get(settings, "oauth_apple_key_id", ""), + private_key: Map.get(settings, "oauth_apple_private_key", "") + } + end + + defp get_github_oauth_credentials_direct do + keys = ["oauth_github_client_id", "oauth_github_client_secret"] + settings = get_settings_direct(keys) + + %{ + client_id: Map.get(settings, "oauth_github_client_id", ""), + client_secret: Map.get(settings, "oauth_github_client_secret", "") + } + end + + defp get_facebook_oauth_credentials_direct do + keys = ["oauth_facebook_app_id", "oauth_facebook_app_secret"] + settings = get_settings_direct(keys) + + %{ + app_id: Map.get(settings, "oauth_facebook_app_id", ""), + app_secret: Map.get(settings, "oauth_facebook_app_secret", "") + } + end + + @doc """ + Gets multiple settings directly from database, bypassing cache. + + Use this for security-critical operations where fresh data is required. + Returns a map with setting keys and their values. + + ## Examples + + iex> PhoenixKit.Settings.get_settings_direct(["oauth_google_client_id", "oauth_google_client_secret"]) + %{"oauth_google_client_id" => "client-id", "oauth_google_client_secret" => "secret"} + """ + def get_settings_direct(keys) when is_list(keys) do + if repo_available?() do + Setting + |> where([s], s.key in ^keys) + |> select([s], {s.key, s.value}) + |> repo().all() + |> Map.new() + else + %{} + end + rescue + error -> + Logger.warning("Failed to get settings directly from DB: #{inspect(error)}") + %{} + end + @doc """ Checks if OAuth credentials are configured for a provider. + Uses cache for performance - suitable for non-critical checks. + ## Examples iex> PhoenixKit.Settings.has_oauth_credentials?(:google) @@ -564,6 +669,29 @@ defmodule PhoenixKit.Settings do end end + @doc """ + Checks if OAuth credentials are configured for a provider, reading directly from database. + + Bypasses cache to ensure fresh data. Use this when configuring OAuth providers + after settings update to avoid race conditions. + + ## Examples + + iex> PhoenixKit.Settings.has_oauth_credentials_direct?(:google) + true + """ + def has_oauth_credentials_direct?(provider) + when provider in [:google, :apple, :github, :facebook] do + credentials = get_oauth_credentials_direct(provider) + + case provider do + :google -> validate_google_credentials(credentials) + :apple -> validate_apple_credentials(credentials) + :github -> validate_github_credentials(credentials) + :facebook -> validate_facebook_credentials(credentials) + end + end + defp validate_google_credentials(credentials) do credentials.client_id != "" and credentials.client_secret != "" end diff --git a/lib/phoenix_kit/users/oauth_config.ex b/lib/phoenix_kit/users/oauth_config.ex index 94a954620..a1c34d72d 100644 --- a/lib/phoenix_kit/users/oauth_config.ex +++ b/lib/phoenix_kit/users/oauth_config.ex @@ -80,39 +80,40 @@ defmodule PhoenixKit.Users.OAuthConfig do end # Build the list of available providers based on configured credentials + # Uses direct database reads to avoid cache race conditions defp build_provider_list do providers = %{} - # Add Google if credentials exist + # Add Google if credentials exist (direct DB read) providers = - if Settings.has_oauth_credentials?(:google) and + if Settings.has_oauth_credentials_direct?(:google) and Settings.get_boolean_setting("oauth_google_enabled", false) do Map.put(providers, :google, {Ueberauth.Strategy.Google, []}) else providers end - # Add Apple if credentials exist + # Add Apple if credentials exist (direct DB read) providers = - if Settings.has_oauth_credentials?(:apple) and + if Settings.has_oauth_credentials_direct?(:apple) and Settings.get_boolean_setting("oauth_apple_enabled", false) do Map.put(providers, :apple, {Ueberauth.Strategy.Apple, []}) else providers end - # Add GitHub if credentials exist + # Add GitHub if credentials exist (direct DB read) providers = - if Settings.has_oauth_credentials?(:github) and + if Settings.has_oauth_credentials_direct?(:github) and Settings.get_boolean_setting("oauth_github_enabled", false) do Map.put(providers, :github, {Ueberauth.Strategy.Github, []}) else providers end - # Add Facebook if credentials exist + # Add Facebook if credentials exist (direct DB read) providers = - if Settings.has_oauth_credentials?(:facebook) and + if Settings.has_oauth_credentials_direct?(:facebook) and Settings.get_boolean_setting("oauth_facebook_enabled", false) do Map.put(providers, :facebook, {Ueberauth.Strategy.Facebook, []}) else @@ -123,9 +124,10 @@ defmodule PhoenixKit.Users.OAuthConfig do end # Configure Google OAuth + # Uses direct DB read to avoid cache race conditions after settings update defp configure_google do if Settings.get_boolean_setting("oauth_google_enabled", false) do - credentials = Settings.get_oauth_credentials(:google) + credentials = Settings.get_oauth_credentials_direct(:google) if credentials.client_id != "" and credentials.client_secret != "" do config = [ @@ -142,9 +144,10 @@ defmodule PhoenixKit.Users.OAuthConfig do end # Configure Apple OAuth + # Uses direct DB read to avoid cache race conditions after settings update defp configure_apple do if Settings.get_boolean_setting("oauth_apple_enabled", false) do - credentials = Settings.get_oauth_credentials(:apple) + credentials = Settings.get_oauth_credentials_direct(:apple) if credentials.client_id != "" and credentials.team_id != "" and @@ -166,9 +169,10 @@ defmodule PhoenixKit.Users.OAuthConfig do end # Configure GitHub OAuth + # Uses direct DB read to avoid cache race conditions after settings update defp configure_github do if Settings.get_boolean_setting("oauth_github_enabled", false) do - credentials = Settings.get_oauth_credentials(:github) + credentials = Settings.get_oauth_credentials_direct(:github) if credentials.client_id != "" and credentials.client_secret != "" do config = [ @@ -185,9 +189,10 @@ defmodule PhoenixKit.Users.OAuthConfig do end # Configure Facebook OAuth + # Uses direct DB read to avoid cache race conditions after settings update defp configure_facebook do if Settings.get_boolean_setting("oauth_facebook_enabled", false) do - credentials = Settings.get_oauth_credentials(:facebook) + credentials = Settings.get_oauth_credentials_direct(:facebook) if credentials.app_id != "" and credentials.app_secret != "" do config = [ @@ -207,6 +212,7 @@ defmodule PhoenixKit.Users.OAuthConfig do Validates OAuth credentials for a specific provider. Returns `{:ok, provider}` if credentials are valid, or `{:error, reason}` if not. + Uses direct database read for accurate validation. ## Examples @@ -217,7 +223,7 @@ defmodule PhoenixKit.Users.OAuthConfig do {:error, "Missing Apple private key"} """ def validate_credentials(provider) when provider in [:google, :apple, :github, :facebook] do - credentials = Settings.get_oauth_credentials(provider) + credentials = Settings.get_oauth_credentials_direct(provider) case provider do :google -> validate_google_credentials(credentials) From aadceb0f2b9e784ca4d11c992cc8959d6095aaf5 Mon Sep 17 00:00:00 2001 From: timujeen Date: Tue, 25 Nov 2025 17:25:20 +0000 Subject: [PATCH 3/3] Use direct DB reads for OAuth credentials throughout the codebase Extended direct database reads for OAuth credentials to all remaining modules that check credentials availability: - OAuthAvailability.available_providers/0 now uses has_oauth_credentials_direct?/1 - OAuthConfigLoader now uses has_oauth_credentials_direct?/1 for provider checks - warm_critical_cache/0 no longer caches OAuth credentials (they use direct reads) This ensures consistent behavior across all OAuth-related code paths and prevents race conditions when credentials are updated through admin UI. --- lib/phoenix_kit/settings/settings.ex | 31 ++++++------------- lib/phoenix_kit/users/oauth_availability.ex | 19 +++++++----- .../workers/oauth_config_loader.ex | 10 +++--- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/lib/phoenix_kit/settings/settings.ex b/lib/phoenix_kit/settings/settings.ex index 9fbc05ada..d5d6c314e 100644 --- a/lib/phoenix_kit/settings/settings.ex +++ b/lib/phoenix_kit/settings/settings.ex @@ -1461,32 +1461,21 @@ defmodule PhoenixKit.Settings do end @doc """ - Warm cache with critical OAuth settings only. + Warm cache with critical settings only. - Returns map of critical OAuth settings for synchronous cache warming. - This is used during startup to ensure OAuth configuration is available - immediately, preventing race conditions with OAuthConfigLoader. + Returns map of critical settings for synchronous cache warming. + This is used during startup to ensure essential configuration is available + immediately. - Only loads OAuth-related settings that are required for provider configuration. + Note: OAuth credentials are NOT cached here because they are read directly + from the database via get_oauth_credentials_direct/1 to avoid race conditions + when credentials are updated through the admin UI. """ def warm_critical_cache do - # Critical OAuth keys that must be loaded synchronously at startup + # Critical keys that must be loaded synchronously at startup + # OAuth credentials are intentionally NOT included - they use direct DB reads critical_keys = [ - # Google OAuth - "oauth_google_client_id", - "oauth_google_client_secret", - # GitHub OAuth - "oauth_github_client_id", - "oauth_github_client_secret", - # Apple OAuth - "oauth_apple_client_id", - "oauth_apple_team_id", - "oauth_apple_key_id", - "oauth_apple_private_key_path", - # Facebook OAuth - "oauth_facebook_app_id", - "oauth_facebook_app_secret", - # OAuth general settings + # OAuth enabled flag only (not credentials) "oauth_enabled" ] diff --git a/lib/phoenix_kit/users/oauth_availability.ex b/lib/phoenix_kit/users/oauth_availability.ex index 73717b878..5d9dd9ecc 100644 --- a/lib/phoenix_kit/users/oauth_availability.ex +++ b/lib/phoenix_kit/users/oauth_availability.ex @@ -49,34 +49,37 @@ defmodule PhoenixKit.Users.OAuthAvailability do try do providers = [] - # Check Google + # Check Google (direct DB read to avoid cache race conditions) providers = - if provider_enabled?(:google) and PhoenixKit.Settings.has_oauth_credentials?(:google) do + if provider_enabled?(:google) and + PhoenixKit.Settings.has_oauth_credentials_direct?(:google) do [:google | providers] else providers end - # Check Apple + # Check Apple (direct DB read to avoid cache race conditions) providers = - if provider_enabled?(:apple) and PhoenixKit.Settings.has_oauth_credentials?(:apple) do + if provider_enabled?(:apple) and + PhoenixKit.Settings.has_oauth_credentials_direct?(:apple) do [:apple | providers] else providers end - # Check GitHub + # Check GitHub (direct DB read to avoid cache race conditions) providers = - if provider_enabled?(:github) and PhoenixKit.Settings.has_oauth_credentials?(:github) do + if provider_enabled?(:github) and + PhoenixKit.Settings.has_oauth_credentials_direct?(:github) do [:github | providers] else providers end - # Check Facebook + # Check Facebook (direct DB read to avoid cache race conditions) providers = if provider_enabled?(:facebook) and - PhoenixKit.Settings.has_oauth_credentials?(:facebook) do + PhoenixKit.Settings.has_oauth_credentials_direct?(:facebook) do [:facebook | providers] else providers diff --git a/lib/phoenix_kit/workers/oauth_config_loader.ex b/lib/phoenix_kit/workers/oauth_config_loader.ex index 2d8a35c06..b10ed3b41 100644 --- a/lib/phoenix_kit/workers/oauth_config_loader.ex +++ b/lib/phoenix_kit/workers/oauth_config_loader.ex @@ -173,12 +173,12 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do # OAuth settings are already loaded via sync_init oauth_enabled = PhoenixKit.Settings.get_setting("oauth_enabled", "false") - # Check that at least one provider's credentials are accessible + # Check that at least one provider's credentials are accessible (direct DB read) has_any_oauth_data = - PhoenixKit.Settings.has_oauth_credentials?(:google) or - PhoenixKit.Settings.has_oauth_credentials?(:apple) or - PhoenixKit.Settings.has_oauth_credentials?(:github) or - PhoenixKit.Settings.has_oauth_credentials?(:facebook) + PhoenixKit.Settings.has_oauth_credentials_direct?(:google) or + PhoenixKit.Settings.has_oauth_credentials_direct?(:apple) or + PhoenixKit.Settings.has_oauth_credentials_direct?(:github) or + PhoenixKit.Settings.has_oauth_credentials_direct?(:facebook) Logger.debug( "OAuth configuration: enabled=#{oauth_enabled}, has_oauth_data=#{has_any_oauth_data}"