security: verify GitHub OAuth state (#24)

Author: zcesur

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
       )
 

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} ->