diff --git a/.env.example b/.env.example index 4e3766b95..ad816e744 100644 --- a/.env.example +++ b/.env.example @@ -4,6 +4,8 @@ export CLOUDEX_API_KEY= export CLOUDEX_CLOUD_NAME= export CLOUDEX_SECRET= export CLOUDFRONT_DOMAIN= +export GITHUB_OAUTH_CLIENT_ID= +export GITHUB_OAUTH_CLIENT_SECRET= export POSTMARK_API_KEY= export S3_BUCKET= export SEGMENT_WRITE_KEY= diff --git a/blueprint/api.apib b/blueprint/api.apib index 482bbffd6..c86e99f18 100644 --- a/blueprint/api.apib +++ b/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 diff --git a/config/dev.exs b/config/dev.exs index c36faaf54..70cbcad6e 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -49,6 +49,8 @@ config :guardian, Guardian, config :code_corps, :analytics, CodeCorps.Analytics.InMemoryAPI +config :code_corps, :github_api, CodeCorps.GitHub.API + # Configures stripe for dev mode config :code_corps, :stripe, Stripe config :code_corps, :stripe_env, :dev @@ -71,3 +73,7 @@ if System.get_env("CLOUDEX_API_KEY") == nil do config :code_corps, :cloudex, CloudexTest config :cloudex, api_key: "test_key", secret: "test_secret", cloud_name: "test_cloud_name" end + +config :code_corps, + github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"), + github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET") diff --git a/config/prod.exs b/config/prod.exs index 5959dcb41..9c9b1d7e5 100644 --- a/config/prod.exs +++ b/config/prod.exs @@ -65,6 +65,10 @@ config :code_corps, postmark_project_acceptance_template: "1447041", postmark_receipt_template: "1255222" +config :code_corps, + github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"), + github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET") + # ## SSL Support # # To get SSL working, you will need to add the `https` key diff --git a/config/remote-development.exs b/config/remote-development.exs index be64e416d..e3245e524 100644 --- a/config/remote-development.exs +++ b/config/remote-development.exs @@ -47,6 +47,10 @@ config :code_corps, postmark_project_acceptance_template: "123", postmark_receipt_template: "123" +config :code_corps, + github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"), + github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET") + # ## SSL Support # # To get SSL working, you will need to add the `https` key diff --git a/config/staging.exs b/config/staging.exs index 6e8a48c83..003103e37 100644 --- a/config/staging.exs +++ b/config/staging.exs @@ -63,6 +63,10 @@ config :code_corps, postmark_project_acceptance_template: "1447022", postmark_receipt_template: "1252361" +config :code_corps, + github_oauth_client_id: System.get_env("GITHUB_OAUTH_CLIENT_ID"), + github_oauth_client_secret: System.get_env("GITHUB_OAUTH_CLIENT_SECRET") + # ## SSL Support # # To get SSL working, you will need to add the `https` key diff --git a/config/test.exs b/config/test.exs index 20ba08cd1..8019cc09b 100644 --- a/config/test.exs +++ b/config/test.exs @@ -32,6 +32,8 @@ config :guardian, Guardian, config :code_corps, :analytics, CodeCorps.Analytics.TestAPI +config :code_corps, :github_api, CodeCorps.GitHub.TestAPI + # Configures stripe for test mode config :code_corps, :stripe, CodeCorps.StripeTesting config :code_corps, :stripe_env, :test diff --git a/lib/code_corps/github.ex b/lib/code_corps/github.ex index 6f54622a6..c0d993c7c 100644 --- a/lib/code_corps/github.ex +++ b/lib/code_corps/github.ex @@ -1,15 +1,41 @@ -defmodule CodeCorps.Github do - +defmodule CodeCorps.GitHub do alias CodeCorps.{User, Repo} + @api Application.get_env(:code_corps, :github_api) + @doc """ - Temporary function until the actual behavior is implemented. + POSTs `code` to GitHub to receive an OAuth token, then associates the user + with that OAuth token. + + 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"}` """ - def connect(user, _code), do: {:ok, user} + @spec connect(User.t, String.t, module) :: {:ok, User.t} | {:error, String.t} + def connect(%User{} = user, code, api \\ @api) do + case code |> api.connect do + {:ok, github_auth_token} -> user |> associate(%{github_auth_token: github_auth_token}) + {:error, error} -> {:error, error} + end + end + + @doc """ + Associates user with the GitHub OAuth token. + Returns one of the following: + + - {:ok, %CodeCorps.User{}} + - {:error, %Ecto.Changeset{}} + """ + @spec associate(User.t, map) :: {:ok, User.t} | {:error, Ecto.Changeset.t} def associate(user, params) do user - |> User.github_associate_changeset(params) + |> User.github_association_changeset(params) |> Repo.update() end end diff --git a/lib/code_corps/github/api.ex b/lib/code_corps/github/api.ex new file mode 100644 index 000000000..c46b93629 --- /dev/null +++ b/lib/code_corps/github/api.ex @@ -0,0 +1,44 @@ +defmodule CodeCorps.GitHub.API do + @moduledoc """ + The boundary module which communicates with the GitHub API using either + direct requests, or through Tentacat. + """ + @behaviour CodeCorps.GitHub.APIContract + + @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. + + Returns either an {:ok, access_token}, or an {:error, error_message}. + """ + @spec connect(String.t) :: {:ok, String.t} | {:error, String.t} + def connect(code) do + with {:ok, %HTTPoison.Response{body: response}} <- code |> build_connect_params() |> do_connect(), + {:ok, %{"access_token" => access_token}} <- response |> Poison.decode + do + {:ok, access_token} + else + {:ok, %{"error" => error}} -> {:error, error} + end + end + + @connect_url "https://github.com/login/oauth/access_token" + + @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"}, {"Content-Type", "application/json"}], [params: params]) + end + + @spec build_connect_params(String.t) :: map + defp build_connect_params(code) do + @base_connect_params |> Map.put(:code, code) + end +end diff --git a/lib/code_corps/github/api_contract.ex b/lib/code_corps/github/api_contract.ex new file mode 100644 index 000000000..e18ec6bc5 --- /dev/null +++ b/lib/code_corps/github/api_contract.ex @@ -0,0 +1,21 @@ +defmodule CodeCorps.GitHub.APIContract do + @moduledoc """ + 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` + module, as well as any custom module we inject in tests. + """ + + @doc """ + 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 diff --git a/lib/code_corps/github/test_api.ex b/lib/code_corps/github/test_api.ex new file mode 100644 index 000000000..4e13a615c --- /dev/null +++ b/lib/code_corps/github/test_api.ex @@ -0,0 +1,13 @@ +defmodule CodeCorps.GitHub.TestAPI do + @moduledoc """ + The most basic implementation of an API module for testing. All functions + 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 + + @spec connect(String.t) :: {:ok, String.t} + def connect(code), do: send(self(), {:ok, code}) +end diff --git a/mix.exs b/mix.exs index 9e20641fe..b09dec7d9 100644 --- a/mix.exs +++ b/mix.exs @@ -45,6 +45,7 @@ defmodule CodeCorps.Mixfile do :segment, :sentry, :stripity_stripe, + :tentacat, :timber, :timex_ecto ] @@ -96,6 +97,7 @@ defmodule CodeCorps.Mixfile do {:sentry, "~> 2.0"}, # Sentry error tracking {:stripity_stripe, git: "https://github.com/code-corps/stripity_stripe.git", branch: "2.0"}, # Stripe {:sweet_xml, "~> 0.5"}, + {:tentacat, "~> 0.5"}, {:timber, "~> 0.4"}, # Logging {:timex, "~> 3.0"}, {:timex_ecto, "~> 3.0"}, diff --git a/mix.lock b/mix.lock index 1f8d63ff0..bc5f94354 100644 --- a/mix.lock +++ b/mix.lock @@ -59,6 +59,7 @@ "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], []}, "stripity_stripe": {:git, "https://github.com/code-corps/stripity_stripe.git", "1e9b87d73710ef901dd6d735d6f2d0a2cef75d1c", [branch: "2.0"]}, "sweet_xml": {:hex, :sweet_xml, "0.6.5", "dd9cde443212b505d1b5f9758feb2000e66a14d3c449f04c572f3048c66e6697", [:mix], []}, + "tentacat": {:hex, :tentacat, "0.6.2", "e5e5ad95d577dd441e4dcfcab259c9d92b0049f0481a4be6453769d61a956a3b", [:mix], [{:exjsx, "~> 3.2", [hex: :exjsx, optional: false]}, {:httpoison, "~> 0.8", [hex: :httpoison, optional: false]}]}, "timber": {:hex, :timber, "0.4.7", "df3fcd79bcb4eb4b53874d906ef5f3a212937b4bc7b7c5b244745202cc389443", [:mix], [{:ecto, "~> 2.0", [hex: :ecto, optional: true]}, {:phoenix, "~> 1.2", [hex: :phoenix, optional: true]}, {:plug, "~> 1.2", [hex: :plug, optional: true]}, {:poison, "~> 2.0 or ~> 3.0", [hex: :poison, optional: false]}]}, "timex": {:hex, :timex, "3.0.8", "71d5ebafcdc557c6c866cdc196c5054f587e7cd1118ad8937e2293d51fc85608", [:mix], [{:combine, "~> 0.7", [hex: :combine, optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, optional: false]}]}, "timex_ecto": {:hex, :timex_ecto, "3.1.1", "37d54f6879d96a6789bb497296531cfb853631de78e152969d95cff03c1368dd", [:mix], [{:ecto, "~> 2.1.0", [hex: :ecto, optional: false]}, {:timex, "~> 3.0", [hex: :timex, optional: false]}]}, diff --git a/priv/repo/migrations/20170526095401_add_github_auth_token_to_users.exs b/priv/repo/migrations/20170526095401_add_github_auth_token_to_users.exs new file mode 100644 index 000000000..51a0a216f --- /dev/null +++ b/priv/repo/migrations/20170526095401_add_github_auth_token_to_users.exs @@ -0,0 +1,9 @@ +defmodule CodeCorps.Repo.Migrations.AddGithubAuthTokenToUsers do + use Ecto.Migration + + def change do + alter table(:users) do + add :github_auth_token, :string + end + end +end diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index 42634eed9..1b88c1d93 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -1324,7 +1324,8 @@ CREATE TABLE users ( cloudinary_public_id character varying(255), default_color character varying(255), sign_up_context character varying(255) DEFAULT 'default'::character varying, - github_id character varying(255) + github_id character varying(255), + github_auth_token character varying(255) ); @@ -2598,5 +2599,5 @@ ALTER TABLE ONLY user_tasks -- PostgreSQL database dump complete -- -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); +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); diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index 483baf943..5de0eb2e1 100644 --- a/test/controllers/user_controller_test.exs +++ b/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" => "client generated code"} + 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 diff --git a/test/lib/code_corps/github_test.exs b/test/lib/code_corps/github_test.exs index a1b5c31b4..4777fe26c 100644 --- a/test/lib/code_corps/github_test.exs +++ b/test/lib/code_corps/github_test.exs @@ -1,20 +1,52 @@ -defmodule CodeCorps.GithubTest do +defmodule CodeCorps.GitHubTest do use CodeCorps.ModelCase - alias CodeCorps.Github + + alias CodeCorps.{ + GitHub, User + } + + alias Ecto.Changeset describe "associate/2" do - test "should update the user with the github_id" do + test "updates the user, returns :ok tuple with user" do user = insert(:user) - params = %{github_id: "foobar"} - {:ok, result} = Github.associate(user, params) - assert result.github_id == "foobar" + params = %{github_auth_token: "foobar"} + {:ok, %User{} = returned_user} = GitHub.associate(user, params) + assert user.id == returned_user.id + assert returned_user.github_auth_token == "foobar" end - test "should return the error with a changeset" do + test "returns :error tupple with changeset if there are validation errors" do user = insert(:user) params = %{} - {:error, changeset} = Github.associate(user, params) + {:error, %Changeset{} = changeset} = GitHub.associate(user, params) refute changeset.valid? end end + + defmodule SuccessAPI do + @behaviour CodeCorps.GitHub.APIContract + def connect(_code), do: {:ok, "foo_auth_token"} + end + + defmodule ErrorAPI do + @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) + + 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) + end + end end diff --git a/test/models/donation_goal_test.exs b/test/models/donation_goal_test.exs index 07bb9e768..c8d420ddb 100644 --- a/test/models/donation_goal_test.exs +++ b/test/models/donation_goal_test.exs @@ -26,6 +26,15 @@ defmodule CodeCorps.DonationGoalTest do refute changeset.valid? changeset |> assert_error_message(:project, "does not exist") end + + test "amount must not be negative" do + attrs = %{amount: -100, description: "Cashback for donators", project_id: 2} + donation_goal = insert(:donation_goal) + changeset = DonationGoal.create_changeset(donation_goal, attrs) + + refute changeset.valid? + changeset |> assert_error_message(:amount, "must be greater than %{number}") + end end describe "&update_changeset/2" do diff --git a/test/models/user_test.exs b/test/models/user_test.exs index 23a63eb16..a8fbfdd81 100644 --- a/test/models/user_test.exs +++ b/test/models/user_test.exs @@ -140,16 +140,16 @@ defmodule CodeCorps.UserTest do end end - describe "github_associate_changeset" do - test "should cast github_id" do + describe "github_association_changeset" do + test "should cast github_auth_token" do user = insert(:user) - changeset = user |> User.github_associate_changeset(%{github_id: "foobar"}) + changeset = user |> User.github_association_changeset(%{github_auth_token: "foobar"}) assert changeset.valid? - assert changeset.changes.github_id == "foobar" + assert changeset.changes.github_auth_token == "foobar" end - test "github_id should be required" do + test "github_auth_token should be required" do user = insert(:user) - changeset = user |> User.github_associate_changeset(%{}) + changeset = user |> User.github_association_changeset(%{}) refute changeset.valid? end end diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index 038b4c465..63396d59b 100644 --- a/web/controllers/user_controller.ex +++ b/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) 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 diff --git a/web/models/donation_goal.ex b/web/models/donation_goal.ex index 572ef430f..375c431d0 100644 --- a/web/models/donation_goal.ex +++ b/web/models/donation_goal.ex @@ -30,6 +30,7 @@ defmodule CodeCorps.DonationGoal do struct |> cast(params, [:amount, :description, :project_id]) |> validate_required([:amount, :description, :project_id]) + |> validate_number(:amount, greater_than: 0) |> assoc_constraint(:project) end diff --git a/web/models/project.ex b/web/models/project.ex index a57532099..bdf63c6dd 100644 --- a/web/models/project.ex +++ b/web/models/project.ex @@ -13,6 +13,8 @@ defmodule CodeCorps.Project do alias CodeCorps.Services.MarkdownRendererService alias CodeCorps.TaskList + alias Ecto.Changeset + @type t :: %__MODULE__{} schema "projects" do diff --git a/web/models/user.ex b/web/models/user.ex index cd9ec0797..969ef4e55 100644 --- a/web/models/user.ex +++ b/web/models/user.ex @@ -30,10 +30,12 @@ defmodule CodeCorps.User do field :twitter, :string field :username, :string field :website, :string - field :github_id, :string field :state, :string, default: "signed_up" field :state_transition, :string, virtual: true + field :github_id, :string + field :github_auth_token, :string + has_one :slugged_route, SluggedRoute has_many :project_users, CodeCorps.ProjectUser @@ -95,10 +97,10 @@ defmodule CodeCorps.User do |> apply_state_transition(struct) end - def github_associate_changeset(struct, params) do + def github_association_changeset(struct, params) do struct - |> cast(params, [:github_id]) - |> validate_required([:github_id]) + |> cast(params, [:github_auth_token]) + |> validate_required([:github_auth_token]) end def reset_password_changeset(struct, params) do diff --git a/web/router.ex b/web/router.ex index 5613e99a2..4db1f1fde 100644 --- a/web/router.ex +++ b/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]