Remove redundant functionality

lib/cadet/auth/provider.ex

@@ -1,12 +1,9 @@
 defmodule Cadet.Auth.Provider do
   @moduledoc """
   An identity provider, which takes the OAuth2 authentication code and exchanges
-  it for a token with the OAuth2 provider, and then retrieves the user ID, name,
-  and user role.
+  it for a token with the OAuth2 provider, and then retrieves the user ID and name.
   """
 
-  alias Cadet.Accounts.Role
-
   @type code :: String.t()
   @type token :: String.t()
   @type client_id :: String.t()
@@ -23,9 +20,6 @@ defmodule Cadet.Auth.Provider do
   @doc "Retrieves the name of the user with the associated token."
   @callback get_name(any(), token) :: {:ok, String.t()} | {:error, error(), String.t()}
 
-  @doc "Retrieves the role of the user with the associated token."
-  @callback get_role(any(), token) :: {:ok, Role.t()} | {:error, error(), String.t()}
-
   @spec get_instance_config(provider_instance) :: {module(), any()} | nil
   def get_instance_config(instance) do
     Application.get_env(:cadet, :identity_providers, %{})[instance]
@@ -47,17 +41,4 @@ defmodule Cadet.Auth.Provider do
       _ -> {:error, :other, "Invalid or nonexistent provider config"}
     end
   end
-
-  # no longer used anymore currently
-
-  # coveralls-ignore-start
-  @spec get_role(provider_instance, token) :: {:ok, String.t()} | {:error, error(), String.t()}
-  def get_role(instance, token) do
-    case get_instance_config(instance) do
-      {provider, config} -> provider.get_role(config, token)
-      _ -> {:error, :other, "Invalid or nonexistent provider config"}
-    end
-  end
-
-  # coveralls-ignore-stop
 end

lib/cadet/auth/providers/auth0_claim_extractor.ex

@@ -1,32 +1,19 @@
 defmodule Cadet.Auth.Providers.Auth0ClaimExtractor do
   @moduledoc """
   Extracts fields from Auth0 JWTs.
-
-  Note: an Auth0 Rule that adds the role to the ID token is required. E.g.:
-
-  ```
-  function (user, context, callback) {
-    if (context.idToken && user.app_metadata && user.app_metadata.role) {
-      context.idToken['https://source-academy.github.io/role'] = user.app_metadata.role;
-    }
-    callback(null, user, context);
-  }
-  ```
   """
 
   @behaviour Cadet.Auth.Providers.OpenID.ClaimExtractor
 
-  def get_username(claims) do
+  def get_username(claims, _id_token) do
     if claims["email_verified"] do
       claims["email"]
     else
       nil
     end
   end
 
-  def get_name(claims), do: claims["name"]
-
-  def get_role(claims), do: claims["https://source-academy.github.io/role"]
+  def get_name(claims, _id_token), do: claims["name"]
 
   def get_token_type, do: "id_token"
 end

lib/cadet/auth/providers/cognito_claim_extractor.ex

@@ -5,23 +5,13 @@ defmodule Cadet.Auth.Providers.CognitoClaimExtractor do
 
   @behaviour Cadet.Auth.Providers.OpenID.ClaimExtractor
 
-  def get_username(claims) do
+  def get_username(claims, _access_token) do
     claims["username"]
   end
 
-  def get_name(claims) do
+  def get_name(claims, _access_token) do
     claims["username"]
   end
 
-  def get_role(claims) do
-    case claims["cognito:groups"] do
-      [head | _] when is_atom(head) -> head
-      ["admin" | _] -> :admin
-      ["staff" | _] -> :staff
-      nil -> nil
-      _ -> :student
-    end
-  end
-
   def get_token_type, do: "access_token"
 end

lib/cadet/auth/providers/config.ex

@@ -36,13 +36,4 @@ defmodule Cadet.Auth.Providers.Config do
       _ -> {:error, :invalid_credentials, "Invalid token"}
     end
   end
-
-  @spec get_role(any(), Provider.token()) ::
-          {:ok, Cadet.Accounts.Role.t()} | {:error, Provider.error(), String.t()}
-  def get_role(config, token) do
-    case Enum.find(config, nil, fn %{token: this_token} -> token == this_token end) do
-      %{role: role} -> {:ok, role}
-      _ -> {:error, :invalid_credentials, "Invalid token"}
-    end
-  end
 end

lib/cadet/auth/providers/github.ex

@@ -68,11 +68,6 @@ defmodule Cadet.Auth.Providers.GitHub do
     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}]
 

lib/cadet/auth/providers/google_claim_extractor.ex

@@ -5,19 +5,17 @@ defmodule Cadet.Auth.Providers.GoogleClaimExtractor do
 
   @behaviour Cadet.Auth.Providers.OpenID.ClaimExtractor
 
-  def get_username(claims) do
+  def get_username(claims, _id_token) do
     if claims["email_verified"] do
       claims["email"]
     else
       nil
     end
   end
 
-  def get_name(claims) do
+  def get_name(claims, _id_token) do
     claims["name"]
   end
 
-  def get_role(_claims), do: nil
-
   def get_token_type, do: "id_token"
 end

lib/cadet/auth/providers/luminus.ex

@@ -64,112 +64,6 @@ defmodule Cadet.Auth.Providers.LumiNUS do
     end
   end
 
-  @spec get_role(config(), Provider.token()) ::
-          {:ok, Cadet.Accounts.Role.t()} | {:error, Provider.error(), String.t()}
-  @doc """
-  Get the role of the user corresponding to this token.
-
-  Roles:
-
-  - student permission -> :student
-  - manager / read manager permissions -> :staff
-  - owner / co-owner -> :admin
-
-  ## Returns
-
-  - `{:ok, :student}` - valid token, has student permissions
-  - `{:ok, :staff}` - valid token, has manager or read manager permissions
-  - `{:ok, :admin}` - valid token, has owner or co-owner permissions
-  - `{:error, :invalid_credentials, "User is not part of module"}` - valid token
-    but the user does not currently read the module
-  - `{:error, :upstream, "Status code xxx from LumiNUS"}` - invalid token or
-    luminus_client_secret is invalid
-
-  ## Parameters
-
-  - `token`: String, the OAuth2 token
-
-  ## Examples
-
-      iex> Cadet.Accounts.Luminus.fetch_role("T0K3N...")
-      {:ok, :student}
-  """
-  def get_role(config, token) do
-    case api_call("module", token, config.api_key) do
-      {:ok, modules} ->
-        parse_modules(modules, config.modules)
-
-      {:error, _, _} = error ->
-        error
-    end
-  end
-
-  @student_access %{
-    "access_Full" => false,
-    "access_Create" => false,
-    "access_Read" => true,
-    "access_Update" => false,
-    "access_Delete" => false,
-    "access_Settings_Read" => false,
-    "access_Settings_Update" => false
-  }
-
-  @staff_access %{
-    "access_Full" => false,
-    "access_Settings_Read" => true
-  }
-
-  @admin_access %{
-    "access_Full" => true,
-    "access_Create" => true,
-    "access_Read" => true,
-    "access_Update" => true,
-    "access_Delete" => true,
-    "access_Settings_Read" => true,
-    "access_Settings_Update" => true
-  }
-
-  defp parse_modules(modules, allowed) do
-    roles =
-      modules["data"]
-      |> Enum.filter(&(module_allowed?(&1, allowed) and module_active?(&1["endDate"])))
-      |> Enum.map(&module_to_role/1)
-      # NOTE: this depends on the fact that the correct role order
-      # [:admin, :staff, :student] happens to also be sorted,
-      # and that :unexpected_access sorts after any valid role
-      |> Enum.sort()
-
-    case roles do
-      [] -> {:error, :invalid_credentials, "User is not part of module"}
-      [role | _] when role in [:admin, :staff, :student] -> {:ok, role}
-      [:unexpected_access | _] -> {:error, :other, "Unexpected access combination"}
-    end
-  end
-
-  defp module_to_role(module) do
-    case module do
-      %{"access" => @admin_access} -> :admin
-      %{"access" => @staff_access} -> :staff
-      %{"access" => @student_access} -> :student
-      _ -> :unexpected_access
-    end
-  end
-
-  defp module_allowed?(module, allowed) do
-    allowed_terms = allowed[module["name"]]
-    term = module["term"]
-
-    cond do
-      is_list(allowed_terms) -> term in allowed_terms
-      is_binary(allowed_terms) -> term == allowed_terms
-      true -> false
-    end
-  end
-
-  defp module_active?(end_date) do
-    Timex.before?(Timex.now(), Timex.parse!(end_date, "{ISO:Extended}"))
-  end
-
   defp api_call(method, token, api_key) do
     headers = [{"Ocp-Apim-Subscription-Key", api_key}, {"Authorization", "Bearer #{token}"}]
     options = [timeout: 10_000, recv_timeout: 10_000]

lib/cadet/auth/providers/openid.ex

@@ -29,7 +29,7 @@ defmodule Cadet.Auth.Providers.OpenID do
               claims,
               nil
             )} do
-      case claim_extractor.get_username(claims) do
+      case claim_extractor.get_username(claims, token) do
         nil ->
           {:error, :invalid_credentials, "No username specified in token"}
 
@@ -56,37 +56,25 @@ defmodule Cadet.Auth.Providers.OpenID do
   end
 
   # issue with JOSE's type specifications
-  @dialyzer {:no_fail_call, [get_name: 2, get_role: 2]}
+  @dialyzer {:no_fail_call, [get_name: 2]}
 
   @spec get_name(config, Provider.token()) ::
           {:ok, String.t()} | {:error, Provider.error(), String.t()}
   def get_name(config, token) do
     %{claim_extractor: claim_extractor} = config
     # Assume the token has already been verified by authorise
-    case claim_extractor.get_name(JOSE.JWT.peek(token).fields) do
+    case claim_extractor.get_name(JOSE.JWT.peek(token).fields, token) do
       nil -> {:error, :invalid_credentials, "No name specified in token"}
       name -> {:ok, name}
     end
   end
-
-  @spec get_role(config, Provider.token()) ::
-          {:ok, Cadet.Accounts.Role.t()} | {:error, Provider.error(), String.t()}
-  def get_role(config, token) do
-    %{claim_extractor: claim_extractor} = config
-    # Assume the token has already been verified by authorise
-    case claim_extractor.get_role(JOSE.JWT.peek(token).fields) do
-      nil -> {:error, :invalid_credentials, "No role specified in token"}
-      role -> {:ok, role}
-    end
-  end
 end
 
 defmodule Cadet.Auth.Providers.OpenID.ClaimExtractor do
   @moduledoc """
   A behaviour for modules that extract fields from JWT token claims.
   """
-  @callback get_username(%{}) :: String.t() | nil
-  @callback get_name(%{}) :: String.t() | nil
-  @callback get_role(%{}) :: String.t() | nil
+  @callback get_username(%{}, String.t()) :: String.t() | nil
+  @callback get_name(%{}, String.t()) :: String.t() | nil
   @callback get_token_type() :: String.t() | nil
 end

lib/cadet_web/admin_views/admin_grading_view.ex

@@ -22,19 +22,11 @@ defmodule CadetWeb.AdminGradingView do
   end
 
   defp build_grading_question(answer) do
-    results = build_autograding_results(answer.autograding_results)
-
     %{question: answer.question}
     |> build_question_by_question_config(true)
     |> Map.put(:answer, answer.answer["code"] || answer.answer["choice_id"])
     |> Map.put(:autogradingStatus, answer.autograding_status)
-    |> Map.put(:autogradingResults, results)
-  end
-
-  defp build_autograding_results(nil), do: nil
-
-  defp build_autograding_results(results) do
-    Enum.map(results, &build_result/1)
+    |> Map.put(:autogradingResults, answer.autograding_results)
   end
 
   defp build_grade(answer = %{grader: grader}) do

lib/cadet_web/views/assessments_helpers.ex

@@ -88,45 +88,11 @@ defmodule CadetWeb.AssessmentsHelpers do
       gradedAt: graded_at_builder(grader),
       xp: &((&1.xp || 0) + (&1.xp_adjustment || 0)),
       autogradingStatus: :autograding_status,
-      autogradingResults: build_results(%{results: answer.autograding_results}),
+      autogradingResults: :autograding_results,
       comments: :comments
     })
   end
 
-  defp build_results(%{results: results}) do
-    case results do
-      nil -> nil
-      _ -> &Enum.map(&1.autograding_results, fn result -> build_result(result) end)
-    end
-  end
-
-  def build_result(result) do
-    transform_map_for_view(result, %{
-      resultType: "resultType",
-      expected: "expected",
-      actual: "actual",
-      errorType: "errorType",
-      errors: build_errors(result["errors"])
-    })
-  end
-
-  defp build_errors(errors) do
-    case errors do
-      nil -> nil
-      _ -> &Enum.map(&1["errors"], fn error -> build_error(error) end)
-    end
-  end
-
-  defp build_error(error) do
-    transform_map_for_view(error, %{
-      errorType: "errorType",
-      line: "line",
-      location: "location",
-      errorLine: "errorLine",
-      errorExplanation: "errorExplanation"
-    })
-  end
-
   defp build_contest_entry(entry) do
     transform_map_for_view(entry, %{
       submission_id: :submission_id,