Merge pull request #511 from mozilla/bug/link-redirect

Fix link redirects

lib/ret/hub.ex

@@ -356,8 +356,17 @@ defmodule Ret.Hub do
 
   def get_by_entry_code_string(entry_code_string) when is_binary(entry_code_string) do
     case Integer.parse(entry_code_string) do
-      {entry_code, _} -> Hub |> Repo.get_by(entry_code: entry_code)
-      _ -> nil
+      {entry_code, _} ->
+        hub = Hub |> Repo.get_by(entry_code: entry_code)
+
+        cond do
+          is_nil(hub) -> nil
+          hub |> Hub.entry_code_expired?() -> nil
+          true -> hub
+        end
+
+      _ ->
+        nil
     end
   end
 

lib/ret_web/controllers/page_controller.ex

@@ -514,9 +514,6 @@ defmodule RetWeb.PageController do
 
   # Redirect to the specified hub identifier, which can be a sid or an entry code
   defp redirect_to_hub_identifier(conn, hub_identifier) do
-    # Rate limit requests for redirects.
-    :timer.sleep(500)
-
     hub = Repo.get_by(Hub, hub_sid: hub_identifier) || Hub.get_by_entry_code_string(hub_identifier)
 
     case hub do

lib/ret_web/router.ex

@@ -12,6 +12,10 @@ defmodule RetWeb.Router do
     plug(Plug.SSL, hsts: true, rewrite_on: [:x_forwarded_proto])
   end
 
+  pipeline :rate_limit do
+    plug(RetWeb.Plugs.RateLimit)
+  end
+
   pipeline :parsed_body do
     plug(
       Plug.Parsers,
@@ -193,6 +197,24 @@ defmodule RetWeb.Router do
     get("/files/:id", FileController, :show)
   end
 
+  scope "/", RetWeb do
+    pipe_through(
+      [:secure_headers, :parsed_body, :browser] ++
+        if(Mix.env() == :prod, do: [:ssl_only, :canonicalize_domain], else: [])
+    )
+
+    get("/link", PageController, only: [:index])
+  end
+
+  scope "/", RetWeb do
+    pipe_through(
+      [:secure_headers, :parsed_body, :browser, :rate_limit] ++
+        if(Mix.env() == :prod, do: [:ssl_only, :canonicalize_domain], else: [])
+    )
+
+    get("/link/*path", PageController, only: [:index])
+  end
+
   scope "/", RetWeb do
     pipe_through(
       [:secure_headers, :parsed_body, :browser] ++

test/ret_web/controllers/page_controller_test.exs

@@ -1,9 +1,29 @@
 defmodule RetWeb.PageControllerTest do
   use RetWeb.ConnCase
 
-  # TODO, fix, this has HTTP auth behind it now
-  # test "GET /", %{conn: conn} do
-  #  conn = get conn, "/"
-  #  assert html_response(conn, 200) =~ "Sign in with Google"
-  # end
+  test "redirect to non-existent entry code", %{conn: conn} do
+    resp = conn |> get("/link/123456")
+    assert resp |> response(404)
+  end
+
+  test "redirect with existing entry code", %{conn: conn} do
+    {:ok, hub} = Ret.Hub.create_new_room(%{"name" => "test hub"}, true)
+    resp = conn |> get("/link/#{hub.entry_code}")
+    assert resp |> response(302)
+  end
+
+  test "redirect fails with expired entry code", %{conn: conn} do
+    {:ok, hub} = Ret.Hub.create_new_room(%{"name" => "test hub"}, true)
+
+    hub
+    |> Ecto.Changeset.change(
+      entry_code_expires_at:
+        Timex.now()
+        |> DateTime.truncate(:second)
+    )
+    |> Ret.Repo.update!()
+
+    resp = conn |> get("/link/#{hub.entry_code}")
+    assert resp |> response(404)
+  end
 end