Cast user claim values

Author: danschultzer

CHANGELOG.md

@@ -6,6 +6,14 @@
 
 ### 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.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
@@ -14,6 +22,10 @@
 * `Assent.HTTPAdapter.Mint` removed
 * `Assent.Config` removed
 
+### Changes
+
+* `Assent.Strategy.normalize_userinfo/2` now casts the user claims per OpenID specification
+
 ## v0.2
 
 The CHANGELOG for v0.2 releases can be found [in the v0.2 branch](https://github.com/pow-auth/assent/blob/v0.2/CHANGELOG.md).

lib/assent.ex

@@ -132,6 +132,30 @@ defmodule Assent do
     end
   end
 
+  defmodule CastClaimsError do
+    defexception [:claims]
+
+    def message(exception) do
+      """
+      The following claims couldn't be cast:
+
+      #{exception.claims |> map_claims() |> List.flatten() |> Enum.join("\n")}
+      """
+    end
+
+    defp map_claims(claims, indent \\ "") do
+      claims
+      |> Enum.sort_by(&elem(&1, 0))
+      |> Enum.reduce([], fn
+        {key, %{} = claims}, acc ->
+          acc ++ ["#{indent}- #{key}" | map_claims(claims, indent <> "  ")]
+
+        {key, {type, _value}}, acc ->
+          acc ++ ["#{indent}- `#{key}` as #{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,25 +126,117 @@ defmodule Assent.Strategy do
 
   defp encode_value(value), do: URI.encode_www_form(Kernel.to_string(value))
 
+  @registered_claim_members %{
+    "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_members, 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)
-      )
+    case traverse_claims(@registered_claim_members, claims) do
+      {claims, nil} -> {:ok, Map.merge(prune(extra), claims || %{})}
+      {_claims, invalid_claims} -> {:error, CastClaimsError.exception(claims: invalid_claims)}
+    end
+  end
+
+  defp traverse_claims(claim_members, claims) do
+    claim_members
+    |> Enum.reduce({[], []}, &traverse_claims(&1, &2, Map.get(claims, elem(&1, 0))))
+    |> then(fn {claims, invalid_claims} ->
+      {
+        (claims != [] && Enum.into(claims, %{})) || nil,
+        (invalid_claims != [] && Enum.into(invalid_claims, %{})) || nil
+      }
+    end)
+  end
+
+  defp traverse_claims(_, {valid_claims, invalid_claims}, nil), do: {valid_claims, invalid_claims}
+
+  defp traverse_claims(
+         {key, %{} = claim_members},
+         {parent_valid_claims, parent_invalid_claims},
+         %{} = claims
+       ) do
+    case traverse_claims(claim_members, claims) do
+      {nil, nil} ->
+        {parent_valid_claims, parent_invalid_claims}
+
+      {claims, nil} ->
+        {[{key, claims} | parent_valid_claims], parent_invalid_claims}
+
+      {nil, invalid_claims} ->
+        {parent_valid_claims, [{key, invalid_claims} | parent_invalid_claims]}
 
-    {:ok, prune(Map.merge(extra, standard_claims))}
+      {claims, invalid_claims} ->
+        {[{key, claims} | parent_valid_claims], [{key, invalid_claims} | parent_invalid_claims]}
+    end
+  end
+
+  defp traverse_claims({key, %{}}, {valid_claims, invalid_claims}, claims) do
+    {valid_claims, [{key, {:map, claims}} | invalid_claims]}
   end
 
+  defp traverse_claims({key, type}, {valid_claims, invalid_claims}, value) do
+    case cast_value(value, type) do
+      {:ok, value} -> {[{key, value} | valid_claims], invalid_claims}
+      :error -> {valid_claims, [{key, {type, value}} | 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
+    case Integer.parse(value) do
+      {integer, ""} -> {:ok, integer}
+      _ -> :error
+    end
+  end
+
+  defp cast_value(value, :boolean) when is_boolean(value), do: {:ok, value}
+
+  defp cast_value(_value, _type), do: :error
+
   @doc """
   Recursively prunes map for nil values.
   """

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/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,94 @@ 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
+    @user %{
+      "sub" => "123",
+      "email" => "[email protected]",
+      "email_verified" => true,
+      "address" => %{
+        "formatted" => "456"
+      },
+      "updated_at" => 1_516_239_022
+    }
+
+    @expected %{
+      "sub" => "123",
+      "email" => "[email protected]",
+      "email_verified" => true,
+      "address" => %{
+        "formatted" => "456"
+      },
+      "updated_at" => 1_516_239_022
+    }
+
+    test "with invalid types" do
+      user = %{
+        @user
+        | "sub" => true,
+          "email_verified" => "false",
+          "updated_at" => "invalid",
+          "address" => "invalid"
+      }
+
+      assert {:error, %Assent.CastClaimsError{} = error} = Strategy.normalize_userinfo(user, %{})
+
+      assert Exception.message(error) == """
+             The following claims couldn't be cast:
+
+             - `address` as map
+             - `email_verified` as boolean
+             - `sub` as binary
+             - `updated_at` as integer
+             """
+    end
+
+    test "with invalid nested types" do
+      user = %{@user | "address" => %{"formatted" => true}}
+
+      assert {:error, %Assent.CastClaimsError{} = error} = Strategy.normalize_userinfo(user, %{})
+
+      assert Exception.message(error) == """
+             The following claims couldn't be cast:
+
+             - address
+               - `formatted` as binary
+             """
+    end
+
+    test "casts" do
+      assert Strategy.normalize_userinfo(@user) == {:ok, @expected}
+    end
+
+    test "casts with extra" do
+      extra = %{"a" => 1}
+
+      assert Strategy.normalize_userinfo(@user, extra) == {:ok, Map.merge(@expected, extra)}
+    end
+
+    test "extra with registered claims" do
+      extra = %{"sub" => "different-sub"}
+
+      assert Strategy.normalize_userinfo(@user, extra) == {:ok, @expected}
+    end
+
+    test "casts types" do
+      user = %{
+        @user
+        | "sub" => 123,
+          "address" => %{"formatted" => 456},
+          "updated_at" => "1516239022"
+      }
+
+      assert Strategy.normalize_userinfo(user) == {:ok, @expected}
+    end
+
+    test "with unknown claims" do
+      user =
+        Map.merge(@user, %{"foo" => "bar", "address" => %{"foo" => "bar", "formatted" => "456"}})
+
+      assert Strategy.normalize_userinfo(user) == {:ok, @expected}
+    end
   end
 
   test "prune/1" do