Skip to content

Commit

Permalink
Invite link can't be used to log in after you set a password or sign …
Browse files Browse the repository at this point in the history
…in with 3rd party
  • Loading branch information
nlalonde committed Jan 21, 2014
1 parent 1dbc1c5 commit da82545
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 5 deletions.
3 changes: 3 additions & 0 deletions app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ def create
return
end

# User signed on with username and password, so let's prevent the invite link
# from being used to log in (if one exists).
Invite.invalidate_for_email(user.email)
else
invalid_credentials
return
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ def user_found(user)
# log on any account that is active with forum access
if Guardian.new(user).can_access_forum? && user.active
log_on_user(user)
# don't carry around old auth info, perhaps move elsewhere
session[:authentication] = nil
Invite.invalidate_for_email(user.email) # invite link can't be used to log in anymore
session[:authentication] = nil # don't carry around old auth info, perhaps move elsewhere
@data.authenticated = true
else
if SiteSetting.must_approve_users? && !user.approved?
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ def password_reset
raise Discourse::InvalidParameters.new(:password) unless params[:password].present?
@user.password = params[:password]
@user.password_required!
logon_after_password_reset if @user.save
if @user.save
Invite.invalidate_for_email(@user.email) # invite link can't be used to log in anymore
logon_after_password_reset
end
end
render layout: 'no_js'
end
Expand Down
19 changes: 17 additions & 2 deletions app/models/invite.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ class Invite < ActiveRecord::Base
def user_doesnt_already_exist
@email_already_exists = false
return if email.blank?
if User.where("email = ?", Email.downcase(email)).exists?
u = User.where("email = ?", Email.downcase(email)).first
if u && u.id != self.user_id

This comment has been minimized.

Copy link
@riking

riking Jan 22, 2014

Contributor

What's the purpose of this logic? Allowing users to invite themselves?

This comment has been minimized.

Copy link
@nlalonde

nlalonde Jan 23, 2014

Author Member

self is the invite record, not yourself.

There was a bug before this change. If you try to do the following:

invite.invalidated_at = Time.zone.now
invite.save

That will fail with an error that the email of the invite record is invalid. The above check makes it so you're allowed to update the invite record if it's the one that created the user in the first place. A new invite record will have a null user_id, so you can't invite yourself and you can't create an invite record if a user exists with the same email address.

This comment has been minimized.

Copy link
@riking

riking Jan 23, 2014

Contributor

Aaah, okay! Thanks for explaining it.

@email_already_exists = true
errors.add(:email)
end
Expand All @@ -40,8 +41,13 @@ def expired?
created_at < SiteSetting.invite_expiry_days.days.ago
end

# link_valid? indicates whether the invite link can be used to log in to the site
def link_valid?
invalidated_at.nil?
end

def redeem
InviteRedeemer.new(self).redeem unless expired? || destroyed?
InviteRedeemer.new(self).redeem unless expired? || destroyed? || !link_valid?
end


Expand Down Expand Up @@ -100,6 +106,15 @@ def self.filter_by(email_or_username)
rails4? ? all : scoped
end
end

def self.invalidate_for_email(email)
i = Invite.where(email: Email.downcase(email)).first
if i
i.invalidated_at = Time.zone.now
i.save
end
i
end
end

# == Schema Information
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20140121204628_add_invalidated_at_to_invites.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddInvalidatedAtToInvites < ActiveRecord::Migration
def change
add_column :invites, :invalidated_at, :datetime
end
end
29 changes: 29 additions & 0 deletions spec/models/invite_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@
invite.redeem.should be_blank
end

it "won't redeem an invalidated invite" do
invite.invalidated_at = 1.day.ago
invite.redeem.should be_blank
end

context 'invite trust levels' do

it "returns the trust level in default_invitee_trust_level" do
Expand Down Expand Up @@ -350,4 +355,28 @@
expect(invites.first).to eq redeemed_invite
end
end

describe '.invalidate_for_email' do
let(:email) { 'invite.me@example.com' }
subject { described_class.invalidate_for_email(email) }

it 'returns nil if there is no invite for the given email' do
subject.should == nil
end

it 'sets the matching invite to be invalid' do
invite = Fabricate(:invite, invited_by: Fabricate(:user), user_id: nil, email: email)
subject.should == invite
subject.link_valid?.should == false
subject.should be_valid
end

it 'sets the matching invite to be invalid without being case-sensitive' do
invite = Fabricate(:invite, invited_by: Fabricate(:user), user_id: nil, email: 'invite.me2@Example.COM')
result = described_class.invalidate_for_email('invite.me2@EXAMPLE.com')
result.should == invite
result.link_valid?.should == false
result.should be_valid
end
end
end

0 comments on commit da82545

Please sign in to comment.