diff --git a/lib/algora/integrations/github/github.ex b/lib/algora/integrations/github/github.ex index e2bae769c..dcfb0dcb6 100644 --- a/lib/algora/integrations/github/github.ex +++ b/lib/algora/integrations/github/github.ex @@ -17,7 +17,18 @@ defmodule Algora.Github do "https://github.com/apps/#{app_handle()}/installations/new" end - def authorize_url(_return_to \\ nil) do + defp oauth_state_ttl, do: 600 + defp oauth_state_salt, do: "github-oauth-state" + + def generate_oauth_state(data) do + Phoenix.Token.sign(AlgoraWeb.Endpoint, oauth_state_salt(), data, max_age: oauth_state_ttl()) + end + + def verify_oauth_state(token) do + Phoenix.Token.verify(AlgoraWeb.Endpoint, oauth_state_salt(), token, max_age: oauth_state_ttl()) + end + + def authorize_url(data \\ nil) do # TODO: find out a way to set return_to w/o github complaining about redirect_uri mismatch # redirect_query = if return_to, do: URI.encode_query(return_to: return_to) @@ -28,7 +39,7 @@ defmodule Algora.Github do query = URI.encode_query( client_id: client_id(), - state: Algora.Util.random_string(), + state: generate_oauth_state(data), redirect_uri: redirect_uri ) diff --git a/lib/algora_web/controllers/oauth_callback_controller.ex b/lib/algora_web/controllers/oauth_callback_controller.ex index 6365a721b..9ecfb6a9c 100644 --- a/lib/algora_web/controllers/oauth_callback_controller.ex +++ b/lib/algora_web/controllers/oauth_callback_controller.ex @@ -15,7 +15,8 @@ defmodule AlgoraWeb.OAuthCallbackController do end def new(conn, %{"provider" => "github", "code" => code, "state" => state} = params) do - with {:ok, info} <- Github.OAuth.exchange_access_token(code: code, state: state), + with {:ok, _data} <- Github.verify_oauth_state(state), + {:ok, info} <- Github.OAuth.exchange_access_token(code: code, state: state), %{info: info, primary_email: primary, emails: emails, token: token} = info, {:ok, user} <- Accounts.register_github_user(primary, info, emails, token) do conn = @@ -29,14 +30,21 @@ defmodule AlgoraWeb.OAuthCallbackController do |> put_flash(:info, welcome_message(user)) |> AlgoraWeb.UserAuth.log_in_user(user) else + {:error, :invalid} -> + conn + |> put_flash(:error, "Unable to verify your login request. Please try signing in again.") + |> redirect(to: "/") + + {:error, :expired} -> + conn + |> put_flash(:error, "Your login link has expired. Please request a new one to continue.") + |> redirect(to: "/") + {:error, %Ecto.Changeset{} = changeset} -> Logger.debug("failed GitHub insert #{inspect(changeset.errors)}") conn - |> put_flash( - :error, - "We were unable to fetch the necessary information from your GitHub account" - ) + |> put_flash(:error, "We were unable to fetch the necessary information from your GitHub account") |> redirect(to: "/") {:error, reason} ->