Cast user claim values

CHANGELOG.md

@@ -6,7 +6,15 @@
 
 ### Breaking changes
 
+* `Assent.Strategy.Auth.callback/2` now encodes `updated_at` as an integer instead of string
+* `Assent.Strategy.Basecamp.callback/2` now encodes `sub` as a string instead of an integer
+* `Assent.Strategy.Github.callback/2` now encodes `sub` as a string instead of an integer
+* `Assent.Strategy.Gitlab.callback/2` now encodes `sub` as a string instead of an integer
 * `Assent.Strategy.Google` now return `hd` instead of `google_hd`
+* `Assent.Strategy.Strava.callback/2` now encodes `sub` as a string instead of an integer
+* `Assent.Strategy.Telegram.callback/2` now encodes `sub` as a string instead of an integer
+* `Assent.Strategy.Twitter.callback/2` now encodes `sub` as a string instead of an integer
+* `Assent.Strategy.VK.callback/2` now encodes `sub` as a string instead of an integer
 * `:site` configuration option removed, use `:base_url` instead
 * `Assent.Strategy.OAuth2.authorize_url/2` no longer allows `:state` in `:authorization_params`
 * `Assent.Strategy.decode_response/2`removed, use `Assent.HTTPAdapter.decode_response/2` instead
@@ -18,6 +26,7 @@
 ### Changes
 
 * `Assent.Strategy.Google` now uses OIDC instead of OAuth 2.0 base strategy
+* `Assent.Strategy.normalize_userinfo/2` now casts the user claims per OpenID specification
 
 ## v0.2
 

lib/assent.ex

@@ -132,6 +132,35 @@ defmodule Assent do
     end
   end
 
+  defmodule CastClaimsError do
+    defexception [:claims, :invalid_types]
+
+    @type t :: %__MODULE__{
+            claims: map(),
+            invalid_types: map()
+          }
+
+    def message(exception) do
+      """
+      The following claims couldn't be cast:
+
+      #{exception.invalid_types |> to_lines() |> Enum.join("\n")}
+      """
+    end
+
+    defp to_lines(claim_types, prepend \\ "") do
+      claim_types
+      |> Enum.sort_by(&elem(&1, 0))
+      |> Enum.reduce([], fn
+        {key, %{} = claim_types}, acc ->
+          acc ++ to_lines(claim_types, prepend <> "#{inspect(key)} -> ")
+
+        {key, type}, acc ->
+          acc ++ ["- #{prepend}#{inspect(key)} to #{inspect(type)}"]
+      end)
+    end
+  end
+
   @doc """
   Fetches the key value from the configuration.
 

lib/assent/strategy.ex

@@ -26,6 +26,8 @@ defmodule Assent.Strategy do
         end
       end
   """
+  alias Assent.CastClaimsError
+
   @callback authorize_url(Keyword.t()) ::
               {:ok, %{:url => binary(), optional(atom()) => any()}} | {:error, term()}
   @callback callback(Keyword.t(), map()) ::
@@ -124,23 +126,109 @@ defmodule Assent.Strategy do
 
   defp encode_value(value), do: URI.encode_www_form(Kernel.to_string(value))
 
+  @registered_claim_member_types %{
+    "sub" => :binary,
+    "name" => :binary,
+    "given_name" => :binary,
+    "family_name" => :binary,
+    "middle_name" => :binary,
+    "nickname" => :binary,
+    "preferred_username" => :binary,
+    "profile" => :binary,
+    "picture" => :binary,
+    "website" => :binary,
+    "email" => :binary,
+    "email_verified" => :boolean,
+    "gender" => :binary,
+    "birthdate" => :binary,
+    "zoneinfo" => :binary,
+    "locale" => :binary,
+    "phone_number" => :binary,
+    "phone_number_verified" => :boolean,
+    "address" => %{
+      "formatted" => :binary,
+      "street_address" => :binary,
+      "locality" => :binary,
+      "region" => :binary,
+      "postal_code" => :binary,
+      "country" => :binary
+    },
+    "updated_at" => :integer
+  }
+
   @doc """
   Normalize API user request response into standard claims.
 
+  The function will cast values to adhere to the following types:
+
+  ```
+  #{inspect(@registered_claim_member_types, pretty: true)}
+  ```
+
+  Returns a `Assent.CastClaimsError` if any of the above types can't be casted.
+
   Based on https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.1
   """
-  @spec normalize_userinfo(map(), map()) :: {:ok, map()}
+  @spec normalize_userinfo(map(), map()) :: {:ok, map()} | {:error, term()}
   def normalize_userinfo(claims, extra \\ %{}) do
-    standard_claims =
-      Map.take(
-        claims,
-        ~w(sub name given_name family_name middle_name nickname
-         preferred_username profile picture website email email_verified
-         gender birthdate zoneinfo locale phone_number phone_number_verified
-         address updated_at)
-      )
-
-    {:ok, prune(Map.merge(extra, standard_claims))}
+    case cast_claims(@registered_claim_member_types, claims) do
+      {casted_claims, nil} ->
+        {:ok, extra |> prune() |> Map.merge(casted_claims)}
+
+      {_claims, invalid_claims} ->
+        {:error,
+         CastClaimsError.exception(claims: claims, invalid_types: Enum.into(invalid_claims, %{}))}
+    end
+  end
+
+  defp cast_claims(claim_types, claims) do
+    {casted_claims, invalid_claims} =
+      Enum.reduce(claim_types, {[], []}, fn {key, type}, acc ->
+        cast_claim(key, type, Map.get(claims, key), acc)
+      end)
+
+    {
+      (casted_claims != [] && Enum.into(casted_claims, %{})) || nil,
+      (invalid_claims != [] && Enum.into(invalid_claims, %{})) || nil
+    }
+  end
+
+  defp cast_claim(_key, _type, nil, acc), do: acc
+
+  defp cast_claim(key, %{} = claim_types, %{} = claims, {casted_claims, invalid_claims}) do
+    {casted_sub_claims, invalid_sub_claims} = cast_claims(claim_types, claims)
+
+    {
+      (casted_sub_claims && [{key, casted_sub_claims} | casted_claims]) || casted_claims,
+      (invalid_sub_claims && [{key, invalid_sub_claims} | invalid_claims]) || invalid_claims
+    }
+  end
+
+  defp cast_claim(key, %{}, _value, {casted_claims, invalid_claims}) do
+    {casted_claims, [{key, :map} | invalid_claims]}
+  end
+
+  defp cast_claim(key, type, value, {casted_claims, invalid_claims}) do
+    case cast_value(value, type) do
+      {:ok, value} -> {[{key, value} | casted_claims], invalid_claims}
+      :error -> {casted_claims, [{key, type} | invalid_claims]}
+    end
+  end
+
+  defp cast_value(value, :binary) when is_binary(value), do: {:ok, value}
+  defp cast_value(value, :binary) when is_integer(value), do: {:ok, to_string(value)}
+  defp cast_value(value, :integer) when is_integer(value), do: {:ok, value}
+  defp cast_value(value, :integer) when is_binary(value), do: cast_integer(value)
+  defp cast_value(value, :boolean) when is_boolean(value), do: {:ok, value}
+  defp cast_value("true", :boolean), do: {:ok, true}
+  defp cast_value("false", :boolean), do: {:ok, false}
+  defp cast_value(_value, _type), do: :error
+
+  defp cast_integer(value) do
+    case Integer.parse(value) do
+      {integer, ""} -> {:ok, integer}
+      _ -> :error
+    end
   end
 
   @doc """

test/assent/strategies/auth0_test.exs

@@ -28,7 +28,7 @@ defmodule Assent.Strategy.Auth0Test do
     },
     "updated_at" => "1556845729"
   }
-  @user @user_response
+  @user %{@user_response | "updated_at" => 1_556_845_729}
 
   describe "authorize_url/2" do
     test "requires domain or base_url configuration", %{config: config} do

test/assent/strategies/basecamp_test.exs

@@ -42,7 +42,7 @@ defmodule Assent.Strategy.BasecampTest do
     "family_name" => "Fried",
     "given_name" => "Jason",
     "name" => "Jason Fried",
-    "sub" => 9_999_999
+    "sub" => "9999999"
   }
 
   test "authorize_url/2", %{config: config} do

test/assent/strategies/github_test.exs

@@ -65,7 +65,7 @@ defmodule Assent.Strategy.GithubTest do
     "picture" => "https://github.com/images/error/octocat_happy.gif",
     "preferred_username" => "octocat",
     "profile" => "https://github.com/octocat",
-    "sub" => 1
+    "sub" => "1"
   }
 
   test "authorize_url/2", %{config: config} do

test/assent/strategies/gitlab_test.exs

@@ -45,7 +45,7 @@ defmodule Assent.Strategy.GitlabTest do
     "name" => "John Smith",
     "picture" => "http://localhost:3000/uploads/user/avatar/1/index.jpg",
     "preferred_username" => "john_smith",
-    "sub" => 1
+    "sub" => "1"
   }
 
   test "authorize_url/2", %{config: config} do

test/assent/strategies/google_test.exs

@@ -19,7 +19,7 @@ defmodule Assent.Strategy.GoogleTest do
   }
   @user %{
     "email" => "[email protected]",
-    "email_verified" => "true",
+    "email_verified" => true,
     "hd" => "example.com",
     "sub" => "10769150350006150715113082367"
   }

test/assent/strategies/strava_test.exs

@@ -53,7 +53,7 @@ defmodule Assent.Strategy.StravaTest do
   }
 
   @user %{
-    "sub" => 1_234_567_890_987_654_321,
+    "sub" => "1234567890987654321",
     "given_name" => "Marianne",
     "family_name" => "Teutenberg",
     "preferred_username" => "marianne_t",

test/assent/strategies/telegram_test.exs

@@ -11,7 +11,7 @@ defmodule Assent.Strategy.TelegramTest do
     "username" => "duroff"
   }
   @user %{
-    "sub" => 928_474_348,
+    "sub" => "928474348",
     "family_name" => "Duroff",
     "given_name" => "Paul",
     "preferred_username" => "duroff",

test/assent/strategies/twitter_test.exs

@@ -124,7 +124,7 @@ defmodule Assent.Strategy.TwitterTest do
     "picture" => "https://pbs.twimg.com/profile_images/880136122604507136/xHrnqf1T_normal.jpg",
     "preferred_username" => "TwitterDev",
     "profile" => "https://twitter.com/TwitterDev",
-    "sub" => 2_244_994_945,
+    "sub" => "2244994945",
     "website" => "https://t.co/FGl7VOULyL"
   }
 

test/assent/strategies/vk_test.exs

@@ -19,7 +19,7 @@ defmodule Assent.Strategy.VKTest do
   @user %{
     "given_name" => "Lindsay",
     "family_name" => "Stirling",
-    "sub" => 210_700_286,
+    "sub" => "210700286",
     "email" => "[email protected]"
   }
 

test/assent/strategy_test.exs

@@ -63,12 +63,101 @@ defmodule Assent.StrategyTest do
              "http://example.com/path?a=1&b[c]=2&b[d][e]=3&f[]=4&f[]=5"
   end
 
-  test "normalize_userinfo/2" do
-    user = %{"email" => "[email protected]", "name" => nil, "nickname" => "foo"}
-    extra = %{"a" => "1"}
-    expected = %{"email" => "[email protected]", "nickname" => "foo", "a" => "1"}
-
-    assert Strategy.normalize_userinfo(user, extra) == {:ok, expected}
+  describe "normalize_userinfo/2" do
+    @valid_user %{
+      "sub" => "123",
+      "email" => "[email protected]",
+      "email_verified" => true,
+      "address" => %{
+        "formatted" => "456"
+      },
+      "updated_at" => 1_516_239_022
+    }
+
+    @invalid_user %{
+      "sub" => true,
+      "address" => %{
+        "formatted" => true
+      }
+    }
+
+    @expected_user %{
+      "sub" => "123",
+      "email" => "[email protected]",
+      "email_verified" => true,
+      "address" => %{
+        "formatted" => "456"
+      },
+      "updated_at" => 1_516_239_022
+    }
+
+    test "with incorrect claim type" do
+      assert {:error, %Assent.CastClaimsError{} = error} =
+               Strategy.normalize_userinfo(@invalid_user, %{})
+
+      assert Exception.message(error) == """
+             The following claims couldn't be cast:
+
+             - "address" -> "formatted" to :binary
+             - "sub" to :binary
+             """
+    end
+
+    test "with atom value claim" do
+      user = %{"sub" => :invalid}
+
+      assert {:error, %Assent.CastClaimsError{} = error} = Strategy.normalize_userinfo(user, %{})
+      assert error.invalid_types == %{"sub" => :binary}
+    end
+
+    test "with binary type claim with integer value" do
+      user = %{"sub" => 123}
+
+      assert {:ok, %{"sub" => "123"}} = Strategy.normalize_userinfo(user, %{})
+    end
+
+    test "with integer type claim with invalid binary value" do
+      user = %{"updated_at" => "123a1"}
+
+      assert {:error, %Assent.CastClaimsError{} = error} = Strategy.normalize_userinfo(user, %{})
+      assert error.invalid_types == %{"updated_at" => :integer}
+    end
+
+    test "with integer type claim with valid binary value" do
+      user = %{"updated_at" => "123"}
+
+      assert {:ok, %{"updated_at" => 123}} = Strategy.normalize_userinfo(user, %{})
+    end
+
+    test "with boolean type claim with string binary value" do
+      user = %{"email_verified" => "true"}
+
+      assert {:ok, %{"email_verified" => true}} = Strategy.normalize_userinfo(user, %{})
+
+      user = %{"email_verified" => "false"}
+
+      assert {:ok, %{"email_verified" => false}} = Strategy.normalize_userinfo(user, %{})
+    end
+
+    test "casts" do
+      assert Strategy.normalize_userinfo(@valid_user) == {:ok, @expected_user}
+    end
+
+    test "with unknown claims" do
+      user =
+        @valid_user
+        |> Map.put("foo", "bar")
+        |> Map.put("address", Map.put(@valid_user["address"], "foo", "bar"))
+
+      assert Strategy.normalize_userinfo(user) == {:ok, @expected_user}
+    end
+
+    test "with extra" do
+      extra = %{"a" => 1, "sub" => "different-sub"}
+      expected_user = Map.put(@expected_user, "a", 1)
+
+      assert Strategy.normalize_userinfo(@valid_user, extra) == {:ok, expected_user}
+    end
   end
 
   test "prune/1" do