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

CloudFlare Zero Trust authentication #1938

Merged
merged 19 commits into from
Jun 20, 2023

Conversation

cristineguadelupe
Copy link
Contributor

This PR adds support for CloudFlare Zero Trust authentication

  • The keys are rotated every 7 days, so I'm not storing them
  • In case of error, I'm showing the authentication screen without the input and the submit
  • The email already comes in the token and we are using it as display name

lib/livebook.ex Outdated Show resolved Hide resolved
lib/livebook/zti/keys.ex Outdated Show resolved Hide resolved
lib/livebook/zti/keys.ex Outdated Show resolved Hide resolved
lib/livebook/zti/keys.ex Outdated Show resolved Hide resolved
@josevalim
Copy link
Contributor

Hi @cristineguadelupe!

Just to recap, I think API wise there are two levels:

  1. Livebook.ZTA

  2. The plug and configuration API which is exclusive to Livebook

Livebook.ZTA will be a collection of modules for each provider. So for example, we will have Livebook.ZTA.Cloudflare, which we will mount in our supervision tree:

{Livebook.ZTA.Cloudflare, name: MyLivebookZTA, other: ..., options: ...}

We will add this to our supervision tree based on the value of LIVEBOOK_IDENTITY_PROVIDER.

Then LivebookWeb.UserPlug will check if LIVEBOOK_IDENTITY_PROVIDER and act accordingly. If "cookies", same as today, if "cloudflare", you will call Livebook.ZTA.Cloudflare.authenticate(MyLivebookZTA, conn) which will return :ok or :error.

In other words, Livebook.ZTA will contain the service and the authentication/request APIs. The plug, application environment, are part of Livebook.

README.md Outdated Show resolved Hide resolved
mix.exs Outdated Show resolved Hide resolved
lib/livebook/config.ex Outdated Show resolved Hide resolved
@cristineguadelupe cristineguadelupe marked this pull request as ready for review June 19, 2023 15:43
mix.exs Show resolved Hide resolved
config/config.exs Outdated Show resolved Hide resolved
lib/livebook/config.ex Outdated Show resolved Hide resolved
Copy link
Contributor

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two nitpicks and ship it!

@github-actions
Copy link

github-actions bot commented Jun 20, 2023

Uffizzi Preview deployment-29059 was deleted.

@jonatanklosko jonatanklosko merged commit efb28fb into livebook-dev:main Jun 20, 2023
7 checks passed

def authenticate(name, conn) do
token = get_req_header(conn, @assertion)
GenServer.call(name, {:authenticate, token})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our GenServer stores things in ets, but we never access them in this function, in the caller. Thus I believe ets is an overkill and we might as well store things in server state. To take full advantage of ets and avoid a single genserver bottleneck, I believe we want something like this:

def authenticate(name, conn) do
  token = Plug.Conn.get_req_header(conn, @assertion)
  do_authenticate(name, token) || GenServer.call(name, {:authenticate, token})
end

where both do_authenticate and handle_call({:authenticate fetches things from ets.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find, added to #1941. Using ets also means the table has to be named, which means we need to require the :name option.

@josevalim josevalim mentioned this pull request Jun 21, 2023
3 tasks
hkrutzer referenced this pull request Oct 9, 2023
Co-authored-by: Alexandre de Souza <alexandre@aledsz.com.br>
@cristineguadelupe cristineguadelupe deleted the cg-cloudflare branch November 13, 2023 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants