Skip to content

Commit

Permalink
Fix differences between 401 and 403 throughout the application
Browse files Browse the repository at this point in the history
  • Loading branch information
joshsmith committed Oct 28, 2016
1 parent 51fc5e3 commit 0d9e1e8
Show file tree
Hide file tree
Showing 23 changed files with 109 additions and 81 deletions.
4 changes: 2 additions & 2 deletions blueprint/api.apib
Original file line number Diff line number Diff line change
Expand Up @@ -2259,10 +2259,10 @@ This endpoint allows you to check whether a username is valid (by running a vali

## Forbidden Response (object)
+ errors
+ detail: `You are not authorized to perform this action.` (string)
+ id: `FORBIDDEN` (string)
+ title: `Forbidden` (string)
+ detail: `You are not authorized to perform this action on {subject_name}` (string) - Details of what record was not found.
+ status: 403 (number) - HTTP status code
+ title: `403 Forbidden` (string)

## No Content Response (object)

Expand Down
8 changes: 4 additions & 4 deletions test/controllers/category_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ defmodule CodeCorps.CategoryControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
assert conn |> request_create(@valid_attrs) |> json_response(401)
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
assert conn |> request_create(@valid_attrs) |> json_response(403)
end
end

Expand All @@ -95,8 +95,8 @@ defmodule CodeCorps.CategoryControllerTest do
end

@tag :authenticated
test "does not update resource and renders 401 when not authorized", %{conn: conn} do
assert conn |> request_update(@valid_attrs) |> json_response(401)
test "does not update resource and renders 403 when not authorized", %{conn: conn} do
assert conn |> request_update(@valid_attrs) |> json_response(403)
end
end
end
16 changes: 6 additions & 10 deletions test/controllers/comment_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ defmodule CodeCorps.CommentControllerTest do
test "creates and renders resource when data is valid", %{conn: conn, current_user: current_user} do
task = insert(:task)
attrs = @valid_attrs |> Map.merge(%{task: task, user: current_user})
json = conn |> request_create(attrs) |> json_response(201)
assert json["data"]["id"]
assert Repo.get_by(Comment, @valid_attrs)
assert conn |> request_create(attrs) |> json_response(201)
end

@tag :authenticated
Expand All @@ -66,8 +64,8 @@ defmodule CodeCorps.CommentControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
assert conn |> request_create(@valid_attrs) |> json_response(401)
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
assert conn |> request_create(@valid_attrs) |> json_response(403)
end
end

Expand All @@ -76,9 +74,7 @@ defmodule CodeCorps.CommentControllerTest do
test "updates and renders chosen resource when data is valid", %{conn: conn, current_user: current_user} do
comment = insert(:comment, user: current_user)
attrs = @valid_attrs |> Map.merge(%{user: current_user})
json = conn |> request_update(comment, attrs) |> json_response(200)
assert json["data"]["id"]
assert Repo.get_by(Comment, @valid_attrs)
assert conn |> request_update(comment, attrs) |> json_response(200)
end

@tag :authenticated
Expand All @@ -94,8 +90,8 @@ defmodule CodeCorps.CommentControllerTest do
end

@tag :authenticated
test "does not update resource and renders 401 when not authorized", %{conn: conn} do
assert conn |> request_update(@valid_attrs) |> json_response(401)
test "does not update resource and renders 403 when not authorized", %{conn: conn} do
assert conn |> request_update(@valid_attrs) |> json_response(403)
end
end
end
12 changes: 6 additions & 6 deletions test/controllers/donation_goal_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ defmodule CodeCorps.DonationGoalControllerTest do
end

@tag :authenticated
test "renders 401 when not authorized", %{conn: conn} do
assert conn |> request_create(@valid_attrs) |> json_response(401)
test "renders 403 when not authorized", %{conn: conn} do
assert conn |> request_create(@valid_attrs) |> json_response(403)
end
end

Expand Down Expand Up @@ -80,8 +80,8 @@ defmodule CodeCorps.DonationGoalControllerTest do
end

@tag :authenticated
test "renders 401 when not authorized", %{conn: conn} do
assert conn |> request_update(@valid_attrs) |> json_response(401)
test "renders 403 when not authorized", %{conn: conn} do
assert conn |> request_update(@valid_attrs) |> json_response(403)
end
end

Expand All @@ -103,8 +103,8 @@ defmodule CodeCorps.DonationGoalControllerTest do
end

@tag :authenticated
test "renders 401 when not authorized", %{conn: conn} do
assert conn |> request_delete |> json_response(401)
test "renders 403 when not authorized", %{conn: conn} do
assert conn |> request_delete |> json_response(403)
end
end
end
8 changes: 4 additions & 4 deletions test/controllers/organization_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ defmodule CodeCorps.OrganizationControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
payload =
build_payload
|> put_attributes(@invalid_attrs)

path = conn |> organization_path(:create)
conn = conn |> post(path, payload)

assert json_response(conn, 401)
assert json_response(conn, 403)
end
end

Expand Down Expand Up @@ -158,7 +158,7 @@ defmodule CodeCorps.OrganizationControllerTest do
end

@tag :authenticated
test "does not update resource and renders 401 when not authorized", %{conn: conn, current_user: current_user} do
test "does not update resource and renders 403 when not authorized", %{conn: conn, current_user: current_user} do
organization = insert(:organization)
insert(:organization_membership, organization: organization, member: current_user, role: "member")

Expand All @@ -170,7 +170,7 @@ defmodule CodeCorps.OrganizationControllerTest do
path = conn |> organization_path(:update, organization)
conn = conn |> put(path, payload)

assert json_response(conn, 401)
assert json_response(conn, 403)
end

@tag :requires_env
Expand Down
8 changes: 4 additions & 4 deletions test/controllers/organization_membership_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ defmodule CodeCorps.OrganizationMembershipControllerTest do
end

@tag :authenticated
test "doesn't update and renders 401 when not authorized", %{conn: conn} do
test "doesn't update and renders 403 when not authorized", %{conn: conn} do
membership = insert(:organization_membership)

payload =
Expand All @@ -156,7 +156,7 @@ defmodule CodeCorps.OrganizationMembershipControllerTest do
path = conn |> organization_membership_path(:update, membership)
conn = conn |> put(path, payload)

assert conn |> json_response(401)
assert conn |> json_response(403)
end

@tag :authenticated
Expand Down Expand Up @@ -192,7 +192,7 @@ defmodule CodeCorps.OrganizationMembershipControllerTest do
end

@tag :authenticated
test "doesn't delete and renders 401 when not authorized", %{conn: conn} do
test "doesn't delete and renders 403 when not authorized", %{conn: conn} do
membership = insert(:organization_membership)

payload =
Expand All @@ -203,7 +203,7 @@ defmodule CodeCorps.OrganizationMembershipControllerTest do
path = conn |> organization_membership_path(:delete, membership)
conn = conn |> delete(path, payload)

assert conn |> json_response(401)
assert conn |> json_response(403)
end

@tag :authenticated
Expand Down
8 changes: 4 additions & 4 deletions test/controllers/project_category_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ defmodule CodeCorps.ProjectCategoryControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn, current_user: current_user} do
test "does not create resource and renders 403 when not authorized", %{conn: conn, current_user: current_user} do
organization = insert(:organization)
project = insert(:project, organization: organization)
category = insert(:category)
Expand All @@ -117,7 +117,7 @@ defmodule CodeCorps.ProjectCategoryControllerTest do
payload = build_payload |> put_relationships(project, category)

path = conn |> project_category_path(:create)
assert conn |> post(path, payload) |> json_response(401)
assert conn |> post(path, payload) |> json_response(403)
end
end

Expand All @@ -142,10 +142,10 @@ defmodule CodeCorps.ProjectCategoryControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
project_category = insert(:project_category)
path = conn |> project_category_path(:delete, project_category)
assert conn |> delete(path) |> json_response(401)
assert conn |> delete(path) |> json_response(403)
end

@tag :authenticated
Expand Down
8 changes: 4 additions & 4 deletions test/controllers/project_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ defmodule CodeCorps.ProjectControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn, current_user: current_user} do
test "does not create resource and renders 403 when not authorized", %{conn: conn, current_user: current_user} do
organization = insert(:organization)
insert(:organization_membership, role: "contributor", member: current_user, organization: organization)

Expand All @@ -165,7 +165,7 @@ defmodule CodeCorps.ProjectControllerTest do
|> put_relationships(organization)

path = conn |> project_path(:create)
assert conn |> post(path, payload) |> json_response(401)
assert conn |> post(path, payload) |> json_response(403)
end
end

Expand Down Expand Up @@ -232,10 +232,10 @@ defmodule CodeCorps.ProjectControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
project = insert(:project)
path = conn |> project_path(:update, project)
assert conn |> put(path) |> json_response(401)
assert conn |> put(path) |> json_response(403)
end

@tag :authenticated
Expand Down
8 changes: 4 additions & 4 deletions test/controllers/project_skill_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ defmodule CodeCorps.ProjectSkillControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn, current_user: current_user} do
test "does not create resource and renders 403 when not authorized", %{conn: conn, current_user: current_user} do
organization = insert(:organization)
project = insert(:project, organization: organization)
skill = insert(:skill)
Expand All @@ -111,7 +111,7 @@ defmodule CodeCorps.ProjectSkillControllerTest do
payload = build_payload |> put_relationships(project, skill)

path = conn |> project_skill_path(:create)
assert conn |> post(path, payload) |> json_response(401)
assert conn |> post(path, payload) |> json_response(403)
end
end

Expand All @@ -130,10 +130,10 @@ defmodule CodeCorps.ProjectSkillControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
project_skill = insert(:project_skill)
path = conn |> project_skill_path(:delete, project_skill)
assert conn |> delete(path) |> json_response(401)
assert conn |> delete(path) |> json_response(403)
end

@tag :authenticated
Expand Down
4 changes: 2 additions & 2 deletions test/controllers/role_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ defmodule CodeCorps.RoleControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
path = conn |> role_path(:create)
payload = build_payload |> put_attributes(@valid_attrs)
assert conn |> post(path, payload) |> json_response(401)
assert conn |> post(path, payload) |> json_response(403)
end
end
end
8 changes: 4 additions & 4 deletions test/controllers/role_skill_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ defmodule CodeCorps.RoleSkillControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
path = conn |> role_skill_path(:create)
assert conn |> post(path) |> json_response(401)
assert conn |> post(path) |> json_response(403)
end
end

Expand Down Expand Up @@ -133,10 +133,10 @@ defmodule CodeCorps.RoleSkillControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
role_skill = insert(:role_skill)
path = conn |> role_skill_path(:delete, role_skill)
assert conn |> delete(path) |> json_response(401)
assert conn |> delete(path) |> json_response(403)
end

@tag :authenticated
Expand Down
4 changes: 2 additions & 2 deletions test/controllers/skill_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,10 @@ defmodule CodeCorps.SkillControllerTest do
end

@tag :authenticated
test "does not create resource and renders 401 when not authorized", %{conn: conn} do
test "does not create resource and renders 403 when not authorized", %{conn: conn} do
path = conn |> skill_path(:create)
payload = build_payload |> put_attributes(@valid_attrs)
assert conn |> post(path, payload) |> json_response(401)
assert conn |> post(path, payload) |> json_response(403)
end
end
end
4 changes: 2 additions & 2 deletions test/controllers/stripe_auth_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ defmodule CodeCorps.StripeAuthControllerTest do

@tag :requires_env
@tag :authenticated
test "renders a 401 when not authorized", %{conn: conn} do
test "renders a 403 when not authorized", %{conn: conn} do
project = insert(:project)

conn = get conn, stripe_auth_path(conn, :stripe_auth, project)
assert json_response(conn, 401)
assert json_response(conn, 403)
end
end
end
4 changes: 2 additions & 2 deletions test/controllers/task_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,12 @@ defmodule CodeCorps.TaskControllerTest do
end

@tag :authenticated
test "does not update resource and renders 401 when not authorized", %{conn: conn} do
test "does not update resource and renders 403 when not authorized", %{conn: conn} do
task = insert(:task)
payload = build_payload |> put_id(task.id) |> put_attributes(@invalid_attrs)

path = conn |> task_path(:update, task)
assert conn |> put(path, payload) |> json_response(401)
assert conn |> put(path, payload) |> json_response(403)
end
end

Expand Down
10 changes: 5 additions & 5 deletions test/controllers/token_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ defmodule CodeCorps.TokenControllerTest do
response = json_response(conn, 401)
[error | _] = response["errors"]
assert error["detail"] == "Your password doesn't match the email #{user.email}."
assert is_unauthorized?(error)
assert renders_401_unauthorized?(error)
refute response["token"]
refute response["user_id"]
end
Expand All @@ -45,7 +45,7 @@ defmodule CodeCorps.TokenControllerTest do
response = json_response(conn, 401)
[error | _] = response["errors"]
assert error["detail"] == "We couldn't find a user with the email notauser@test.com."
assert is_unauthorized?(error)
assert renders_401_unauthorized?(error)
refute response["token"]
refute response["user_id"]
end
Expand Down Expand Up @@ -73,11 +73,11 @@ defmodule CodeCorps.TokenControllerTest do
refute response["token"]
refute response["user_id"]
[error | _] = response["errors"]
assert is_unauthorized?(error)
assert renders_401_unauthorized?(error)
assert error["detail"] == "token_expired"
end
end

defp is_unauthorized?(%{"id" => "UNAUTHORIZED", "title" => "401 Unauthorized", "status" => 401}), do: true
defp is_unauthorized?(_), do: false
defp renders_401_unauthorized?(%{"id" => "UNAUTHORIZED", "title" => "401 Unauthorized", "status" => 401}), do: true
defp renders_401_unauthorized?(_), do: false
end

0 comments on commit 0d9e1e8

Please sign in to comment.