analytics icon indicating copy to clipboard operation
analytics copied to clipboard

Draft: Implement OIDC

Open lorenz opened this issue 2 years ago • 15 comments

Changes

This is an initial implementation of OIDC. It's still pretty basic, but it does log in users via OIDC. Automated user creation is not yet implemented, as is automatically assigning sites based on roles.

Mostly posted to gather some early feedback and for others to try.

Fixes #1554

Tests

  • [ ] Automated tests have been added
  • [ ] This PR does not require tests

Changelog

  • [ ] Entry has been added to changelog
  • [ ] This PR does not make a user-facing change

Documentation

  • [ ] Docs have been updated
  • [ ] This change does not need a documentation update

Dark mode

  • [ ] The UI has been tested both in dark and light mode
  • [x] This PR does not change the UI

lorenz avatar Jul 21 '23 12:07 lorenz

BundleMon

Unchanged files (7)
Status Path Size Limits
:white_check_mark: static/css/app.css
492.34KB -
:white_check_mark: static/js/dashboard.js
318.55KB -
:white_check_mark: static/js/app.js
40.1KB -
:white_check_mark: static/js/embed.host.js
5.58KB -
:white_check_mark: static/js/embed.content.js
5.08KB -
:white_check_mark: tracker/js/plausible.js
742B -
:white_check_mark: static/js/applyTheme.js
314B -

No change in files bundle size

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] avatar Jul 21 '23 12:07 bundlemon[bot]

Still working on this?

jannismilz avatar Oct 14 '23 14:10 jannismilz

@jannismilz I am currently using this as-is, as I said in the PR I wanted to get some feedback from the developers on the general approach before investing more time into it. What features do you need?

lorenz avatar Oct 15 '23 14:10 lorenz

@lorenz Tbh I don't have specific needs I was just looking for a SSO login option. The method doesn't matter too much since I can use a IdP to handle that right?

jannismilz avatar Oct 23 '23 07:10 jannismilz

I might be interested in this. I'm evaluating Plausible and having OIDC would be one selling point. I have two questions:

  • Can you resolve conflicts with the main branch? It would be easier to build and try.
  • As far as I understood, with OIDC_IDP configured properly, I still need to manually create users in Plausible but then my co-workers can simply log in with our SSO page. Is that correct? Since I manually create the users and I want to give a feeling that every is handled by our OIDC provider, would you recommend removing Plausible email verification?

relu91 avatar Oct 27 '23 07:10 relu91

I might be interested in this. I'm evaluating Plausible and having OIDC would be one selling point. I have two questions:

  • Can you resolve conflicts with the main branch? It would be easier to build and try.

I'l look into it today or sunday.

  • As far as I understood, with OIDC_IDP configured properly, I still need to manually create users in Plausible but then my co-workers can simply log in with our SSO page. Is that correct? Since I manually create the users and I want to give a feeling that every is handled by our OIDC provider, would you recommend removing Plausible email verification?

Currently yes, but it should be relatively easy to automatically create users from OIDC userinfo. Basically instead of bailing when the user is not found, it needs to call Plausible.Auth.User.new/Repo.insert.

lorenz avatar Oct 27 '23 13:10 lorenz

Tried a image built in this branch but after initialization, it crashed:

warning: :logger :level has been set to :warn in config files, please use :warning instead
  (logger 1.15.7) lib/logger/app.ex:102: Logger.App.default_level/0
  (logger 1.15.7) lib/logger/app.ex:35: Logger.App.start/2
  (kernel 9.1) application_master.erl:293: :application_master.start_it_old/4
Kernel pid terminated (application_controller) ("{application_start_failure,plausible,{{shutdown,{failed_to_start_child,'Elixir.OpenIDConnect.Worker',{#{value => nil,protocol => 'Elixir.Enumerable',description => <<>>,'__struct__' => 'Elixir.Protocol.UndefinedError','__exception__' => true},[{'Elixir.Enumerable','impl_for!',1,[{file,\"lib/enum.ex\"},{line,1}]},{'Elixir.Enumerable',reduce,3,[{file,\"lib/enum.ex\"},{line,166}]},{'Elixir.Enum',map,2,[{file,\"lib/enum.ex\"},{line,4387}]},{'Elixir.Enum',into,3,[{file,\"lib/enum.ex\"},{line,1587}]},{'Elixir.OpenIDConnect.Worker',init,1,[{file,\"lib/openid_connect/worker.ex\"},{line,22}]},{gen_server,init_it,2,[{file,\"gen_server.erl\"},{line,962}]},{gen_server,init_it,6,[{file,\"gen_server.erl\"},{line,917}]},{proc_lib,init_p_do_apply,3,[{file,\"proc_lib.erl\"},{line,241}]}]}}},{'Elixir.Plausible.Application',start,[normal,[]]}}}")
Crash dump is being written to: erl_crash.dump...

Sadly, I can't easily get erl_crash.dump, I hope that that log is enough to understand the problem.

relu91 avatar Nov 20 '23 14:11 relu91

Is this with an OIDC provider set up? I think I see why it crashes if you don't. I'll get to fixing that at some point.

lorenz avatar Nov 20 '23 16:11 lorenz

@lorenz @relu91 👋

The error means that the worker is started with provider_configs = nil, and since they are fetched from :openid_connect_providers app env on startup, I'd guess use_oidc = oidc_idp != "" evaluates to false in runtime.exs which means that OIDC_IDP env var was not set.

ruslandoga avatar Nov 21 '23 05:11 ruslandoga

Ops my bad, I assumed the configuration was optional and I could add it later. Ok now everything starts but when I try to log in I get some good requests back and forth with our idp and then a 500 error from plausible. Here's the exeception raised in the log:

2023-11-21T11:57:53.500174999Z Request: GET /oidc/callback?code=<Code here>
2023-11-21T11:57:53.500179897Z ** (exit) an exception was raised:
2023-11-21T11:57:53.500183881Z     ** (ArgumentError) comparison with nil is forbidden as it is unsafe. If you want to check if a value is nil, use is_nil/1 instead
2023-11-21T11:57:53.500188081Z         (ecto 3.10.3) lib/ecto/query/builder.ex:1048: Ecto.Query.Builder.not_nil!/1
2023-11-21T11:57:53.500192285Z         (plausible 0.0.1) lib/plausible_web/controllers/auth_controller.ex:279: PlausibleWeb.AuthController.find_user/1
2023-11-21T11:57:53.500196544Z         (plausible 0.0.1) lib/plausible_web/controllers/auth_controller.ex:539: PlausibleWeb.AuthController.oidc_callback/2
2023-11-21T11:57:53.500216749Z         (plausible 0.0.1) lib/plausible_web/controllers/auth_controller.ex:1: PlausibleWeb.AuthController.action/2
2023-11-21T11:57:53.500221590Z         (plausible 0.0.1) lib/plausible_web/controllers/auth_controller.ex:1: PlausibleWeb.AuthController.phoenix_controller_pipeline/2
2023-11-21T11:57:53.500225910Z         (phoenix 1.7.7) lib/phoenix/router.ex:430: Phoenix.Router.__call__/5
2023-11-21T11:57:53.500229946Z         (plausible 0.0.1) lib/plausible_web/endpoint.ex:1: PlausibleWeb.Endpoint.plug_builder_call/2
2023-11-21T11:57:53.500234007Z         (plausible 0.0.1) lib/plausible_web/endpoint.ex:1: PlausibleWeb.Endpoint."call (overridable 3)"/2

Did I configure our IDP in the wrong way?

relu91 avatar Nov 21 '23 14:11 relu91

The error means that email is nil here. And that it shouldn't be. It's nil probably because claims doesn't contain "email" field.

I don't know anything about OIDC but from just looking at the code, the IDP didn't respond with all the required scopes. In particular, email is missing.

ruslandoga avatar Nov 21 '23 15:11 ruslandoga

The error means that email is nil here. And that it shouldn't be. It's nil probably because claims doesn't contain "email" field.

I don't know anything about OIDC but from just looking at the code, the IDP didn't respond with all the required scopes. In particular, email is missing.

exactly the problem was that zitadel (our idp) didn't send the user details by the defult in the token. You have to manually enable it in the project dashboard. Works like a charm now! The only thing left for me is user creation. I'm struggling to find a sort of admin dashboard where I can manually create users. Is it there? meanwhile I'll try to navigate the docs.

relu91 avatar Nov 21 '23 17:11 relu91

I don't think there is an admin dashboard like that.

ruslandoga avatar Nov 21 '23 18:11 ruslandoga

Update

I was able to get users to automatically register when using our IDP. As you might have guessed I've never coded in Elixir, so I had to kinda guess how it works (plus a little help from chatGPT - to be honest this time it got me sideway multiple times). For reference this is how the function coded by @lorenz looks now:

  def oidc_callback(conn, %{"code" => code}) do
    if !Application.get_env(:plausible, :use_oidc) do
      render_error(
          conn,
          400,
          "OIDC is not active"
        )
    else
      with {:ok, tokens} <- OpenIDConnect.fetch_tokens(:default, %{code: code}),
          {:ok, claims} <- OpenIDConnect.verify(:default, tokens["id_token"]),
          {{:ok, user}, claims} <- {find_user(claims["email"]), claims} do
            login_dest = get_session(conn, :login_dest) || Routes.site_path(conn, :index)

            conn
            |> put_session(:current_user_id, user.id)
            |> put_resp_cookie("logged_in", "true",
              http_only: false,
              max_age: 60 * 60 * 24
            )
            |> put_session(:login_dest, nil)
            |> redirect(to: login_dest)
      else
        result ->
          case result do
            {:user_not_found, claims} ->
              IO.inspect(claims)
              with {:ok, user} <- Plausible.Auth.create_user(claims["given_name"],claims["email"],"cP943<@Tti'B") do
                login_dest = get_session(conn, :login_dest) || Routes.site_path(conn, :index)
                conn
                  |> put_session(:current_user_id, user.id)
                  |> put_resp_cookie("logged_in", "true",
                    http_only: false,
                    max_age: 60 * 60 * 24
                  )
                  |> put_session(:login_dest, nil)
                  |> redirect(to: login_dest)
              else
                e -> render_error(
                  conn,
                  400,
                  "Error creating the user: #{inspect(e)}"
                )
              end
            {e,_} -> render_error(
              conn,
              400,
              "OIDC login failed: #{inspect(e)}"
            )
            e -> render_error(
              conn,
              400,
              "OIDC login failed: #{inspect(e)}"
            )
          end
      end

It is kinda ugly and it has duplicated code, but it works for our use case. I would say that before going to production what I would like to see is:

  • UI coherence: the landing page of plausible still contains the register link which does not really make sense when you have the IDP enabled.
  • Ability to restrict the users that can use plausible. This can achieved by looking for a specific role or something else?
  • Ability to disable Add website button to some users. Maybe this is already a feature but I couldn't find it in the doc.

relu91 avatar Nov 22 '23 15:11 relu91

We're considering adopting Plausible as the analytics tool for a non-profit website (with 10+ millions visits monthly), but having SSO is a pretty strong requirement for us.

We use keycloak to manage user permissions across all the tools we use, and having this feature being considered would make it much easier for the board to accept Plausible as the tool of choice

xavivars avatar Dec 24 '23 17:12 xavivars

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Mar 22 '24 14:03 CLAassistant

I already closed the other OIDC pull request, will do the same with this one. See https://github.com/plausible/analytics/pull/3706#issuecomment-2019761745 for reasoning. In short: we consider team accounts as a blocker for SSO

ukutaht avatar Apr 23 '24 08:04 ukutaht

For everyone's clarity - does that mean that the features that enable Self-Hosted users to run CE and use their own IdP aren't being merged at all, or aren't being merged yet? Critically, I'd like to know I can self-host and use SSO with my own provider - even if I do have to make use of Traefik and headers to do so.

vai avatar Jun 02 '24 18:06 vai