Skip to content
Snippets Groups Projects
Unverified Commit 95ccea3f authored by David Taylor's avatar David Taylor Committed by GitHub
Browse files

DEV: Move `saml_request_method` handling into the OmniAuth strategy (#44) 

Putting this logic into the omniauth strategy is much cleaner because:
- we no longer need a Rails controller and the associated `custom_url` parameter
- we can re-use the `authn_request` instance which is automatically generated by the omniauth strategy, rather than re-implementing that logic
- the behavior is decided at runtime, rather than during initialization. This makes it testable, and is another step on the way to making the plugin multisite-compatible

This commit also introduces a spec for the feature.
parent 12cae03c
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
lib/discourse_saml/saml_omniauth_strategy.rb 0 → 100644
+ 53
− 0
View file @ 95ccea3f
# frozen_string_literal: true
class ::DiscourseSaml::SamlOmniauthStrategy < OmniAuth::Strategies::SAML
option :request_method, "GET"
def request_phase
if options[:request_method] == "POST"
render_auto_submitted_form
else
super
end
end
private
def render_auto_submitted_form
authn_request = OneLogin::RubySaml::Authrequest.new
with_settings do |settings|
saml_req = authn_request.create_params(settings, additional_params_for_authn_request)["SAMLRequest"]
destination_url = settings.idp_sso_service_url
script_url = UrlHelper.absolute('/plugins/discourse-saml/javascripts/submit-form-on-load.js', GlobalSetting.cdn_url)
html = <<~HTML
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
<body>
<noscript>
<p>
<strong>Note:</strong> Since your browser does not support JavaScript,
you must press the Continue button once to proceed.
</p>
</noscript>
<form action="#{destination_url}" method="post">
<div>
<input type="hidden" name="SAMLRequest" value="#{saml_req}"/>
</div>
<noscript>
<div>
<input type="submit" value="Continue"/>
</div>
</noscript>
</form>
<script src="#{script_url}"></script>
</body>
</html>
HTML
r = Rack::Response.new
r.write(html)
r.finish
end
end
end
lib/saml_authenticator.rb
+ 2
− 2
View file @ 95ccea3f
...@@ -49,7 +49,7 @@ class SamlAuthenticator < ::Auth::OAuth2Authenticator ...@@ -49,7 +49,7 @@ class SamlAuthenticator < ::Auth::OAuth2Authenticator
end end
def register_middleware(omniauth) def register_middleware(omniauth)
omniauth.provider :saml, omniauth.provider ::DiscourseSaml::SamlOmniauthStrategy,
name: name, name: name,
setup: lambda { |env| setup: lambda { |env|
setup_strategy(env["omniauth.strategy"]) setup_strategy(env["omniauth.strategy"])
...@@ -71,7 +71,7 @@ class SamlAuthenticator < ::Auth::OAuth2Authenticator ...@@ -71,7 +71,7 @@ class SamlAuthenticator < ::Auth::OAuth2Authenticator
assertion_consumer_service_url: SamlAuthenticator.saml_base_url + "/auth/#{name}/callback", assertion_consumer_service_url: SamlAuthenticator.saml_base_url + "/auth/#{name}/callback",
single_logout_service_url: SamlAuthenticator.saml_base_url + "/auth/#{name}/slo", single_logout_service_url: SamlAuthenticator.saml_base_url + "/auth/#{name}/slo",
name_identifier_format: GlobalSetting.try(:saml_name_identifier_format), name_identifier_format: GlobalSetting.try(:saml_name_identifier_format),
custom_url: (GlobalSetting.try(:saml_request_method) == 'post') ? "/discourse_saml" : nil, request_method: (GlobalSetting.try(:saml_request_method)&.downcase == 'post') ? "POST" : "GET",
certificate: GlobalSetting.try(:saml_sp_certificate), certificate: GlobalSetting.try(:saml_sp_certificate),
private_key: GlobalSetting.try(:saml_sp_private_key), private_key: GlobalSetting.try(:saml_sp_private_key),
security: { security: {
......
plugin.rb
+ 3
− 89
View file @ 95ccea3f
...@@ -14,8 +14,6 @@ gem 'rexml', '3.2.5' ...@@ -14,8 +14,6 @@ gem 'rexml', '3.2.5'
gem 'ruby-saml', '1.13.0' gem 'ruby-saml', '1.13.0'
gem "omniauth-saml", '1.9.0' gem "omniauth-saml", '1.9.0'
require_relative "lib/saml_authenticator"
on(:before_session_destroy) do |data| on(:before_session_destroy) do |data|
next if !GlobalSetting.try(:saml_slo_target_url).present? next if !GlobalSetting.try(:saml_slo_target_url).present?
data[:redirect_url] = Discourse.base_path + "/auth/saml/spslo" data[:redirect_url] = Discourse.base_path + "/auth/saml/spslo"
...@@ -67,96 +65,12 @@ after_initialize do ...@@ -67,96 +65,12 @@ after_initialize do
end end
end end
request_method = GlobalSetting.try(:saml_request_method) || 'get' require_relative "lib/discourse_saml/saml_omniauth_strategy"
require_relative "lib/saml_authenticator"
if request_method == 'post'
after_initialize do
module ::DiscourseSaml
class Engine < ::Rails::Engine
engine_name "discourse_saml"
isolate_namespace DiscourseSaml
end
end
class DiscourseSaml::DiscourseSamlController < ::ApplicationController
skip_before_action :check_xhr
skip_before_action :redirect_to_login_if_required, only: [:index]
def index
authn_request = OneLogin::RubySaml::Authrequest.new
metadata_url = GlobalSetting.try(:saml_metadata_url)
settings = nil
if metadata_url
idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new
settings = idp_metadata_parser.parse_remote(metadata_url)
settings.idp_sso_target_url ||= GlobalSetting.saml_target_url
settings.idp_cert ||= GlobalSetting.try(:saml_cert)
else
settings = OneLogin::RubySaml::Settings.new(
idp_sso_target_url: GlobalSetting.saml_target_url,
idp_cert_fingerprint: GlobalSetting.try(:saml_cert_fingerprint),
idp_cert_fingerprint_algorithm: GlobalSetting.try(:saml_cert_fingerprint_algorithm),
idp_cert: GlobalSetting.try(:saml_cert),
)
end
settings.compress_request = false
settings.passive = false
settings.issuer = SamlAuthenticator.saml_base_url
settings.assertion_consumer_service_url = SamlAuthenticator.saml_base_url + "/auth/saml/callback"
settings.name_identifier_format = GlobalSetting.try(:saml_name_identifier_format) || "urn:oasis:names:tc:SAML:2.0:protocol"
saml_params = authn_request.create_params(settings, {})
@saml_req = saml_params['SAMLRequest']
script_path = '/plugins/discourse-saml/javascripts/submit-form-on-load.js'
html = <<~HTML
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
<body>
<noscript>
<p>
<strong>Note:</strong> Since your browser does not support JavaScript,
you must press the Continue button once to proceed.
</p>
</noscript>
<form action="#{GlobalSetting.saml_target_url}" method="post">
<div>
<input type="hidden" name="SAMLRequest" value="#{@saml_req}"/>
</div>
<noscript>
<div>
<input type="submit" value="Continue"/>
</div>
</noscript>
</form>
<script src="#{UrlHelper.absolute(script_path, GlobalSetting.cdn_url)}"></script>
</body>
</html>
HTML
render html: html.html_safe
end
end
DiscourseSaml::Engine.routes.draw do
get '/' => 'discourse_saml#index'
end
Discourse::Application.routes.append do
mount ::DiscourseSaml::Engine, at: "/discourse_saml"
end
end
end
pretty_name = GlobalSetting.try(:saml_title) || "SAML" pretty_name = GlobalSetting.try(:saml_title) || "SAML"
button_title = GlobalSetting.try(:saml_button_title) || GlobalSetting.try(:saml_title) || "with SAML" button_title = GlobalSetting.try(:saml_button_title) || GlobalSetting.try(:saml_title) || "with SAML"
auth_provider title: button_title, auth_provider title: button_title,
pretty_name: pretty_name, pretty_name: pretty_name,
authenticator: SamlAuthenticator.new('saml'), authenticator: SamlAuthenticator.new('saml')
custom_url: request_method == 'post' ? "/discourse_saml" : nil
spec/integration/saml_post_mode_spec.rb 0 → 100644
+ 40
− 0
View file @ 95ccea3f
# frozen_string_literal: true
require 'rails_helper'
describe "SAML POST-mode functionality", type: :request do
before do
OmniAuth.config.test_mode = false
global_setting :saml_target_url, "https://example.com/samlidp"
end
it "does not affect functionality when disabled" do
global_setting :saml_request_method, "GET"
post "/auth/saml"
expect(response.status).to eq(302)
expect(response.location).to start_with("https://example.com/samlidp")
end
it "serves an auto-submitting POST form when enabled" do
global_setting :saml_request_method, "POST"
post "/auth/saml"
expect(response.status).to eq(200)
expect(response.body).to have_tag(
"form",
with: {
"action" => "https://example.com/samlidp",
"method" => "post",
}
)
expect(response.body).to have_tag(
"form input",
with: {
"name" => "SAMLRequest",
"type" => "hidden",
}
)
expect(response.body).to have_tag("script")
end
end
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment