add mutex around offline user token upsert requests

This can cause bugs if more then one of these requests go out at once, only one per app is valid and if more then one simultaneous request happens only the last requested from shopify is valid.

This moves the get_user_token from JWTSessionToken to UserToken

Author: hez

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Unreleased
 
+- Refactor: adds a mutex around the offline shop token and online user token upsert calls.
+
 ## 0.16.4
 
 - Fix: Add support for larger webhook payload bodies (15MB vs. the previous 8MB)

lib/shopify_api/application.ex

@@ -10,7 +10,10 @@ defmodule ShopifyAPI.Application do
     RateLimiting.GraphQLTracker.init()
 
     # Define workers and child supervisors to be supervised
-    children = []
+    children = [
+      {Mutex, name: ShopifyAPI.OfflineToken},
+      {Mutex, name: ShopifyAPI.OfflineUserToken}
+    ]
 
     # See https://hexdocs.pm/elixir/Supervisor.html
     # for other strategies and supported options

lib/shopify_api/auth_token.ex

@@ -1,4 +1,6 @@
 defmodule ShopifyAPI.AuthToken do
+  require Logger
+
   @derive {Jason.Encoder, only: [:code, :app_name, :shop_name, :token, :timestamp, :plus]}
   defstruct code: "",
             app_name: "",
@@ -32,7 +34,7 @@ defmodule ShopifyAPI.AuthToken do
   def create_key(shop, app), do: "#{shop}:#{app}"
 
   @spec new(App.t(), String.t(), String.t(), String.t()) :: t()
-  def new(app, myshopify_domain, auth_code, token) do
+  def new(%App{} = app, myshopify_domain, auth_code, token) do
     %__MODULE__{
       app_name: app.name,
       shop_name: myshopify_domain,
@@ -42,7 +44,39 @@ defmodule ShopifyAPI.AuthToken do
   end
 
   @spec from_auth_request(App.t(), String.t(), String.t(), map()) :: t()
-  def from_auth_request(app, myshopify_domain, code \\ "", attrs) when is_struct(app, App) do
-    new(app, myshopify_domain, code, attrs["access_token"])
+  def from_auth_request(%App{} = app, myshopify_domain, code \\ "", %{} = attrs),
+    do: new(app, myshopify_domain, code, attrs["access_token"])
+
+  @spec get_offline_token(App.t(), String.t(), String.t()) ::
+          ok_t() | {:error, :failed_fetching_online_token}
+  def get_offline_token(%App{} = app, myshopify_domain, token) do
+    case ShopifyAPI.AuthTokenServer.get(myshopify_domain, app.name) do
+      {:ok, _} = resp -> resp
+      _ -> mutexed_get_offline_token(app, myshopify_domain, token)
+    end
+  end
+
+  defp mutexed_get_offline_token(app, myshopify_domain, token) do
+    mutex_key = {app.name, myshopify_domain}
+
+    Mutex.with_lock(ShopifyAPI.OfflineToken, mutex_key, fn ->
+      # Try fetching a valid token from cache, a new one may have been put in here since the
+      # first call
+      case ShopifyAPI.AuthTokenServer.get(myshopify_domain, app.name) do
+        {:ok, _} = resp -> resp
+        _ -> request_offline_token(app, myshopify_domain, token)
+      end
+    end)
+  end
+
+  defp request_offline_token(app, myshopify_domain, token) do
+    case ShopifyAPI.AuthRequest.request_offline_access_token(app, myshopify_domain, token) do
+      {:ok, token} ->
+        Task.async(fn -> ShopifyAPI.Shop.post_login(token) end)
+        {:ok, token}
+
+      error ->
+        error
+    end
   end
 end

lib/shopify_api/jwt_session_token.ex

@@ -42,68 +42,4 @@ defmodule ShopifyAPI.JWTSessionToken do
 
   def user_id(_),
     do: {:error, "Invalid user token or no id"}
-
-  @spec get_offline_token(JOSE.JWT.t(), String.t()) ::
-          {:ok, ShopifyAPI.AuthToken.t()}
-          | {:error, :invalid_session_token}
-          | {:error, :failed_fetching_online_token}
-  def get_offline_token(%JOSE.JWT{} = jwt, token) do
-    with {:ok, myshopify_domain} <- myshopify_domain(jwt),
-         {:ok, app} <- app(jwt) do
-      case ShopifyAPI.AuthTokenServer.get(myshopify_domain, app.name) do
-        {:ok, _} = resp ->
-          resp
-
-        {:error, _} ->
-          Logger.warning("No token found, exchanging for new")
-
-          case ShopifyAPI.AuthRequest.request_offline_access_token(app, myshopify_domain, token) do
-            {:ok, token} ->
-              fire_post_login_hook(token)
-              {:ok, token}
-
-            error ->
-              error
-          end
-      end
-    else
-      error ->
-        Logger.warning("failed getting required informatio from the JWT #{inspect(error)}")
-        {:error, :invalid_session_token}
-    end
-  end
-
-  @spec get_user_token(JOSE.JWT.t(), String.t()) ::
-          {:ok, ShopifyAPI.UserToken.t()}
-          | {:error, :invalid_session_token}
-          | {:error, :failed_fetching_online_token}
-  def get_user_token(%JOSE.JWT{} = jwt, token) do
-    with {:ok, myshopify_domain} <- myshopify_domain(jwt),
-         {:ok, app} <- app(jwt),
-         {:ok, user_id} <- user_id(jwt) do
-      case ShopifyAPI.UserTokenServer.get_valid(myshopify_domain, app.name, user_id) do
-        {:ok, _} = resp ->
-          resp
-
-        {:error, :invalid_user_token} ->
-          Logger.debug("Expired or no user token found, exchanging for new")
-
-          case ShopifyAPI.AuthRequest.request_online_access_token(app, myshopify_domain, token) do
-            {:ok, user_token} ->
-              fire_post_login_hook(user_token)
-              {:ok, user_token}
-
-            error ->
-              error
-          end
-      end
-    else
-      error ->
-        Logger.warning("failed getting required informatio from the JWT #{inspect(error)}")
-        {:error, :invalid_session_token}
-    end
-  end
-
-  defp fire_post_login_hook(user_token),
-    do: Task.async(fn -> ShopifyAPI.Shop.post_login(user_token) end)
 end

lib/shopify_api/plugs/admin_authenticator.ex

@@ -34,7 +34,9 @@ defmodule ShopifyAPI.Plugs.AdminAuthenticator do
 
   alias Plug.Conn
 
+  alias ShopifyAPI.AuthToken
   alias ShopifyAPI.JWTSessionToken
+  alias ShopifyAPI.UserToken
 
   @defaults [shopify_mount_path: "/shop"]
 
@@ -56,8 +58,8 @@ defmodule ShopifyAPI.Plugs.AdminAuthenticator do
          :ok <- validate_hmac(app, conn.query_params),
          {:ok, myshopify_domain} <- JWTSessionToken.myshopify_domain(jwt),
          {:ok, shop} <- ShopifyAPI.ShopServer.get_or_create(myshopify_domain, true),
-         {:ok, auth_token} <- JWTSessionToken.get_offline_token(jwt, token),
-         {:ok, user_token} <- JWTSessionToken.get_user_token(jwt, token) do
+         {:ok, auth_token} <- get_offline_token(jwt, token),
+         {:ok, user_token} <- get_user_token(jwt, token) do
       conn
       |> assign_app(app)
       |> assign_shop(shop)
@@ -131,4 +133,27 @@ defmodule ShopifyAPI.Plugs.AdminAuthenticator do
       _ -> {:error, :invalid_hmac}
     end)
   end
+
+  defp get_user_token(jwt, token) do
+    with {:ok, app} <- JWTSessionToken.app(jwt),
+         {:ok, myshopify_domain} <- JWTSessionToken.myshopify_domain(jwt),
+         {:ok, user_id} <- JWTSessionToken.user_id(jwt) do
+      UserToken.get_user_token(app, myshopify_domain, user_id, token)
+    else
+      error ->
+        Logger.warning("failed getting required information from the JWT #{inspect(error)}")
+        {:error, :invalid_session_token}
+    end
+  end
+
+  defp get_offline_token(jwt, token) do
+    with {:ok, app} <- JWTSessionToken.app(jwt),
+         {:ok, myshopify_domain} <- JWTSessionToken.myshopify_domain(jwt) do
+      AuthToken.get_offline_token(app, myshopify_domain, token)
+    else
+      error ->
+        Logger.warning("failed getting required information from the JWT #{inspect(error)}")
+        {:error, :invalid_session_token}
+    end
+  end
 end

lib/shopify_api/plugs/auth_shop_session_token.ex

@@ -21,6 +21,7 @@ defmodule ShopifyAPI.Plugs.AuthShopSessionToken do
   alias ShopifyAPI.AuthTokenServer
   alias ShopifyAPI.JWTSessionToken
   alias ShopifyAPI.ShopServer
+  alias ShopifyAPI.UserToken
 
   def init(opts), do: opts
 
@@ -32,7 +33,7 @@ defmodule ShopifyAPI.Plugs.AuthShopSessionToken do
          {:ok, user_id} <- JWTSessionToken.user_id(jwt),
          {:ok, shop} <- ShopServer.get(myshopify_domain),
          {:ok, auth_token} <- AuthTokenServer.get(myshopify_domain, app.name),
-         {:ok, user_token} <- JWTSessionToken.get_user_token(jwt, token) do
+         {:ok, user_token} <- get_user_token(jwt, token) do
       conn
       |> assign(:app, app)
       |> assign(:shop, shop)
@@ -48,4 +49,16 @@ defmodule ShopifyAPI.Plugs.AuthShopSessionToken do
         |> halt()
     end
   end
+
+  defp get_user_token(jwt, token) do
+    with {:ok, app} <- JWTSessionToken.app(jwt),
+         {:ok, myshopify_domain} <- JWTSessionToken.myshopify_domain(jwt),
+         {:ok, user_id} <- JWTSessionToken.user_id(jwt) do
+      UserToken.get_user_token(app, myshopify_domain, user_id, token)
+    else
+      error ->
+        Logger.warning("failed getting required information from the JWT #{inspect(error)}")
+        {:error, :invalid_session_token}
+    end
+  end
 end

lib/shopify_api/user_token.ex

@@ -3,6 +3,7 @@ defmodule ShopifyAPI.UserToken do
   Represents the auth token for individual users, Shopify documentation for the auth process
   is here https://shopify.dev/docs/apps/auth/oauth/getting-started#online-access-mode
   """
+  require Logger
 
   @derive {Jason.Encoder,
            only: [
@@ -79,4 +80,37 @@ defmodule ShopifyAPI.UserToken do
       scope: attrs["scope"]
     )
   end
+
+  @spec get_user_token(App.t(), String.t(), integer(), String.t()) ::
+          {:ok, ShopifyAPI.UserToken.t()} | {:error, :failed_fetching_online_token}
+  def get_user_token(app, myshopify_domain, user_id, token) do
+    case ShopifyAPI.UserTokenServer.get_valid(myshopify_domain, app.name, user_id) do
+      {:ok, _} = resp -> resp
+      _ -> mutexed_get_user_token(app, myshopify_domain, user_id, token)
+    end
+  end
+
+  defp mutexed_get_user_token(app, myshopify_domain, user_id, token) do
+    mutex_key = {app.name, user_id}
+
+    Mutex.with_lock(ShopifyAPI.OfflineUserToken, mutex_key, fn ->
+      # Try fetching a valid token from cache, a new one may have been put in here since the
+      # get_user_token/4 call
+      case ShopifyAPI.UserTokenServer.get_valid(myshopify_domain, app.name, user_id) do
+        {:ok, _} = resp -> resp
+        _ -> request_offline_token(app, myshopify_domain, token)
+      end
+    end)
+  end
+
+  defp request_offline_token(app, myshopify_domain, token) do
+    case ShopifyAPI.AuthRequest.request_online_access_token(app, myshopify_domain, token) do
+      {:ok, user_token} ->
+        Task.async(fn -> ShopifyAPI.Shop.post_login(user_token) end)
+        {:ok, user_token}
+
+      error ->
+        error
+    end
+  end
 end