From 670b38224b2f223d1d6d80c7483babfb7a40e435 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sun, 3 Sep 2023 15:33:47 +0800
Subject: [PATCH 01/24] Add samly dependency

---
 mix.exs  | 1 +
 mix.lock | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/mix.exs b/mix.exs
index 8a248575d..c8edaad2a 100644
--- a/mix.exs
+++ b/mix.exs
@@ -79,6 +79,7 @@ defmodule Cadet.Mixfile do
       {:quantum, "~> 3.0"},
       {:que, "~> 0.10"},
       {:recase, "~> 0.7", override: true},
+      {:samly, "~> 1.0"},
       {:sentry, "~> 8.0"},
       {:sweet_xml, "~> 0.6"},
       {:timex, "~> 3.7"},
diff --git a/mix.lock b/mix.lock
index 87b92af28..bcd2a88d6 100644
--- a/mix.lock
+++ b/mix.lock
@@ -29,6 +29,7 @@
   "ecto_sql": {:hex, :ecto_sql, "3.11.1", "e9abf28ae27ef3916b43545f9578b4750956ccea444853606472089e7d169470", [:mix], [{:db_connection, "~> 2.5 or ~> 2.4.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.11.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.6.0", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.16.0 or ~> 0.17.0 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:tds, "~> 2.1.1 or ~> 2.2", [hex: :tds, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ce14063ab3514424276e7e360108ad6c2308f6d88164a076aac8a387e1fea634"},
   "eiconv": {:hex, :eiconv, "1.0.0", "ee1e47ee37799a05beff7a68d61f63cccc93101833c4fb94b454c23b12a21629", [:rebar3], [], "hexpm", "8c80851decf72fc4571a70278d7932e9a87437770322077ecf797533fbb792cd"},
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
+  "esaml": {:hex, :esaml, "4.5.0", "4c79a47511b212deab235816b10f89cf884a7da662ef50956176bff8bca1f8e8", [:rebar3], [{:cowboy, "< 3.0.0", [hex: :cowboy, repo: "hexpm", optional: false]}], "hexpm", "4697e5cdd70c9ea0fd7ff8ff1c4049566bfa9202588b56924b181c3b2976d67d"},
   "ex_aws": {:hex, :ex_aws, "2.5.3", "9c2d05ba0c057395b12c7b5ca6267d14cdaec1d8e65bdf6481fe1fd245accfb4", [:mix], [{:configparser_ex, "~> 4.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8 or ~> 3.0", [hex: :jsx, repo: "hexpm", optional: true]}, {:mime, "~> 1.2 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "67115f1d399d7ec4d191812ee565c6106cb4b1bbf19a9d4db06f265fd87da97e"},
   "ex_aws_lambda": {:hex, :ex_aws_lambda, "2.1.0", "f28bffae6dde34ba17ef815ef50a82a1e7f3af26d36fe31dbda3a7657e0de449", [:mix], [{:ex_aws, "~> 2.0", [hex: :ex_aws, repo: "hexpm", optional: false]}], "hexpm", "25608630b8b45fe22b0237696662f88be724bc89f77ee42708a5871511f531a0"},
   "ex_aws_s3": {:hex, :ex_aws_s3, "2.4.0", "ce8decb6b523381812798396bc0e3aaa62282e1b40520125d1f4eff4abdff0f4", [:mix], [{:ex_aws, "~> 2.0", [hex: :ex_aws, repo: "hexpm", optional: false]}, {:sweet_xml, ">= 0.0.0", [hex: :sweet_xml, repo: "hexpm", optional: true]}], "hexpm", "85dda6e27754d94582869d39cba3241d9ea60b6aa4167f9c88e309dc687e56bb"},
@@ -85,7 +86,12 @@
   "que": {:hex, :que, "0.10.1", "788ed0ec92ed69bdf9cfb29bf41a94ca6355b8d44959bd0669cf706e557ac891", [:mix], [{:ex_utils, "~> 0.1.6", [hex: :ex_utils, repo: "hexpm", optional: false]}, {:memento, "~> 0.3.0", [hex: :memento, repo: "hexpm", optional: false]}], "hexpm", "a737b365253e75dbd24b2d51acc1d851049e87baae08cd0c94e2bc5cd65088d5"},
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
   "recase": {:hex, :recase, "0.7.0", "3f2f719f0886c7a3b7fe469058ec539cb7bbe0023604ae3bce920e186305e5ae", [:mix], [], "hexpm", "36f5756a9f552f4a94b54a695870e32f4e72d5fad9c25e61bc4a3151c08a4e0c"},
+<<<<<<< HEAD
   "sentry": {:hex, :sentry, "8.1.0", "8d235b62fce5f8e067ea1644e30939405b71a5e1599d9529ff82899d11d03f2b", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "f9fc7641ef61e885510f5e5963c2948b9de1de597c63f781e9d3d6c9c8681ab4"},
+=======
+  "samly": {:hex, :samly, "1.3.0", "4ff4d9fe4e71d0d153ac83e460c6dd1c2e24d3a4c756b3f40ec363da80bd6c8d", [:mix], [{:esaml, "~> 4.3", [hex: :esaml, repo: "hexpm", optional: false]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.6", [hex: :sweet_xml, repo: "hexpm", optional: false]}], "hexpm", "b21632cad5ecf86046d3c5eaaf9f3e4fba0b690ba19924db2b0b9555d84388a7"},
+  "sentry": {:hex, :sentry, "8.0.6", "c8de1bf0523bc120ec37d596c55260901029ecb0994e7075b0973328779ceef7", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "051a2d0472162f3137787c7c9d6e6e4ef239de9329c8c45b1f1bf1e9379e1883"},
+>>>>>>> 1178c5c (Add samly dependency)
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
   "sweet_xml": {:hex, :sweet_xml, "0.7.4", "a8b7e1ce7ecd775c7e8a65d501bc2cd933bff3a9c41ab763f5105688ef485d08", [:mix], [], "hexpm", "e7c4b0bdbf460c928234951def54fe87edf1a170f6896675443279e2dbeba167"},
   "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},

From 858b92e83cf0251d201a7a48cf1e39b15abc57d6 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sun, 3 Sep 2023 16:36:38 +0800
Subject: [PATCH 02/24] Set up samly

---
 lib/cadet/application.ex | 3 ++-
 lib/cadet_web/router.ex  | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/cadet/application.ex b/lib/cadet/application.ex
index f2249418a..619ec74dc 100644
--- a/lib/cadet/application.ex
+++ b/lib/cadet/application.ex
@@ -22,7 +22,8 @@ defmodule Cadet.Application do
       # Start the Quantum scheduler
       worker(Cadet.Jobs.Scheduler, []),
       # Start the Oban instance
-      {Oban, Application.fetch_env!(:cadet, Oban)}
+      {Oban, Application.fetch_env!(:cadet, Oban)},
+      {Samly.Provider, []}
     ]
 
     children =
diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex
index b54bb9f69..705a62d76 100644
--- a/lib/cadet_web/router.ex
+++ b/lib/cadet_web/router.ex
@@ -28,6 +28,10 @@ defmodule CadetWeb.Router do
     get("/.well-known/jwks.json", JWKSController, :index)
   end
 
+  scope "/sso" do
+    forward("/", Samly.Router)
+  end
+
   # V2 API
 
   # Public Pages

From 28e49e35e327adaa6ba03ed1ec09fd7024d91ae3 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 06:15:27 +0700
Subject: [PATCH 03/24] Fix incorrect documentation

---
 lib/cadet_web/controllers/auth_controller.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 99db02c7d..3988b931c 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -47,7 +47,7 @@ defmodule CadetWeb.AuthController do
         |> text("Unknown error: #{reason}")
 
       {:signin, {:error, status, reason}} ->
-        # reason can be :bad_request or :internal_server_error
+        # status can be :bad_request or :internal_server_error
         conn
         |> put_status(status)
         |> text("Unable to retrieve user: #{reason}")

From 5990278c3e55bef80ab15b1bbd87f1afb25357d7 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 06:16:48 +0700
Subject: [PATCH 04/24] Make code param optional in auth controller

---
 lib/cadet_web/controllers/auth_controller.ex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 3988b931c..1084178ec 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -19,10 +19,11 @@ defmodule CadetWeb.AuthController do
   def create(
         conn,
         params = %{
-          "code" => code,
           "provider" => provider
         }
       ) do
+    # Code is optional as SAML flow is mainly handled by backend
+    code = Map.get(params, "code")
     client_id = Map.get(params, "client_id")
     redirect_uri = Map.get(params, "redirect_uri")
 

From 7452da4d536eeb62135d4cf39711e45ce443d142 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 06:20:55 +0700
Subject: [PATCH 05/24] Create test SAML provider

---
 lib/cadet/auth/providers/simplesaml.ex | 52 ++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 lib/cadet/auth/providers/simplesaml.ex

diff --git a/lib/cadet/auth/providers/simplesaml.ex b/lib/cadet/auth/providers/simplesaml.ex
new file mode 100644
index 000000000..5ccfd511a
--- /dev/null
+++ b/lib/cadet/auth/providers/simplesaml.ex
@@ -0,0 +1,52 @@
+defmodule Cadet.Auth.Providers.SimpleSAML do
+  @moduledoc """
+  Provides identity using SimpleSAMLphp.
+  """
+  alias Cadet.Auth.Provider
+  alias Samly.Assertion
+
+  require Logger
+
+  @behaviour Provider
+
+  @type config :: %{idp_id: String.t()}
+
+  @spec authorise(
+          any(),
+          Provider.code() | Plug.Conn.t(),
+          Provider.client_id(),
+          Provider.redirect_uri()
+        ) ::
+          {:ok, %{token: Provider.token(), username: String.t()}}
+          | {:error, Provider.error(), String.t()}
+  def authorise(config, conn, _client_id, _redirect_uri) do
+    # Logger.debug("test")
+    assertion = Samly.get_active_assertion(conn)
+    # Logger.debug("assertion: #{inspect(assertion)}")
+    expected_id = config.idp_id
+
+    case get_assertion_idp_id(assertion) do
+      nil ->
+        {:error, :invalid_credentials, "Missing SAML assertion"}
+
+      ^expected_id ->
+        {:ok, %{token: "testtoken", username: "testuser"}}
+
+      _ ->
+        {:error, :bad_request, "Invalid authentication provider"}
+    end
+  end
+
+  @spec get_name(any(), Provider.token()) ::
+          {:ok, String.t()} | {:error, Provider.error(), String.t()}
+  def get_name(_config, _token) do
+    # case Enum.find(config, nil, fn %{token: this_token} -> token == this_token end) do
+    #   %{name: name} -> {:ok, name}
+    #   _ -> {:error, :invalid_credentials, "Invalid token"}
+    # end
+    {:ok, "testuser"}
+  end
+
+  defp get_assertion_idp_id(%Assertion{idp_id: idp_id}), do: idp_id
+  defp get_assertion_idp_id(nil), do: nil
+end

From f8e1698065d8d1e3b099ac9793ff0afc97d975d2 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 06:22:56 +0700
Subject: [PATCH 06/24] Update example config to configure SAML IdP

---
 config/dev.secrets.exs.example | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config/dev.secrets.exs.example b/config/dev.secrets.exs.example
index ded86e074..15c4d2786 100644
--- a/config/dev.secrets.exs.example
+++ b/config/dev.secrets.exs.example
@@ -29,6 +29,7 @@ config :cadet,
     #       token_url: "https://github.com/login/oauth/access_token",
     #       user_api: "https://api.github.com/user"
     #    }},
+    "test_saml" => {Cadet.Auth.Providers.SimpleSAML, %{idp_id: "testidp"}},
     "test" =>
       {Cadet.Auth.Providers.Config,
        [

From df01025d884b48064f66d5ff2e402522da9f1497 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 06:26:33 +0700
Subject: [PATCH 07/24] Fallback to connection for code

Hacky fallback because SAML does not require a code to be sent in the
authentication flow.
---
 lib/cadet_web/controllers/auth_controller.ex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 1084178ec..e68579955 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -23,7 +23,8 @@ defmodule CadetWeb.AuthController do
         }
       ) do
     # Code is optional as SAML flow is mainly handled by backend
-    code = Map.get(params, "code")
+    # FIXME: HACKY fallback for SAML (because no code is needed)
+    code = Map.get(params, "code", conn)
     client_id = Map.get(params, "client_id")
     redirect_uri = Map.get(params, "redirect_uri")
 

From 72598c9beca53eaf9bfb39405159b9acdd5e8f57 Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 06:33:41 +0700
Subject: [PATCH 08/24] Replace FIXME comment with TODO

FIXME was causing `mix credo` to fail.
---
 lib/cadet_web/controllers/auth_controller.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index e68579955..3b38b46b5 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -23,7 +23,7 @@ defmodule CadetWeb.AuthController do
         }
       ) do
     # Code is optional as SAML flow is mainly handled by backend
-    # FIXME: HACKY fallback for SAML (because no code is needed)
+    # TODO: Potentially HACKY fallback for SAML (because no code is needed)
     code = Map.get(params, "code", conn)
     client_id = Map.get(params, "client_id")
     redirect_uri = Map.get(params, "redirect_uri")

From 82bfbfac7f9d6cabec95c19a0fe87eb1687119fc Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Sat, 9 Dec 2023 10:02:19 +0700
Subject: [PATCH 09/24] Fix typing

---
 lib/cadet/auth/provider.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cadet/auth/provider.ex b/lib/cadet/auth/provider.ex
index 7e399f203..9a8bb8905 100644
--- a/lib/cadet/auth/provider.ex
+++ b/lib/cadet/auth/provider.ex
@@ -4,7 +4,7 @@ defmodule Cadet.Auth.Provider do
   it for a token with the OAuth2 provider, and then retrieves the user ID and name.
   """
 
-  @type code :: String.t()
+  @type code :: String.t() | Plug.Conn.t()
   @type token :: String.t()
   @type client_id :: String.t()
   @type redirect_uri :: String.t()

From bbd9923b6bea486f53574eec463a775db31c6bd7 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Fri, 10 May 2024 15:32:55 +0800
Subject: [PATCH 10/24] Test student saml config

---
 lib/cadet/auth/providers/simplesaml.ex | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/cadet/auth/providers/simplesaml.ex b/lib/cadet/auth/providers/simplesaml.ex
index 5ccfd511a..78cffa5f0 100644
--- a/lib/cadet/auth/providers/simplesaml.ex
+++ b/lib/cadet/auth/providers/simplesaml.ex
@@ -22,15 +22,22 @@ defmodule Cadet.Auth.Providers.SimpleSAML do
   def authorise(config, conn, _client_id, _redirect_uri) do
     # Logger.debug("test")
     assertion = Samly.get_active_assertion(conn)
+    IO.inspect(assertion)
+    IO.inspect(assertion.attributes)
     # Logger.debug("assertion: #{inspect(assertion)}")
     expected_id = config.idp_id
 
     case get_assertion_idp_id(assertion) do
       nil ->
-        {:error, :invalid_credentials, "Missing SAML assertion"}
+        {:error, :invalid_credentials, "Missing SAML assertion!"}
 
       ^expected_id ->
-        {:ok, %{token: "testtoken", username: "testuser"}}
+        # TODO: Add a assertion extractor to get the different fields
+        {:ok,
+         %{
+           token: Map.get(assertion.attributes, "displayname"),
+           username: Map.get(assertion.attributes, "samaccountname")
+         }}
 
       _ ->
         {:error, :bad_request, "Invalid authentication provider"}
@@ -39,12 +46,12 @@ defmodule Cadet.Auth.Providers.SimpleSAML do
 
   @spec get_name(any(), Provider.token()) ::
           {:ok, String.t()} | {:error, Provider.error(), String.t()}
-  def get_name(_config, _token) do
+  def get_name(_config, token_as_name) do
     # case Enum.find(config, nil, fn %{token: this_token} -> token == this_token end) do
     #   %{name: name} -> {:ok, name}
     #   _ -> {:error, :invalid_credentials, "Invalid token"}
     # end
-    {:ok, "testuser"}
+    {:ok, token_as_name}
   end
 
   defp get_assertion_idp_id(%Assertion{idp_id: idp_id}), do: idp_id

From 92b20c57305f5f1e326812bdb50655b2622a37b1 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Fri, 10 May 2024 15:45:18 +0800
Subject: [PATCH 11/24] Fix rebase conflict

---
 mix.lock | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/mix.lock b/mix.lock
index bcd2a88d6..48fd8d55b 100644
--- a/mix.lock
+++ b/mix.lock
@@ -86,12 +86,8 @@
   "que": {:hex, :que, "0.10.1", "788ed0ec92ed69bdf9cfb29bf41a94ca6355b8d44959bd0669cf706e557ac891", [:mix], [{:ex_utils, "~> 0.1.6", [hex: :ex_utils, repo: "hexpm", optional: false]}, {:memento, "~> 0.3.0", [hex: :memento, repo: "hexpm", optional: false]}], "hexpm", "a737b365253e75dbd24b2d51acc1d851049e87baae08cd0c94e2bc5cd65088d5"},
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
   "recase": {:hex, :recase, "0.7.0", "3f2f719f0886c7a3b7fe469058ec539cb7bbe0023604ae3bce920e186305e5ae", [:mix], [], "hexpm", "36f5756a9f552f4a94b54a695870e32f4e72d5fad9c25e61bc4a3151c08a4e0c"},
-<<<<<<< HEAD
-  "sentry": {:hex, :sentry, "8.1.0", "8d235b62fce5f8e067ea1644e30939405b71a5e1599d9529ff82899d11d03f2b", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "f9fc7641ef61e885510f5e5963c2948b9de1de597c63f781e9d3d6c9c8681ab4"},
-=======
   "samly": {:hex, :samly, "1.3.0", "4ff4d9fe4e71d0d153ac83e460c6dd1c2e24d3a4c756b3f40ec363da80bd6c8d", [:mix], [{:esaml, "~> 4.3", [hex: :esaml, repo: "hexpm", optional: false]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.6", [hex: :sweet_xml, repo: "hexpm", optional: false]}], "hexpm", "b21632cad5ecf86046d3c5eaaf9f3e4fba0b690ba19924db2b0b9555d84388a7"},
-  "sentry": {:hex, :sentry, "8.0.6", "c8de1bf0523bc120ec37d596c55260901029ecb0994e7075b0973328779ceef7", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "051a2d0472162f3137787c7c9d6e6e4ef239de9329c8c45b1f1bf1e9379e1883"},
->>>>>>> 1178c5c (Add samly dependency)
+  "sentry": {:hex, :sentry, "8.1.0", "8d235b62fce5f8e067ea1644e30939405b71a5e1599d9529ff82899d11d03f2b", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "f9fc7641ef61e885510f5e5963c2948b9de1de597c63f781e9d3d6c9c8681ab4"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
   "sweet_xml": {:hex, :sweet_xml, "0.7.4", "a8b7e1ce7ecd775c7e8a65d501bc2cd933bff3a9c41ab763f5105688ef485d08", [:mix], [], "hexpm", "e7c4b0bdbf460c928234951def54fe87edf1a170f6896675443279e2dbeba167"},
   "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},

From 4f994a8fedff5b031d3e2f0960484422f56326aa Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sat, 11 May 2024 01:56:36 +0800
Subject: [PATCH 12/24] Add assertion extractor

---
 config/cadet.exs.example                      | 32 ++++++++++
 config/dev.secrets.exs.example                | 35 ++++++++++-
 .../saml/nusstf_assertion_extractor.ex        | 15 +++++
 .../saml/nusstu_assertion_extractor.ex        | 15 +++++
 lib/cadet/auth/providers/saml/saml.ex         | 48 +++++++++++++++
 lib/cadet/auth/providers/simplesaml.ex        | 59 -------------------
 6 files changed, 143 insertions(+), 61 deletions(-)
 create mode 100644 lib/cadet/auth/providers/saml/nusstf_assertion_extractor.ex
 create mode 100644 lib/cadet/auth/providers/saml/nusstu_assertion_extractor.ex
 create mode 100644 lib/cadet/auth/providers/saml/saml.ex
 delete mode 100644 lib/cadet/auth/providers/simplesaml.ex

diff --git a/config/cadet.exs.example b/config/cadet.exs.example
index 1040e3c0e..2faf4f1a9 100644
--- a/config/cadet.exs.example
+++ b/config/cadet.exs.example
@@ -56,6 +56,12 @@ config :cadet,
     #      # You may need to write your own claim extractor for other providers
     #      claim_extractor: Cadet.Auth.Providers.CognitoClaimExtractor
     #    }},
+
+    # # Example SAML authentication with NUS Student IdP
+    # "test_saml" =>
+    #   {Cadet.Auth.Providers.SAML,
+    #   %{assertion_extractor: Cadet.Auth.Providers.NusstuAssertionExtractor}},
+
     "test" =>
       {Cadet.Auth.Providers.Config,
        [
@@ -142,3 +148,29 @@ config :cadet,
 # You may also want to change the timezone used for scheduled jobs
 # config :cadet, Cadet.Jobs.Scheduler,
 #   timezone: "Asia/Singapore",
+
+# # Additional configuration for SAML authentication
+# # For more details, see https://github.com/handnot2/samly
+# config :samly, Samly.Provider,
+#   idp_id_from: :path_segment,
+#   service_providers: [
+#     %{
+#       id: "source-academy-backend",
+#       certfile: "example_path/certfile.pem",
+#       keyfile: "example_path/keyfile.pem"
+#     }
+#   ],
+#   identity_providers: [
+#     %{
+#       id: "student",
+#       sp_id: "source-academy-backend",
+#       base_url: "https://example_backend/sso",
+#       metadata_file: "student_idp_metadata.xml"
+#     },
+#     %{
+#       id: "staff",
+#       sp_id: "source-academy-backend",
+#       base_url: "https://example_backend/sso",
+#       metadata_file: "staff_idp_metadata.xml"
+#     }
+#   ]
diff --git a/config/dev.secrets.exs.example b/config/dev.secrets.exs.example
index 15c4d2786..c1db7e0ee 100644
--- a/config/dev.secrets.exs.example
+++ b/config/dev.secrets.exs.example
@@ -29,7 +29,12 @@ config :cadet,
     #       token_url: "https://github.com/login/oauth/access_token",
     #       user_api: "https://api.github.com/user"
     #    }},
-    "test_saml" => {Cadet.Auth.Providers.SimpleSAML, %{idp_id: "testidp"}},
+
+    # # Example SAML authentication with NUS Student IdP
+    # "test_saml" =>
+    #   {Cadet.Auth.Providers.SAML,
+    #   %{assertion_extractor: Cadet.Auth.Providers.NusstuAssertionExtractor}},
+
     "test" =>
       {Cadet.Auth.Providers.Config,
        [
@@ -91,10 +96,36 @@ config :cadet,
   config :openai,
   # find it at https://platform.openai.com/account/api-keys
   api_key: "the actual api key",
-  # For source academy deployment, leave this as empty string.Ingeneral could find it at https://platform.openai.com/account/org-settings under "Organization ID". 
+  # For source academy deployment, leave this as empty string.Ingeneral could find it at https://platform.openai.com/account/org-settings under "Organization ID".
   organization_key: "",
   # optional, passed to [HTTPoison.Request](https://hexdocs.pm/httpoison/HTTPoison.Request.html) options
   http_options: [recv_timeout: 170_0000]
 
 config :sentry,
   dsn: "https://public_key/sentry.io/somethingsomething"
+
+# # Additional configuration for SAML authentication
+# # For more details, see https://github.com/handnot2/samly
+# config :samly, Samly.Provider,
+#   idp_id_from: :path_segment,
+#   service_providers: [
+#     %{
+#       id: "source-academy-backend",
+#       certfile: "example_path/certfile.pem",
+#       keyfile: "example_path/keyfile.pem"
+#     }
+#   ],
+#   identity_providers: [
+#     %{
+#       id: "student",
+#       sp_id: "source-academy-backend",
+#       base_url: "https://example_backend/sso",
+#       metadata_file: "student_idp_metadata.xml"
+#     },
+#     %{
+#       id: "staff",
+#       sp_id: "source-academy-backend",
+#       base_url: "https://example_backend/sso",
+#       metadata_file: "staff_idp_metadata.xml"
+#     }
+#   ]
diff --git a/lib/cadet/auth/providers/saml/nusstf_assertion_extractor.ex b/lib/cadet/auth/providers/saml/nusstf_assertion_extractor.ex
new file mode 100644
index 000000000..ac191823e
--- /dev/null
+++ b/lib/cadet/auth/providers/saml/nusstf_assertion_extractor.ex
@@ -0,0 +1,15 @@
+defmodule Cadet.Auth.Providers.NusstfAssertionExtractor do
+  @moduledoc """
+  Extracts fields from NUS Staff IdP SAML assertions.
+  """
+
+  @behaviour Cadet.Auth.Providers.AssertionExtractor
+
+  def get_username(assertion) do
+    Map.get(assertion.attributes, "SamAccountName")
+  end
+
+  def get_name(assertion) do
+    Map.get(assertion.attributes, "DisplayName")
+  end
+end
diff --git a/lib/cadet/auth/providers/saml/nusstu_assertion_extractor.ex b/lib/cadet/auth/providers/saml/nusstu_assertion_extractor.ex
new file mode 100644
index 000000000..d7332d909
--- /dev/null
+++ b/lib/cadet/auth/providers/saml/nusstu_assertion_extractor.ex
@@ -0,0 +1,15 @@
+defmodule Cadet.Auth.Providers.NusstuAssertionExtractor do
+  @moduledoc """
+  Extracts fields from NUS Student IdP SAML assertions.
+  """
+
+  @behaviour Cadet.Auth.Providers.AssertionExtractor
+
+  def get_username(assertion) do
+    Map.get(assertion.attributes, "samaccountname")
+  end
+
+  def get_name(assertion) do
+    Map.get(assertion.attributes, "samaccountname")
+  end
+end
diff --git a/lib/cadet/auth/providers/saml/saml.ex b/lib/cadet/auth/providers/saml/saml.ex
new file mode 100644
index 000000000..cc2e63f6f
--- /dev/null
+++ b/lib/cadet/auth/providers/saml/saml.ex
@@ -0,0 +1,48 @@
+defmodule Cadet.Auth.Providers.SAML do
+  @moduledoc """
+  Provides identity using SAML.
+  """
+  alias Cadet.Auth.Provider
+  alias Samly.Assertion
+
+  @behaviour Provider
+
+  @type config :: %{assertion_extractor: module()}
+
+  @spec authorise(
+          any(),
+          Provider.code() | Plug.Conn.t(),
+          Provider.client_id(),
+          Provider.redirect_uri()
+        ) ::
+          {:ok, %{token: Provider.token(), username: String.t()}}
+          | {:error, Provider.error(), String.t()}
+  def authorise(config, conn, _client_id, _redirect_uri) do
+    %{assertion_extractor: assertion_extractor} = config
+
+    with {:assertion, assertion} when not is_nil(assertion) <-
+           {:assertion, Samly.get_attribute(conn)} do
+      {:ok,
+       %{
+         token: Jason.encode!(%{name: assertion_extractor.get_name(assertion)}),
+         username: assertion_extractor.get_username(assertion)
+       }}
+    else
+      {:assertion, nil} -> {:error, :invalid_credentials, "Missing SAML assertion!"}
+    end
+  end
+
+  @spec get_name(any(), Provider.token()) ::
+          {:ok, String.t()} | {:error, Provider.error(), String.t()}
+  def get_name(_config, token) do
+    {:ok, Jason.decode!(token).name}
+  end
+end
+
+defmodule Cadet.Auth.Providers.AssertionExtractor do
+  @moduledoc """
+  A behaviour for modules that extract fields from SAML assertions.
+  """
+  @callback get_username(Samly.Assertion) :: String.t() | nil
+  @callback get_name(Samly.Assertion) :: String.t() | nil
+end
diff --git a/lib/cadet/auth/providers/simplesaml.ex b/lib/cadet/auth/providers/simplesaml.ex
deleted file mode 100644
index 78cffa5f0..000000000
--- a/lib/cadet/auth/providers/simplesaml.ex
+++ /dev/null
@@ -1,59 +0,0 @@
-defmodule Cadet.Auth.Providers.SimpleSAML do
-  @moduledoc """
-  Provides identity using SimpleSAMLphp.
-  """
-  alias Cadet.Auth.Provider
-  alias Samly.Assertion
-
-  require Logger
-
-  @behaviour Provider
-
-  @type config :: %{idp_id: String.t()}
-
-  @spec authorise(
-          any(),
-          Provider.code() | Plug.Conn.t(),
-          Provider.client_id(),
-          Provider.redirect_uri()
-        ) ::
-          {:ok, %{token: Provider.token(), username: String.t()}}
-          | {:error, Provider.error(), String.t()}
-  def authorise(config, conn, _client_id, _redirect_uri) do
-    # Logger.debug("test")
-    assertion = Samly.get_active_assertion(conn)
-    IO.inspect(assertion)
-    IO.inspect(assertion.attributes)
-    # Logger.debug("assertion: #{inspect(assertion)}")
-    expected_id = config.idp_id
-
-    case get_assertion_idp_id(assertion) do
-      nil ->
-        {:error, :invalid_credentials, "Missing SAML assertion!"}
-
-      ^expected_id ->
-        # TODO: Add a assertion extractor to get the different fields
-        {:ok,
-         %{
-           token: Map.get(assertion.attributes, "displayname"),
-           username: Map.get(assertion.attributes, "samaccountname")
-         }}
-
-      _ ->
-        {:error, :bad_request, "Invalid authentication provider"}
-    end
-  end
-
-  @spec get_name(any(), Provider.token()) ::
-          {:ok, String.t()} | {:error, Provider.error(), String.t()}
-  def get_name(_config, token_as_name) do
-    # case Enum.find(config, nil, fn %{token: this_token} -> token == this_token end) do
-    #   %{name: name} -> {:ok, name}
-    #   _ -> {:error, :invalid_credentials, "Invalid token"}
-    # end
-    {:ok, token_as_name}
-  end
-
-  defp get_assertion_idp_id(%Assertion{idp_id: idp_id}), do: idp_id
-  defp get_assertion_idp_id(nil), do: nil
-end

From bd479210839ee029c20700c15d7b53586490afa2 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sat, 11 May 2024 11:52:36 +0800
Subject: [PATCH 13/24] Restructure auth provider directory

---
 lib/cadet/auth/providers/{ => openid}/auth0_claim_extractor.ex    | 0
 lib/cadet/auth/providers/{ => openid}/cognito_claim_extractor.ex  | 0
 lib/cadet/auth/providers/{ => openid}/google_claim_extractor.ex   | 0
 lib/cadet/auth/providers/{ => openid}/mit_claim_extractor.ex      | 0
 .../auth/providers/{ => openid}/mit_csail_claim_extractor.ex      | 0
 lib/cadet/auth/providers/{ => openid}/openid.ex                   | 0
 6 files changed, 0 insertions(+), 0 deletions(-)
 rename lib/cadet/auth/providers/{ => openid}/auth0_claim_extractor.ex (100%)
 rename lib/cadet/auth/providers/{ => openid}/cognito_claim_extractor.ex (100%)
 rename lib/cadet/auth/providers/{ => openid}/google_claim_extractor.ex (100%)
 rename lib/cadet/auth/providers/{ => openid}/mit_claim_extractor.ex (100%)
 rename lib/cadet/auth/providers/{ => openid}/mit_csail_claim_extractor.ex (100%)
 rename lib/cadet/auth/providers/{ => openid}/openid.ex (100%)

diff --git a/lib/cadet/auth/providers/auth0_claim_extractor.ex b/lib/cadet/auth/providers/openid/auth0_claim_extractor.ex
similarity index 100%
rename from lib/cadet/auth/providers/auth0_claim_extractor.ex
rename to lib/cadet/auth/providers/openid/auth0_claim_extractor.ex
diff --git a/lib/cadet/auth/providers/cognito_claim_extractor.ex b/lib/cadet/auth/providers/openid/cognito_claim_extractor.ex
similarity index 100%
rename from lib/cadet/auth/providers/cognito_claim_extractor.ex
rename to lib/cadet/auth/providers/openid/cognito_claim_extractor.ex
diff --git a/lib/cadet/auth/providers/google_claim_extractor.ex b/lib/cadet/auth/providers/openid/google_claim_extractor.ex
similarity index 100%
rename from lib/cadet/auth/providers/google_claim_extractor.ex
rename to lib/cadet/auth/providers/openid/google_claim_extractor.ex
diff --git a/lib/cadet/auth/providers/mit_claim_extractor.ex b/lib/cadet/auth/providers/openid/mit_claim_extractor.ex
similarity index 100%
rename from lib/cadet/auth/providers/mit_claim_extractor.ex
rename to lib/cadet/auth/providers/openid/mit_claim_extractor.ex
diff --git a/lib/cadet/auth/providers/mit_csail_claim_extractor.ex b/lib/cadet/auth/providers/openid/mit_csail_claim_extractor.ex
similarity index 100%
rename from lib/cadet/auth/providers/mit_csail_claim_extractor.ex
rename to lib/cadet/auth/providers/openid/mit_csail_claim_extractor.ex
diff --git a/lib/cadet/auth/providers/openid.ex b/lib/cadet/auth/providers/openid/openid.ex
similarity index 100%
rename from lib/cadet/auth/providers/openid.ex
rename to lib/cadet/auth/providers/openid/openid.ex

From 15f9700ac01e081fcb80fd8fd750f033859a8a22 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 01:58:22 +0800
Subject: [PATCH 14/24] Update SAML provider authorise

---
 lib/cadet/auth/providers/saml/saml.ex | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/cadet/auth/providers/saml/saml.ex b/lib/cadet/auth/providers/saml/saml.ex
index cc2e63f6f..fade8429a 100644
--- a/lib/cadet/auth/providers/saml/saml.ex
+++ b/lib/cadet/auth/providers/saml/saml.ex
@@ -21,21 +21,26 @@ defmodule Cadet.Auth.Providers.SAML do
     %{assertion_extractor: assertion_extractor} = config
 
     with {:assertion, assertion} when not is_nil(assertion) <-
-           {:assertion, Samly.get_attribute(conn)} do
+           {:assertion, Samly.get_active_assertion(conn)},
+         {:name, name} when not is_nil(name) <- {:name, assertion_extractor.get_name(assertion)},
+         {:username, username} when not is_nil(username) <-
+           {:username, assertion_extractor.get_username(assertion)} do
       {:ok,
        %{
-         token: Jason.encode!(%{name: assertion_extractor.get_name(assertion)}),
-         username: assertion_extractor.get_username(assertion)
+         token: Jason.encode!(%{name: name}),
+         username: username
        }}
     else
       {:assertion, nil} -> {:error, :invalid_credentials, "Missing SAML assertion!"}
+      {:name, nil} -> {:error, :invalid_credentials, "Missing name attribute!"}
+      {:username, nil} -> {:error, :invalid_credentials, "Missing username attribute!"}
     end
   end
 
   @spec get_name(any(), Provider.token()) ::
           {:ok, String.t()} | {:error, Provider.error(), String.t()}
   def get_name(_config, token) do
-    {:ok, Jason.decode!(token).name}
+    {:ok, Jason.decode!(token)["name"]}
   end
 end
 

From 6efaea2344c3ab41b56202c714c2c21492005c24 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 17:24:04 +0800
Subject: [PATCH 15/24] Add SAML redirect flow

---
 config/cadet.exs.example                     |  5 +-
 config/dev.secrets.exs.example               |  5 +-
 lib/cadet/auth/providers/saml/saml.ex        |  1 -
 lib/cadet_web/controllers/auth_controller.ex | 54 ++++++++++++++++++--
 lib/cadet_web/router.ex                      |  1 +
 5 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/config/cadet.exs.example b/config/cadet.exs.example
index 2faf4f1a9..f20aae580 100644
--- a/config/cadet.exs.example
+++ b/config/cadet.exs.example
@@ -60,7 +60,10 @@ config :cadet,
     # # Example SAML authentication with NUS Student IdP
     # "test_saml" =>
     #   {Cadet.Auth.Providers.SAML,
-    #   %{assertion_extractor: Cadet.Auth.Providers.NusstuAssertionExtractor}},
+    #    %{
+    #      assertion_extractor: Cadet.Auth.Providers.NusstuAssertionExtractor,
+    #      client_redirect_url: "http://cadet.frontend:8000/login/callback"
+    #    }},
 
     "test" =>
       {Cadet.Auth.Providers.Config,
diff --git a/config/dev.secrets.exs.example b/config/dev.secrets.exs.example
index c1db7e0ee..088f27727 100644
--- a/config/dev.secrets.exs.example
+++ b/config/dev.secrets.exs.example
@@ -33,7 +33,10 @@ config :cadet,
     # # Example SAML authentication with NUS Student IdP
     # "test_saml" =>
     #   {Cadet.Auth.Providers.SAML,
-    #   %{assertion_extractor: Cadet.Auth.Providers.NusstuAssertionExtractor}},
+    #    %{
+    #      assertion_extractor: Cadet.Auth.Providers.NusstuAssertionExtractor,
+    #      client_redirect_url: "http://cadet.frontend:8000/login/callback"
+    #    }},
 
     "test" =>
       {Cadet.Auth.Providers.Config,
diff --git a/lib/cadet/auth/providers/saml/saml.ex b/lib/cadet/auth/providers/saml/saml.ex
index fade8429a..015be3738 100644
--- a/lib/cadet/auth/providers/saml/saml.ex
+++ b/lib/cadet/auth/providers/saml/saml.ex
@@ -3,7 +3,6 @@ defmodule Cadet.Auth.Providers.SAML do
   Provides identity using SAML.
   """
   alias Cadet.Auth.Provider
-  alias Samly.Assertion
 
   @behaviour Provider
 
diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 3b38b46b5..6fb86d418 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -28,10 +28,58 @@ defmodule CadetWeb.AuthController do
     client_id = Map.get(params, "client_id")
     redirect_uri = Map.get(params, "redirect_uri")
 
+    case create_user_and_tokens(conn, provider, code, client_id, redirect_uri) do
+      {:ok, tokens} ->
+        render(conn, "token.json", tokens)
+
+      conn ->
+        conn
+    end
+  end
+
+  def create(conn, _params) do
+    send_resp(conn, :bad_request, "Missing parameter")
+  end
+
+  @doc """
+  Callback URL which processes a SAML redirect from the Assertion Consumer Service (ACS).
+  """
+  def saml_redirect(
+        conn,
+        %{
+          "provider" => provider_instance
+        }
+      ) do
+    case create_user_and_tokens(conn, provider_instance, conn, nil, nil) do
+      {:ok, tokens} ->
+        {_provider, %{client_redirect_url: client_redirect_url}} =
+          Application.get_env(:cadet, :identity_providers, %{})[provider_instance]
+
+        encoded_tokens = tokens |> Jason.encode!()
+
+        conn
+        |> put_resp_cookie("jwts", encoded_tokens,
+          domain: "cadet.ap",
+          http_only: false
+        )
+        |> put_resp_header("location", URI.encode(client_redirect_url))
+        |> send_resp(302, "")
+        |> halt()
+
+      conn ->
+        conn
+    end
+  end
+
+  def saml_redirect(conn, _params) do
+    send_resp(conn, :bad_request, "Missing parameter")
+  end
+
+  defp create_user_and_tokens(conn, provider, code, client_id, redirect_uri) do
     with {:authorise, {:ok, %{token: token, username: username}}} <-
            {:authorise, Provider.authorise(provider, code, client_id, redirect_uri)},
          {:signin, {:ok, user}} <- {:signin, Accounts.sign_in(username, token, provider)} do
-      render(conn, "token.json", generate_tokens(user))
+      {:ok, generate_tokens(user)}
     else
       {:authorise, {:error, :upstream, reason}} ->
         conn
@@ -56,10 +104,6 @@ defmodule CadetWeb.AuthController do
     end
   end
 
-  def create(conn, _params) do
-    send_resp(conn, :bad_request, "Missing parameter")
-  end
-
   @doc """
   Receives a /refresh request with valid attribute.
 
diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex
index 705a62d76..a5081bbbf 100644
--- a/lib/cadet_web/router.ex
+++ b/lib/cadet_web/router.ex
@@ -42,6 +42,7 @@ defmodule CadetWeb.Router do
     post("/auth/refresh", AuthController, :refresh)
     post("/auth/login", AuthController, :create)
     post("/auth/logout", AuthController, :logout)
+    get("/auth/saml_redirect", AuthController, :saml_redirect)
   end
 
   scope "/v2", CadetWeb do

From 125d1a8544c8575877a298ff8e3ded052304ec79 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 18:17:54 +0800
Subject: [PATCH 16/24] Refactor providers with param map

---
 lib/cadet/auth/provider.ex                   | 24 ++++++++++-----
 lib/cadet/auth/providers/adfs.ex             |  8 +++--
 lib/cadet/auth/providers/config.ex           |  6 ++--
 lib/cadet/auth/providers/github.ex           |  8 +++--
 lib/cadet/auth/providers/openid/openid.ex    |  7 +++--
 lib/cadet/auth/providers/saml/saml.ex        | 11 +++----
 lib/cadet_web/controllers/auth_controller.ex | 32 ++++++++++++++------
 7 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/lib/cadet/auth/provider.ex b/lib/cadet/auth/provider.ex
index 9a8bb8905..5d3cae5fb 100644
--- a/lib/cadet/auth/provider.ex
+++ b/lib/cadet/auth/provider.ex
@@ -4,32 +4,40 @@ defmodule Cadet.Auth.Provider do
   it for a token with the OAuth2 provider, and then retrieves the user ID and name.
   """
 
-  @type code :: String.t() | Plug.Conn.t()
   @type token :: String.t()
-  @type client_id :: String.t()
-  @type redirect_uri :: String.t()
   @type error :: :upstream | :invalid_credentials | :other
   @type provider_instance :: String.t()
   @type username :: String.t()
   @type prefix :: String.t()
+  @type authorise_params :: %{
+          provider_instance: provider_instance,
+          code: String.t(),
+          conn: Plug.Conn.t(),
+          client_id: String.t(),
+          redirect_uri: String.t()
+        }
 
   @doc "Exchanges the OAuth2 authorisation code for a token and the user ID."
-  @callback authorise(any(), code, client_id, redirect_uri) ::
+  @callback authorise(any(), authorise_params()) ::
               {:ok, %{token: token, username: String.t()}} | {:error, error(), String.t()}
 
   @doc "Retrieves the name of the user with the associated token."
   @callback get_name(any(), token) :: {:ok, String.t()} | {:error, error(), String.t()}
 
-  @spec get_instance_config(provider_instance) :: {module(), any()} | nil
+  @spec get_instance_config(provider_instance()) :: {module(), any()} | nil
   def get_instance_config(instance) do
     Application.get_env(:cadet, :identity_providers, %{})[instance]
   end
 
-  @spec authorise(provider_instance, code, client_id, redirect_uri) ::
+  @spec authorise(authorise_params) ::
           {:ok, %{token: token, username: String.t()}} | {:error, error(), String.t()}
-  def authorise(instance, code, client_id, redirect_uri) do
+  def authorise(
+        params = %{
+          provider_instance: instance
+        }
+      ) do
     case get_instance_config(instance) do
-      {provider, config} -> provider.authorise(config, code, client_id, redirect_uri)
+      {provider, config} -> provider.authorise(config, params)
       _ -> {:error, :other, "Invalid or nonexistent provider config"}
     end
   end
diff --git a/lib/cadet/auth/providers/adfs.ex b/lib/cadet/auth/providers/adfs.ex
index 79db851d1..06ae2baf4 100644
--- a/lib/cadet/auth/providers/adfs.ex
+++ b/lib/cadet/auth/providers/adfs.ex
@@ -9,10 +9,14 @@ defmodule Cadet.Auth.Providers.ADFS do
 
   @type config :: %{token_endpoint: String.t(), modules: %{}}
 
-  @spec authorise(config(), Provider.code(), Provider.client_id(), Provider.redirect_uri()) ::
+  @spec authorise(config(), Provider.authorise_params()) ::
           {:ok, %{token: Provider.token(), username: String.t()}}
           | {:error, Provider.error(), String.t()}
-  def authorise(config, code, client_id, redirect_uri) do
+  def authorise(config, %{
+        code: code,
+        client_id: client_id,
+        redirect_uri: redirect_uri
+      }) do
     query =
       URI.encode_query(%{
         client_id: client_id,
diff --git a/lib/cadet/auth/providers/config.ex b/lib/cadet/auth/providers/config.ex
index 726a71658..7f878690a 100644
--- a/lib/cadet/auth/providers/config.ex
+++ b/lib/cadet/auth/providers/config.ex
@@ -15,10 +15,12 @@ defmodule Cadet.Auth.Providers.Config do
 
   @behaviour Provider
 
-  @spec authorise(any(), Provider.code(), Provider.client_id(), Provider.redirect_uri()) ::
+  @spec authorise(any(), Provider.authorise_params()) ::
           {:ok, %{token: Provider.token(), username: String.t()}}
           | {:error, Provider.error(), String.t()}
-  def authorise(config, code, _client_id, _redirect_uri) do
+  def authorise(config, %{
+        code: code
+      }) do
     case Enum.find(config, nil, fn %{code: this_code} -> code == this_code end) do
       %{token: token, username: username} ->
         {:ok, %{token: token, username: username}}
diff --git a/lib/cadet/auth/providers/github.ex b/lib/cadet/auth/providers/github.ex
index 2879cec3a..e418cedc0 100644
--- a/lib/cadet/auth/providers/github.ex
+++ b/lib/cadet/auth/providers/github.ex
@@ -12,10 +12,14 @@ defmodule Cadet.Auth.Providers.GitHub do
           user_api: String.t()
         }
 
-  @spec authorise(config(), Provider.code(), Provider.client_id(), Provider.redirect_uri()) ::
+  @spec authorise(config(), Provider.authorise_params()) ::
           {:ok, %{token: Provider.token(), username: String.t()}}
           | {:error, Provider.error(), String.t()}
-  def authorise(config, code, client_id, redirect_uri) do
+  def authorise(config, %{
+        code: code,
+        client_id: client_id,
+        redirect_uri: redirect_uri
+      }) do
     token_headers = [
       {"Content-Type", "application/x-www-form-urlencoded"},
       {"Accept", "application/json"}
diff --git a/lib/cadet/auth/providers/openid/openid.ex b/lib/cadet/auth/providers/openid/openid.ex
index 317fe2c00..6a993849c 100644
--- a/lib/cadet/auth/providers/openid/openid.ex
+++ b/lib/cadet/auth/providers/openid/openid.ex
@@ -9,10 +9,13 @@ defmodule Cadet.Auth.Providers.OpenID do
 
   @type config :: %{openid_provider: atom(), claim_extractor: module()}
 
-  @spec authorise(config, Provider.code(), Provider.client_id(), Provider.redirect_uri()) ::
+  @spec authorise(config(), Provider.params()) ::
           {:ok, %{token: Provider.token(), username: String.t()}}
           | {:error, Provider.error(), String.t()}
-  def authorise(config, code, _client_id, redirect_uri) do
+  def authorise(config, %{
+        code: code,
+        redirect_uri: redirect_uri
+      }) do
     %{openid_provider: openid_provider, claim_extractor: claim_extractor} = config
 
     with {:token, {:ok, token_map}} <-
diff --git a/lib/cadet/auth/providers/saml/saml.ex b/lib/cadet/auth/providers/saml/saml.ex
index 015be3738..a5af6fa94 100644
--- a/lib/cadet/auth/providers/saml/saml.ex
+++ b/lib/cadet/auth/providers/saml/saml.ex
@@ -8,15 +8,12 @@ defmodule Cadet.Auth.Providers.SAML do
 
   @type config :: %{assertion_extractor: module()}
 
-  @spec authorise(
-          any(),
-          Provider.code() | Plug.Conn.t(),
-          Provider.client_id(),
-          Provider.redirect_uri()
-        ) ::
+  @spec authorise(config(), Provider.authorise_params()) ::
           {:ok, %{token: Provider.token(), username: String.t()}}
           | {:error, Provider.error(), String.t()}
-  def authorise(config, conn, _client_id, _redirect_uri) do
+  def authorise(config, %{
+        conn: conn
+      }) do
     %{assertion_extractor: assertion_extractor} = config
 
     with {:assertion, assertion} when not is_nil(assertion) <-
diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 6fb86d418..41dfcde1c 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -19,16 +19,20 @@ defmodule CadetWeb.AuthController do
   def create(
         conn,
         params = %{
+          "code" => code,
           "provider" => provider
         }
       ) do
-    # Code is optional as SAML flow is mainly handled by backend
-    # TODO: Potentially HACKY fallback for SAML (because no code is needed)
-    code = Map.get(params, "code", conn)
     client_id = Map.get(params, "client_id")
     redirect_uri = Map.get(params, "redirect_uri")
 
-    case create_user_and_tokens(conn, provider, code, client_id, redirect_uri) do
+    case create_user_and_tokens(%{
+           conn: conn,
+           provider_instance: provider,
+           code: code,
+           client_id: client_id,
+           redirect_uri: redirect_uri
+         }) do
       {:ok, tokens} ->
         render(conn, "token.json", tokens)
 
@@ -47,13 +51,16 @@ defmodule CadetWeb.AuthController do
   def saml_redirect(
         conn,
         %{
-          "provider" => provider_instance
+          "provider" => provider
         }
       ) do
-    case create_user_and_tokens(conn, provider_instance, conn, nil, nil) do
+    case create_user_and_tokens(%{
+           conn: conn,
+           provider_instance: provider
+         }) do
       {:ok, tokens} ->
         {_provider, %{client_redirect_url: client_redirect_url}} =
-          Application.get_env(:cadet, :identity_providers, %{})[provider_instance]
+          Application.get_env(:cadet, :identity_providers, %{})[provider]
 
         encoded_tokens = tokens |> Jason.encode!()
 
@@ -75,9 +82,16 @@ defmodule CadetWeb.AuthController do
     send_resp(conn, :bad_request, "Missing parameter")
   end
 
-  defp create_user_and_tokens(conn, provider, code, client_id, redirect_uri) do
+  @spec create_user_and_tokens(Provider.authorise_params()) ::
+          {:ok, %{access_token: String.t(), refresh_token: String.t()}} | Plug.Conn.t()
+  defp create_user_and_tokens(
+         params = %{
+           conn: conn,
+           provider_instance: provider
+         }
+       ) do
     with {:authorise, {:ok, %{token: token, username: username}}} <-
-           {:authorise, Provider.authorise(provider, code, client_id, redirect_uri)},
+           {:authorise, Provider.authorise(params)},
          {:signin, {:ok, user}} <- {:signin, Accounts.sign_in(username, token, provider)} do
       {:ok, generate_tokens(user)}
     else

From 4660f7acabfaa16d11f16c08a575d2b09716b7d6 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 22:17:44 +0800
Subject: [PATCH 17/24] Update tests

---
 test/cadet/auth/provider_test.exs             |  5 ++-
 test/cadet/auth/providers/adfs_test.exs       | 43 ++++++++++++++++---
 test/cadet/auth/providers/config_test.exs     |  4 +-
 test/cadet/auth/providers/github_test.exs     | 31 ++++++++++---
 test/cadet/auth/providers/openid_test.exs     | 12 +++---
 .../controllers/auth_controller_test.exs      |  4 +-
 6 files changed, 75 insertions(+), 24 deletions(-)

diff --git a/test/cadet/auth/provider_test.exs b/test/cadet/auth/provider_test.exs
index 8546cd5a6..eecd66fc9 100644
--- a/test/cadet/auth/provider_test.exs
+++ b/test/cadet/auth/provider_test.exs
@@ -8,13 +8,14 @@ defmodule Cadet.Auth.ProviderTest do
   alias Cadet.Auth.Provider
 
   test "with valid provider" do
-    assert {:ok, _} = Provider.authorise("test", "student_code", nil, nil)
+    assert {:ok, _} = Provider.authorise(%{provider_instance: "test", code: "student_code"})
+
     assert {:ok, _} = Provider.get_name("test", "student_token")
   end
 
   test "with invalid provider" do
     assert {:error, :other, "Invalid or nonexistent provider config"} =
-             Provider.authorise("3452345", "student_code", nil, nil)
+             Provider.authorise(%{provider_instance: "3452345", code: "student_code"})
 
     assert {:error, :other, "Invalid or nonexistent provider config"} =
              Provider.get_name("32523453", "student_token")
diff --git a/test/cadet/auth/providers/adfs_test.exs b/test/cadet/auth/providers/adfs_test.exs
index 8511c05b4..4d6632b80 100644
--- a/test/cadet/auth/providers/adfs_test.exs
+++ b/test/cadet/auth/providers/adfs_test.exs
@@ -44,7 +44,12 @@ defmodule Cadet.Auth.Providers.ADFSTest do
   describe "authorise" do
     test "using a valid code" do
       use_cassette "adfs/authorise#1", custom: true do
-        assert {:ok, _} = ADFS.authorise(@config, @code, @client_id, @redirect_uri)
+        assert {:ok, _} =
+                 ADFS.authorise(@config, %{
+                   code: @code,
+                   client_id: @client_id,
+                   redirect_uri: @redirect_uri
+                 })
       end
     end
 
@@ -52,7 +57,11 @@ defmodule Cadet.Auth.Providers.ADFSTest do
       use_cassette "adfs/authorise#2", custom: true do
         assert {:error, :upstream,
                 "Status code 400 from ADFS: {\"error\":\"invalid_grant\",\"error_description\":\"MSIS9612: The authorization code received in \\u0027code\\u0027 parameter is invalid. \"}"} =
-                 ADFS.authorise(@config, @code <> "_invalid", @client_id, @redirect_uri)
+                 ADFS.authorise(@config, %{
+                   code: @code <> "_invalid",
+                   client_id: @client_id,
+                   redirect_uri: @redirect_uri
+                 })
       end
     end
 
@@ -60,7 +69,11 @@ defmodule Cadet.Auth.Providers.ADFSTest do
       use_cassette "adfs/authorise#3", custom: true do
         assert {:error, :upstream,
                 "Status code 400 from ADFS: {\"error\":\"invalid_request\",\"error_description\":\"MSIS9609: The \\u0027redirect_uri\\u0027 parameter is invalid. No redirect uri with the specified value is registered for the received \\u0027client_id\\u0027. \"}"} =
-                 ADFS.authorise(@config, @code, @client_id, @redirect_uri <> "_invalid")
+                 ADFS.authorise(@config, %{
+                   code: @code,
+                   client_id: @client_id,
+                   redirect_uri: @redirect_uri <> "_invalid"
+                 })
       end
     end
 
@@ -68,7 +81,11 @@ defmodule Cadet.Auth.Providers.ADFSTest do
       use_cassette "adfs/authorise#4", custom: true do
         assert {:error, :upstream,
                 "Status code 400 from ADFS: {\"error\":\"invalid_client\",\"error_description\":\"MSIS9607: The \\u0027client_id\\u0027 parameter in the request is invalid. No registered client is found with this identifier.\"}"} =
-                 ADFS.authorise(@config, @code, @client_id <> "_invalid", @redirect_uri)
+                 ADFS.authorise(@config, %{
+                   code: @code,
+                   client_id: @client_id <> "_invalid",
+                   redirect_uri: @redirect_uri
+                 })
       end
     end
 
@@ -76,21 +93,33 @@ defmodule Cadet.Auth.Providers.ADFSTest do
       use_cassette "adfs/authorise#5", custom: true do
         assert {:error, :upstream,
                 "Status code 500 from ADFS: {\"error\":\"invalid_client\",\"error_description\":\"boom.\"}"} =
-                 ADFS.authorise(@config, @code, @client_id, @redirect_uri)
+                 ADFS.authorise(@config, %{
+                   code: @code,
+                   client_id: @client_id,
+                   redirect_uri: @redirect_uri
+                 })
       end
     end
 
     test "no username in token from upstream" do
       use_cassette "adfs/authorise#6", custom: true do
         assert {:error, :invalid_credentials, "Could not retrieve username from token"} =
-                 ADFS.authorise(@config, @code, @client_id, @redirect_uri)
+                 ADFS.authorise(@config, %{
+                   code: @code,
+                   client_id: @client_id,
+                   redirect_uri: @redirect_uri
+                 })
       end
     end
 
     test "expired token from upstream" do
       use_cassette "adfs/authorise#7", custom: true do
         assert {:error, :invalid_credentials, "Failed to verify token claims (token expired?)"} =
-                 ADFS.authorise(@config, @code, @client_id, @redirect_uri)
+                 ADFS.authorise(@config, %{
+                   code: @code,
+                   client_id: @client_id,
+                   redirect_uri: @redirect_uri
+                 })
       end
     end
   end
diff --git a/test/cadet/auth/providers/config_test.exs b/test/cadet/auth/providers/config_test.exs
index b49d39d03..7c5bb4e78 100644
--- a/test/cadet/auth/providers/config_test.exs
+++ b/test/cadet/auth/providers/config_test.exs
@@ -22,11 +22,11 @@ defmodule Cadet.Auth.Providers.ConfigTest do
   describe "authorise" do
     test "successfully" do
       assert {:ok, %{token: @token, username: @username}} =
-               Config.authorise(@config, @code, nil, nil)
+               Config.authorise(@config, %{code: @code})
     end
 
     test "with wrong code" do
-      assert {:error, _, _} = Config.authorise(@config, @code <> "dflajhdfs", nil, nil)
+      assert {:error, _, _} = Config.authorise(@config, %{code: @code <> "dflajhdfs"})
     end
   end
 
diff --git a/test/cadet/auth/providers/github_test.exs b/test/cadet/auth/providers/github_test.exs
index 3b19b89ce..72cbdde83 100644
--- a/test/cadet/auth/providers/github_test.exs
+++ b/test/cadet/auth/providers/github_test.exs
@@ -45,7 +45,11 @@ defmodule Cadet.Auth.Providers.GitHubTest do
     bypass_api_call(bypass)
 
     assert {:ok, %{token: @dummy_access_token, username: @username}} ==
-             GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+             GitHub.authorise(config(bypass), %{
+               code: "",
+               client_id: "dummy_client_id",
+               redirect_uri: ""
+             })
   end
 
   test "invalid github client id", %{bypass: bypass} do
@@ -53,7 +57,11 @@ defmodule Cadet.Auth.Providers.GitHubTest do
     bypass_api_call(bypass)
 
     assert {:error, :invalid_credentials, "Invalid client id"} ==
-             GitHub.authorise(config(bypass), "", "invalid_client_id", "")
+             GitHub.authorise(config(bypass), %{
+               code: "",
+               client_id: "invalid_client_id",
+               redirect_uri: ""
+             })
   end
 
   test "non-successful HTTP status (access token)", %{bypass: bypass} do
@@ -62,7 +70,11 @@ defmodule Cadet.Auth.Providers.GitHubTest do
     end)
 
     assert {:error, :upstream, "Status code 403 from GitHub"} ==
-             GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+             GitHub.authorise(config(bypass), %{
+               code: "",
+               client_id: "dummy_client_id",
+               redirect_uri: ""
+             })
   end
 
   test "error token response", %{bypass: bypass} do
@@ -72,7 +84,11 @@ defmodule Cadet.Auth.Providers.GitHubTest do
       |> 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", "")
+               GitHub.authorise(config(bypass), %{
+                 code: "",
+                 client_id: "dummy_client_id",
+                 redirect_uri: ""
+               })
     end)
   end
 
@@ -84,7 +100,12 @@ defmodule Cadet.Auth.Providers.GitHubTest do
     end)
 
     assert {:error, :upstream, "Status code 401 from GitHub"}
-    GitHub.authorise(config(bypass), "", "dummy_client_id", "")
+
+    GitHub.authorise(config(bypass), %{
+      code: "",
+      client_id: "dummy_client_id",
+      redirect_uri: ""
+    })
   end
 
   test "get_name successful", %{bypass: bypass} do
diff --git a/test/cadet/auth/providers/openid_test.exs b/test/cadet/auth/providers/openid_test.exs
index fef336ddd..c07d2dfa4 100644
--- a/test/cadet/auth/providers/openid_test.exs
+++ b/test/cadet/auth/providers/openid_test.exs
@@ -103,7 +103,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
     bypass_return_token(bypass, @okay_token)
 
     assert {:ok, %{token: @okay_token, username: @username}} =
-             OpenID.authorise(@config, "dummy_code", "", "")
+             OpenID.authorise(@config, %{code: "dummy_code", redirect_uri: ""})
 
     assert {:ok, @username} == OpenID.get_name(@config, @okay_token)
   end
@@ -114,7 +114,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
     bypass_return_token(bypass, @no_username_token)
 
     assert {:error, :invalid_credentials, "No username specified in token"} ==
-             OpenID.authorise(@config, "dummy_code", "", "")
+             OpenID.authorise(@config, %{code: "dummy_code", redirect_uri: ""})
   end
 
   test "get_name with no name in token" do
@@ -128,7 +128,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
     bypass_return_token(bypass, @expired_token)
 
     assert {:error, :invalid_credentials, "Failed to verify token claims (token expired?)"} ==
-             OpenID.authorise(@config, "dummy_code", "", "")
+             OpenID.authorise(@config, %{code: "dummy_code", redirect_uri: ""})
   end
 
   @invalid_signature_token "eyJraWQiOiIxIiwiYWxnIjoiUlMyNTYifQ.eyJjb2duaXRvOmdyb3VwcyI6WyJhZG1pbiJdLCJ1c2VybmFtZSI6InVzZXJuYW1lIiwiZXhwIjowfQ.M6VMo5_xtFAl75kwpLWsDRArYJLpqIp3qkks3TvExQXqOZ9eI98bm_R1VFMkbJ0-URQqURiBO6SYA1uNdXoEMoDtv0tV-P26fEy5pcdGC-sYXkbgXsw4iKAuSUfc4GSwDLEcYb9n6gQAG6pqbHAvft_L3f6RLLhP6GWWnOZ3upRDwsYcZSyYvDdpuVpCtHOfsMPANeXmZPHbYoRWQULU2okklPmXrc4sO0HO_zVocMus9DDf9hF0NmGl0diUQBA-w3i9hkcqsuhmxhOLApPre49TK37HDd1ileo9GIMGMPuOfXd44d8fHg5yr5MVqJLeH2RoHh9ZD_myPnTBG0GptA"
@@ -137,7 +137,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
     bypass_return_token(bypass, @invalid_signature_token)
 
     assert {:error, :invalid_credentials, "Failed to verify token"} ==
-             OpenID.authorise(@config, "dummy_code", "", "")
+             OpenID.authorise(@config, %{code: "dummy_code", redirect_uri: ""})
   end
 
   test "non-successful HTTP status", %{bypass: bypass} do
@@ -146,7 +146,7 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
     end)
 
     assert {:error, :invalid_credentials, "Failed to fetch token from OpenID provider"} ==
-             OpenID.authorise(@config, "dummy_code", "", "")
+             OpenID.authorise(@config, %{code: "dummy_code", redirect_uri: ""})
   end
 
   test "missing token", %{bypass: bypass} do
@@ -157,6 +157,6 @@ defmodule Cadet.Auth.Providers.OpenIDTest do
     end)
 
     assert {:error, :invalid_credentials, "Missing token in response from OpenID provider"} ==
-             OpenID.authorise(@config, "dummy_code", "", "")
+             OpenID.authorise(@config, %{code: "dummy_code", redirect_uri: ""})
   end
 end
diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs
index 1a4a9a45e..09c93f5a1 100644
--- a/test/cadet_web/controllers/auth_controller_test.exs
+++ b/test/cadet_web/controllers/auth_controller_test.exs
@@ -54,7 +54,7 @@ defmodule CadetWeb.AuthControllerTest do
                    %{conn: conn},
                    Cadet.Auth.Provider,
                    [],
-                   authorise: fn _, _, _, _ -> {:error, :upstream, "Upstream error"} end do
+                   authorise: fn _ -> {:error, :upstream, "Upstream error"} end do
       conn =
         post(conn, "/v2/auth/login", %{
           "code" => "invalid code",
@@ -71,7 +71,7 @@ defmodule CadetWeb.AuthControllerTest do
                    %{conn: conn},
                    Cadet.Auth.Provider,
                    [],
-                   authorise: fn _, _, _, _ -> {:error, :other, "Unknown error"} end do
+                   authorise: fn _ -> {:error, :other, "Unknown error"} end do
       conn =
         post(conn, "/v2/auth/login", %{
           "code" => "code",

From 5fd79d481b644571690ec3f79df080d4bff02696 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 22:21:40 +0800
Subject: [PATCH 18/24] Update swagger

---
 lib/cadet_web/controllers/auth_controller.ex | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 41dfcde1c..e9ad1ff63 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -228,6 +228,16 @@ defmodule CadetWeb.AuthController do
     response(401, "Invalid token")
   end
 
+  swagger_path :saml_redirect do
+    get("/auth/saml_redirect")
+
+    summary(
+      "SAML redirect endpoint after Assertion Consumer Service validation. Generates JWT tokens before redirecting again to the frontend."
+    )
+
+    response(302, "Found")
+  end
+
   def swagger_definitions do
     %{
       Tokens:

From 0e689621dfb7e419bed14517ed0698c0a757aff8 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 22:30:54 +0800
Subject: [PATCH 19/24] Restructure test providers folder

---
 .../auth/providers/{ => openid}/auth0_claim_extractor_test.exs    | 0
 .../auth/providers/{ => openid}/cognito_claim_extractor_test.exs  | 0
 .../auth/providers/{ => openid}/google_claim_extractor_test.exs   | 0
 test/cadet/auth/providers/{ => openid}/openid_test.exs            | 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename test/cadet/auth/providers/{ => openid}/auth0_claim_extractor_test.exs (100%)
 rename test/cadet/auth/providers/{ => openid}/cognito_claim_extractor_test.exs (100%)
 rename test/cadet/auth/providers/{ => openid}/google_claim_extractor_test.exs (100%)
 rename test/cadet/auth/providers/{ => openid}/openid_test.exs (100%)

diff --git a/test/cadet/auth/providers/auth0_claim_extractor_test.exs b/test/cadet/auth/providers/openid/auth0_claim_extractor_test.exs
similarity index 100%
rename from test/cadet/auth/providers/auth0_claim_extractor_test.exs
rename to test/cadet/auth/providers/openid/auth0_claim_extractor_test.exs
diff --git a/test/cadet/auth/providers/cognito_claim_extractor_test.exs b/test/cadet/auth/providers/openid/cognito_claim_extractor_test.exs
similarity index 100%
rename from test/cadet/auth/providers/cognito_claim_extractor_test.exs
rename to test/cadet/auth/providers/openid/cognito_claim_extractor_test.exs
diff --git a/test/cadet/auth/providers/google_claim_extractor_test.exs b/test/cadet/auth/providers/openid/google_claim_extractor_test.exs
similarity index 100%
rename from test/cadet/auth/providers/google_claim_extractor_test.exs
rename to test/cadet/auth/providers/openid/google_claim_extractor_test.exs
diff --git a/test/cadet/auth/providers/openid_test.exs b/test/cadet/auth/providers/openid/openid_test.exs
similarity index 100%
rename from test/cadet/auth/providers/openid_test.exs
rename to test/cadet/auth/providers/openid/openid_test.exs

From 5fcae93cab5437cb1151f0b9c755bf56a490ae29 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 23:29:11 +0800
Subject: [PATCH 20/24] Fix dialyzer warnings

---
 lib/cadet/auth/provider.ex                   | 8 ++++----
 lib/cadet/auth/providers/openid/openid.ex    | 2 +-
 lib/cadet_web/controllers/auth_controller.ex | 5 ++++-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/cadet/auth/provider.ex b/lib/cadet/auth/provider.ex
index 5d3cae5fb..ad801aa43 100644
--- a/lib/cadet/auth/provider.ex
+++ b/lib/cadet/auth/provider.ex
@@ -10,11 +10,11 @@ defmodule Cadet.Auth.Provider do
   @type username :: String.t()
   @type prefix :: String.t()
   @type authorise_params :: %{
-          provider_instance: provider_instance,
-          code: String.t(),
           conn: Plug.Conn.t(),
-          client_id: String.t(),
-          redirect_uri: String.t()
+          provider_instance: provider_instance,
+          code: String.t() | nil,
+          client_id: String.t() | nil,
+          redirect_uri: String.t() | nil
         }
 
   @doc "Exchanges the OAuth2 authorisation code for a token and the user ID."
diff --git a/lib/cadet/auth/providers/openid/openid.ex b/lib/cadet/auth/providers/openid/openid.ex
index 6a993849c..558978161 100644
--- a/lib/cadet/auth/providers/openid/openid.ex
+++ b/lib/cadet/auth/providers/openid/openid.ex
@@ -9,7 +9,7 @@ defmodule Cadet.Auth.Providers.OpenID do
 
   @type config :: %{openid_provider: atom(), claim_extractor: module()}
 
-  @spec authorise(config(), Provider.params()) ::
+  @spec authorise(config(), Provider.authorise_params()) ::
           {:ok, %{token: Provider.token(), username: String.t()}}
           | {:error, Provider.error(), String.t()}
   def authorise(config, %{
diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index e9ad1ff63..1c20123c8 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -56,7 +56,10 @@ defmodule CadetWeb.AuthController do
       ) do
     case create_user_and_tokens(%{
            conn: conn,
-           provider_instance: provider
+           provider_instance: provider,
+           code: nil,
+           client_id: nil,
+           redirect_uri: nil
          }) do
       {:ok, tokens} ->
         {_provider, %{client_redirect_url: client_redirect_url}} =

From ef949032b21f9c3561c4141a3aa1fdf51dcd04f1 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Sun, 12 May 2024 23:36:34 +0800
Subject: [PATCH 21/24] Add SAML provider tests

---
 .../saml/nusstf_assertion_extractor_test.exs  | 14 +++++
 .../saml/nusstu_assertion_extractor_test.exs  | 13 +++++
 test/cadet/auth/providers/saml/saml_test.exs  | 52 +++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 test/cadet/auth/providers/saml/nusstf_assertion_extractor_test.exs
 create mode 100644 test/cadet/auth/providers/saml/nusstu_assertion_extractor_test.exs
 create mode 100644 test/cadet/auth/providers/saml/saml_test.exs

diff --git a/test/cadet/auth/providers/saml/nusstf_assertion_extractor_test.exs b/test/cadet/auth/providers/saml/nusstf_assertion_extractor_test.exs
new file mode 100644
index 000000000..a09a9151b
--- /dev/null
+++ b/test/cadet/auth/providers/saml/nusstf_assertion_extractor_test.exs
@@ -0,0 +1,14 @@
+defmodule Cadet.Auth.Providers.NusstfAssertionExtractorTest do
+  use ExUnit.Case, async: true
+
+  alias Cadet.Auth.Providers.NusstfAssertionExtractor, as: Testee
+
+  @username "JohnT"
+  @name "John Tan"
+  @assertion %{attributes: %{"SamAccountName" => @username, "DisplayName" => @name}}
+
+  test "success" do
+    assert @username == Testee.get_username(@assertion)
+    assert @name == Testee.get_name(@assertion)
+  end
+end
diff --git a/test/cadet/auth/providers/saml/nusstu_assertion_extractor_test.exs b/test/cadet/auth/providers/saml/nusstu_assertion_extractor_test.exs
new file mode 100644
index 000000000..e5d86bee3
--- /dev/null
+++ b/test/cadet/auth/providers/saml/nusstu_assertion_extractor_test.exs
@@ -0,0 +1,13 @@
+defmodule Cadet.Auth.Providers.NusstuAssertionExtractorTest do
+  use ExUnit.Case, async: true
+
+  alias Cadet.Auth.Providers.NusstuAssertionExtractor, as: Testee
+
+  @username "JohnT"
+  @assertion %{attributes: %{"samaccountname" => @username}}
+
+  test "success" do
+    assert @username == Testee.get_username(@assertion)
+    assert @username == Testee.get_name(@assertion)
+  end
+end
diff --git a/test/cadet/auth/providers/saml/saml_test.exs b/test/cadet/auth/providers/saml/saml_test.exs
new file mode 100644
index 000000000..d621b1331
--- /dev/null
+++ b/test/cadet/auth/providers/saml/saml_test.exs
@@ -0,0 +1,52 @@
+defmodule Cadet.Auth.Providers.SamlTest do
+  @moduledoc """
+  Tests the SAML authentication provider by simulating validated assertions from the SAML IdP.
+  """
+  use ExUnit.Case, async: false
+
+  alias Cadet.Auth.Providers.SAML
+
+  import Mock
+
+  @config %{
+    assertion_extractor: Cadet.Auth.Providers.NusstfAssertionExtractor,
+    client_redirect_url: "http://example.com/login/callback"
+  }
+
+  @username_field "SamAccountName"
+  @username "JohnT"
+  @name_field "DisplayName"
+  @name "John Tan"
+  @token "{\"name\":\"John Tan\"}"
+
+  test_with_mock "success", Samly,
+    get_active_assertion: fn _ ->
+      %{attributes: %{@username_field => @username, @name_field => @name}}
+    end do
+    assert {:ok, %{token: @token, username: @username}} =
+             SAML.authorise(@config, %{conn: %{}})
+
+    assert {:ok, @name} == SAML.get_name(@config, @token)
+  end
+
+  test_with_mock "Missing SAML assertion", Samly, get_active_assertion: fn _ -> nil end do
+    assert {:error, :invalid_credentials, "Missing SAML assertion!"} =
+             SAML.authorise(@config, %{conn: %{}})
+  end
+
+  test_with_mock "Missing name attribute", Samly,
+    get_active_assertion: fn _ ->
+      %{attributes: %{@username_field => @username}}
+    end do
+    assert {:error, :invalid_credentials, "Missing name attribute!"} =
+             SAML.authorise(@config, %{conn: %{}})
+  end
+
+  test_with_mock "Missing username attribute", Samly,
+    get_active_assertion: fn _ ->
+      %{attributes: %{@name_field => @name}}
+    end do
+    assert {:error, :invalid_credentials, "Missing username attribute!"} =
+             SAML.authorise(@config, %{conn: %{}})
+  end
+end

From cee56eb98b12e36f9a52577199e8835159f5706c Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Mon, 13 May 2024 00:26:53 +0800
Subject: [PATCH 22/24] Add auth_controller tests for SAML redirect endpoint

---
 config/test.exs                               |  8 +++-
 lib/cadet_web/controllers/auth_controller.ex  |  2 +-
 .../controllers/auth_controller_test.exs      | 37 +++++++++++++++++++
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/config/test.exs b/config/test.exs
index 05ca74630..75824dfc5 100644
--- a/config/test.exs
+++ b/config/test.exs
@@ -68,7 +68,13 @@ config :cadet,
            username: "E1234564"
            #  role: :student
          }
-       ]}
+       ]},
+      "saml" =>
+        {Cadet.Auth.Providers.SAML,
+          %{
+            assertion_extractor: Cadet.Auth.Providers.NusstfAssertionExtractor,
+            client_redirect_url: "https://cadet.frontend/login/callback"
+          }}
   },
   autograder: [
     lambda_name: "dummy"
diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex
index 1c20123c8..d51089785 100644
--- a/lib/cadet_web/controllers/auth_controller.ex
+++ b/lib/cadet_web/controllers/auth_controller.ex
@@ -69,7 +69,7 @@ defmodule CadetWeb.AuthController do
 
         conn
         |> put_resp_cookie("jwts", encoded_tokens,
-          domain: "cadet.ap",
+          domain: URI.new!(client_redirect_url).host,
           http_only: false
         )
         |> put_resp_header("location", URI.encode(client_redirect_url))
diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs
index 09c93f5a1..87419d20a 100644
--- a/test/cadet_web/controllers/auth_controller_test.exs
+++ b/test/cadet_web/controllers/auth_controller_test.exs
@@ -100,6 +100,43 @@ defmodule CadetWeb.AuthControllerTest do
     end
   end
 
+  describe "GET /auth/saml_redirect" do
+    test_with_mock "success", %{conn: conn}, Samly, [],
+      get_active_assertion: fn _ -> %{attributes: %{"SamAccountName" => "username", "DisplayName" => "name"}} end do
+        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+
+        assert response(conn, 302)
+    end
+
+    test_with_mock "missing parameter", %{conn: conn}, Samly, [],
+      get_active_assertion: fn _ -> %{attributes: %{"SamAccountName" => "username", "DisplayName" => "name"}} end do
+        conn = get(conn, "/v2/auth/saml_redirect", %{})
+
+        assert response(conn, 400) == "Missing parameter"
+      end
+
+    test_with_mock "missing SAML assertion", %{conn: conn}, Samly, [],
+      get_active_assertion: fn _ -> nil end do
+        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+
+        assert response(conn, 400) == "Unable to validate token: Missing SAML assertion!"
+      end
+
+    test_with_mock "missing name attribute", %{conn: conn}, Samly, [],
+      get_active_assertion: fn _ ->  %{attributes: %{"SamAccountName" => "username"}} end do
+        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+
+        assert response(conn, 400) == "Unable to validate token: Missing name attribute!"
+      end
+
+    test_with_mock "missing username attribute", %{conn: conn}, Samly, [],
+      get_active_assertion: fn _ ->  %{attributes: %{"DisplayName" => "name"}} end do
+        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+
+        assert response(conn, 400) == "Unable to validate token: Missing username attribute!"
+      end
+  end
+
   describe "POST /auth/refresh" do
     test "missing parameter", %{conn: conn} do
       conn = post(conn, "/v2/auth/refresh", %{})

From 3115e7d85333482c2d26a140dc65433687e86cbb Mon Sep 17 00:00:00 2001
From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Date: Mon, 13 May 2024 00:29:29 +0800
Subject: [PATCH 23/24] Ran format

---
 config/test.exs                               | 12 +++---
 test/cadet/auth/providers/saml/saml_test.exs  |  3 +-
 .../controllers/auth_controller_test.exs      | 40 ++++++++++---------
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/config/test.exs b/config/test.exs
index 75824dfc5..73c0e3451 100644
--- a/config/test.exs
+++ b/config/test.exs
@@ -69,12 +69,12 @@ config :cadet,
            #  role: :student
          }
        ]},
-      "saml" =>
-        {Cadet.Auth.Providers.SAML,
-          %{
-            assertion_extractor: Cadet.Auth.Providers.NusstfAssertionExtractor,
-            client_redirect_url: "https://cadet.frontend/login/callback"
-          }}
+    "saml" =>
+      {Cadet.Auth.Providers.SAML,
+       %{
+         assertion_extractor: Cadet.Auth.Providers.NusstfAssertionExtractor,
+         client_redirect_url: "https://cadet.frontend/login/callback"
+       }}
   },
   autograder: [
     lambda_name: "dummy"
diff --git a/test/cadet/auth/providers/saml/saml_test.exs b/test/cadet/auth/providers/saml/saml_test.exs
index d621b1331..166ff6e16 100644
--- a/test/cadet/auth/providers/saml/saml_test.exs
+++ b/test/cadet/auth/providers/saml/saml_test.exs
@@ -23,8 +23,7 @@ defmodule Cadet.Auth.Providers.SamlTest do
     get_active_assertion: fn _ ->
       %{attributes: %{@username_field => @username, @name_field => @name}}
     end do
-    assert {:ok, %{token: @token, username: @username}} =
-             SAML.authorise(@config, %{conn: %{}})
+    assert {:ok, %{token: @token, username: @username}} = SAML.authorise(@config, %{conn: %{}})
 
     assert {:ok, @name} == SAML.get_name(@config, @token)
   end
diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs
index 87419d20a..df63098a3 100644
--- a/test/cadet_web/controllers/auth_controller_test.exs
+++ b/test/cadet_web/controllers/auth_controller_test.exs
@@ -102,39 +102,43 @@ defmodule CadetWeb.AuthControllerTest do
 
   describe "GET /auth/saml_redirect" do
     test_with_mock "success", %{conn: conn}, Samly, [],
-      get_active_assertion: fn _ -> %{attributes: %{"SamAccountName" => "username", "DisplayName" => "name"}} end do
-        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+      get_active_assertion: fn _ ->
+        %{attributes: %{"SamAccountName" => "username", "DisplayName" => "name"}}
+      end do
+      conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
 
-        assert response(conn, 302)
+      assert response(conn, 302)
     end
 
     test_with_mock "missing parameter", %{conn: conn}, Samly, [],
-      get_active_assertion: fn _ -> %{attributes: %{"SamAccountName" => "username", "DisplayName" => "name"}} end do
-        conn = get(conn, "/v2/auth/saml_redirect", %{})
+      get_active_assertion: fn _ ->
+        %{attributes: %{"SamAccountName" => "username", "DisplayName" => "name"}}
+      end do
+      conn = get(conn, "/v2/auth/saml_redirect", %{})
 
-        assert response(conn, 400) == "Missing parameter"
-      end
+      assert response(conn, 400) == "Missing parameter"
+    end
 
     test_with_mock "missing SAML assertion", %{conn: conn}, Samly, [],
       get_active_assertion: fn _ -> nil end do
-        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+      conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
 
-        assert response(conn, 400) == "Unable to validate token: Missing SAML assertion!"
-      end
+      assert response(conn, 400) == "Unable to validate token: Missing SAML assertion!"
+    end
 
     test_with_mock "missing name attribute", %{conn: conn}, Samly, [],
-      get_active_assertion: fn _ ->  %{attributes: %{"SamAccountName" => "username"}} end do
-        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+      get_active_assertion: fn _ -> %{attributes: %{"SamAccountName" => "username"}} end do
+      conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
 
-        assert response(conn, 400) == "Unable to validate token: Missing name attribute!"
-      end
+      assert response(conn, 400) == "Unable to validate token: Missing name attribute!"
+    end
 
     test_with_mock "missing username attribute", %{conn: conn}, Samly, [],
-      get_active_assertion: fn _ ->  %{attributes: %{"DisplayName" => "name"}} end do
-        conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
+      get_active_assertion: fn _ -> %{attributes: %{"DisplayName" => "name"}} end do
+      conn = get(conn, "/v2/auth/saml_redirect", %{"provider" => "saml"})
 
-        assert response(conn, 400) == "Unable to validate token: Missing username attribute!"
-      end
+      assert response(conn, 400) == "Unable to validate token: Missing username attribute!"
+    end
   end
 
   describe "POST /auth/refresh" do

From 60990593bedcadbca33d118616e684a8c6e48c31 Mon Sep 17 00:00:00 2001
From: En Rong <53928333+chownces@users.noreply.github.com>
Date: Mon, 13 May 2024 00:36:34 +0800
Subject: [PATCH 24/24] Fix sigil warning

---
 test/cadet/auth/providers/saml/saml_test.exs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/cadet/auth/providers/saml/saml_test.exs b/test/cadet/auth/providers/saml/saml_test.exs
index 166ff6e16..c36e3cbdd 100644
--- a/test/cadet/auth/providers/saml/saml_test.exs
+++ b/test/cadet/auth/providers/saml/saml_test.exs
@@ -17,7 +17,7 @@ defmodule Cadet.Auth.Providers.SamlTest do
   @username "JohnT"
   @name_field "DisplayName"
   @name "John Tan"
-  @token "{\"name\":\"John Tan\"}"
+  @token ~s({"name":"John Tan"})
 
   test_with_mock "success", Samly,
     get_active_assertion: fn _ ->