From 651d037194b2204493fea5aa9c99b8be2bca6e88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Bara=C3=BAna?= Date: Tue, 19 Nov 2024 16:27:06 -0300 Subject: [PATCH] Bug fix: fix redirect_to URL when authenticating via livebook teams (#2857) --- lib/livebook/zta/livebook_teams.ex | 29 ++++++++--- .../zta/livebook_teams_test.exs | 52 +++++++++---------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/lib/livebook/zta/livebook_teams.ex b/lib/livebook/zta/livebook_teams.ex index 56355374f..80d3d3899 100644 --- a/lib/livebook/zta/livebook_teams.ex +++ b/lib/livebook/zta/livebook_teams.ex @@ -103,14 +103,31 @@ defmodule Livebook.ZTA.LivebookTeams do defp request_user_authentication(conn, team) do case Teams.Requests.create_auth_request(team) do {:ok, %{"authorize_uri" => authorize_uri}} -> - current_url = LivebookWeb.Endpoint.url() <> conn.request_path <> "?teams_identity" + # We have the browser do the redirect because the browser + # knows the current page location. Unfortunately, it is quite + # complex to know the actual host on the server, because the + # user may be running inside a proxy. So in order to make the + # feature more accessible, we do the redirecting on the client. + conn = + html(conn, """ + + + + + Redirecting... + + + + """) - {conn |> redirect(external: url) |> halt(), nil} + {halt(conn), nil} _ -> {conn diff --git a/test/livebook_teams/zta/livebook_teams_test.exs b/test/livebook_teams/zta/livebook_teams_test.exs index 01c5c7396..866161b90 100644 --- a/test/livebook_teams/zta/livebook_teams_test.exs +++ b/test/livebook_teams/zta/livebook_teams_test.exs @@ -1,7 +1,6 @@ defmodule Livebook.ZTA.LivebookTeamsTest do # Not async, because we alter global config (teams auth) use Livebook.TeamsIntegrationCase, async: false - use Plug.Test alias Livebook.ZTA.LivebookTeams @@ -19,50 +18,51 @@ defmodule Livebook.ZTA.LivebookTeamsTest do assert_receive {:agent_joined, %{hub_id: ^hub_id, org_id: ^org_id, deployment_group_id: ^deployment_group_id}} - {:ok, - deployment_group: deployment_group, team: team, opts: [name: test, identity_key: team.id]} + start_supervised!({LivebookTeams, name: test, identity_key: team.id}) + {:ok, deployment_group: deployment_group, team: team} end describe "authenticate/3" do - test "redirects the user to Livebook Teams for authentication", - %{conn: conn, test: test, opts: opts} do - start_supervised({LivebookTeams, opts}) - conn = Plug.Test.init_test_session(conn, %{}) - - assert {%{status: 302, halted: true}, nil} = LivebookTeams.authenticate(test, conn, []) + test "renders HTML with JavaScript redirect", %{conn: conn, test: test} do + conn = init_test_session(conn, %{}) + assert {conn, nil} = LivebookTeams.authenticate(test, conn, []) + assert conn.halted + assert html_response(conn, 200) =~ "window.location.href = " end test "gets the user information from Livebook Teams", - %{conn: conn, node: node, test: test, opts: opts} do - start_supervised({LivebookTeams, opts}) - conn = Plug.Test.init_test_session(conn, %{}) + %{conn: conn, node: node, test: test} do + # Step 1: Get redirected to Livebook Teams + conn = init_test_session(conn, %{}) {conn, nil} = LivebookTeams.authenticate(test, conn, []) - [location] = get_resp_header(conn, "location") + [_, location] = Regex.run(~r/URL\("(.*?)"\)/, html_response(conn, 200)) uri = URI.parse(location) assert uri.path == "/identity/authorize" - - redirect_to = LivebookWeb.Endpoint.url() <> "/?teams_identity" - assert %{"code" => code, "redirect_to" => ^redirect_to} = URI.decode_query(uri.query) + assert %{"code" => code} = URI.decode_query(uri.query) erpc_call(node, :allow_auth_request, [code]) + # Step 2: Emulate the redirect back with the code for validation conn = - conn(:get, "/", %{teams_identity: "", code: code}) - |> Plug.Test.init_test_session(%{}) + build_conn(:get, "/", %{teams_identity: "", code: code}) + |> init_test_session(%{}) assert {conn, %{id: _id, name: _, email: _, payload: %{"access_token" => _}} = metadata} = LivebookTeams.authenticate(test, conn, []) - assert conn.status == 302 - assert get_resp_header(conn, "location") == ["/"] + assert redirected_to(conn, 302) == "/" + + # Step 3: Confirm the token/metadata is valid for future requests + conn = + build_conn(:get, "/") + |> init_test_session(%{identity_data: metadata}) - conn = Plug.Test.init_test_session(conn(:get, "/"), %{identity_data: metadata}) assert {%{halted: false}, ^metadata} = LivebookTeams.authenticate(test, conn, []) end test "redirects to Livebook Teams with invalid access token", - %{conn: conn, test: test, opts: opts} do + %{conn: conn, test: test} do identity_data = %{ id: "11", name: "Ada Lovelace", @@ -70,10 +70,10 @@ defmodule Livebook.ZTA.LivebookTeamsTest do email: "user95387220@example.com" } - start_supervised({LivebookTeams, opts}) - conn = Plug.Test.init_test_session(conn, %{identity_data: identity_data}) - - assert {%{status: 302}, nil} = LivebookTeams.authenticate(test, conn, []) + conn = init_test_session(conn, %{identity_data: identity_data}) + assert {conn, nil} = LivebookTeams.authenticate(test, conn, []) + assert conn.halted + assert html_response(conn, 200) =~ "window.location.href = " end end end