From 1eeb777c8d9622d6638d64d125a1a990e07cf377 Mon Sep 17 00:00:00 2001 From: Maarten Jacobs Date: Mon, 19 Feb 2024 00:31:36 +0100 Subject: [PATCH 1/3] do not return a valid access token when it expires within 5 minutes --- lib/ex_oauth2_provider/access_tokens/access_tokens.ex | 2 +- .../access_tokens/access_tokens_test.exs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/ex_oauth2_provider/access_tokens/access_tokens.ex b/lib/ex_oauth2_provider/access_tokens/access_tokens.ex index bc35d24e..f95a17be 100644 --- a/lib/ex_oauth2_provider/access_tokens/access_tokens.ex +++ b/lib/ex_oauth2_provider/access_tokens/access_tokens.ex @@ -120,7 +120,7 @@ defmodule ExOauth2Provider.AccessTokens do queryable |> where([a], is_nil(a.revoked_at)) - |> where([a], is_nil(a.expires_in) or datetime_add(a.inserted_at, a.expires_in, "second") > ^now) + |> where([a], is_nil(a.expires_in) or datetime_add(a.inserted_at, a.expires_in, "second") > datetime_add(^now, 5, "minute")) |> order_by([a], desc: a.inserted_at, desc: :id) |> Config.repo(config).all() |> Enum.filter(&is_accessible?/1) diff --git a/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs b/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs index 6ebe0c07..1a71abe5 100644 --- a/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs +++ b/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs @@ -128,6 +128,17 @@ defmodule ExOauth2Provider.AccessTokensTest do refute AccessTokens.get_application_token_for(Fixtures.application(uid: "application-2"), nil, otp_app: :ex_oauth2_provider) end + + test "does not get the token when it expires within 5 minutes", %{application: application} do + {:ok, %{expires_in: expires_in} = access_token} = + AccessTokens.create_application_token(application, %{}, otp_app: :ex_oauth2_provider) + + inserted_at = QueryHelpers.timestamp(OauthAccessToken, :inserted_at, seconds: -1 * expires_in + 299) + # the token will expire in 4min 59s + QueryHelpers.change!(access_token, inserted_at: inserted_at) + + refute AccessTokens.get_application_token_for(application, nil, otp_app: :ex_oauth2_provider) + end end test "get_authorized_tokens_for/2", %{user: user, application: application} do From b40285c695d4408c37419e772df8ad2d1dfb6d4b Mon Sep 17 00:00:00 2001 From: Maarten Jacobs Date: Mon, 19 Feb 2024 08:38:43 +0100 Subject: [PATCH 2/3] instead of shortening the period an existing valid token will be returned, just always generate a new one when requesting one --- .../token/strategy/client_credentials.ex | 8 +----- .../access_tokens/access_tokens_test.exs | 25 ------------------- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/lib/ex_oauth2_provider/oauth2/token/strategy/client_credentials.ex b/lib/ex_oauth2_provider/oauth2/token/strategy/client_credentials.ex index 4da262bb..845fa958 100644 --- a/lib/ex_oauth2_provider/oauth2/token/strategy/client_credentials.ex +++ b/lib/ex_oauth2_provider/oauth2/token/strategy/client_credentials.ex @@ -38,13 +38,7 @@ defmodule ExOauth2Provider.Token.ClientCredentials do scopes: scopes } - application - |> AccessTokens.get_application_token_for(scopes, config) - |> case do - nil -> AccessTokens.create_application_token(application, token_params, config) - access_token -> {:ok, access_token} - end - |> case do + case AccessTokens.create_application_token(application, token_params, config) do {:ok, access_token} -> {:ok, Map.merge(params, %{access_token: access_token})} {:error, error} -> Error.add_error({:ok, params}, error) end diff --git a/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs b/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs index 1a71abe5..d292861c 100644 --- a/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs +++ b/test/ex_oauth2_provider/access_tokens/access_tokens_test.exs @@ -116,31 +116,6 @@ defmodule ExOauth2Provider.AccessTokensTest do end end - describe "get_application_token_for/3" do - test "fetches", %{application: application} do - {:ok, access_token1} = AccessTokens.create_application_token(application, %{}, otp_app: :ex_oauth2_provider) - inserted_at = QueryHelpers.timestamp(OauthAccessToken, :inserted_at, seconds: -1) - QueryHelpers.change!(access_token1, inserted_at: inserted_at) - {:ok, access_token2} = AccessTokens.create_application_token(application, %{}, otp_app: :ex_oauth2_provider) - - assert %OauthAccessToken{id: id} = AccessTokens.get_application_token_for(application, nil, otp_app: :ex_oauth2_provider) - assert id == access_token2.id - - refute AccessTokens.get_application_token_for(Fixtures.application(uid: "application-2"), nil, otp_app: :ex_oauth2_provider) - end - - test "does not get the token when it expires within 5 minutes", %{application: application} do - {:ok, %{expires_in: expires_in} = access_token} = - AccessTokens.create_application_token(application, %{}, otp_app: :ex_oauth2_provider) - - inserted_at = QueryHelpers.timestamp(OauthAccessToken, :inserted_at, seconds: -1 * expires_in + 299) - # the token will expire in 4min 59s - QueryHelpers.change!(access_token, inserted_at: inserted_at) - - refute AccessTokens.get_application_token_for(application, nil, otp_app: :ex_oauth2_provider) - end - end - test "get_authorized_tokens_for/2", %{user: user, application: application} do {:ok, access_token} = AccessTokens.create_token(user, %{application: application}, otp_app: :ex_oauth2_provider) From ca6e5d08c38511644c514ecbfd3d9951377000bf Mon Sep 17 00:00:00 2001 From: Maarten Jacobs Date: Mon, 19 Feb 2024 08:44:28 +0100 Subject: [PATCH 3/3] revert the 5 minute check on fetching tokens --- .../access_tokens/access_tokens.ex | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/lib/ex_oauth2_provider/access_tokens/access_tokens.ex b/lib/ex_oauth2_provider/access_tokens/access_tokens.ex index f95a17be..a5edeff8 100644 --- a/lib/ex_oauth2_provider/access_tokens/access_tokens.ex +++ b/lib/ex_oauth2_provider/access_tokens/access_tokens.ex @@ -90,26 +90,6 @@ defmodule ExOauth2Provider.AccessTokens do |> load_matching_token_for(application, scopes, config) end - @doc """ - Gets the most recent, acccessible, matching access token for an application. - - ## Examples - - iex> get_application_token_for(application, "read write", otp_app: :my_app) - %OauthAccessToken{} - - iex> get_application_token_for(application, "read invalid", otp_app: :my_app) - nil - """ - @spec get_application_token_for(Application.t(), binary(), keyword()) :: AccessToken.t() | nil - def get_application_token_for(application, scopes, config \\ []) do - config - |> Config.access_token() - |> scope_belongs_to(:resource_owner_id, nil) - |> scope_belongs_to(:application_id, application) - |> load_matching_token_for(application, scopes, config) - end - defp load_matching_token_for(queryable, application, scopes, config) do now = config @@ -120,7 +100,7 @@ defmodule ExOauth2Provider.AccessTokens do queryable |> where([a], is_nil(a.revoked_at)) - |> where([a], is_nil(a.expires_in) or datetime_add(a.inserted_at, a.expires_in, "second") > datetime_add(^now, 5, "minute")) + |> where([a], is_nil(a.expires_in) or datetime_add(a.inserted_at, a.expires_in, "second") > ^now) |> order_by([a], desc: a.inserted_at, desc: :id) |> Config.repo(config).all() |> Enum.filter(&is_accessible?/1)