FEATURE: Add prompt=none functionality to SSO Provider protocol (#22393)

This commit adds support for an optional `prompt` parameter in the
payload of the /session/sso_provider endpoint.  If an SSO Consumer
adds a `prompt=none` parameter to the encoded/signed `sso` payload,
then Discourse will avoid trying to login a not-logged-in user:

 * If the user is already logged in, Discourse will immediately
   redirect back to the Consumer with the user's credentials in a
   signed payload, as usual.

 * If the user is not logged in, Discourse will immediately redirect
   back to the Consumer with a signed payload bearing the parameter
   `failed=true`.

This allows the SSO Consumer to simply test whether or not a user is
logged in, without forcing the user to try to log in.  This is useful
when the SSO Consumer allows both anonymous and authenticated access.
(E.g., users that are already logged-in to Discourse can be seamlessly
logged-in to the Consumer site, and anonymous users can remain
anonymous until they explicitly ask to log in.)

This feature is similar to the `prompt=none` functionality in an
OpenID Connect Authentication Request; see
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
This commit is contained in:
Matt Marjanović 2023-09-28 04:53:28 -07:00 committed by GitHub
parent b72ed3cb38
commit 619d43ea47
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 88 additions and 7 deletions

View File

@ -59,10 +59,15 @@ class SessionController < ApplicationController
end end
if data[:no_current_user] if data[:no_current_user]
if data[:prompt] == "none"
redirect_to data[:sso_redirect_url], allow_other_host: true
return
else
cookies[:sso_payload] = payload || request.query_string cookies[:sso_payload] = payload || request.query_string
redirect_to path("/login") redirect_to path("/login")
return return
end end
end
if request.xhr? if request.xhr?
# for the login modal # for the login modal
@ -88,6 +93,8 @@ class SessionController < ApplicationController
render plain: I18n.t("discourse_connect.login_error"), status: 422 render plain: I18n.t("discourse_connect.login_error"), status: 422
rescue DiscourseConnectProvider::BlankReturnUrl rescue DiscourseConnectProvider::BlankReturnUrl
render plain: "return_sso_url is blank, it must be provided", status: 400 render plain: "return_sso_url is blank, it must be provided", status: 400
rescue DiscourseConnectProvider::InvalidParameterValueError => e
render plain: I18n.t("discourse_connect.invalid_parameter_value", param: e.param), status: 400
end end
# For use in development mode only when login options could be limited or disabled. # For use in development mode only when login options could be limited or disabled.

View File

@ -2543,6 +2543,7 @@ en:
email_error: "An account could not be registered with the email address <b>%{email}</b>. Please contact the site's administrator." email_error: "An account could not be registered with the email address <b>%{email}</b>. Please contact the site's administrator."
missing_secret: "Authentication failed due to missing secret. Contact the site administrators to fix this problem." missing_secret: "Authentication failed due to missing secret. Contact the site administrators to fix this problem."
invite_redeem_failed: "Invite redemption failed. Please contact the site's administrator." invite_redeem_failed: "Invite redemption failed. Please contact the site's administrator."
invalid_parameter_value: "Authentication failed due to invalid value for `%{param}` parameter. Contact the site administrators to fix this problem."
original_poster: "Original Poster" original_poster: "Original Poster"
most_recent_poster: "Most Recent Poster" most_recent_poster: "Most Recent Poster"

View File

@ -7,7 +7,6 @@ class DiscourseConnectBase
ACCESSORS = %i[ ACCESSORS = %i[
add_groups add_groups
admin admin
moderator
avatar_force_update avatar_force_update
avatar_url avatar_url
bio bio
@ -15,14 +14,17 @@ class DiscourseConnectBase
confirmed_2fa confirmed_2fa
email email
external_id external_id
failed
groups groups
locale locale
locale_force_update locale_force_update
location location
logout logout
moderator
name name
no_2fa_methods no_2fa_methods
nonce nonce
prompt
profile_background_url profile_background_url
remove_groups remove_groups
require_2fa require_2fa
@ -40,6 +42,7 @@ class DiscourseConnectBase
admin admin
avatar_force_update avatar_force_update
confirmed_2fa confirmed_2fa
failed
locale_force_update locale_force_update
logout logout
moderator moderator

View File

@ -5,8 +5,17 @@ class DiscourseConnectProvider < DiscourseConnectBase
end end
class BlankReturnUrl < RuntimeError class BlankReturnUrl < RuntimeError
end end
class InvalidParameterValueError < RuntimeError
attr_reader :param
def initialize(param)
@param = param
super("Invalid value for parameter `#{param}`")
end
end
def self.parse(payload, sso_secret = nil, **init_kwargs) def self.parse(payload, sso_secret = nil, **init_kwargs)
# We extract the return_sso_url parameter early; we need the URL's host
# in order to lookup the correct SSO secret in our site settings.
parsed_payload = Rack::Utils.parse_query(payload) parsed_payload = Rack::Utils.parse_query(payload)
return_sso_url = lookup_return_sso_url(parsed_payload) return_sso_url = lookup_return_sso_url(parsed_payload)
@ -32,7 +41,12 @@ class DiscourseConnectProvider < DiscourseConnectBase
raise BlankSecret raise BlankSecret
end end
super(payload, sso_secret, **init_kwargs) sso = super(payload, sso_secret, **init_kwargs)
# Do general parameter validation now, after signature-verification has succeeded.
raise InvalidParameterValueError.new("prompt") if (sso.prompt != nil) && (sso.prompt != "none")
sso
end end
def self.lookup_return_sso_url(parsed_payload) def self.lookup_return_sso_url(parsed_payload)

View File

@ -10,7 +10,22 @@ module SecondFactor::Actions
def second_factor_auth_skipped!(params) def second_factor_auth_skipped!(params)
sso = get_sso(payload(params)) sso = get_sso(payload(params))
return { logout: true, return_sso_url: sso.return_sso_url } if sso.logout return { logout: true, return_sso_url: sso.return_sso_url } if sso.logout
return { no_current_user: true } if !current_user if !current_user
if sso.prompt == "none"
# 'prompt=none' was requested, so just return a failed authentication
# without putting up a login dialog and interrogating the user.
sso.failed = true
return(
{
no_current_user: true,
prompt: sso.prompt,
sso_redirect_url: sso.to_url(sso.return_sso_url),
}
)
end
# ...otherwise, trigger the usual redirect to login dialog.
return { no_current_user: true }
end
populate_user_data(sso) populate_user_data(sso)
sso.confirmed_2fa = true if @opts[:confirmed_2fa_during_login] sso.confirmed_2fa = true if @opts[:confirmed_2fa_during_login]
{ sso_redirect_url: sso.to_url(sso.return_sso_url) } { sso_redirect_url: sso.to_url(sso.return_sso_url) }

View File

@ -1462,7 +1462,7 @@ RSpec.describe SessionController do
expect(redirect_query["sig"][0]).to eq(expected_sig) expect(redirect_query["sig"][0]).to eq(expected_sig)
end end
it "it fails to log in if secret is wrong" do it "fails to log in if secret is wrong" do
get "/session/sso_provider", get "/session/sso_provider",
params: Rack::Utils.parse_query(@sso.payload("secretForRandomSite")) params: Rack::Utils.parse_query(@sso.payload("secretForRandomSite"))
expect(response.status).to eq(422) expect(response.status).to eq(422)
@ -1515,6 +1515,47 @@ RSpec.describe SessionController do
expect(sso2.no_2fa_methods).to eq(nil) expect(sso2.no_2fa_methods).to eq(nil)
end end
it "fails with a nice error message if `prompt` parameter has an invalid value" do
@sso.prompt = "xyzpdq"
get "/session/sso_provider",
params: Rack::Utils.parse_query(@sso.payload("secretForOverRainbow"))
expect(response.status).to eq(400)
expect(response.body).to eq(
I18n.t("discourse_connect.invalid_parameter_value", param: "prompt"),
)
end
it "redirects browser to return_sso_url with auth failure when prompt=none is requested and the user is not logged in" do
@sso.prompt = "none"
get "/session/sso_provider",
params: Rack::Utils.parse_query(@sso.payload("secretForOverRainbow"))
location = response.header["Location"]
expect(location).to match(%r{^http://somewhere.over.rainbow/sso})
payload = location.split("?")[1]
sso2 = DiscourseConnectProvider.parse(payload)
expect(sso2.failed).to eq(true)
expect(sso2.email).to eq(nil)
expect(sso2.name).to eq(nil)
expect(sso2.username).to eq(nil)
expect(sso2.external_id).to eq(nil)
expect(sso2.admin).to eq(nil)
expect(sso2.moderator).to eq(nil)
expect(sso2.groups).to eq(nil)
expect(sso2.avatar_url).to eq(nil)
expect(sso2.profile_background_url).to eq(nil)
expect(sso2.card_background_url).to eq(nil)
expect(sso2.confirmed_2fa).to eq(nil)
expect(sso2.no_2fa_methods).to eq(nil)
end
it "handles non local content correctly" do it "handles non local content correctly" do
SiteSetting.avatar_sizes = "100|49" SiteSetting.avatar_sizes = "100|49"
setup_s3 setup_s3