Merge branch 'master' into cat

.credo.exs

@@ -100,7 +100,7 @@
         {Credo.Check.Refactor.MatchInCondition},
         {Credo.Check.Refactor.NegatedConditionsInUnless},
         {Credo.Check.Refactor.NegatedConditionsWithElse},
-        {Credo.Check.Refactor.Nesting},
+        {Credo.Check.Refactor.Nesting, max_nesting: 5},
         {Credo.Check.Refactor.PipeChainStart},
         {Credo.Check.Refactor.UnlessWithElse},
         {Credo.Check.Warning.BoolOperationOnSameValues},

lib/cadet/assessments/assessments.ex

@@ -268,7 +268,7 @@ defmodule Cadet.Assessments do
       assessment = assessment |> Map.put(:questions, questions)
       {:ok, assessment}
     else
-      {:error, {:unauthorized, "Assessment not open"}}
+      {:error, {:forbidden, "Assessment not open"}}
     end
   end
 
@@ -1158,7 +1158,7 @@ defmodule Cadet.Assessments do
   a boolean which is false by default.
 
   The return value is {:ok, submissions} if no errors, else it is {:error,
-  {:unauthorized, "Forbidden."}}
+  {:forbidden, "Forbidden."}}
   """
   @spec all_submissions_by_grader_for_index(CourseRegistration.t()) ::
           {:ok, String.t()}
@@ -1270,7 +1270,7 @@ defmodule Cadet.Assessments do
   end
 
   @spec get_answers_in_submission(integer() | String.t()) ::
-          {:ok, [Answer.t()]} | {:error, {:bad_request | :unauthorized, String.t()}}
+          {:ok, [Answer.t()]} | {:error, {:bad_request, String.t()}}
   def get_answers_in_submission(id) when is_ecto_id(id) do
     answer_query =
       Answer
@@ -1338,7 +1338,7 @@ defmodule Cadet.Assessments do
           CourseRegistration.t()
         ) ::
           {:ok, nil}
-          | {:error, {:unauthorized | :bad_request | :internal_server_error, String.t()}}
+          | {:error, {:forbidden | :bad_request | :internal_server_error, String.t()}}
   def update_grading_info(
         %{submission_id: submission_id, question_id: question_id},
         attrs,
@@ -1393,7 +1393,7 @@ defmodule Cadet.Assessments do
         _,
         _
       ) do
-    {:error, {:unauthorized, "User is not permitted to grade."}}
+    {:error, {:forbidden, "User is not permitted to grade."}}
   end
 
   @spec force_regrade_submission(integer() | String.t(), CourseRegistration.t()) ::

lib/cadet/auth/guardian.ex

@@ -8,16 +8,24 @@ defmodule Cadet.Auth.Guardian do
   alias Guardian.DB
 
   def subject_for_token(user, _claims) do
-    {:ok, to_string(user.id)}
+    {:ok,
+     URI.encode_query(%{
+       id: user.id,
+       username: user.username,
+       provider: user.provider
+     })}
   end
 
   def resource_from_claims(claims) do
-    user = Accounts.get_user(claims["sub"])
-
-    if user == nil do
-      {:error, :not_found}
-    else
-      {:ok, user}
+    case claims["sub"] |> URI.decode_query() |> Map.fetch("id") do
+      :error ->
+        {:error, :bad_request}
+
+      {:ok, id} ->
+        case user = Accounts.get_user(id) do
+          nil -> {:error, :not_found}
+          _ -> {:ok, user}
+        end
     end
   end
 

mix.exs

@@ -7,8 +7,7 @@ defmodule Cadet.Mixfile do
       version: "0.0.1",
       elixir: "~> 1.10",
       elixirc_paths: elixirc_paths(Mix.env()),
-      # compilers: [:phoenix, :gettext] ++ Mix.compilers() ++ [:phoenix_swagger],
-      compilers: [:phoenix] ++ Mix.compilers() ++ [:phoenix_swagger],
+      compilers: Mix.compilers() ++ [:phoenix_swagger],
       start_permanent: Mix.env() == :prod,
       test_coverage: [tool: ExCoveralls],
       preferred_cli_env: [
@@ -71,6 +70,7 @@ defmodule Cadet.Mixfile do
       {:jason, "~> 1.2"},
       {:openid_connect, "~> 0.2"},
       {:phoenix, "~> 1.5"},
+      {:phoenix_view, "~> 2.0"},
       {:phoenix_ecto, "~> 4.0"},
       {:phoenix_swagger, "~> 0.8"},
       {:plug_cowboy, "~> 2.0"},

test/cadet/auth/guardian_test.exs

@@ -6,23 +6,37 @@ defmodule Cadet.Auth.GuardianTest do
 
   test "token subject is user id" do
     user = insert(:user)
-    assert Guardian.subject_for_token(user, nil) == {:ok, to_string(user.id)}
+
+    assert Guardian.subject_for_token(user, nil) ==
+             {:ok,
+              URI.encode_query(%{
+                id: user.id,
+                username: user.username,
+                provider: user.provider
+              })}
   end
 
   test "get user from claims" do
     user = insert(:user)
 
     good_claims = %{
-      "sub" => to_string(user.id)
+      # Username and provider are only used for microservices
+      # The main backend only checks the user ID
+      "sub" => URI.encode_query(%{id: user.id})
+    }
+
+    bad_claims_user_not_found = %{
+      "sub" => URI.encode_query(%{id: 2000})
     }
 
-    bad_claims = %{
-      "sub" => "2000"
+    bad_claims_bad_sub = %{
+      "sub" => "bad"
     }
 
     assert Guardian.resource_from_claims(good_claims) ==
              {:ok, remove_preload(user, :latest_viewed_course)}
 
-    assert Guardian.resource_from_claims(bad_claims) == {:error, :not_found}
+    assert Guardian.resource_from_claims(bad_claims_user_not_found) == {:error, :not_found}
+    assert Guardian.resource_from_claims(bad_claims_bad_sub) == {:error, :bad_request}
   end
 end

test/cadet_web/controllers/assessments_controller_test.exs

@@ -895,7 +895,7 @@ defmodule CadetWeb.AssessmentsControllerTest do
         |> sign_in(student.user)
         |> get(build_url(course1.id, mission.assessment.id))
 
-      assert response(conn, 401) == "Assessment not open"
+      assert response(conn, 403) == "Assessment not open"
     end
 
     test "it does not permit access to unpublished assessments", %{