ueberauth / ueberauth

An Elixir Authentication System for Plug-based Web Applications
MIT License
1.62k stars 120 forks source link

Latest release breaks existing paths #194

Closed wkirschbaum closed 10 months ago

wkirschbaum commented 11 months ago

Steps to Reproduce

On the user session controller we have a new action with a login screen, as well as the callback actions.,

Visiting http://localhost:4000/users/log_in crashes the page with the following error:

Ueberauth.NoProviderError at GET /users/log_in

Provider users/log_in was not found

Expected Result

The existing routes should not crash, even if it renders on the same controller as the callback action, or the changelog should specify this breaking change.

Actual Result

It crashes the existing routes.

FelixFortis commented 11 months ago

My provider is defined, but I am still getting the error in my test: Providers in IEx:

Ueberauth.init([otp_app: :my_app])
#=>
[
  {{"/auth/edm", "GET"},
   {Ueberauth.Strategy.EDM, :run_request,
    %{
      callback_methods: ["GET"],
      callback_params: nil,
      callback_path: "/auth/edm/callback",
      callback_port: nil,
      callback_scheme: nil,
      callback_url: nil,
      options: [],
      request_path: "/auth/edm",
      request_port: nil,
      request_scheme: nil,
      strategy: Ueberauth.Strategy.EDM,
      strategy_name: :edm
    }}},
  {{"/auth/edm/callback", "GET"},
   {Ueberauth.Strategy.EDM, :run_callback,
    %{
      callback_methods: ["GET"],
      callback_params: nil,
      callback_path: "/auth/edm/callback",
      callback_port: nil,
      callback_scheme: nil,
      callback_url: nil,
      options: [],
      request_path: "/auth/edm",
      request_port: nil,
      request_scheme: nil,
      strategy: Ueberauth.Strategy.EDM,
      strategy_name: :edm
    }}}
]

Test:

describe "logout/2" do
    test "clears the session and redirects to sign-in", %{conn: conn} do
      use_cassette "discovery_url" do
        conn =
          conn
          |> sign_in_user("user.admin@example.com")
          |> delete("/auth/edm")

        assert get_session(conn) == %{}

        assert redirected_to(conn, 302) ==
                 "https://localhost:4443/openid/v1/end_session?post_logout_redirect_uri=http%3A%2F%2Flocalhost%3A4002%2Fadmin"
      end
    end
  end

Failure:

  ** (Ueberauth.NoProviderError) Provider auth/edm was not found
     code: |> delete("/auth/edm")
     stacktrace:
       (ueberauth 0.10.6) lib/ueberauth.ex:303: Ueberauth.call/2
yordis commented 11 months ago

@wkirschbaum I am gonna retire the version, it was not intended to break changes. My apologies!

Related to https://github.com/ueberauth/ueberauth/pull/193

cc: @Hajto

yordis commented 11 months ago

I would like to add a test case to replicate the issue, but also,

It is tricky the existing situation about #193 and https://github.com/ueberauth/ueberauth/issues/173

It comes down to "what should Ueberauth expect people to do?", I am open to suggestions here.

yordis commented 11 months ago

https://github.com/ueberauth/ueberauth/pull/191 I should merge that one before accepting the PR, it was passing before!

😢 My apologies folks, you live you learn!

nathanalderson commented 10 months ago

I think you need to publish a version with the rollback in addition to retiring the release. Retiring the release doesn't seem to stop mix deps from pulling it in if there's no newer version.

yordis commented 10 months ago

uuufff I thought I forgot about it, but it is marked as "retired" thou

Screenshot 2024-01-02 at 11 54 41 AM