From faf1b06c567524da38a16870647696a35d085263 Mon Sep 17 00:00:00 2001 From: Yann VERY Date: Thu, 5 Nov 2020 09:54:34 +0100 Subject: [PATCH 1/4] Load configuration from %Plug.Conn{} --- lib/ueberauth/strategy/cas.ex | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/ueberauth/strategy/cas.ex b/lib/ueberauth/strategy/cas.ex index d69f880..ddcf36f 100644 --- a/lib/ueberauth/strategy/cas.ex +++ b/lib/ueberauth/strategy/cas.ex @@ -220,4 +220,32 @@ defmodule Ueberauth.Strategy.CAS do value end end + + defp redirect_url(conn) do + login_url(conn) <> "?service=" <> callback_url(conn) + end + + defp validate_url(conn) do + base_url(conn) <> validation_path(conn) + end + + defp login_url(conn) do + base_url(conn) <> "/login" + end + + defp base_url(conn) do + Keyword.get(settings(conn), :base_url) + end + + defp validation_path(conn) do + Keyword.get(settings(conn), :validation_path, "/serviceValidate") + end + + defp attributes(conn) do + Keyword.get(settings(conn), :attributes, %{}) + end + + defp settings(conn) do + Ueberauth.Strategy.Helpers.options(conn) || [] + end end From 332b76743bcc570c95fe33a2e061a00f33531a35 Mon Sep 17 00:00:00 2001 From: Yann VERY Date: Wed, 4 Nov 2020 16:00:02 +0100 Subject: [PATCH 2/4] Use the configuration extracted from %Plug.Conn{} --- lib/ueberauth/strategy/cas.ex | 36 ++++----- lib/ueberauth/strategy/cas/api.ex | 24 +----- test/ueberauth/strategy/cas/api_test.exs | 21 +++--- test/ueberauth/strategy/cas_test.exs | 95 +++++++++++++++++------- 4 files changed, 97 insertions(+), 79 deletions(-) diff --git a/lib/ueberauth/strategy/cas.ex b/lib/ueberauth/strategy/cas.ex index ddcf36f..0074a1d 100644 --- a/lib/ueberauth/strategy/cas.ex +++ b/lib/ueberauth/strategy/cas.ex @@ -99,8 +99,7 @@ defmodule Ueberauth.Strategy.CAS do Ueberauth `request` handler. Redirects to the CAS server's login page. """ def handle_request!(conn) do - conn - |> redirect!(redirect_url(conn)) + redirect!(conn, redirect_url(conn)) end @doc """ @@ -148,17 +147,18 @@ defmodule Ueberauth.Strategy.CAS do """ def info(conn) do user = conn.private.cas_user - attributes = user.attributes + user_attributes = user.attributes + attribute_mapping = attributes(conn) %Info{ name: user.name, - email: get_attribute(attributes, :email), - birthday: get_attribute(attributes, :birthday), - description: get_attribute(attributes, :description), - first_name: get_attribute(attributes, :first_name), - last_name: get_attribute(attributes, :last_name), - nickname: get_attribute(attributes, :nickname), - phone: get_attribute(attributes, :phone) + email: get_attribute(attribute_mapping, user_attributes, :email), + birthday: get_attribute(attribute_mapping, user_attributes, :birthday), + description: get_attribute(attribute_mapping, user_attributes, :description), + first_name: get_attribute(attribute_mapping, user_attributes, :first_name), + last_name: get_attribute(attribute_mapping, user_attributes, :last_name), + nickname: get_attribute(attribute_mapping, user_attributes, :nickname), + phone: get_attribute(attribute_mapping, user_attributes, :phone) } end @@ -174,10 +174,6 @@ defmodule Ueberauth.Strategy.CAS do } end - defp redirect_url(conn) do - CAS.API.login_url() <> "?service=#{callback_url(conn)}" - end - defp handle_ticket(conn, ticket) do conn |> put_private(:cas_ticket, ticket) @@ -186,7 +182,7 @@ defmodule Ueberauth.Strategy.CAS do defp fetch_user(conn, ticket) do ticket - |> CAS.API.validate_ticket(conn) + |> CAS.API.validate_ticket(validate_url(conn), callback_url(conn)) |> handle_validate_ticket_response(conn) end @@ -205,14 +201,10 @@ defmodule Ueberauth.Strategy.CAS do |> put_private(:cas_user, user) end - defp get_attribute(attributes, key) do - {_, settings} = Application.get_env(:ueberauth, Ueberauth)[:providers][:cas] - - name = - Keyword.get(settings, :attributes, %{}) - |> Map.get(key, Atom.to_string(key)) + defp get_attribute(attribute_mapping, user_attributes, key) do + name = Map.get(attribute_mapping, key, Atom.to_string(key)) - value = Map.get(attributes, name) + value = Map.get(user_attributes, name) if is_list(value) do Enum.at(value, 0) diff --git a/lib/ueberauth/strategy/cas/api.ex b/lib/ueberauth/strategy/cas/api.ex index 4dec364..db9269d 100644 --- a/lib/ueberauth/strategy/cas/api.ex +++ b/lib/ueberauth/strategy/cas/api.ex @@ -3,19 +3,14 @@ defmodule Ueberauth.Strategy.CAS.API do CAS server API implementation. """ - use Ueberauth.Strategy alias Ueberauth.Strategy.CAS import SweetXml - @doc "Returns the URL to this CAS server's login page." - def login_url do - settings(:base_url) <> "/login" - end - @doc "Validate a CAS Service Ticket with the CAS server." - def validate_ticket(ticket, conn) do - HTTPoison.get(validate_url(), [], params: %{ticket: ticket, service: callback_url(conn)}) + def validate_ticket(ticket, validate_url, service) do + validate_url + |> HTTPoison.get([], params: %{ticket: ticket, service: service}) |> handle_validate_ticket_response() end @@ -52,17 +47,4 @@ defmodule Ueberauth.Strategy.CAS.API do {error_code || "unknown_error", message || "Unknown error"} end - - def validate_url do - settings(:base_url) <> validate_path() - end - - defp validate_path do - settings(:validation_path) || "/serviceValidate" - end - - defp settings(key) do - {_, settings} = Application.get_env(:ueberauth, Ueberauth)[:providers][:cas] - settings[key] - end end diff --git a/test/ueberauth/strategy/cas/api_test.exs b/test/ueberauth/strategy/cas/api_test.exs index 48aa6e1..23060c7 100644 --- a/test/ueberauth/strategy/cas/api_test.exs +++ b/test/ueberauth/strategy/cas/api_test.exs @@ -4,10 +4,6 @@ defmodule Ueberauth.Strategy.CAS.API.Test do alias Ueberauth.Strategy.CAS.API - test "generates a cas login url" do - assert API.login_url() == "http://cas.example.com/login" - end - test "validates a valid ticket response" do ok_xml = """ @@ -33,7 +29,7 @@ defmodule Ueberauth.Strategy.CAS.API.Test do {:ok, %HTTPoison.Response{status_code: 200, body: ok_xml, headers: []}} end do {:ok, %Ueberauth.Strategy.CAS.User{name: name}} = - API.validate_ticket("ST-XXXXX", %Plug.Conn{}) + API.validate_ticket("ST-XXXXX", "http://cas.example.com/serviceValidate", "service_name") assert name == "mail@marceldegraaf.net" end @@ -50,7 +46,8 @@ defmodule Ueberauth.Strategy.CAS.API.Test do get: fn _url, _opts, _params -> {:ok, %HTTPoison.Response{status_code: 200, body: error_xml, headers: []}} end do - {:error, {code, message}} = API.validate_ticket("ST-XXXXX", %Plug.Conn{}) + {:error, {code, message}} = + API.validate_ticket("ST-XXXXX", "http://cas.example.com/serviceValidate", "service_name") assert code == "INVALID_TICKET" assert message == "Ticket 'ST-XXXXX' already consumed" @@ -68,7 +65,8 @@ defmodule Ueberauth.Strategy.CAS.API.Test do get: fn _url, _opts, _params -> {:ok, %HTTPoison.Response{status_code: 200, body: unknown_error_xml, headers: []}} end do - {:error, {code, message}} = API.validate_ticket("ST-XXXXX", %Plug.Conn{}) + {:error, {code, message}} = + API.validate_ticket("ST-XXXXX", "http://cas.example.com/serviceValidate", "service_name") assert code == "unknown_error" assert message == "An unknown error occurred" @@ -86,7 +84,8 @@ defmodule Ueberauth.Strategy.CAS.API.Test do get: fn _url, _opts, _params -> {:ok, %HTTPoison.Response{status_code: 200, body: unknown_error_xml, headers: []}} end do - {:error, {code, message}} = API.validate_ticket("ST-XXXXX", %Plug.Conn{}) + {:error, {code, message}} = + API.validate_ticket("ST-XXXXX", "http://cas.example.com/serviceValidate", "service_name") assert code == "CONNECTION_ERROR" assert message == "Unknown error" @@ -104,7 +103,8 @@ defmodule Ueberauth.Strategy.CAS.API.Test do get: fn _url, _opts, _params -> {:ok, %HTTPoison.Response{status_code: 200, body: unknown_error_xml, headers: []}} end do - {:error, {code, message}} = API.validate_ticket("ST-XXXXX", %Plug.Conn{}) + {:error, {code, message}} = + API.validate_ticket("ST-XXXXX", "http://cas.example.com/serviceValidate", "service_name") assert code == "unknown_error" assert message == "Unknown error" @@ -116,7 +116,8 @@ defmodule Ueberauth.Strategy.CAS.API.Test do get: fn _url, _opts, _params -> {:ok, %HTTPoison.Response{status_code: 200, body: "blip blob", headers: []}} end do - {:error, {code, _}} = API.validate_ticket("ST-XXXXX", %Plug.Conn{}) + {:error, {code, _}} = + API.validate_ticket("ST-XXXXX", "http://cas.example.com/serviceValidate", "service_name") assert code == "malformed_xml" end diff --git a/test/ueberauth/strategy/cas_test.exs b/test/ueberauth/strategy/cas_test.exs index 9088ffd..dd722bf 100644 --- a/test/ueberauth/strategy/cas_test.exs +++ b/test/ueberauth/strategy/cas_test.exs @@ -6,10 +6,26 @@ defmodule Ueberauth.Strategy.CAS.Test do alias Ueberauth.Strategy.CAS setup do + ueberauth_request_options = %{ + callback_url: "http://service.com/auth/provider/callback", + options: [ + base_url: "http://cas.example.com", + validate_path: "serviceValidate" + ] + } + conn = %Plug.Conn{ private: %{ - cas_user: %CAS.User{name: "Marcel de Graaf", attributes: %{"email" => "mail@marceldegraaf.net", "roles" => ["developer"], "first_name" => ["Joe", "Example"]}}, - cas_ticket: "ST-XXXXX", + ueberauth_request_options: ueberauth_request_options, + cas_user: %CAS.User{ + name: "Marcel de Graaf", + attributes: %{ + "email" => "mail@marceldegraaf.net", + "roles" => ["developer"], + "first_name" => ["Joe", "Example"] + } + }, + cas_ticket: "ST-XXXXX" } } @@ -39,13 +55,24 @@ defmodule Ueberauth.Strategy.CAS.Test do conn: conn, ok_xml: ok_xml, error_xml: error_xml, + ueberauth_request_options: ueberauth_request_options } end - test "redirect callback redirects to login url" do - conn = conn(:get, "/login") |> CAS.handle_request! + test "redirect callback redirects to login url", %{ + ueberauth_request_options: ueberauth_request_options + } do + conn = + conn(:get, "/login") + |> Plug.Conn.put_private(:ueberauth_request_options, ueberauth_request_options) + |> CAS.handle_request!() assert conn.status == 302 + + assert Plug.Conn.get_resp_header(conn, "location") == + [ + "http://cas.example.com/login?service=http://service.com/auth/provider/callback" + ] end test "login callback without service ticket shows an error" do @@ -53,13 +80,18 @@ defmodule Ueberauth.Strategy.CAS.Test do assert Map.has_key?(conn.assigns, :ueberauth_failure) end - test "successful login callback validates the ticket", %{ok_xml: xml} do - with_mock HTTPoison, [ - get: fn(_url, _opts, _params) -> - {:ok, %HTTPoison.Response{status_code: 200, body: xml, headers: []} - } end - ] do - conn = CAS.handle_callback!(%Plug.Conn{params: %{"ticket" => "ST-XXXXX"}}) + test "successful login callback validates the ticket", %{ + ok_xml: xml, + ueberauth_request_options: ueberauth_request_options + } do + with_mock HTTPoison, + get: fn _url, _opts, _params -> + {:ok, %HTTPoison.Response{status_code: 200, body: xml, headers: []}} + end do + conn = + %Plug.Conn{params: %{"ticket" => "ST-XXXXX"}} + |> Plug.Conn.put_private(:ueberauth_request_options, ueberauth_request_options) + |> CAS.handle_callback!() assert conn.private.cas_ticket == "ST-XXXXX" assert conn.private.cas_user.name == "mail@marceldegraaf.net" @@ -67,29 +99,40 @@ defmodule Ueberauth.Strategy.CAS.Test do end end - test "invalid login callback returns an error", %{error_xml: xml} do - with_mock HTTPoison, [ - get: fn(_url, _opts, _params) -> - {:ok, %HTTPoison.Response{status_code: 200, body: xml, headers: []} - } end - ] do - conn = CAS.handle_callback!(%Plug.Conn{params: %{"ticket" => "ST-XXXXX"}}) + test "invalid login callback returns an error", %{ + error_xml: xml, + ueberauth_request_options: ueberauth_request_options + } do + with_mock HTTPoison, + get: fn _url, _opts, _params -> + {:ok, %HTTPoison.Response{status_code: 200, body: xml, headers: []}} + end do + conn = + %Plug.Conn{params: %{"ticket" => "ST-XXXXX"}} + |> Plug.Conn.put_private(:ueberauth_request_options, ueberauth_request_options) + |> CAS.handle_callback!() assert List.first(conn.assigns.ueberauth_failure.errors).message_key == "INVALID_TICKET" - assert List.first(conn.assigns.ueberauth_failure.errors).message == "Ticket 'ST-XXXXX' already consumed" + + assert List.first(conn.assigns.ueberauth_failure.errors).message == + "Ticket 'ST-XXXXX' already consumed" end end - test "network error propagates" do - with_mock HTTPoison, [ - get: fn(_url, _opts, _params) -> + test "network error propagates", %{ueberauth_request_options: ueberauth_request_options} do + with_mock HTTPoison, + get: fn _url, _opts, _params -> {:error, %HTTPoison.Error{reason: :timeout, id: nil}} - end - ] do - conn = CAS.handle_callback!(%Plug.Conn{params: %{"ticket" => "ST-XXXXX"}}) + end do + conn = + %Plug.Conn{params: %{"ticket" => "ST-XXXXX"}} + |> Plug.Conn.put_private(:ueberauth_request_options, ueberauth_request_options) + |> CAS.handle_callback!() assert List.first(conn.assigns.ueberauth_failure.errors).message_key == "NETWORK_ERROR" - assert List.first(conn.assigns.ueberauth_failure.errors).message == "An error occurred: timeout" + + assert List.first(conn.assigns.ueberauth_failure.errors).message == + "An error occurred: timeout" end end From 151e2de20ab947329cf7599b14701ea8e91d33c3 Mon Sep 17 00:00:00 2001 From: Yann VERY Date: Wed, 4 Nov 2020 18:20:02 +0100 Subject: [PATCH 3/4] Rename callback param to callback_url --- README.md | 4 ++-- lib/ueberauth/strategy/cas.ex | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7946806..ee3a3c9 100644 --- a/README.md +++ b/README.md @@ -34,12 +34,12 @@ Central Authentication Service strategy for Überauth. config :ueberauth, Ueberauth, providers: [cas: {Ueberauth.Strategy.CAS, [ base_url: "http://cas.example.com", - callback: "http://your-app.example.com/auth/cas/callback", + callback_url: "http://your-app.example.com/auth/cas/callback", ]}] ``` 4. Include the Überauth plug in your controller: - + ```elixir defmodule MyApp.AuthController do use MyApp.Web, :controller diff --git a/lib/ueberauth/strategy/cas.ex b/lib/ueberauth/strategy/cas.ex index 0074a1d..5fdabd7 100644 --- a/lib/ueberauth/strategy/cas.ex +++ b/lib/ueberauth/strategy/cas.ex @@ -77,7 +77,7 @@ defmodule Ueberauth.Strategy.CAS do providers: [cas: {Ueberauth.Strategy.CAS, [ base_url: "http://cas.example.com", validation_path: "/serviceValidate", - callback: "http://your-app.example.com/auth/cas/callback", + callback_url: "http://your-app.example.com/auth/cas/callback", attributes: %{ last_name: "surname" }, From e5213167831937562a96751735a6034e2e1d763a Mon Sep 17 00:00:00 2001 From: Yann VERY Date: Wed, 4 Nov 2020 16:00:40 +0100 Subject: [PATCH 4/4] Test configuration is no longer need --- config/test.exs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/config/test.exs b/config/test.exs index ed28fe5..d2d855e 100644 --- a/config/test.exs +++ b/config/test.exs @@ -1,6 +1 @@ use Mix.Config - -config :ueberauth, Ueberauth, - providers: [ - cas: {Ueberauth.Strategy.CAS, [base_url: "http://cas.example.com", service: "http://svc.example.com"]} - ]