Added auth provider namespacing, and Google and GitHub auth

config/dev.secrets.exs.example

@@ -26,6 +26,17 @@ config :cadet,
     #      # You may need to write your own claim extractor for other providers
     #      claim_extractor: Cadet.Auth.Providers.CognitoClaimExtractor
     #    }},
+    # # To use authentication with GitHub
+    # "github" =>
+    #   {Cadet.Auth.Providers.GitHub,
+    #    %{
+    #       # A map of GitHub client_id => client_secret
+    #       clients: %{
+    #         "client_id" => "client_secret"
+    #       },
+    #       token_url: "https://github.com/login/oauth/access_token",
+    #       user_api: "https://api.github.com/user"
+    #    }},
     "test" =>
       {Cadet.Auth.Providers.Config,
        [

lib/cadet/auth/provider.ex

@@ -13,6 +13,8 @@ defmodule Cadet.Auth.Provider do
   @type redirect_uri :: String.t()
   @type error :: :upstream | :invalid_credentials | :other
   @type provider_instance :: String.t()
+  @type username :: String.t()
+  @type prefix :: String.t()
 
   @doc "Exchanges the OAuth2 authorisation code for a token and the user ID."
   @callback authorise(any(), code, client_id, redirect_uri) ::
@@ -53,4 +55,9 @@ defmodule Cadet.Auth.Provider do
       _ -> {:error, :other, "Invalid or nonexistent provider config"}
     end
   end
+
+  @spec namespace(username, prefix) :: String.t()
+  def namespace(username, prefix) do
+    prefix <> "/" <> username
+  end
 end

lib/cadet/auth/providers/config.ex

@@ -20,8 +20,11 @@ defmodule Cadet.Auth.Providers.Config do
           | {:error, Provider.error(), String.t()}
   def authorise(config, code, _client_id, _redirect_uri) do
     case Enum.find(config, nil, fn %{code: this_code} -> code == this_code end) do
-      %{token: token, username: username} -> {:ok, %{token: token, username: username}}
-      _ -> {:error, :invalid_credentials, "Invalid code"}
+      %{token: token, username: username} ->
+        {:ok, %{token: token, username: Provider.namespace(username, "test")}}
+
+      _ ->
+        {:error, :invalid_credentials, "Invalid code"}
     end
   end
 

lib/cadet/auth/providers/github.ex

@@ -0,0 +1,87 @@
+defmodule Cadet.Auth.Providers.GitHub do
+  @moduledoc """
+  Provides identity using GitHub OAuth.
+  """
+  alias Cadet.Auth.Provider
+
+  @behaviour Provider
+
+  @type config :: %{
+          clients: %{},
+          token_url: String.t(),
+          user_api: String.t()
+        }
+
+  @spec authorise(config(), Provider.code(), Provider.client_id(), Provider.redirect_uri()) ::
+          {:ok, %{token: Provider.token(), username: String.t()}}
+          | {:error, Provider.error(), String.t()}
+  def authorise(config, code, client_id, redirect_uri) do
+    token_headers = [
+      {"Content-Type", "application/x-www-form-urlencoded"},
+      {"Accept", "application/json"}
+    ]
+
+    token_url = config.token_url
+    user_api = config.user_api
+
+    with {:validate_client, {:ok, client_secret}} <-
+           {:validate_client, Map.fetch(config.clients, client_id)},
+         {:token_query, token_query} <-
+           {:token_query,
+            URI.encode_query(%{
+              client_id: client_id,
+              client_secret: client_secret,
+              code: code,
+              redirect_uri: redirect_uri
+            })},
+         {:token, {:ok, %{body: body, status_code: 200}}} <-
+           {:token, HTTPoison.post(token_url, token_query, token_headers)},
+         {:token_response, %{"access_token" => token}} <- {:token_response, Jason.decode!(body)},
+         {:user, {:ok, %{"login" => username}}} <- {:user, api_call(user_api, token)} do
+      {:ok, %{token: token, username: Provider.namespace(username, "github")}}
+    else
+      {:validate_client, :error} ->
+        {:error, :invalid_credentials, "Invalid client id"}
+
+      {:token, {:ok, %{status_code: status}}} ->
+        {:error, :upstream, "Status code #{status} from GitHub"}
+
+      {:token_response, %{"error" => error}} ->
+        {:error, :invalid_credentials, "Error from GitHub: #{error}"}
+
+      {:user, {:error, _, _} = error} ->
+        error
+    end
+  end
+
+  @spec get_name(config(), Provider.token()) ::
+          {:ok, String.t()} | {:error, Provider.error(), String.t()}
+  def get_name(config, token) do
+    user_api = config.user_api
+
+    case api_call(user_api, token) do
+      {:ok, %{"name" => name}} ->
+        {:ok, name}
+
+      {:error, _, _} = error ->
+        error
+    end
+  end
+
+  def get_role(_config, _claims) do
+    # There is no role specified for the GitHub provider
+    {:error, :invalid_credentials, "No role specified in token"}
+  end
+
+  defp api_call(url, token) do
+    headers = [{"Authorization", "token " <> token}]
+
+    case HTTPoison.get(url, headers) do
+      {:ok, %{body: body, status_code: 200}} ->
+        {:ok, Jason.decode!(body)}
+
+      {:ok, %{status_code: status}} ->
+        {:error, :upstream, "Status code #{status} from GitHub"}
+    end
+  end
+end

lib/cadet/auth/providers/google_claim_extractor.ex

@@ -13,7 +13,9 @@ defmodule Cadet.Auth.Providers.GoogleClaimExtractor do
     end
   end
 
-  def get_name(_claims), do: nil
+  def get_name(claims) do
+    claims["name"]
+  end
 
   def get_role(_claims), do: nil
 

lib/cadet/auth/providers/luminus.ex

@@ -36,7 +36,7 @@ defmodule Cadet.Auth.Providers.LumiNUS do
          {:verify_jwt, {:ok, _}} <-
            {:verify_jwt,
             Guardian.Token.Jwt.Verify.verify_claims(Cadet.Auth.EmptyGuardian, claims, nil)} do
-      {:ok, %{token: token, username: username}}
+      {:ok, %{token: token, username: Provider.namespace(username, "luminus")}}
     else
       {:token, {:ok, %{body: body, status_code: status}}} ->
         {:error, :upstream, "Status code #{status} from LumiNUS: #{body}"}

lib/cadet/auth/providers/openid.ex

@@ -30,8 +30,15 @@ defmodule Cadet.Auth.Providers.OpenID do
               nil
             )} do
       case claim_extractor.get_username(claims) do
-        nil -> {:error, :invalid_credentials, "No username specified in token"}
-        username -> {:ok, %{token: token, username: username}}
+        nil ->
+          {:error, :invalid_credentials, "No username specified in token"}
+
+        username ->
+          {:ok,
+           %{
+             token: token,
+             username: Provider.namespace(username, Atom.to_string(openid_provider))
+           }}
       end
     else
       {:token, {:error, _, _}} ->

lib/cadet_web/controllers/auth_controller.ex

@@ -34,7 +34,7 @@ defmodule CadetWeb.AuthController do
       {:authorise, {:error, :upstream, reason}} ->
         conn
         |> put_status(:bad_request)
-        |> text("Unable to retrieve token from ADFS: #{reason}")
+        |> text("Unable to retrieve token from authentication provider: #{reason}")
 
       {:authorise, {:error, :invalid_credentials, reason}} ->
         conn

test/cadet/auth/guardian_test.exs

@@ -1,6 +1,7 @@
 defmodule Cadet.Auth.GuardianTest do
   use Cadet.DataCase
 
+  import Cadet.TestHelper
   alias Cadet.Auth.Guardian
 
   test "token subject is user id" do
@@ -19,7 +20,9 @@ defmodule Cadet.Auth.GuardianTest do
       "sub" => "2000"
     }
 
-    assert Guardian.resource_from_claims(good_claims) == {:ok, user}
+    assert Guardian.resource_from_claims(good_claims) ==
+             {:ok, remove_preload(user, :latest_viewed)}
+
     assert Guardian.resource_from_claims(bad_claims) == {:error, :not_found}
   end
 end

test/cadet/auth/provider_test.exs

@@ -10,7 +10,6 @@ defmodule Cadet.Auth.ProviderTest do
   test "with valid provider" do
     assert {:ok, _} = Provider.authorise("test", "student_code", nil, nil)
     assert {:ok, _} = Provider.get_name("test", "student_token")
-    assert {:ok, _} = Provider.get_role("test", "student_token")
   end
 
   test "with invalid provider" do

test/cadet/auth/providers/config_test.exs

@@ -7,6 +7,7 @@ defmodule Cadet.Auth.Providers.ConfigTest do
   @token "token"
   @name "Test Name"
   @username "testusername"
+  @namespaced_username "test/testusername"
   @role :student
 
   @config [
@@ -21,7 +22,7 @@ defmodule Cadet.Auth.Providers.ConfigTest do
 
   describe "authorise" do
     test "successfully" do
-      assert {:ok, %{token: @token, username: @username}} =
+      assert {:ok, %{token: @token, username: @namespaced_username}} =
                Config.authorise(@config, @code, nil, nil)
     end
 

test/cadet/auth/providers/github_test.exs

@@ -0,0 +1,105 @@
+defmodule Cadet.Auth.Providers.GitHubTest do
+  use ExUnit.Case, async: false
+
+  alias Cadet.Auth.Providers.GitHub
+  alias Plug.Conn, as: PlugConn
+
+  @username "username"
+  @namespaced_username "github/username"
+  @name "name"
+
+  @dummy_access_token "dummy_access_token"
+
+  setup_all do
+    Application.ensure_all_started(:bypass)
+    bypass = Bypass.open()
+
+    {:ok, bypass: bypass}
+  end
+
+  defp config(bypass) do
+    %{
+      clients: %{"dummy_client_id" => "dummy_client_secret"},
+      token_url: "http://localhost:#{bypass.port}/login/oauth/access_token",
+      user_api: "http://localhost:#{bypass.port}/user"
+    }
+  end
+
+  defp bypass_return_token(bypass) do
+    Bypass.stub(bypass, "POST", "login/oauth/access_token", fn conn ->
+      conn
+      |> PlugConn.put_resp_header("content-type", "application/json")
+      |> PlugConn.resp(200, ~s({"access_token":"#{@dummy_access_token}"}))
+    end)
+  end
+
+  defp bypass_api_call(bypass) do
+    Bypass.stub(bypass, "GET", "user", fn conn ->
+      conn
+      |> PlugConn.put_resp_header("content-type", "application/json")
+      |> PlugConn.resp(200, ~s({"login":"#{@username}","name":"#{@name}"}))
+    end)
+  end
+
+  test "successful", %{bypass: bypass} do
+    bypass_return_token(bypass)
+    bypass_api_call(bypass)
+
+    assert {:ok, %{token: @dummy_access_token, username: @namespaced_username}} ==
+             GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+  end
+
+  test "invalid github client id", %{bypass: bypass} do
+    bypass_return_token(bypass)
+    bypass_api_call(bypass)
+
+    assert {:error, :invalid_credentials, "Invalid client id"} ==
+             GitHub.authorise(config(bypass), "", "invalid_client_id", "")
+  end
+
+  test "non-successful HTTP status (access token)", %{bypass: bypass} do
+    Bypass.stub(bypass, "POST", "login/oauth/access_token", fn conn ->
+      PlugConn.resp(conn, 403, "")
+    end)
+
+    assert {:error, :upstream, "Status code 403 from GitHub"} ==
+             GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+  end
+
+  test "error token response", %{bypass: bypass} do
+    Bypass.stub(bypass, "POST", "login/oauth/access_token", fn conn ->
+      conn
+      |> PlugConn.put_resp_header("content-type", "application/json")
+      |> PlugConn.resp(200, ~s({"error":"bad_verification_code"}))
+
+      assert {:error, :invalid_credentials, "Error from GitHub: bad_verification_code"} ==
+               GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+    end)
+  end
+
+  test "non-successful HTTP status (user api call)", %{bypass: bypass} do
+    bypass_return_token(bypass)
+
+    Bypass.stub(bypass, "GET", "user", fn conn ->
+      PlugConn.resp(conn, 401, "")
+    end)
+
+    assert {:error, :upstream, "Status code 401 from GitHub"}
+    GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+  end
+
+  test "get_name successful", %{bypass: bypass} do
+    bypass_api_call(bypass)
+
+    assert {:ok, @name} == GitHub.get_name(config(bypass), @dummy_access_token)
+  end
+
+  test "get_name non-successful HTTP status", %{bypass: bypass} do
+    Bypass.stub(bypass, "GET", "user", fn conn ->
+      PlugConn.resp(conn, 401, "")
+    end)
+
+    assert {:error, :upstream, "Status code 401 from GitHub"} ==
+             GitHub.get_name(config(bypass), "invalid_access_token")
+  end
+end

test/cadet/auth/providers/openid_test.exs

@@ -33,6 +33,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
   """
 
   @username "username"
+  @namespaced_username "test/username"
   @role :admin
 
   @openid_provider_name :test
@@ -103,7 +104,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
 
     bypass_return_token(bypass, @okay_token)
 
-    assert {:ok, %{token: @okay_token, username: @username}} =
+    assert {:ok, %{token: @okay_token, username: @namespaced_username}} =
              OpenID.authorise(@config, "dummy_code", "", "")
 
     assert {:ok, @username} == OpenID.get_name(@config, @okay_token)

test/factories/accounts/user_factory.ex

@@ -14,7 +14,7 @@ defmodule Cadet.Accounts.UserFactory do
           username:
             sequence(
               :nusnet_id,
-              &"E#{&1 |> Integer.to_string() |> String.pad_leading(7, "0")}"
+              &"test/E#{&1 |> Integer.to_string() |> String.pad_leading(7, "0")}"
             ),
           game_states: %{}
         }
@@ -27,7 +27,7 @@ defmodule Cadet.Accounts.UserFactory do
           username:
             sequence(
               :nusnet_id,
-              &"E#{&1 |> Integer.to_string() |> String.pad_leading(7, "0")}"
+              &"test/E#{&1 |> Integer.to_string() |> String.pad_leading(7, "0")}"
             ),
           game_states: %{}
         }

test/test_helper.exs

@@ -6,3 +6,19 @@ ExUnit.start()
 Faker.start()
 
 Ecto.Adapters.SQL.Sandbox.mode(Cadet.Repo, :manual)
+
+defmodule Cadet.TestHelper do
+  @doc """
+  Removes a preloaded Ecto association.
+  """
+  def remove_preload(struct, field, cardinality \\ :one) do
+    %{
+      struct
+      | field => %Ecto.Association.NotLoaded{
+          __field__: field,
+          __owner__: struct.__struct__,
+          __cardinality__: cardinality
+        }
+    }
+  end
+end