diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index df795e125..1547ddbbb 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -36,23 +36,20 @@ defmodule CodeCorpsWeb.UserController do {:ok, :authorized} <- current_user |> Policy.authorize(:update, user), {:ok, user, _, _} <- user |> UserService.update(params) do - conn |> render("show.json-api", data: user) + conn |> render("show.json-api", data: user) end end @doc """ Differs from other resources by path: `/oauth/github` """ - def github_oauth(conn, %{"code" => code, "state" => state}) do + @spec github_oauth(Conn.t, map) :: Conn.t + def github_oauth(%Conn{} = conn, %{"code" => code, "state" => state}) do current_user = Guardian.Plug.current_resource(conn) with {:ok, user} <- GitHub.User.connect(current_user, code, state) do conn |> render("show.json-api", data: user) else - {:error, %Ecto.Changeset{} = changeset} -> - conn - |> put_status(:unprocessable_entity) - |> render(CodeCorpsWeb.ChangesetView, "error.json-api", changeset: changeset) {:error, _error} -> conn |> put_status(:internal_server_error) @@ -60,12 +57,14 @@ defmodule CodeCorpsWeb.UserController do end end - def email_available(conn, %{"email" => email}) do + @spec email_available(Conn.t, map) :: Conn.t + def email_available(%Conn{} = conn, %{"email" => email}) do hash = User.check_email_availability(email) conn |> json(hash) end - def username_available(conn, %{"username" => username}) do + @spec username_available(Conn.t, map) :: Conn.t + def username_available(%Conn{} = conn, %{"username" => username}) do hash = User.check_username_availability(username) conn |> json(hash) end diff --git a/test/lib/code_corps/github/user_test.exs b/test/lib/code_corps/github/user_test.exs index e4d664ad7..7558221e5 100644 --- a/test/lib/code_corps/github/user_test.exs +++ b/test/lib/code_corps/github/user_test.exs @@ -49,7 +49,7 @@ defmodule CodeCorps.GitHub.UserTest do error = GitHub.APIError.new({404, %{"message" => "{\"error\":\"Not Found\"}"}}) with_mock_api(NotFoundRequest) do - assert {:error, error } == GitHub.User.connect(user, "foo_code", "foo_state") + assert {:error, error} == GitHub.User.connect(user, "foo_code", "foo_state") end end end diff --git a/test/lib/code_corps_web/controllers/user_controller_test.exs b/test/lib/code_corps_web/controllers/user_controller_test.exs index f3640bee7..52690c5ad 100644 --- a/test/lib/code_corps_web/controllers/user_controller_test.exs +++ b/test/lib/code_corps_web/controllers/user_controller_test.exs @@ -3,6 +3,8 @@ defmodule CodeCorpsWeb.UserControllerTest do use CodeCorpsWeb.ApiCase, resource_name: :user + import CodeCorps.GitHub.TestHelpers + alias CodeCorps.{User, Repo} @valid_attrs %{ @@ -243,26 +245,30 @@ defmodule CodeCorpsWeb.UserControllerTest do end end - describe "github_oauth" do - test "return the user when current user connects successfully", %{conn: conn} do - user = insert(:user) - - json = %{"code" => "valid_code", "state" => "valid_state"} - - path = user_path(conn, :github_oauth, json) + @attrs %{"code" => "foo", "state" => "bar"} + @tag :authenticated + test "return the user when current user connects successfully", %{conn: conn, current_user: current_user} do + path = user_path(conn, :github_oauth) - json = conn |> authenticate(user) |> post(path) |> json_response(200) + json = conn |> post(path, @attrs) |> json_response(200) - assert json["data"]["id"] |> String.to_integer == user.id + assert json["data"]["id"] |> String.to_integer == current_user.id + assert json["data"]["attributes"]["github-id"] end - test "return unauthenticated error code when no current user", %{conn: conn} do - json = %{"code" => "client generated code", "state" => "state"} + test "requires authentication", %{conn: conn} do + path = user_path(conn, :github_oauth) + assert conn |> post(path, @attrs) |> json_response(401) + end - path = user_path(conn, :github_oauth, json) + @tag :authenticated + test "renders 500 if there's a GitHub API error", %{conn: conn} do + path = user_path(conn, :github_oauth) - conn |> post(path) |> json_response(401) + with_mock_api(CodeCorps.GitHub.FailureAPI) do + assert conn |> post(path, @attrs) |> json_response(500) + end end end diff --git a/test/support/github/failure_api.ex b/test/support/github/failure_api.ex new file mode 100644 index 000000000..c7eeb9c27 --- /dev/null +++ b/test/support/github/failure_api.ex @@ -0,0 +1,37 @@ +defmodule CodeCorps.GitHub.FailureAPI do + @moduledoc ~S""" + A basic GitHub API mock which returns a 401 forbidden for all requests. + + Should be good enough for any tests that simply assert a piece of code is able + to recover from a generic request error. + + For any tests that cover handling of specific errors, a non-default API should + be defined inline. + + Since our GitHub requests are often forced to start with an installation + access token request, that one is set to succeed here as well. + """ + import CodeCorps.GitHub.TestHelpers + + alias CodeCorps.GitHub.SuccessAPI + + def request(method, url, headers, body, options) do + case {method, url} |> for_access_token?() do + true -> SuccessAPI.request(method, url, headers, body, options) + false -> + send(self(), {method, url, headers, body, options}) + body = load_endpoint_fixture("forbidden") + error = CodeCorps.GitHub.APIError.new({401, body}) + {:error, error} + end + end + + defp for_access_token?({:post, url}), do: url |> access_token_url?() + defp for_access_token?({_method, _url}), do: false + + defp access_token_url?("https://api.github.com/" <> path), do: path |> String.split("/") |> access_token_parts?() + defp access_token_url?(_), do: false + + defp access_token_parts?(["installations", _, "access_tokens"]), do: true + defp access_token_parts?(_), do: false +end