From 4129aa969a50fdee068cd11d3d733440afa71732 Mon Sep 17 00:00:00 2001 From: James Harton <59449+jimsynz@users.noreply.github.com> Date: Thu, 12 Jan 2023 17:23:40 +1300 Subject: [PATCH] feat(GitHub)!: Add GitHub authentication strategy. (#125) --- .formatter.exs | 6 +- config/dev.exs | 11 ++- config/test.exs | 6 +- lib/ash_authentication/dsl.ex | 67 +++++++++++-------- lib/ash_authentication/strategies/oauth2.ex | 18 ++--- .../strategies/oauth2/default.ex | 18 ----- .../strategies/oauth2/plug.ex | 23 ++----- .../strategies/oauth2/transformer.ex | 5 -- .../strategies/oauth2/verifier.ex | 8 +-- .../strategies/oauth2/plug_test.exs | 5 -- test/support/example/generic_oauth_change.ex | 2 +- test/support/example/user.ex | 29 ++++++-- 12 files changed, 97 insertions(+), 101 deletions(-) delete mode 100644 lib/ash_authentication/strategies/oauth2/default.ex diff --git a/.formatter.exs b/.formatter.exs index 6bf07ed..5f432c2 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -5,7 +5,7 @@ spark_locals_without_parens = [ auth0: 2, auth_method: 1, authorization_params: 1, - authorize_path: 1, + authorize_url: 1, client_id: 1, client_secret: 1, confirm_action_name: 1, @@ -60,9 +60,9 @@ spark_locals_without_parens = [ store_token_action_name: 1, subject_name: 1, token_lifetime: 1, - token_path: 1, + token_url: 1, token_resource: 1, - user_path: 1 + user_url: 1 ] [ diff --git a/config/dev.exs b/config/dev.exs index a7be239..a7d0982 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -34,15 +34,20 @@ config :ash_authentication, redirect_uri: "http://localhost:4000/auth", client_secret: System.get_env("OAUTH2_CLIENT_SECRET"), site: System.get_env("OAUTH2_SITE"), - authorize_path: "/authorize", - token_path: "/oauth/token", - user_path: "/userinfo" + authorize_url: "#{System.get_env("OAUTH2_SITE")}/authorize", + token_url: "#{System.get_env("OAUTH2_SITE")}/oauth/token", + user_url: "#{System.get_env("OAUTH2_SITE")}/userinfo" ], auth0: [ client_id: System.get_env("OAUTH2_CLIENT_ID"), redirect_uri: "http://localhost:4000/auth", client_secret: System.get_env("OAUTH2_CLIENT_SECRET"), site: System.get_env("OAUTH2_SITE") + ], + github: [ + client_id: System.get_env("GITHUB_CLIENT_ID"), + client_secret: System.get_env("GITHUB_CLIENT_SECRET"), + redirect_uri: "http://localhost:4000/auth" ] ], tokens: [ diff --git a/config/test.exs b/config/test.exs index 9f152e1..0780c66 100644 --- a/config/test.exs +++ b/config/test.exs @@ -27,9 +27,9 @@ config :ash_authentication, redirect_uri: "http://localhost:4000/auth", client_secret: "pretend client secret", site: "https://example.com/", - authorize_path: "/authorize", - token_path: "/oauth/token", - user_path: "/userinfo" + authorize_url: "https://example.com/authorize", + token_url: "https://example.com/oauth/token", + user_url: "https://example.com/userinfo" ] ], tokens: [ diff --git a/lib/ash_authentication/dsl.ex b/lib/ash_authentication/dsl.ex index b694b94..785e20a 100644 --- a/lib/ash_authentication/dsl.ex +++ b/lib/ash_authentication/dsl.ex @@ -22,7 +22,7 @@ defmodule AshAuthentication.Dsl do OptionsHelpers } - @type strategy :: :confirmation | :oauth2 | :password | :auth0 + @type strategy :: :confirmation | :oauth2 | :password | :auth0 | :github @shared_strategy_options [ name: [ @@ -193,7 +193,8 @@ defmodule AshAuthentication.Dsl do entities: [ strategy(:password), strategy(:oauth2), - strategy(:auth0) + strategy(:auth0), + strategy(:github) ] }, %Section{ @@ -319,15 +320,15 @@ defmodule AshAuthentication.Dsl do args: [{:optional, :name, :oauth2}], target: OAuth2, modules: [ - :authorize_path, + :authorize_url, :client_id, :client_secret, :identity_resource, :private_key, :redirect_uri, :site, - :token_path, - :user_path + :token_url, + :user_url ], schema: OptionsHelpers.merge_schemas( @@ -415,59 +416,56 @@ defmodule AshAuthentication.Dsl do """, required: false ], - authorize_path: [ + authorize_url: [ type: @secret_type, doc: """ - The API path to the OAuth2 authorize endpoint. + The API url to the OAuth2 authorize endpoint. Relative to the value of `site`. - If not set, it defaults to `#{inspect(OAuth2.Default.default(:authorize_path))}`. #{@secret_doc} Example: ```elixir - authorize_path fn _, _ -> {:ok, "/authorize"} end + authorize_url fn _, _ -> {:ok, "https://exampe.com/authorize"} end ``` """, - required: false + required: true ], - token_path: [ + token_url: [ type: @secret_type, doc: """ - The API path to access the token endpoint. + The API url to access the token endpoint. Relative to the value of `site`. - If not set, it defaults to `#{inspect(OAuth2.Default.default(:token_path))}`. #{@secret_doc} Example: ```elixir - token_path fn _, _ -> {:ok, "/oauth_token"} end + token_url fn _, _ -> {:ok, "https://example.com/oauth_token"} end ``` """, - required: false + required: true ], - user_path: [ + user_url: [ type: @secret_type, doc: """ - The API path to access the user endpoint. + The API url to access the user endpoint. Relative to the value of `site`. - If not set, it defaults to `#{inspect(OAuth2.Default.default(:user_path))}`. #{@secret_doc} Example: ```elixir - user_path fn _, _ -> {:ok, "/userinfo"} end + user_url fn _, _ -> {:ok, "https://example.com/userinfo"} end ``` """, - required: false + required: true ], private_key: [ type: @secret_type, @@ -587,7 +585,8 @@ defmodule AshAuthentication.Dsl do ], @shared_strategy_options, "Shared options" - ) + ), + auto_set_fields: [assent_strategy: Assent.Strategy.OAuth2] } end @@ -710,14 +709,24 @@ defmodule AshAuthentication.Dsl do name: :auth0, args: [{:optional, :name, :auth0}], describe: "Auth0 authentication", - auto_set_fields: [ - authorization_params: [scope: "openid profile email"], - auth_method: :client_secret_post, - authorize_path: "/authorize", - token_path: "/oauth/token", - user_path: "/userinfo", - icon: :auth0 - ] + auto_set_fields: strategy_fields(Assent.Strategy.Auth0, icon: :auth0) }) end + + def strategy(:github) do + :oauth2 + |> strategy() + |> Map.merge(%{ + name: :github, + args: [{:optional, :name, :github}], + describe: "GitHub authentication", + auto_set_fields: strategy_fields(Assent.Strategy.Github, icon: :github) + }) + end + + defp strategy_fields(strategy, params) do + params + |> Keyword.put(:assent_strategy, strategy) + |> strategy.default_config() + end end diff --git a/lib/ash_authentication/strategies/oauth2.ex b/lib/ash_authentication/strategies/oauth2.ex index 0a6b7b5..63b930a 100644 --- a/lib/ash_authentication/strategies/oauth2.ex +++ b/lib/ash_authentication/strategies/oauth2.ex @@ -223,9 +223,9 @@ defmodule AshAuthentication.Strategy.OAuth2 do site: nil, auth_method: :client_secret_post, client_secret: nil, - authorize_path: nil, - token_path: nil, - user_path: nil, + authorize_url: nil, + token_url: nil, + user_url: nil, private_key: nil, redirect_uri: nil, authorization_params: [], @@ -238,7 +238,8 @@ defmodule AshAuthentication.Strategy.OAuth2 do provider: :oauth2, name: nil, resource: nil, - icon: nil + icon: nil, + assent_strategy: Assent.Strategy.OAuth2 alias AshAuthentication.Strategy.OAuth2 @@ -254,9 +255,9 @@ defmodule AshAuthentication.Strategy.OAuth2 do | :client_secret_jwt | :private_key_jwt, client_secret: secret, - authorize_path: secret, - token_path: secret, - user_path: secret, + authorize_url: secret, + token_url: secret, + user_url: secret, private_key: secret, redirect_uri: secret, authorization_params: keyword, @@ -269,6 +270,7 @@ defmodule AshAuthentication.Strategy.OAuth2 do provider: atom, name: atom, resource: module, - icon: nil | atom + icon: nil | atom, + assent_strategy: module } end diff --git a/lib/ash_authentication/strategies/oauth2/default.ex b/lib/ash_authentication/strategies/oauth2/default.ex deleted file mode 100644 index 143fc12..0000000 --- a/lib/ash_authentication/strategies/oauth2/default.ex +++ /dev/null @@ -1,18 +0,0 @@ -defmodule AshAuthentication.Strategy.OAuth2.Default do - @moduledoc """ - Sets default values for values which can be configured at runtime and are not set. - """ - - use AshAuthentication.Secret - - @doc false - @impl true - @spec secret_for([atom], Ash.Resource.t(), keyword) :: {:ok, String.t()} | :error - def secret_for(path, _resource, _opts), do: path |> Enum.reverse() |> List.first() |> default() - - @doc false - @spec default(atom) :: {:ok, String.t()} - def default(:authorize_path), do: {:ok, "/oauth/authorize"} - def default(:token_path), do: {:ok, "/oauth/access_token"} - def default(:user_path), do: {:ok, "/user"} -end diff --git a/lib/ash_authentication/strategies/oauth2/plug.ex b/lib/ash_authentication/strategies/oauth2/plug.ex index e2955d6..1589c67 100644 --- a/lib/ash_authentication/strategies/oauth2/plug.ex +++ b/lib/ash_authentication/strategies/oauth2/plug.ex @@ -6,7 +6,6 @@ defmodule AshAuthentication.Strategy.OAuth2.Plug do alias Ash.Error.Framework.AssumptionFailed alias AshAuthentication.{Errors, Info, Strategy, Strategy.OAuth2} alias Assent.{Config, HTTPAdapter.Mint} - alias Assent.Strategy.OAuth2, as: Assent alias Plug.Conn import Ash.PlugHelpers, only: [get_actor: 1, get_tenant: 1] import AshAuthentication.Plug.Helpers, only: [store_authentication_result: 2] @@ -22,7 +21,8 @@ defmodule AshAuthentication.Strategy.OAuth2.Plug do def request(conn, strategy) do with {:ok, config} <- config_for(strategy), {:ok, session_key} <- session_key(strategy), - {:ok, %{session_params: session_params, url: url}} <- Assent.authorize_url(config) do + {:ok, %{session_params: session_params, url: url}} <- + strategy.assent_strategy.authorize_url(config) do conn |> put_session(session_key, session_params) |> put_resp_header("location", url) @@ -46,7 +46,8 @@ defmodule AshAuthentication.Strategy.OAuth2.Plug do session_params when is_map(session_params) <- get_session(conn, session_key), conn <- delete_session(conn, session_key), config <- Config.put(config, :session_params, session_params), - {:ok, %{user: user, token: token}} <- Assent.callback(config, conn.params), + {:ok, %{user: user, token: token}} <- + strategy.assent_strategy.callback(config, conn.params), action_opts <- action_opts(conn), {:ok, user} <- register_or_sign_in_user( @@ -70,9 +71,9 @@ defmodule AshAuthentication.Strategy.OAuth2.Plug do with {:ok, client_id} <- fetch_secret(strategy, :client_id), {:ok, site} <- fetch_secret(strategy, :site), {:ok, redirect_uri} <- build_redirect_uri(strategy), - {:ok, authorize_url} <- build_uri(strategy, :authorize_path), - {:ok, token_url} <- build_uri(strategy, :token_path), - {:ok, user_url} <- build_uri(strategy, :user_path) do + {:ok, authorize_url} <- fetch_secret(strategy, :authorize_url), + {:ok, token_url} <- fetch_secret(strategy, :token_url), + {:ok, user_url} <- fetch_secret(strategy, :user_url) do config = [ auth_method: strategy.auth_method, @@ -154,14 +155,4 @@ defmodule AshAuthentication.Strategy.OAuth2.Plug do {:error, reason} end end - - defp build_uri(strategy, secret_name) do - with {:ok, site} <- fetch_secret(strategy, :site), - {:ok, uri} <- URI.new(site), - {:ok, path} <- fetch_secret(strategy, secret_name) do - path = Path.join(uri.path || "/", path) - - {:ok, to_string(%URI{uri | path: path})} - end - end end diff --git a/lib/ash_authentication/strategies/oauth2/transformer.ex b/lib/ash_authentication/strategies/oauth2/transformer.ex index ee1c756..79fb0df 100644 --- a/lib/ash_authentication/strategies/oauth2/transformer.ex +++ b/lib/ash_authentication/strategies/oauth2/transformer.ex @@ -82,12 +82,7 @@ defmodule AshAuthentication.Strategy.OAuth2.Transformer do end defp set_defaults(strategy) do - default_secret = {OAuth2.Default, []} - strategy - |> maybe_set_field(:authorize_path, default_secret) - |> maybe_set_field(:token_path, default_secret) - |> maybe_set_field(:user_path, default_secret) |> maybe_set_field_lazy(:register_action_name, &:"register_with_#{&1.name}") |> maybe_set_field_lazy(:sign_in_action_name, &:"sign_in_with_#{&1.name}") end diff --git a/lib/ash_authentication/strategies/oauth2/verifier.ex b/lib/ash_authentication/strategies/oauth2/verifier.ex index 76cf1d4..f8f8099 100644 --- a/lib/ash_authentication/strategies/oauth2/verifier.ex +++ b/lib/ash_authentication/strategies/oauth2/verifier.ex @@ -44,13 +44,13 @@ defmodule AshAuthentication.Strategy.OAuth2.Verifier do end defp transform_strategy(strategy) do - with :ok <- validate_secret(strategy, :authorize_path), + with :ok <- validate_secret(strategy, :authorize_url), :ok <- validate_secret(strategy, :client_id), :ok <- validate_secret(strategy, :client_secret), :ok <- validate_secret(strategy, :redirect_uri), :ok <- validate_secret(strategy, :site), - :ok <- validate_secret(strategy, :token_path), - :ok <- validate_secret(strategy, :user_path) do + :ok <- validate_secret(strategy, :token_url), + :ok <- validate_secret(strategy, :user_url) do validate_secret(strategy, :private_key, strategy.auth_method != :private_key_jwt) end end @@ -71,7 +71,7 @@ defmodule AshAuthentication.Strategy.OAuth2.Verifier do DslError.exception( path: [:authentication, :strategies, :oauth2], message: - "Expected `#{inspect(option)}` to be either a string or a module which implements the `AshAuthentication.Sender` behaviour." + "Expected `#{inspect(option)}` to be either a string or a module which implements the `AshAuthentication.Secret` behaviour." )} end end diff --git a/test/ash_authentication/strategies/oauth2/plug_test.exs b/test/ash_authentication/strategies/oauth2/plug_test.exs index 558a4b4..3b18376 100644 --- a/test/ash_authentication/strategies/oauth2/plug_test.exs +++ b/test/ash_authentication/strategies/oauth2/plug_test.exs @@ -23,9 +23,4 @@ defmodule AshAuthentication.Strategy.OAuth2.PlugTest do assert session.state =~ ~r/.+/ end end - - describe "callback/2" do - @tag skip: "not exactly sure the best way to test this" - test "it signs in or registers the user" - end end diff --git a/test/support/example/generic_oauth_change.ex b/test/support/example/generic_oauth_change.ex index 5a7fc49..93d89d0 100644 --- a/test/support/example/generic_oauth_change.ex +++ b/test/support/example/generic_oauth_change.ex @@ -10,6 +10,6 @@ defmodule Example.GenericOAuth2Change do user_info = Changeset.get_argument(changeset, :user_info) changeset - |> Changeset.change_attribute(:username, user_info["nickname"]) + |> Changeset.change_attribute(:username, user_info["nickname"] || user_info["login"]) end end diff --git a/test/support/example/user.ex b/test/support/example/user.ex index fafbcb8..2dda717 100644 --- a/test/support/example/user.ex +++ b/test/support/example/user.ex @@ -77,6 +77,17 @@ defmodule Example.User do filter expr(username == get_path(^arg(:user_info), [:nickname])) end + + create :register_with_github do + argument :user_info, :map, allow_nil?: false + argument :oauth_tokens, :map, allow_nil?: false + upsert? true + upsert_identity :username + + change AshAuthentication.GenerateTokenChange + change Example.GenericOAuth2Change + change AshAuthentication.Strategy.OAuth2.IdentityChange + end end graphql do @@ -147,9 +158,9 @@ defmodule Example.User do redirect_uri &get_config/2 client_secret &get_config/2 site &get_config/2 - authorize_path &get_config/2 - token_path &get_config/2 - user_path &get_config/2 + authorize_url &get_config/2 + token_url &get_config/2 + user_url &get_config/2 authorization_params scope: "openid profile email" auth_method :client_secret_post identity_resource Example.UserIdentity @@ -160,9 +171,15 @@ defmodule Example.User do redirect_uri &get_config/2 client_secret &get_config/2 site &get_config/2 - authorize_path &get_config/2 - token_path &get_config/2 - user_path &get_config/2 + authorize_url &get_config/2 + token_url &get_config/2 + user_url &get_config/2 + end + + github do + client_id &get_config/2 + redirect_uri &get_config/2 + client_secret &get_config/2 end end end