From f79236e823a086d3c3181b2cc06788f33f5bd09d Mon Sep 17 00:00:00 2001 From: Brian Peiris Date: Tue, 13 Jul 2021 16:59:02 -0400 Subject: [PATCH 1/2] Fix link redirects --- lib/ret/hub.ex | 13 ++++++-- lib/ret_web/controllers/page_controller.ex | 3 -- lib/ret_web/router.ex | 23 ++++++++++++++ .../controllers/page_controller_test.exs | 30 +++++++++++++++---- 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/lib/ret/hub.ex b/lib/ret/hub.ex index b6d0b8f2c..066d118bb 100644 --- a/lib/ret/hub.ex +++ b/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 diff --git a/lib/ret_web/controllers/page_controller.ex b/lib/ret_web/controllers/page_controller.ex index dddb0f0ba..85717874c 100644 --- a/lib/ret_web/controllers/page_controller.ex +++ b/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 diff --git a/lib/ret_web/router.ex b/lib/ret_web/router.ex index 7a3ac49ef..a1f9e3ca5 100644 --- a/lib/ret_web/router.ex +++ b/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,25 @@ 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]) + 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] ++ diff --git a/test/ret_web/controllers/page_controller_test.exs b/test/ret_web/controllers/page_controller_test.exs index 4b71a1ffe..655048878 100644 --- a/test/ret_web/controllers/page_controller_test.exs +++ b/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 From cc36c77be9a3b7f6bbbf58ef3ad54b33b115ad7c Mon Sep 17 00:00:00 2001 From: Brian Peiris Date: Tue, 13 Jul 2021 17:12:39 -0400 Subject: [PATCH 2/2] fix lint warnings --- lib/ret_web/router.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ret_web/router.ex b/lib/ret_web/router.ex index a1f9e3ca5..9332f5189 100644 --- a/lib/ret_web/router.ex +++ b/lib/ret_web/router.ex @@ -203,7 +203,6 @@ defmodule RetWeb.Router do if(Mix.env() == :prod, do: [:ssl_only, :canonicalize_domain], else: []) ) - get("/link/", PageController, only: [:index]) get("/link", PageController, only: [:index]) end