Skip to content

Commit

Permalink
Merge pull request #1881 from Logflare/fix/switch-teams-prompting-403
Browse files Browse the repository at this point in the history
fix: prompt user to switch teams on 403
  • Loading branch information
Ziinc authored Dec 12, 2023
2 parents d11a506 + cacfbcd commit 9a50c18
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 37 deletions.
16 changes: 16 additions & 0 deletions assets/css/dashboard.scss
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,23 @@
background-color: #1d1d1d;
border: 0;
overflow: hidden;
&.active {
background-color: $brand-black;

}
}

.badge {
&.badge-primary {
background-color: $brand-fuscia;
color: $brand-black;
}
&.badge-secondary {
background-color: $brand-light-gray;
color: $brand-black;
}
}

/* Fixes artifacts in Chrome */
.list-group-item:focus,
.list-group-item:hover {
Expand Down
11 changes: 6 additions & 5 deletions lib/logflare/teams.ex
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,13 @@ defmodule Logflare.Teams do
|> Repo.one()
end

defp query_teams_by_user_access(%User{email: email, id: id}) do
defp query_teams_by_user_access(%User{id: id, provider_uid: uid}) do
from t in Team,
join: tu in TeamUser,
on: tu.email == ^email,
where: t.user_id == ^id or tu.email == ^email,
left_join: tu in TeamUser,
on: t.id == tu.team_id,
where: t.user_id == ^id or tu.provider_uid == ^uid,
distinct: true,
preload: [:user, :team_users]
preload: [:user, :team_users],
select: t
end
end
3 changes: 3 additions & 0 deletions lib/logflare_web/controllers/plugs/set_team.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ defmodule LogflareWeb.Plugs.SetTeam do
Teams.get_team_by(user_id: user.id)
|> Teams.preload_team_users()

teams = Teams.list_teams_by_user_access(user)

conn
|> assign(:team, team)
|> assign(:teams, teams)
end
end
2 changes: 1 addition & 1 deletion lib/logflare_web/controllers/plugs/set_verify_source.ex
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ defmodule LogflareWeb.Plugs.SetVerifySource do
set_source_for_public(public_token, conn, opts)
end

def call(%{assigns: %{user: user}, params: params} = conn, _opts) do
def call(%{assigns: %{user: user, teams: _teams}, params: params} = conn, _opts) do
id = params["source_id"] || params["id"]
source = Sources.get_by_and_preload(id: id)

Expand Down
37 changes: 23 additions & 14 deletions lib/logflare_web/controllers/team_user_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,44 @@ defmodule LogflareWeb.TeamUserController do
end
end

def change_team(%{assigns: %{team_user: _team_user, user: _user}} = conn, %{
"user_id" => user_id,
"team_user_id" => team_user_id
}) do
def change_team(
%{assigns: %{team_user: _team_user, user: _user}} = conn,
%{
"user_id" => user_id,
"team_user_id" => team_user_id
} = params
) do
conn
|> put_session(:user_id, user_id)
|> put_session(:team_user_id, team_user_id)
|> put_resp_cookie("_logflare_user_id", user_id, max_age: 2_592_000)
|> put_resp_cookie("_logflare_team_user_id", team_user_id, max_age: 2_592_000)
|> put_flash(:info, "Welcome to this Logflare team!")
|> redirect(to: Routes.source_path(conn, :dashboard))
|> redirect(to: Map.get(params, "redirect_to", ~p"/dashboard"))
end

def change_team(%{assigns: %{team_user: _team_user, user: _user}} = conn, %{
"user_id" => user_id
}) do
def change_team(
%{assigns: %{team_user: _team_user, user: _user}} = conn,
%{
"user_id" => user_id
} = params
) do
conn
|> put_session(:user_id, user_id)
|> delete_session(:team_user_id)
|> delete_resp_cookie("_logflare_user_id")
|> delete_resp_cookie("_logflare_team_user_id")
|> put_flash(:info, "Welcome to this Logflare team!")
|> redirect(to: Routes.source_path(conn, :dashboard))
|> redirect(to: Map.get(params, "redirect_to", ~p"/dashboard"))
end

def change_team(%{assigns: %{user: user}} = conn, %{
"user_id" => user_id,
"team_user_id" => team_user_id
}) do
def change_team(
%{assigns: %{user: user}} = conn,
%{
"user_id" => user_id,
"team_user_id" => team_user_id
} = params
) do
{:ok, _team_user} = TeamUsers.update_team_user_on_change_team(user, team_user_id)

conn
Expand All @@ -102,6 +111,6 @@ defmodule LogflareWeb.TeamUserController do
|> put_resp_cookie("_logflare_user_id", user_id, max_age: 2_592_000)
|> put_resp_cookie("_logflare_team_user_id", team_user_id, max_age: 2_592_000)
|> put_flash(:info, "Welcome to this Logflare team!")
|> redirect(to: Routes.source_path(conn, :dashboard))
|> redirect(to: Map.get(params, "redirect_to", ~p"/dashboard"))
end
end
8 changes: 0 additions & 8 deletions lib/logflare_web/templates/error/403_page.html.eex

This file was deleted.

29 changes: 29 additions & 0 deletions lib/logflare_web/templates/error/403_page.html.heex
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<%= render_in "error_container.html", assigns do %>
<div class="col-lg-7">
<h1>403</h1>
<h2 class="text-white mb-3">Forbidden</h2>
<p>You may need to switch teams to access this resource</p>
<ul class="list-group">
<li
:for={t <- assigns[:teams] || []}
class={[
"list-group-item d-flex justify-content-between align-items-center",
if(t.id == @team.id, do: "active"),
"tw-mx-auto tw-w-96"
]}
>
<%= link(t.name,
to:
~p"/profile/switch?#{cond do
t.user_id == @user.id -> %{user_id: t.user_id, redirect_to: @conn.request_path}
true -> tu = Enum.find(t.team_users, &(&1.provider_uid == @user.provider_uid))
%{user_id: t.user_id, team_user_id: tu.id, redirect_to: @conn.request_path}
end}"
) %>
<small :if={t.id == @team.id} class="badge badge-secondary badge-pill">
Current
</small>
</li>
</ul>
</div>
<% end %>
12 changes: 6 additions & 6 deletions test/logflare/teams_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,21 @@ defmodule Logflare.TeamsTest do
end

test "list_teams_by_user_access/1 lists all teams of a given user" do
insert(:team, user: build(:user))
user = insert(:user)
teams = insert_list(2, :team_user, email: user.email)
insert_list(2, :team_user, provider_uid: user.provider_uid)

expected = Enum.map(teams, & &1.team.id) |> Enum.sort()
result = Enum.map(Teams.list_teams_by_user_access(user), & &1.id) |> Enum.sort()

assert expected == result
# 2 items, :team_users preloaded
assert [%{team_users: [_|_]}, _] = Teams.list_teams_by_user_access(user)
end

test "get_team_by_user_access/2 gets a team for a given user and token" do
user = insert(:user)
team = insert(:team)
_team_user = insert(:team_user, email: user.email, team: team)
_team_user = insert(:team_user, provider_uid: user.provider_uid, team: team)
insert_list(2, :team_user)

assert team.id == Teams.get_team_by_user_access(user, team.token).id

end
end
4 changes: 2 additions & 2 deletions test/logflare_web/controllers/api/team_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ defmodule LogflareWeb.Api.TeamControllerTest do
main_team = insert(:team, user: user)

another_user_team_with_main_user = insert(:team, user: insert(:user))
insert(:team_user, team: another_user_team_with_main_user, email: user.email)
insert(:team_user, team: another_user_team_with_main_user, provider_uid: user.provider_uid)

_non_relevant_to_main_user = insert(:team_user, team: main_team, email: insert(:user).email)
_non_relevant_to_main_user = insert(:team_user, team: main_team, provider_uid: insert(:user).provider_uid)

Counters.start_link()

Expand Down
33 changes: 33 additions & 0 deletions test/logflare_web/controllers/source_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ defmodule LogflareWeb.SourceControllerTest do
alias Logflare.Logs.RejectedLogEvents
alias Logflare.SingleTenant

setup do

Logflare.Sources.Counters
|> stub(:get_inserts, fn _token -> {:ok, 123} end)
|> stub(:get_bq_inserts, fn _token -> {:ok, 456} end)
:ok
end
describe "list" do
setup %{conn: conn} do
Logflare.Sources.Counters
Expand Down Expand Up @@ -91,6 +98,32 @@ defmodule LogflareWeb.SourceControllerTest do
assert html =~ "403"
assert html =~ "Forbidden"
end

end

test "prompt to switch team if not found and part of many teams", %{conn: conn} do
hidden_team = insert(:team, user: build(:user))

insert(:plan)
user = insert(:user)
source = insert(:source, user: user)
main_team = insert(:team, user: user)

other_user = insert(:user)
other_team = insert(:team, user: other_user)
insert(:team_user, team: main_team, provider_uid: other_user.provider_uid)

# main team has 2 users now
html = conn
|> login_user(other_user)
|> get(~p"/sources/#{source.id}")
|> html_response(403)

assert html =~ other_team.name
assert html =~ main_team.name
refute html =~ hidden_team.name
assert html =~ "You may need to switch teams"

end

describe "Premium only features" do
Expand Down
2 changes: 1 addition & 1 deletion test/support/factory.ex
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ defmodule Logflare.Factory do
bigquery_processed_bytes_limit: 10_000_000_000,
token: TestUtils.random_string(64),
api_key: TestUtils.random_string(10),
provider_uid: "provider_uid",
provider_uid: "provider_uid_#{TestUtils.random_string()}",
bigquery_udfs_hash: ""
}
end
Expand Down

0 comments on commit 9a50c18

Please sign in to comment.