Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix link redirects #511

Merged
merged 2 commits into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions lib/ret/hub.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 0 additions & 3 deletions lib/ret_web/controllers/page_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions lib/ret_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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] ++
Expand Down
30 changes: 25 additions & 5 deletions test/ret_web/controllers/page_controller_test.exs
Original file line number Diff line number Diff line change
@@ -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