Redirect after logout with full screen login

Bug
1

This is a bit of a weird one. It requires a discourse install using “full screen login” for authentication, and the omniauth origin param set to an external url (this is done either through a referrer or the ?origin= param).

Steps to reproduce:

  1. Log into a Discourse site using FSL, setting the origin to an external site
    e.g. https://discourse.mozilla.org/auth/auth0?origin=http://example.com/ (I’d suggest logging in via passwordless by entering your email at the top, GitHub authentication currently requires 2FA)
  2. Log out of the Discourse site after a successful login

Result:

You’re redirected to the external site.

Video:

What’s going on here:

During login, in omniauth_callbacks_controller.rb:

# origin is e.g. "http://example.com/"
parsed = URI.parse(origin)
@origin = "#{parsed.path}?#{parsed.query}"
cookies[:authentication_data] = {
  destination_url: origin
}
redirect_to @origin

@origin is a host-less url, but origin is absolute. So in this case @origin = "/"

Also note, the cookie is set to the absolute origin, but the redirect is to the host-less @origin.

After logout, thanks to this in application.html.erb (which triggers a call to Discourse.authenticationComplete with the data), the user is redirected to the destination_url set above:

<%- if !current_user && cookies[:authentication_data] %>
  <meta id="data-authentication" data-authentication-data="<%= cookies.delete(:authentication_data) %>">
<%- end %>

So, there’s a couple of problems here:

  1. That authentcation_data cookie seems to exist for login only. It then triggering Discourse.authenticationComplete on logout seems wrong to me. The cookie should probably be nuked on logout. This would prevent the post-logout redirect to origin.

  2. The code in omniauth_callbacks_controller.rb is written to assume that the origin has the same domain as the Discourse instance, but that’s not always the case. I don’t know if the cookie-origin/redirect-@origin discrepancy is deliberate, but clearing origin prior to generating @origin if it’s not a Discourse url should fix this problem.

I’m happy to work on a PR to fix these two issues.

6 Likes
2

Sounds good to me, go for it :+1:

3 Likes
3

PR submitted:

https://github.com/discourse/discourse/pull/7170

8 Likes
4

Is this related to your recent work @techAPJ

1 Like
5

This is not related to my recent “full screen login” work but the PR to fix this was merged by David and all the specs are passing. So we’re good here. :+1:

4 Likes