diff --git a/app/assets/javascripts/discourse/app/controllers/invites-show.js b/app/assets/javascripts/discourse/app/controllers/invites-show.js index 875820daf77..0e640be3949 100644 --- a/app/assets/javascripts/discourse/app/controllers/invites-show.js +++ b/app/assets/javascripts/discourse/app/controllers/invites-show.js @@ -63,6 +63,11 @@ export default Controller.extend( this.setProperties(props); }, + @discourseComputed + discourseConnectEnabled() { + return this.siteSettings.enable_discourse_connect; + }, + @discourseComputed welcomeTitle() { return I18n.t("invites.welcome_to", { @@ -83,10 +88,17 @@ export default Controller.extend( @discourseComputed externalAuthsOnly() { return ( - !this.siteSettings.enable_local_logins && this.externalAuthsEnabled + !this.siteSettings.enable_local_logins && + this.externalAuthsEnabled && + !this.siteSettings.enable_discourse_connect ); }, + @discourseComputed("externalAuthsOnly", "discourseConnectEnabled") + showSocialLoginAvailable(externalAuthsOnly, discourseConnectEnabled) { + return !externalAuthsOnly && !discourseConnectEnabled; + }, + @discourseComputed( "externalAuthsOnly", "authOptions", @@ -170,6 +182,9 @@ export default Controller.extend( @discourseComputed wavingHandURL: () => wavingHandURL(), + @discourseComputed + ssoPath: () => getUrl("/session/sso"), + actions: { submit() { const userFields = this.userFields; diff --git a/app/assets/javascripts/discourse/app/templates/invites/show.hbs b/app/assets/javascripts/discourse/app/templates/invites/show.hbs index d4a0f5afc9e..9a45776efa2 100644 --- a/app/assets/javascripts/discourse/app/templates/invites/show.hbs +++ b/app/assets/javascripts/discourse/app/templates/invites/show.hbs @@ -21,15 +21,16 @@

{{user-info user=invitedBy}}

{{#unless isInviteLink}} -

+

{{html-safe yourEmailMessage}} - {{#unless externalAuthsOnly}} + {{#if showSocialLoginAvailable}} {{i18n "invites.social_login_available"}} - {{/unless}} + {{/if}}

{{/unless}} {{#if externalAuthsOnly}} + {{! authOptions are present once the user has followed the OmniAuth flow (e.g. twitter/google/etc) }} {{#if authOptions}} {{#unless isInviteLink}} {{input-tip validation=emailValidation id="account-email-validation"}} @@ -39,6 +40,12 @@ {{/if}} {{/if}} + {{#if discourseConnectEnabled}} + + {{i18n "continue"}} + + {{/if}} + {{#if shouldDisplayForm}}
{{#if isInviteLink}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js b/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js index 1265053c1cb..472e9cb50c5 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js @@ -170,6 +170,53 @@ acceptance("Invite accept when local login is disabled", function (needs) { }); }); +acceptance( + "Invite accept when DiscourseConnect SSO is enabled and local login is disabled", + function (needs) { + needs.settings({ + enable_local_logins: false, + enable_discourse_connect: true, + }); + + test("invite link", async function (assert) { + preloadInvite({ link: true }); + + await visit("/invites/myvalidinvitetoken"); + + assert.ok( + !exists(".btn-social.facebook"), + "does not show Facebook login button" + ); + assert.ok(!exists("form"), "does not display the form"); + assert.ok( + !exists(".email-message"), + "does not show the email message with the prefilled email" + ); + assert.ok(exists(".discourse-connect"), "shows the Continue button"); + }); + + test("email invite link", async function (assert) { + preloadInvite(); + + await visit("/invites/myvalidinvitetoken"); + + assert.ok( + !exists(".btn-social.facebook"), + "does not show Facebook login button" + ); + assert.ok(!exists("form"), "does not display the form"); + assert.ok( + exists(".email-message"), + "shows the email message with the prefilled email" + ); + assert.ok(exists(".discourse-connect"), "shows the Continue button"); + assert.ok( + queryAll(".email-message").text().includes("foobar@example.com") + ); + }); + } +); + acceptance("Invite link with authentication data", function (needs) { needs.settings({ enable_local_logins: false }); diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 8d2becaaae2..0d39f9a8f9b 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -336,14 +336,6 @@ class GroupsController < ApplicationController raise Discourse::InvalidParameters.new(I18n.t("groups.errors.usernames_or_emails_required")) end - if emails.any? - if SiteSetting.enable_discourse_connect? - raise Discourse::InvalidParameters.new(I18n.t("groups.errors.no_invites_with_discourse_connect")) - elsif !SiteSetting.enable_local_logins? - raise Discourse::InvalidParameters.new(I18n.t("groups.errors.no_invites_without_local_logins")) - end - end - if users.length > ADD_MEMBERS_LIMIT return render_json_error( I18n.t("groups.errors.adding_too_many_users", count: ADD_MEMBERS_LIMIT) diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 8843efee479..98210b775fe 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -16,7 +16,7 @@ class InvitesController < ApplicationController expires_now invite = Invite.find_by(invite_key: params[:id]) - if invite.present? && !invite.expired? && !invite.redeemed? + if invite.present? && invite.redeemable? email = Email.obfuscate(invite.email) # Show email if the user already authenticated their email @@ -34,14 +34,16 @@ class InvitesController < ApplicationController is_invite_link: invite.is_invite_link? )) + secure_session["invite-key"] = invite.invite_key + render layout: 'application' else - flash.now[:error] = if invite&.expired? - I18n.t('invite.expired', base_url: Discourse.base_url) - elsif invite&.redeemed? - I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url) - else + flash.now[:error] = if invite.blank? I18n.t('invite.not_found', base_url: Discourse.base_url) + elsif invite.redeemed? + I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url) + elsif invite.expired? + I18n.t('invite.expired', base_url: Discourse.base_url) end render layout: 'no_ember' @@ -165,6 +167,8 @@ class InvitesController < ApplicationController render json: success_json end + # For DiscourseConnect SSO, all invite acceptance is done + # via the SessionController#sso_login route def perform_accept_invitation params.require(:id) params.permit(:email, :username, :name, :password, :timezone, user_custom_fields: {}) @@ -190,7 +194,7 @@ class InvitesController < ApplicationController invite.email end - user = invite.redeem(attrs) + user = invite.redeem(**attrs) rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e return render json: failed_json.merge(errors: e.record&.errors&.to_hash, message: I18n.t('invite.error_message')), status: 412 rescue Invite::UserExists => e @@ -296,7 +300,7 @@ class InvitesController < ApplicationController private def ensure_invites_allowed - if SiteSetting.enable_discourse_connect || (!SiteSetting.enable_local_logins && Discourse.enabled_auth_providers.count == 0) + if (!SiteSetting.enable_local_logins && Discourse.enabled_auth_providers.count == 0 && !SiteSetting.enable_discourse_connect) raise Discourse::NotFound end end diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 1b5cf7c2761..ffd938165a5 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -172,6 +172,8 @@ class SessionController < ApplicationController sso.expire_nonce! begin + invite = validate_invitiation!(sso) + if user = sso.lookup_or_create_user(request.remote_ip) if user.suspended? @@ -179,25 +181,38 @@ class SessionController < ApplicationController return end - if SiteSetting.must_approve_users? && !user.approved? + # users logging in via SSO using an invite do not need to be approved, + # they are already pre-approved because they have been invited + if SiteSetting.must_approve_users? && !user.approved? && invite.blank? if SiteSetting.discourse_connect_not_approved_url.present? redirect_to SiteSetting.discourse_connect_not_approved_url else render_sso_error(text: I18n.t("discourse_connect.account_not_approved"), status: 403) end return + + # we only want to redeem the invite if + # the user has not already redeemed an invite + # (covers the same SSO user visiting an invite link) + elsif invite.present? && user.invited_user.blank? + redeem_invitation(invite, sso) + + # we directly call user.activate here instead of going + # through the UserActivator path because we assume the account + # is valid from the SSO provider's POV and do not need to + # send an activation email to the user + user.activate + login_sso_user(sso, user) + + topic = invite.topics.first + return_path = topic.present? ? path(topic.relative_url) : path("/") elsif !user.active? activation = UserActivator.new(user, request, session, cookies) activation.finish session["user_created_message"] = activation.message - redirect_to(users_account_created_path) && (return) + return redirect_to(users_account_created_path) else - if SiteSetting.verbose_discourse_connect_logging - Rails.logger.warn("Verbose SSO log: User was logged on #{user.username}\n\n#{sso.diagnostics}") - end - if user.id != current_user&.id - log_on_user user - end + login_sso_user(sso, user) end # If it's not a relative URL check the host @@ -252,11 +267,14 @@ class SessionController < ApplicationController end render_sso_error(text: text || I18n.t("discourse_connect.unknown_error"), status: 500) - rescue DiscourseSingleSignOn::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) + rescue Invite::RedemptionFailed => e + render_sso_error(text: I18n.t("discourse_connect.invite_redeem_failed"), status: 412) + rescue Invite::UserExists => e + render_sso_error(text: e.message, status: 412) rescue => e message = +"Failed to create or lookup user: #{e}." message << " " @@ -270,6 +288,13 @@ class SessionController < ApplicationController end end + def login_sso_user(sso, user) + if SiteSetting.verbose_discourse_connect_logging + Rails.logger.warn("Verbose SSO log: User was logged on #{user.username}\n\n#{sso.diagnostics}") + end + log_on_user(user) if user.id != current_user&.id + end + def create params.require(:login) params.require(:password) @@ -612,4 +637,49 @@ class SessionController < ApplicationController def sso_url(sso) sso.to_url end + + # the invite_key will be present if set in InvitesController + # when the user visits an /invites/xxxx link; however we do + # not want to complete the SSO process of creating a user + # and redeeming the invite if the invite is not redeemable or + # for the wrong user + def validate_invitiation!(sso) + invite_key = secure_session["invite-key"] + return if invite_key.blank? + + invite = Invite.find_by(invite_key: invite_key) + + if invite.blank? + raise Invite::ValidationFailed.new(I18n.t("invite.not_found", base_url: Discourse.base_url)) + end + + if invite.redeemable? + if !invite.is_invite_link? && sso.email != invite.email + raise Invite::ValidationFailed.new(I18n.t("invite.not_matching_email")) + end + elsif invite.expired? + raise Invite::ValidationFailed.new(I18n.t('invite.expired', base_url: Discourse.base_url)) + elsif invite.redeemed? + raise Invite::ValidationFailed.new(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)) + end + + invite + end + + def redeem_invitation(invite, sso) + InviteRedeemer.new( + invite: invite, + username: sso.username, + name: sso.name, + ip_address: request.remote_ip, + session: session, + email: sso.email + ).redeem + secure_session["invite-key"] = nil + + # note - more specific errors are handled in the sso_login method + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e + Rails.logger.warn("SSO invite redemption failed: #{e}") + raise Invite::RedemptionFailed + end end diff --git a/app/models/invite.rb b/app/models/invite.rb index f376db0e653..da46b38dcd8 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -2,6 +2,8 @@ class Invite < ActiveRecord::Base class UserExists < StandardError; end + class RedemptionFailed < StandardError; end + class ValidationFailed < StandardError; end include RateLimiter::OnCreateRecord include Trashable @@ -31,7 +33,6 @@ class Invite < ActiveRecord::Base validates :email, email: true, allow_blank: true validate :ensure_max_redemptions_allowed validate :user_doesnt_already_exist - validate :ensure_no_invalid_email_invites before_create do self.invite_key ||= SecureRandom.hex @@ -68,6 +69,10 @@ class Invite < ActiveRecord::Base email.blank? end + def redeemable? + !redeemed? && !expired? && !destroyed? && link_valid? + end + def redeemed? if is_invite_link? redemption_count >= max_redemptions_allowed @@ -163,20 +168,23 @@ class Invite < ActiveRecord::Base end def redeem(email: nil, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil, session: nil) - if !expired? && !destroyed? && link_valid? - raise UserExists.new I18n.t("invite_link.email_taken") if is_invite_link? && UserEmail.exists?(email: email) - email = self.email if email.blank? && !is_invite_link? - InviteRedeemer.new( - invite: self, - email: email, - username: username, - name: name, - password: password, - user_custom_fields: user_custom_fields, - ip_address: ip_address, - session: session - ).redeem + return if !redeemable? + + if is_invite_link? && UserEmail.exists?(email: email) + raise UserExists.new I18n.t("invite_link.email_taken") end + + email = self.email if email.blank? && !is_invite_link? + InviteRedeemer.new( + invite: self, + email: email, + username: username, + name: name, + password: password, + user_custom_fields: user_custom_fields, + ip_address: ip_address, + session: session + ).redeem end def self.redeem_from_email(email) @@ -254,14 +262,6 @@ class Invite < ActiveRecord::Base end end end - - def ensure_no_invalid_email_invites - return if email.blank? - - if SiteSetting.enable_discourse_connect? - errors.add(:email, I18n.t("invite.disabled_errors.discourse_connect_enabled")) - end - end end # == Schema Information diff --git a/app/services/user_activator.rb b/app/services/user_activator.rb index 49672b04ffe..6a7d8ad1b05 100644 --- a/app/services/user_activator.rb +++ b/app/services/user_activator.rb @@ -33,7 +33,7 @@ class UserActivator if !user.active? EmailActivator - elsif SiteSetting.must_approve_users? && !(invite.present? && !invite.expired? && !invite.destroyed? && invite.link_valid?) + elsif SiteSetting.must_approve_users? && !(invite.present? && invite.redeemable?) ApprovalActivator else LoginActivator diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 53541b81a1f..fbed6eee272 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -229,6 +229,7 @@ en: expired: "Your invite token has expired. Please contact staff." not_found: "Your invite token is invalid. Please contact staff." not_found_json: "Your invite token is invalid. Please contact staff." + not_matching_email: "Your email address and the email address associated with the invite token do not match. Please contact staff." not_found_template: |

Your invite to %{site_name} has already been redeemed.

@@ -2359,6 +2360,7 @@ en: blank_id_error: "The `external_id` is required but was blank" email_error: "An account could not be registered with the email address %{email}. 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." original_poster: "Original Poster" most_posts: "Most Posts" diff --git a/lib/guardian.rb b/lib/guardian.rb index f65029bd1b7..f9488f951ce 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -351,14 +351,21 @@ class Guardian end def can_invite_to_forum?(groups = nil) - authenticated? && - (SiteSetting.max_invites_per_day.to_i > 0 || is_staff?) && - !SiteSetting.enable_discourse_connect && - ( - (!SiteSetting.must_approve_users? && @user.has_trust_level?(SiteSetting.min_trust_level_to_allow_invite.to_i)) || - is_staff? - ) && - (groups.blank? || is_admin? || groups.all? { |g| can_edit_group?(g) }) + return false if !authenticated? + + invites_available = SiteSetting.max_invites_per_day.to_i.positive? + trust_level_requirement_met = !SiteSetting.must_approve_users? && @user.has_trust_level?(SiteSetting.min_trust_level_to_allow_invite.to_i) + + if !is_staff? + return false if !invites_available + return false if !trust_level_requirement_met + end + + if groups.present? + return is_admin? || groups.all? { |g| can_edit_group?(g) } + end + + true end def can_invite_to?(object, groups = nil) @@ -390,7 +397,8 @@ class Guardian def can_invite_via_email?(object) return false unless can_invite_to?(object) - !SiteSetting.enable_discourse_connect && SiteSetting.enable_local_logins && (!SiteSetting.must_approve_users? || is_staff?) + (SiteSetting.enable_local_logins || SiteSetting.enable_discourse_connect) && + (!SiteSetting.must_approve_users? || is_staff?) end def can_bulk_invite_to_forum?(user) diff --git a/lib/single_sign_on.rb b/lib/single_sign_on.rb index 729b3df2006..a4c0bab2e75 100644 --- a/lib/single_sign_on.rb +++ b/lib/single_sign_on.rb @@ -117,6 +117,10 @@ class SingleSignOn OpenSSL::HMAC.hexdigest("sha256", secret, payload) end + def to_json + self.to_h.to_json + end + def to_url(base_url = nil) base = "#{base_url || sso_url}" "#{base}#{base.include?('?') ? '&' : '?'}#{payload}" @@ -128,6 +132,10 @@ class SingleSignOn end def unsigned_payload + Rack::Utils.build_query(self.to_h) + end + + def to_h payload = {} ACCESSORS.each do |k| @@ -139,7 +147,6 @@ class SingleSignOn payload["custom.#{k}"] = v.to_s end - Rack::Utils.build_query(payload) + payload end - end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 04a051ad3e5..da2b9b59725 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -511,14 +511,6 @@ describe Guardian do expect(Guardian.new(user).can_invite_to_forum?).to be_falsey end - it 'returns false when DiscourseConnect is enabled' do - SiteSetting.discourse_connect_url = "https://www.example.com/sso" - SiteSetting.enable_discourse_connect = true - - expect(Guardian.new(user).can_invite_to_forum?).to be_falsey - expect(Guardian.new(moderator).can_invite_to_forum?).to be_falsey - end - context 'with groups' do let(:groups) { [group, another_group] } @@ -691,13 +683,13 @@ describe Guardian do expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_truthy end - it 'returns false for all users when sso is enabled' do + it 'returns true for all users when sso is enabled' do SiteSetting.discourse_connect_url = "https://www.example.com/sso" SiteSetting.enable_discourse_connect = true - expect(Guardian.new(trust_level_2).can_invite_via_email?(topic)).to be_falsey - expect(Guardian.new(moderator).can_invite_via_email?(topic)).to be_falsey - expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_falsey + expect(Guardian.new(trust_level_2).can_invite_via_email?(topic)).to be_truthy + expect(Guardian.new(moderator).can_invite_via_email?(topic)).to be_truthy + expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_truthy end it 'returns false for all users when local logins are disabled' do diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index a84427a3f97..bda281c9309 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -50,17 +50,6 @@ describe Invite do end end - context "DiscourseConnect validation" do - it "prevents creating an email invite when DiscourseConnect is enabled" do - SiteSetting.discourse_connect_url = "https://www.example.com/sso" - SiteSetting.enable_discourse_connect = true - - invite = Fabricate.build(:invite, email: "test@mail.com") - expect(invite).not_to be_valid - expect(invite.errors.details[:email].first[:error]).to eq(I18n.t("invite.disabled_errors.discourse_connect_enabled")) - end - end - context '#create' do context 'saved' do subject { Fabricate(:invite) } diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index f428518328a..8ff7c606bd8 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -1374,23 +1374,6 @@ describe GroupsController do expect(response.status).to eq(200) end - it "rejects unknown emails when DiscourseConnect is enabled" do - SiteSetting.discourse_connect_url = "https://www.example.com/sso" - SiteSetting.enable_discourse_connect = true - put "/groups/#{group.id}/members.json", params: { emails: "newuser@example.com" } - - expect(response.status).to eq(400) - expect(response.parsed_body["error_type"]).to eq("invalid_parameters") - end - - it "rejects unknown emails when local logins are disabled" do - SiteSetting.enable_local_logins = false - put "/groups/#{group.id}/members.json", params: { emails: "newuser@example.com" } - - expect(response.status).to eq(400) - expect(response.parsed_body["error_type"]).to eq("invalid_parameters") - end - it "will find users by email, and invite the correct user" do new_user = Fabricate(:user) expect(new_user.group_ids.include?(group.id)).to eq(false) diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 942bb6046ad..8fd37b887ff 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -51,6 +51,13 @@ describe InvitesController do expect(CGI.unescapeHTML(body)).to_not include(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)) end + it "stores the invite key in the secure session if invite exists" do + get "/invites/#{invite.invite_key}" + expect(response.status).to eq(200) + invite_key = read_secure_session["invite-key"] + expect(invite_key).to eq(invite.invite_key) + end + it "returns error if invite has already been redeemed" do Fabricate(:invited_user, invite: invite, user: Fabricate(:user)) get "/invites/#{invite.invite_key}" @@ -382,16 +389,6 @@ describe InvitesController do expect(response.status).to eq(404) end - it 'returns the right response when DiscourseConnect is enabled' do - invite - SiteSetting.discourse_connect_url = "https://www.example.com/sso" - SiteSetting.enable_discourse_connect = true - - put "/invites/show/#{invite.invite_key}.json" - - expect(response.status).to eq(404) - end - describe 'with authentication session' do let(:authenticated_email) { "foobar@example.com" } diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 9fd1ae9edd0..b065fd7e50e 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -659,7 +659,7 @@ RSpec.describe SessionController do def sso_for_ip_specs sso = get_sso('/a/') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -694,7 +694,7 @@ RSpec.describe SessionController do it 'respects email restrictions' do sso = get_sso('/a/') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -708,7 +708,7 @@ RSpec.describe SessionController do it 'allows you to create an admin account' do sso = get_sso('/a/') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -735,7 +735,7 @@ RSpec.describe SessionController do it 'redirects to a non-relative url' do sso = get_sso("#{Discourse.base_url}/b/") - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -748,7 +748,7 @@ RSpec.describe SessionController do SiteSetting.discourse_connect_allows_all_return_paths = true sso = get_sso('https://gusundtrout.com') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -759,7 +759,7 @@ RSpec.describe SessionController do it 'redirects to root if the host of the return_path is different' do sso = get_sso('//eviltrout.com') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -770,7 +770,7 @@ RSpec.describe SessionController do it 'redirects to root if the host of the return_path is different' do sso = get_sso('http://eviltrout.com') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -783,7 +783,7 @@ RSpec.describe SessionController do group = Fabricate(:group, name: :bob, automatic_membership_email_domains: 'bob.com') sso = get_sso('/a/') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -820,11 +820,106 @@ RSpec.describe SessionController do expect(logged_on_user.custom_fields["bla"]).to eq(nil) end + context "when an invitation is used" do + let(:invite) { Fabricate(:invite, email: invite_email, invited_by: Fabricate(:admin)) } + let(:invite_email) { nil } + + def login_with_sso_and_invite(invite_key = invite.invite_key) + write_secure_session("invite-key", invite_key) + sso = get_sso("/") + sso.external_id = "666" + sso.email = "bob@bob.com" + sso.name = "Sam Saffron" + sso.username = "sam" + + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + end + + it "errors if the invite key is invalid" do + login_with_sso_and_invite("wrong") + expect(response.status).to eq(400) + expect(response.body).to include(I18n.t("invite.not_found", base_url: Discourse.base_url)) + expect(invite.reload.redeemed?).to eq(false) + expect(User.find_by_email("bob@bob.com")).to eq(nil) + end + + it "errors if the invite has expired" do + invite.update!(expires_at: 3.days.ago) + login_with_sso_and_invite + expect(response.status).to eq(400) + expect(response.body).to include(I18n.t("invite.expired", base_url: Discourse.base_url)) + expect(invite.reload.redeemed?).to eq(false) + expect(User.find_by_email("bob@bob.com")).to eq(nil) + end + + it "errors if the invite has been redeemed already" do + invite.update!(max_redemptions_allowed: 1, redemption_count: 1) + login_with_sso_and_invite + expect(response.status).to eq(400) + expect(response.body).to include(I18n.t("invite.not_found_template", site_name: SiteSetting.title, base_url: Discourse.base_url)) + expect(invite.reload.redeemed?).to eq(true) + expect(User.find_by_email("bob@bob.com")).to eq(nil) + end + + it "errors if the invite is for a specific email and that email does not match the sso email" do + invite.update!(email: "someotheremail@dave.com") + login_with_sso_and_invite + expect(response.status).to eq(400) + expect(response.body).to include(I18n.t("invite.not_matching_email", base_url: Discourse.base_url)) + expect(invite.reload.redeemed?).to eq(false) + expect(User.find_by_email("bob@bob.com")).to eq(nil) + end + + it "allows you to create an account and redeems the invite successfully, clearing the invite-key session" do + login_with_sso_and_invite + + expect(response.status).to eq(302) + expect(response).to redirect_to("/") + expect(invite.reload.redeemed?).to eq(true) + + user = User.find_by_email("bob@bob.com") + expect(user.active).to eq(true) + expect(session[:current_user_id]).to eq(user.id) + expect(read_secure_session["invite-key"]).to eq(nil) + end + + it "allows you to create an account and redeems the invite successfully even if must_approve_users is enabled" do + SiteSetting.must_approve_users = true + + login_with_sso_and_invite + + expect(response.status).to eq(302) + expect(response).to redirect_to("/") + expect(invite.reload.redeemed?).to eq(true) + + user = User.find_by_email("bob@bob.com") + expect(user.active).to eq(true) + end + + it "redirects to the topic associated to the invite" do + topic_invite = TopicInvite.create!(invite: invite, topic: Fabricate(:topic)) + login_with_sso_and_invite + + expect(response.status).to eq(302) + expect(response).to redirect_to(topic_invite.topic.relative_url) + end + + it "adds the user to the appropriate invite groups" do + invited_group = InvitedGroup.create!(invite: invite, group: Fabricate(:group)) + login_with_sso_and_invite + + expect(invite.reload.redeemed?).to eq(true) + + user = User.find_by_email("bob@bob.com") + expect(GroupUser.exists?(user: user, group: invited_group.group)).to eq(true) + end + end + context 'when sso emails are not trusted' do context 'if you have not activated your account' do it 'does not log you in' do sso = get_sso('/a/') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' @@ -838,7 +933,7 @@ RSpec.describe SessionController do it 'sends an activation email' do sso = get_sso('/a/') - sso.external_id = '666' # the number of the beast + sso.external_id = '666' sso.email = 'bob@bob.com' sso.name = 'Sam Saffron' sso.username = 'sam' diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index ce974f3e1dd..e0e7dda210e 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1757,25 +1757,6 @@ describe UsersController do end end - context 'when DiscourseConnect has been enabled' do - before do - SiteSetting.discourse_connect_url = "https://www.example.com/sso" - SiteSetting.enable_discourse_connect = true - end - - it 'explains why invites are disabled to staff users' do - inviter = sign_in(Fabricate(:admin)) - Fabricate(:invite, invited_by: inviter, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) - - get "/u/#{inviter.username}/invited/pending.json" - expect(response.status).to eq(200) - - expect(response.parsed_body['error']).to include(I18n.t( - 'invite.disabled_errors.discourse_connect_enabled' - )) - end - end - context 'with redeemed invites' do it 'returns invites' do sign_in(Fabricate(:moderator)) diff --git a/spec/support/integration_helpers.rb b/spec/support/integration_helpers.rb index 182a5514ee4..45a7b3cfbc6 100644 --- a/spec/support/integration_helpers.rb +++ b/spec/support/integration_helpers.rb @@ -46,4 +46,10 @@ module IntegrationHelpers SecureSession.new(session[:secure_session_id]) end + + def write_secure_session(key, value) + secure_session = read_secure_session + secure_session[key] = value + secure_session + end end