From 16d9b2755cb9f38b2f4f794a657f482fd03f6b26 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Thu, 6 Jan 2022 13:28:46 +0100 Subject: [PATCH] DEV: rename single_sign_on classes to discourse_connect (#15332) --- app/controllers/admin/users_controller.rb | 6 +-- app/controllers/session_controller.rb | 16 +++---- ...single_sign_on.rb => discourse_connect.rb} | 8 ++-- ...e_sign_on.rb => discourse_connect_base.rb} | 4 +- ...vider.rb => discourse_connect_provider.rb} | 2 +- ...n_on_spec.rb => discourse_connect_spec.rb} | 48 +++++++++---------- spec/requests/admin/users_controller_spec.rb | 4 +- .../omniauth_callbacks_controller_spec.rb | 4 +- spec/requests/session_controller_spec.rb | 18 +++---- 9 files changed, 55 insertions(+), 55 deletions(-) rename app/models/{discourse_single_sign_on.rb => discourse_connect.rb} (98%) rename lib/{single_sign_on.rb => discourse_connect_base.rb} (96%) rename lib/{single_sign_on_provider.rb => discourse_connect_provider.rb} (96%) rename spec/models/{discourse_single_sign_on_spec.rb => discourse_connect_spec.rb} (94%) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 1c411fae985..b29ceda5fbd 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -462,8 +462,8 @@ class Admin::UsersController < Admin::AdminController return render body: nil, status: 404 unless SiteSetting.enable_discourse_connect begin - sso = DiscourseSingleSignOn.parse("sso=#{params[:sso]}&sig=#{params[:sig]}", secure_session: secure_session) - rescue DiscourseSingleSignOn::ParseError + sso = DiscourseConnect.parse("sso=#{params[:sso]}&sig=#{params[:sig]}", secure_session: secure_session) + rescue DiscourseConnect::ParseError return render json: failed_json.merge(message: I18n.t("discourse_connect.login_error")), status: 422 end @@ -472,7 +472,7 @@ class Admin::UsersController < Admin::AdminController render_serialized(user, AdminDetailedUserSerializer, root: false) rescue ActiveRecord::RecordInvalid => ex render json: failed_json.merge(message: ex.message), status: 403 - rescue DiscourseSingleSignOn::BlankExternalId => ex + rescue DiscourseConnect::BlankExternalId => ex render json: failed_json.merge(message: I18n.t('discourse_connect.blank_id_error')), status: 422 end end diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index bf901e1a141..5586a7e408c 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -25,7 +25,7 @@ class SessionController < ApplicationController cookies.delete(:destination_url) if SiteSetting.enable_discourse_connect? - sso = DiscourseSingleSignOn.generate_sso(return_path, secure_session: secure_session) + sso = DiscourseConnect.generate_sso(return_path, secure_session: secure_session) if SiteSetting.verbose_discourse_connect_logging Rails.logger.warn("Verbose SSO log: Started SSO process\n\n#{sso.diagnostics}") end @@ -42,11 +42,11 @@ class SessionController < ApplicationController params.require(:sso) payload = request.query_string end - sso = SingleSignOnProvider.parse(payload) - rescue SingleSignOnProvider::BlankSecret + sso = DiscourseConnectProvider.parse(payload) + rescue DiscourseConnectProvider::BlankSecret render plain: I18n.t("discourse_connect.missing_secret"), status: 400 return - rescue SingleSignOnProvider::ParseError => e + rescue DiscourseConnectProvider::ParseError => e if SiteSetting.verbose_discourse_connect_logging Rails.logger.warn("Verbose SSO log: Signature parse error\n\n#{e.message}\n\n#{sso&.diagnostics}") end @@ -144,8 +144,8 @@ class SessionController < ApplicationController params.require(:sig) begin - sso = DiscourseSingleSignOn.parse(request.query_string, secure_session: secure_session) - rescue DiscourseSingleSignOn::ParseError => e + sso = DiscourseConnect.parse(request.query_string, secure_session: secure_session) + rescue DiscourseConnect::ParseError => e if SiteSetting.verbose_discourse_connect_logging Rails.logger.warn("Verbose SSO log: Signature parse error\n\n#{e.message}\n\n#{sso&.diagnostics}") end @@ -248,7 +248,7 @@ class SessionController < ApplicationController #{e.record.errors.to_h} Attributes: - #{e.record.attributes.slice(*SingleSignOn::ACCESSORS.map(&:to_s))} + #{e.record.attributes.slice(*DiscourseConnectBase::ACCESSORS.map(&:to_s))} SSO Diagnostics: #{sso.diagnostics} @@ -267,7 +267,7 @@ class SessionController < ApplicationController end render_sso_error(text: text || I18n.t("discourse_connect.unknown_error"), status: 500) - rescue DiscourseSingleSignOn::BlankExternalId + rescue DiscourseConnect::BlankExternalId render_sso_error(text: I18n.t("discourse_connect.blank_id_error"), status: 500) rescue Invite::ValidationFailed => e render_sso_error(text: e.message, status: 400) diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_connect.rb similarity index 98% rename from app/models/discourse_single_sign_on.rb rename to app/models/discourse_connect.rb index b8ba38fee31..90ae9f3fc16 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_connect.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class DiscourseSingleSignOn < SingleSignOn +class DiscourseConnect < DiscourseConnectBase class BlankExternalId < StandardError; end class BannedExternalId < StandardError; end @@ -32,9 +32,9 @@ class DiscourseSingleSignOn < SingleSignOn def register_nonce(return_path) if nonce if SiteSetting.discourse_connect_csrf_protection - @secure_session.set(nonce_key, return_path, expires: SingleSignOn.nonce_expiry_time) + @secure_session.set(nonce_key, return_path, expires: DiscourseConnectBase.nonce_expiry_time) else - Discourse.cache.write(nonce_key, return_path, expires_in: SingleSignOn.nonce_expiry_time) + Discourse.cache.write(nonce_key, return_path, expires_in: DiscourseConnectBase.nonce_expiry_time) end end end @@ -73,7 +73,7 @@ class DiscourseSingleSignOn < SingleSignOn Discourse.cache.delete nonce_key end - Discourse.cache.write(used_nonce_key, return_path, expires_in: SingleSignOn.used_nonce_expiry_time) + Discourse.cache.write(used_nonce_key, return_path, expires_in: DiscourseConnectBase.used_nonce_expiry_time) end end diff --git a/lib/single_sign_on.rb b/lib/discourse_connect_base.rb similarity index 96% rename from lib/single_sign_on.rb rename to lib/discourse_connect_base.rb index 3cdfff08271..8264b7ede96 100644 --- a/lib/single_sign_on.rb +++ b/lib/discourse_connect_base.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class SingleSignOn +class DiscourseConnectBase class ParseError < RuntimeError; end @@ -101,7 +101,7 @@ class SingleSignOn end def diagnostics - SingleSignOn::ACCESSORS.map { |a| "#{a}: #{public_send(a)}" }.join("\n") + DiscourseConnectBase::ACCESSORS.map { |a| "#{a}: #{public_send(a)}" }.join("\n") end def sso_secret diff --git a/lib/single_sign_on_provider.rb b/lib/discourse_connect_provider.rb similarity index 96% rename from lib/single_sign_on_provider.rb rename to lib/discourse_connect_provider.rb index 3e744430ba2..20c10797e1b 100644 --- a/lib/single_sign_on_provider.rb +++ b/lib/discourse_connect_provider.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class SingleSignOnProvider < SingleSignOn +class DiscourseConnectProvider < DiscourseConnectBase class BlankSecret < RuntimeError; end def self.parse(payload, sso_secret = nil) diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_connect_spec.rb similarity index 94% rename from spec/models/discourse_single_sign_on_spec.rb rename to spec/models/discourse_connect_spec.rb index 0556ca62514..78b917d94ef 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_connect_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" -describe DiscourseSingleSignOn do +describe DiscourseConnect do before do @discourse_connect_url = "http://example.com/discourse_sso" @discourse_connect_secret = "shjkfdhsfkjh" @@ -15,7 +15,7 @@ describe DiscourseSingleSignOn do end def make_sso - sso = SingleSignOn.new + sso = DiscourseConnectBase.new sso.sso_url = "http://meta.discorse.org/topics/111" sso.sso_secret = "supersecret" sso.nonce = "testing" @@ -39,7 +39,7 @@ describe DiscourseSingleSignOn do end def new_discourse_sso - DiscourseSingleSignOn.new(secure_session: secure_session) + DiscourseConnect.new(secure_session: secure_session) end def test_parsed(parsed, sso) @@ -63,13 +63,13 @@ describe DiscourseSingleSignOn do end it "can do round trip parsing correctly" do - sso = SingleSignOn.new + sso = DiscourseConnectBase.new sso.sso_secret = "test" sso.name = "sam saffron" sso.username = "sam" sso.email = "sam@sam.com" - sso = SingleSignOn.parse(sso.payload, "test") + sso = DiscourseConnectBase.parse(sso.payload, "test") expect(sso.name).to eq "sam saffron" expect(sso.username).to eq "sam" @@ -90,20 +90,20 @@ describe DiscourseSingleSignOn do expect do sso.lookup_or_create_user(ip_address) - end.to raise_error(DiscourseSingleSignOn::BlankExternalId) + end.to raise_error(DiscourseConnect::BlankExternalId) sso.external_id = nil expect do sso.lookup_or_create_user(ip_address) - end.to raise_error(DiscourseSingleSignOn::BlankExternalId) + end.to raise_error(DiscourseConnect::BlankExternalId) # going for slight duplication here so our intent is crystal clear %w{none nil Blank null}.each do |word| sso.external_id = word expect do sso.lookup_or_create_user(ip_address) - end.to raise_error(DiscourseSingleSignOn::BannedExternalId) + end.to raise_error(DiscourseConnect::BannedExternalId) end end @@ -572,7 +572,7 @@ describe DiscourseSingleSignOn do url, payload = sso.to_url.split("?") expect(url).to eq sso.sso_url - parsed = SingleSignOn.parse(payload, "supersecret") + parsed = DiscourseConnectBase.parse(payload, "supersecret") test_parsed(parsed, sso) end @@ -585,18 +585,18 @@ describe DiscourseSingleSignOn do url, payload = sso.to_url.split("?") expect(url).to eq "http://tcdev7.wpengine.com/" - parsed = SingleSignOn.parse(payload, "supersecret") + parsed = DiscourseConnectBase.parse(payload, "supersecret") test_parsed(parsed, sso) end it "validates nonce" do - _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + _ , payload = DiscourseConnect.generate_url(secure_session: secure_session).split("?") - sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + sso = DiscourseConnect.parse(payload, secure_session: secure_session) expect(sso.nonce_valid?).to eq true - other_session_sso = DiscourseSingleSignOn.parse(payload, secure_session: SecureSession.new("differentsession")) + other_session_sso = DiscourseConnect.parse(payload, secure_session: SecureSession.new("differentsession")) expect(other_session_sso.nonce_valid?).to eq false sso.expire_nonce! @@ -606,12 +606,12 @@ describe DiscourseSingleSignOn do it "allows disabling CSRF protection" do SiteSetting.discourse_connect_csrf_protection = false - _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + _ , payload = DiscourseConnect.generate_url(secure_session: secure_session).split("?") - sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + sso = DiscourseConnect.parse(payload, secure_session: secure_session) expect(sso.nonce_valid?).to eq true - other_session_sso = DiscourseSingleSignOn.parse(payload, secure_session: SecureSession.new("differentsession")) + other_session_sso = DiscourseConnect.parse(payload, secure_session: SecureSession.new("differentsession")) expect(other_session_sso.nonce_valid?).to eq true sso.expire_nonce! @@ -620,18 +620,18 @@ describe DiscourseSingleSignOn do end it "generates a correct sso url" do - url, payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + url, payload = DiscourseConnect.generate_url(secure_session: secure_session).split("?") expect(url).to eq @discourse_connect_url - sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + sso = DiscourseConnect.parse(payload, secure_session: secure_session) expect(sso.nonce).to_not be_nil end context 'nonce error' do it "generates correct error message when nonce has already been used" do - _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + _ , payload = DiscourseConnect.generate_url(secure_session: secure_session).split("?") - sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + sso = DiscourseConnect.parse(payload, secure_session: secure_session) expect(sso.nonce_valid?).to eq true sso.expire_nonce! @@ -639,9 +639,9 @@ describe DiscourseSingleSignOn do end it "generates correct error message when nonce is expired" do - _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + _ , payload = DiscourseConnect.generate_url(secure_session: secure_session).split("?") - sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + sso = DiscourseConnect.parse(payload, secure_session: secure_session) expect(sso.nonce_valid?).to eq true Discourse.cache.delete(sso.used_nonce_key) @@ -650,9 +650,9 @@ describe DiscourseSingleSignOn do it "generates correct error message when nonce is expired, and csrf protection disabled" do SiteSetting.discourse_connect_csrf_protection = false - _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + _ , payload = DiscourseConnect.generate_url(secure_session: secure_session).split("?") - sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + sso = DiscourseConnect.parse(payload, secure_session: secure_session) expect(sso.nonce_valid?).to eq true Discourse.cache.delete(sso.used_nonce_key) diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 9c2b249d339..7ece24fd80d 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -922,7 +922,7 @@ RSpec.describe Admin::UsersController do end describe '#sync_sso' do - let(:sso) { SingleSignOn.new } + let(:sso) { DiscourseConnectBase.new } let(:sso_secret) { "sso secret" } before do @@ -942,7 +942,7 @@ RSpec.describe Admin::UsersController do sso.email = "bob@bob.com" sso.external_id = "1" - user = DiscourseSingleSignOn.parse(sso.payload, secure_session: read_secure_session).lookup_or_create_user + user = DiscourseConnect.parse(sso.payload, secure_session: read_secure_session).lookup_or_create_user sso.name = "Bill" sso.username = "Hokli$$!!" diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb index f03ec5b49b0..4f7cbd4fe07 100644 --- a/spec/requests/omniauth_callbacks_controller_spec.rb +++ b/spec/requests/omniauth_callbacks_controller_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'rails_helper' -require 'single_sign_on' +require 'discourse_connect_base' RSpec.describe Users::OmniauthCallbacksController do fab!(:user) { Fabricate(:user) } @@ -543,7 +543,7 @@ RSpec.describe Users::OmniauthCallbacksController do SiteSetting.enable_discourse_connect_provider = true SiteSetting.discourse_connect_secret = "topsecret" - @sso = SingleSignOn.new + @sso = DiscourseConnectBase.new @sso.nonce = "mynonce" @sso.sso_secret = SiteSetting.discourse_connect_secret @sso.return_sso_url = "http://somewhere.over.rainbow/sso" diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index a59537cf789..45ecfb2a94a 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -535,11 +535,11 @@ describe SessionController do def get_sso(return_path) nonce = SecureRandom.hex - dso = DiscourseSingleSignOn.new(secure_session: read_secure_session) + dso = DiscourseConnect.new(secure_session: read_secure_session) dso.nonce = nonce dso.register_nonce(return_path) - sso = SingleSignOn.new + sso = DiscourseConnectBase.new sso.nonce = nonce sso.sso_secret = @sso_secret sso @@ -684,7 +684,7 @@ describe SessionController do ScreenedIpAddress.all.destroy_all get "/" sso = sso_for_ip_specs - DiscourseSingleSignOn.parse(sso.payload, secure_session: read_secure_session).lookup_or_create_user(request.remote_ip) + DiscourseConnect.parse(sso.payload, secure_session: read_secure_session).lookup_or_create_user(request.remote_ip) sso = sso_for_ip_specs _screened_ip = Fabricate(:screened_ip_address, ip_address: request.remote_ip, action_type: ScreenedIpAddress.actions[:block]) @@ -1132,7 +1132,7 @@ describe SessionController do "somewhere.over.rainbow|secretForOverRainbow", ].join("\n") - @sso = SingleSignOnProvider.new + @sso = DiscourseConnectProvider.new @sso.nonce = "mynonce" @sso.return_sso_url = "http://somewhere.over.rainbow/sso" @@ -1164,7 +1164,7 @@ describe SessionController do expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) payload = location.split("?")[1] - sso2 = SingleSignOnProvider.parse(payload) + sso2 = DiscourseConnectProvider.parse(payload) expect(sso2.email).to eq(@user.email) expect(sso2.name).to eq(@user.name) @@ -1191,7 +1191,7 @@ describe SessionController do it "fails with a nice error message if secret is blank" do SiteSetting.discourse_connect_provider_secrets = "" - sso = SingleSignOnProvider.new + sso = DiscourseConnectProvider.new sso.nonce = "mynonce" sso.return_sso_url = "http://website.without.secret.com/sso" get "/session/sso_provider", params: Rack::Utils.parse_query(sso.payload("aasdasdasd")) @@ -1201,7 +1201,7 @@ describe SessionController do it "returns a 422 if no return_sso_url" do SiteSetting.discourse_connect_provider_secrets = "abcdefghij" - sso = SingleSignOnProvider.new + sso = DiscourseConnectProvider.new get "/session/sso_provider?sso=asdf&sig=abcdefghij" expect(response.status).to eq(422) end @@ -1215,7 +1215,7 @@ describe SessionController do expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) payload = location.split("?")[1] - sso2 = SingleSignOnProvider.parse(payload) + sso2 = DiscourseConnectProvider.parse(payload) expect(sso2.email).to eq(@user.email) expect(sso2.name).to eq(@user.name) @@ -1279,7 +1279,7 @@ describe SessionController do expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) payload = location.split("?")[1] - sso2 = SingleSignOnProvider.parse(payload) + sso2 = DiscourseConnectProvider.parse(payload) expect(sso2.avatar_url.blank?).to_not eq(true) expect(sso2.profile_background_url.blank?).to_not eq(true)