Skip to content

Commit

Permalink
Apply review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
aleDsz committed Jan 7, 2025
1 parent 0052293 commit f6c59fa
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 65 deletions.
6 changes: 2 additions & 4 deletions lib/livebook/config.ex
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
defmodule Livebook.Config do
alias Livebook.FileSystem

@type authentication_mode :: :token | :password | :disabled

@type authentication ::
%{mode: :password, secret: String.t()}
| %{mode: :token, secret: String.t()}
Expand Down Expand Up @@ -68,7 +66,7 @@ defmodule Livebook.Config do
@doc """
Returns the authentication configuration.
"""
@spec authentication() :: authentication_mode()
@spec authentication() :: authentication()
def authentication() do
case Application.fetch_env!(:livebook, :authentication) do
{:password, password} -> %{mode: :password, secret: password}
Expand Down Expand Up @@ -276,7 +274,7 @@ defmodule Livebook.Config do
@spec logout_enabled?() :: boolean()
def logout_enabled?() do
{_type, module, _key} = Livebook.Config.identity_provider()
authentication().mode != :disabled or module.logout_supported?()
Code.ensure_loaded?(module) and function_exported?(module, :logout, 2)
end

@doc """
Expand Down
8 changes: 2 additions & 6 deletions lib/livebook/zta.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ defmodule Livebook.ZTA do
@doc """
Logouts against the given name.
"""
@callback logout(name(), Phoenix.LiveView.Socket.t()) :: :ok | :error | :unsupported

@doc """
Checks if the logout is supported against the given name.
"""
@callback logout_supported?() :: boolean()
@callback logout(name(), Phoenix.LiveView.Socket.t()) :: :ok | {:error, String.t()}
@optional_callbacks logout: 2

@doc false
def init do
Expand Down
6 changes: 0 additions & 6 deletions lib/livebook/zta/basic_auth.ex
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,4 @@ defmodule Livebook.ZTA.BasicAuth do
{conn, %{}}
end
end

@impl true
def logout(_name, _socket), do: raise("not implemented")

@impl true
def logout_supported?, do: false
end
6 changes: 0 additions & 6 deletions lib/livebook/zta/cloudflare.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ defmodule Livebook.ZTA.Cloudflare do
{conn, authenticate_user(token, identity, keys)}
end

@impl true
def logout(_name, _socket), do: raise("not implemented")

@impl true
def logout_supported?, do: false

@impl true
def init(options) do
state = struct!(__MODULE__, options)
Expand Down
6 changes: 0 additions & 6 deletions lib/livebook/zta/google_iap.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ defmodule Livebook.ZTA.GoogleIAP do
{conn, authenticate_user(token, identity, keys)}
end

@impl true
def logout(_name, _socket), do: raise("not implemented")

@impl true
def logout_supported?, do: false

@impl true
def init(options) do
state = struct!(__MODULE__, options)
Expand Down
6 changes: 2 additions & 4 deletions lib/livebook/zta/livebook_teams.ex
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,11 @@ defmodule Livebook.ZTA.LivebookTeams do

case Teams.Requests.logout_identity_provider(team, token) do
{:ok, _no_content} -> :ok
_otherwise -> :error
{:error, _} = error -> error
{:transport_error, reason} -> {:error, reason}
end
end

@impl true
def logout_supported?, do: true

defp handle_request(conn, team, %{"teams_identity" => _, "code" => code}) do
with {:ok, access_token} <- retrieve_access_token(team, code),
{:ok, metadata} <- get_user_info(team, access_token) do
Expand Down
6 changes: 0 additions & 6 deletions lib/livebook/zta/pass_through.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,4 @@ defmodule Livebook.ZTA.PassThrough do
def authenticate(_name, conn, _opts) do
{conn, %{}}
end

@impl true
def logout(_name, _socket), do: raise("not implemented")

@impl true
def logout_supported?, do: false
end
6 changes: 0 additions & 6 deletions lib/livebook/zta/tailscale.ex
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ defmodule Livebook.ZTA.Tailscale do
{conn, user}
end

@impl true
def logout(_name, _socket), do: raise("not implemented")

@impl true
def logout_supported?, do: false

defp authenticate_ip(remote_ip, address) do
{url, options} =
if String.starts_with?(address, "http") do
Expand Down
29 changes: 15 additions & 14 deletions lib/livebook_web/components/layout_components.ex
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ defmodule LivebookWeb.LayoutComponents do
to={~p"/settings"}
current={@current_page}
/>
<button
:if={Livebook.Config.logout_enabled?()}
class="h-7 flex items-center text-gray-400 hover:text-white border-l-4 border-transparent hover:border-white"
aria-label="logout"
phx-click="logout"
>
<.remix_icon
icon="logout-box-line"
class="text-lg leading-6 w-[56px] flex justify-center"
/>
<span class="text-sm font-medium">
Logout
</span>
</button>
</div>
<.hub_section hubs={@saved_hubs} current_page={@current_page} />
</div>
Expand All @@ -126,20 +140,7 @@ defmodule LivebookWeb.LayoutComponents do
Shut Down
</span>
</button>
<button
:if={@current_user.email}
class="h-7 flex items-center text-gray-400 hover:text-white border-l-4 border-transparent hover:border-white"
aria-label="logout"
phx-click="logout"
>
<.remix_icon
icon="logout-box-line"
class="text-lg leading-6 w-[56px] flex justify-center"
/>
<span class="text-sm font-medium">
Logout
</span>
</button>
<button
class="mt-6 flex items-center group border-l-4 border-transparent"
aria_label="user profile"
Expand Down
10 changes: 3 additions & 7 deletions lib/livebook_web/live/hooks/sidebar_hook.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,10 @@ defmodule LivebookWeb.SidebarHook do
case module.logout(LivebookWeb.ZTA, socket) do
:ok ->
Livebook.Users.unsubscribe(socket.assigns.current_user.id)
{:halt, redirect(socket, to: ~p"/logout")}

{:halt,
socket
|> assign(current_user: nil)
|> redirect(to: ~p"/logout")}

:error ->
{:cont, put_flash(socket, :error, Livebook.Teams.Requests.error_message())}
{:error, reason} ->
{:cont, put_flash(socket, :error, reason)}
end
end

Expand Down

0 comments on commit f6c59fa

Please sign in to comment.