Fix GitHub OAuth connect flow

.env.example

@@ -4,8 +4,8 @@ export CLOUDEX_API_KEY=
 export CLOUDEX_CLOUD_NAME=
 export CLOUDEX_SECRET=
 export CLOUDFRONT_DOMAIN=
-export GITHUB_CLIENT_ID=
-export GITHUB_CLIENT_SECRET=
+export GITHUB_OAUTH_CLIENT_ID=
+export GITHUB_OAUTH_CLIENT_SECRET=
 export POSTMARK_API_KEY=
 export S3_BUCKET=
 export SEGMENT_WRITE_KEY=

blueprint/api.apib

@@ -2306,9 +2306,9 @@ This endpoint handles the User resources for Code Corps.
 
     + Attributes (Record Not Found Response)
 
-### Connect user to GitHub [POST /github_connect]
+### Connect user to GitHub [POST /oauth/github]
 
-This endpoint allows a user to connect their CodeCorps account to their respective GitHub account.
+This endpoint allows a user to connect their Code Corps account to their respective GitHub account.
 
 + Request
 

config/dev.exs

@@ -49,7 +49,7 @@ config :guardian, Guardian,
 
 config :code_corps, :analytics, CodeCorps.Analytics.InMemoryAPI
 
-config :code_corps, :github_api, CodeCorps.Github.API
+config :code_corps, :github_api, CodeCorps.GitHub.API
 
 # Configures stripe for dev mode
 config :code_corps, :stripe, Stripe
@@ -75,5 +75,5 @@ if System.get_env("CLOUDEX_API_KEY") == nil do
 end
 
 config :code_corps,
-  github_client_id: System.get_env("GITHUB_CLIENT_ID"),
-  github_client_secret: System.get_env("GITHUB_CLIENT_SECRET")
+  github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"),
+  github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET")

config/prod.exs

@@ -66,8 +66,8 @@ config :code_corps,
   postmark_receipt_template: "1255222"
 
 config :code_corps,
-  github_client_id: System.get_env("GITHUB_CLIENT_ID"),
-  github_client_secret: System.get_env("GITHUB_CLIENT_SECRET")
+  github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"),
+  github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET")
 
 # ## SSL Support
 #

config/remote-development.exs

@@ -48,8 +48,8 @@ config :code_corps,
   postmark_receipt_template: "123"
 
 config :code_corps,
-  github_client_id: System.get_env("GITHUB_CLIENT_ID"),
-  github_client_secret: System.get_env("GITHUB_CLIENT_SECRET")
+  github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"),
+  github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET")
 
 # ## SSL Support
 #

config/staging.exs

@@ -64,8 +64,8 @@ config :code_corps,
   postmark_receipt_template: "1252361"
 
 config :code_corps,
-  github_client_id: System.get_env("GITHUB_CLIENT_ID"),
-  github_client_secret: System.get_env("GITHUB_CLIENT_SECRET")
+  github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"),
+  github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET")
 
 # ## SSL Support
 #

config/test.exs

@@ -32,7 +32,7 @@ config :guardian, Guardian,
 
 config :code_corps, :analytics, CodeCorps.Analytics.TestAPI
 
-config :code_corps, :github_api, CodeCorps.Github.TestAPI
+config :code_corps, :github_api, CodeCorps.GitHub.TestAPI
 
 # Configures stripe for test mode
 config :code_corps, :stripe, CodeCorps.StripeTesting

lib/code_corps/github.ex

@@ -1,20 +1,20 @@
-defmodule CodeCorps.Github do
+defmodule CodeCorps.GitHub do
   alias CodeCorps.{User, Repo}
 
   @api Application.get_env(:code_corps, :github_api)
 
   @doc """
-  Posts code to github to receive an auth token, associates user with that
-  auth token.
+  POSTs `code` to GitHub to receive an OAuth token, then associates the user
+  with that OAuth token.
 
-  Accepts a third parameter, which is a custom API module, for the purposes of
+  Accepts a third parameter – a custom API module – for the purposes of
   explicit dependency injection during testing.
 
   Returns one of the following:
 
-  - {:ok, %CodeCorps.User{}}
-  - {:error, %Ecto.Changeset{}}
-  - {:error, "some_github_error"}
+  - `{:ok, %CodeCorps.User{}}`
+  - `{:error, %Ecto.Changeset{}}`
+  - `{:error, "some_github_error"}`
   """
   @spec connect(User.t, String.t, module) :: {:ok, User.t} | {:error, String.t}
   def connect(%User{} = user, code, api \\ @api) do
@@ -25,7 +25,7 @@ defmodule CodeCorps.Github do
   end
 
   @doc """
-  Associates user with an auth token
+  Associates user with the GitHub OAuth token.
 
   Returns one of the following:
 

lib/code_corps/github/api.ex

@@ -1,21 +1,21 @@
-defmodule CodeCorps.Github.API do
+defmodule CodeCorps.GitHub.API do
   @moduledoc """
-  The boundary module which communicates with the Github API using either
-  direct requests, or through Tentacat
+  The boundary module which communicates with the GitHub API using either
+  direct requests, or through Tentacat.
   """
-  @behaviour CodeCorps.Github.APIContract
+  @behaviour CodeCorps.GitHub.APIContract
 
-  @client_secret Application.get_env(:code_corps, :github_client_secret)
-  @client_id Application.get_env(:code_corps, :github_client_id)
+  @client_secret Application.get_env(:code_corps, :github_oauth_client_secret)
+  @client_id Application.get_env(:code_corps, :github_oauth_client_id)
 
   @base_connect_params %{
     client_id: @client_id,
     client_secret: @client_secret
   }
 
   @doc """
-  Receives a code generated through the client-side github connect process and
-  posts it to github.
+  Receives a code generated through the client-side GitHub connect process
+  and posts it to GitHub.
 
   Returns either an {:ok, access_token}, or an {:error, error_message}.
   """
@@ -34,7 +34,7 @@ defmodule CodeCorps.Github.API do
 
   @spec do_connect(map) :: {:ok, HTTPoison.Response.t | HTTPoison.AsyncResponse.t} | {:error, HTTPoison.Error.t}
   defp do_connect(params) do
-    HTTPoison.post(@connect_url, "", [{"Accept", "application/json"}], [params: params])
+    HTTPoison.post(@connect_url, "", [{"Accept", "application/json"}, {"Content-Type", "application/json"}], [params: params])
   end
 
   @spec build_connect_params(String.t) :: map

lib/code_corps/github/api_contract.ex

@@ -1,17 +1,21 @@
-defmodule CodeCorps.Github.APIContract do
+defmodule CodeCorps.GitHub.APIContract do
   @moduledoc """
-  Defines a contract for a github API module, listing all functions the module
+  Defines a contract for a GitHub API module, listing all functions the module
   should implement.
 
-  This contract should be specified as behaviour for the default `Github.API`
+  This contract should be specified as behaviour for the default `GitHub.API`
   module, as well as any custom module we inject in tests.
   """
 
   @doc """
-  Receives a code string, created in the client part of the github connect process,
-  returns either an :ok tupple indicating a successful connect process, where
-  the second element is the auth token string, or an :error tuple, where the
-  second element is an error message, or a struct
+  Receives a code string, created on the client side of the GitHub connect
+  process.
+
+  Returns one of:
+
+  - an `:ok` tuple indicating a successful connect process, where
+  the second element is the OAuth token string
+  - an `:error` tuple, where the second element is an error message or a struct
   """
   @callback connect(code :: String.t) :: {:ok, auth_token :: String.t} | {:error, error :: String.t}
 end

lib/code_corps/github/test_api.ex

@@ -1,12 +1,12 @@
-defmodule CodeCorps.Github.TestAPI do
+defmodule CodeCorps.GitHub.TestAPI do
   @moduledoc """
   The most basic implementation of an API module for testing. All functions
-  here should return successes.
+  here should return `:ok` tuples.
 
   If we want to test the wrapper module, we can specify custom API modules
   during function calls.
   """
-  @behaviour CodeCorps.Github.APIContract
+  @behaviour CodeCorps.GitHub.APIContract
 
   @spec connect(String.t) :: {:ok, String.t}
   def connect(code), do: send(self(), {:ok, code})

priv/repo/structure.sql

@@ -2600,3 +2600,4 @@ ALTER TABLE ONLY user_tasks
 --
 
 INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170526095401);
+

test/controllers/user_controller_test.exs

@@ -242,13 +242,13 @@ defmodule CodeCorps.UserControllerTest do
     end
   end
 
-  describe "github_connect" do
+  describe "github_oauth" do
     test "return the user when current user connects successfully", %{conn: conn} do
       user = insert(:user)
 
       code = %{"code" => "valid_code"}
 
-      path = user_path(conn, :github_connect, code)
+      path = user_path(conn, :github_oauth, code)
 
       json = conn |> authenticate(user) |> post(path) |> json_response(200)
 
@@ -258,7 +258,7 @@ defmodule CodeCorps.UserControllerTest do
     test "return unauthenticated error code when no current user", %{conn: conn} do
       code = %{"code" => "client generated code"}
 
-      path = user_path(conn, :github_connect, code)
+      path = user_path(conn, :github_oauth, code)
 
       conn |> post(path) |> json_response(401)
     end

test/lib/code_corps/github_test.exs

@@ -1,8 +1,8 @@
-defmodule CodeCorps.GithubTest do
+defmodule CodeCorps.GitHubTest do
   use CodeCorps.ModelCase
 
   alias CodeCorps.{
-    Github, User
+    GitHub, User
   }
 
   alias Ecto.Changeset
@@ -11,42 +11,42 @@ defmodule CodeCorps.GithubTest do
     test "updates the user, returns :ok tuple with user" do
       user = insert(:user)
       params = %{github_auth_token: "foobar"}
-      {:ok, %User{} = returned_user} = Github.associate(user, params)
+      {:ok, %User{} = returned_user} = GitHub.associate(user, params)
       assert user.id == returned_user.id
       assert returned_user.github_auth_token == "foobar"
     end
 
     test "returns :error tupple with changeset if there are validation errors" do
       user = insert(:user)
       params = %{}
-      {:error, %Changeset{} = changeset} = Github.associate(user, params)
+      {:error, %Changeset{} = changeset} = GitHub.associate(user, params)
       refute changeset.valid?
     end
   end
 
   defmodule SuccessAPI do
-    @behaviour CodeCorps.Github.APIContract
+    @behaviour CodeCorps.GitHub.APIContract
     def connect(_code), do: {:ok, "foo_auth_token"}
   end
 
   defmodule ErrorAPI do
-    @behaviour CodeCorps.Github.APIContract
+    @behaviour CodeCorps.GitHub.APIContract
     def connect(_code), do: {:error, "foo_error"}
   end
 
   describe "connect/2" do
     test "posts to github, updates user if reply is ok, returns updated user" do
       user = insert(:user)
 
-      {:ok, %User{} = returned_user} = Github.connect(user, "foo", SuccessAPI)
+      {:ok, %User{} = returned_user} = GitHub.connect(user, "foo", SuccessAPI)
 
       assert returned_user.id == user.id
       assert returned_user.github_auth_token == "foo_auth_token"
     end
 
     test "posts to github, returns error if reply is not ok" do
       user = insert(:user)
-      assert {:error, "foo_error"} == Github.connect(user, "foo", ErrorAPI)
+      assert {:error, "foo_error"} == GitHub.connect(user, "foo", ErrorAPI)
     end
   end
 end

web/controllers/user_controller.ex

@@ -6,7 +6,7 @@ defmodule CodeCorps.UserController do
 
   alias CodeCorps.User
   alias CodeCorps.Services.UserService
-  alias CodeCorps.Github
+  alias CodeCorps.GitHub
 
   plug :load_and_authorize_resource, model: User, only: [:update]
   plug JaResource
@@ -35,11 +35,14 @@ defmodule CodeCorps.UserController do
     end
   end
 
-  def github_connect(conn, %{"code" => code}) do
+  @doc """
+  Differs from other resources by path: `/oauth/github`
+  """
+  def github_oauth(conn, %{"code" => code}) do
     current_user = Guardian.Plug.current_resource(conn)
-    with {:ok, user} <- Github.connect(current_user, code)
+    with {:ok, user} <- GitHub.connect(current_user, code)
     do
-      conn |> render(:show, data: user)
+      conn |> render("show.json-api", data: user)
     else
       {:error, %Ecto.Changeset{} = changeset} ->
         conn

web/router.ex

@@ -63,7 +63,7 @@ defmodule CodeCorps.Router do
     resources "/categories", CategoryController, only: [:create, :update]
     resources "/comments", CommentController, only: [:create, :update]
     resources "/donation-goals", DonationGoalController, only: [:create, :update, :delete]
-    post "/github-connect", UserController, :github_connect
+    post "/oauth/github", UserController, :github_oauth
     resources "/organizations", OrganizationController, only: [:create, :update]
     resources "/previews", PreviewController, only: [:create]
     resources "/project-categories", ProjectCategoryController, only: [:create, :delete]