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

FEATURE: Add probe-only functionality to SSO Provider protocol #22393

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,14 @@ def sso_provider(payload = nil, confirmed_2fa_during_login = false)
end

if data[:no_current_user]
cookies[:sso_payload] = payload || request.query_string
redirect_to path("/login")
return
if data[:prompt] == "none"
redirect_to data[:sso_redirect_url], allow_other_host: true
return
else
cookies[:sso_payload] = payload || request.query_string
redirect_to path("/login")
return
end
end

if request.xhr?
Expand All @@ -88,6 +93,8 @@ def sso_provider(payload = nil, confirmed_2fa_during_login = false)
render plain: I18n.t("discourse_connect.login_error"), status: 422
rescue DiscourseConnectProvider::BlankReturnUrl
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

# For use in development mode only when login options could be limited or disabled.
Expand Down
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2520,6 +2520,7 @@ en:
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."
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"
most_recent_poster: "Most Recent Poster"
Expand Down
5 changes: 4 additions & 1 deletion lib/discourse_connect_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,24 @@ class ParseError < RuntimeError
ACCESSORS = %i[
add_groups
admin
moderator
avatar_force_update
avatar_url
bio
card_background_url
confirmed_2fa
email
external_id
failed
groups
locale
locale_force_update
location
logout
moderator
name
no_2fa_methods
nonce
prompt
profile_background_url
remove_groups
require_2fa
Expand All @@ -40,6 +42,7 @@ class ParseError < RuntimeError
admin
avatar_force_update
confirmed_2fa
failed
locale_force_update
logout
moderator
Expand Down
16 changes: 15 additions & 1 deletion lib/discourse_connect_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,17 @@ class BlankSecret < RuntimeError
end
class BlankReturnUrl < RuntimeError
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)
# 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)
return_sso_url = lookup_return_sso_url(parsed_payload)

Expand All @@ -32,7 +41,12 @@ def self.parse(payload, sso_secret = nil, **init_kwargs)
raise BlankSecret
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

def self.lookup_return_sso_url(parsed_payload)
Expand Down
17 changes: 16 additions & 1 deletion lib/second_factor/actions/discourse_connect_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,22 @@ def skip_second_factor_auth?(params)
def second_factor_auth_skipped!(params)
sso = get_sso(payload(params))
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)
sso.confirmed_2fa = true if @opts[:confirmed_2fa_during_login]
{ sso_redirect_url: sso.to_url(sso.return_sso_url) }
Expand Down
43 changes: 42 additions & 1 deletion spec/requests/session_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
expect(redirect_query["sig"][0]).to eq(expected_sig)
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",
params: Rack::Utils.parse_query(@sso.payload("secretForRandomSite"))
expect(response.status).to eq(422)
Expand Down Expand Up @@ -1513,6 +1513,47 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
expect(sso2.no_2fa_methods).to eq(nil)
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
SiteSetting.avatar_sizes = "100|49"
setup_s3
Expand Down