devise_invitable icon indicating copy to clipboard operation
devise_invitable copied to clipboard

Prevent resending of confirmation mail before invitation is accepted

Open TheFlow0360 opened this issue 8 years ago • 9 comments
trafficstars

I'm using confirmable together with invitable. Users can only be invited, sign up is disabled. Confirmable is thus only needed to confirm changes of the email. The problem is, Newly invited users can "resend" the confirmation mail and click the link, instead of accepting the invitation, thus logging them in without letting them change the password.

I found this issue regarding disabling the "reset password", I need something similar for the "resend confirmation". How do I do this? Also I'd like to show a message, instead of silently failing this as in the issue mentioned.

Maybe both features (disabling password reset and resend confirmation confirm) could be added as options to invitable...

TheFlow0360 avatar Mar 08 '17 15:03 TheFlow0360

I looked into the devise sources and found resend_confirmation_instructions which can do what I want. But, I still let this fail silently. How do I prevent that the success message is displayed?

TheFlow0360 avatar Mar 08 '17 15:03 TheFlow0360

Probably it could be fixed with this method:

def pending_any_confirmation
  super if invitation_token.nil?
end

In that case users which didn't accept invitation are not allowed to resend confirmation. It doesn't make sense to resend confirmation if invitation not accepted, and devise_invitable should never allow it, I think invited users were confirmed previously, but now they are confirmed only when invitation is accepted.

If you can try that code and works, it would be very nice to get a pull request with that method and a test which tries to resend confirmation and verify no email is sent, I don't have time to do it by myself now.

scambra avatar Mar 08 '17 17:03 scambra

Thanks for your fast response. Sadly, this didn't have any effect for me. I'm currently using this in my user model:

def send_reset_password_instructions
  super if invitation_token.nil?
end

def resend_confirmation_instructions
  super if invitation_token.nil?
end

But as I said, this fails silently, meaning devise still tells the user "you will receive a mail ..." even though it should give a warning / an error. I won't create a pull request until I know a way to fix this. Maybe there is a good way to do this, but since this isn't my top priority right now it could take a while until I have the time to tackle this problem myself.

TheFlow0360 avatar Mar 09 '17 08:03 TheFlow0360

confirmations controller check if anything was sent with this method:

def successfully_sent?(resource)
    notice = if Devise.paranoid
      resource.errors.clear
      :send_paranoid_instructions
    elsif resource.errors.empty?
      :send_instructions
    end

    if notice
      set_flash_message! :notice, notice
      true
    end
  end

So an error needs to be added to avoid displaying success message. I still thinks pending_any_confirmation is the right method to override, error could be added there:

def pending_any_confirmation
  if invitation_token.present?
    errors.add ...
    false
  else
    super
  end
end

scambra avatar Mar 09 '17 10:03 scambra

This way it works, thanks a lot. I'm going to look into a way to do this for password reset, too. When I have a solution I'll create a pull request. Thanks for your help so far.

TheFlow0360 avatar Mar 10 '17 08:03 TheFlow0360

I was able to use exactly the same solution for send_reset_password_instructions, too, so now it works as intended.

But I have a problem: I'm working on a Windows and I can't seem to get the tests running... So no pull request for now.

TheFlow0360 avatar Mar 10 '17 08:03 TheFlow0360

@TheFlow0360 just encountered this issue today, so looks like it's still a problem. Don't suppose you still have that code to hand? 😬

jalada avatar Sep 21 '21 09:09 jalada

@jalada Sorry, a bit late to see this - do you still need the code? what I did was adding the following to my user model:

  def send_reset_password_instructions
    if invitation_token.present?
      errors.add :base, 'There is an open invitation for this account.'
      false
    else
      super
    end
  end

  def pending_any_confirmation
    if invitation_token.present?
      errors.add :base, 'There is an open invitation for this account.'
       false
    else
      super
    end
  end

However, I'm not entirely sure if that's everything that was needed. In case you need further help I could give you access to the full code (it was just an university project).

TheFlow0360 avatar Nov 09 '21 08:11 TheFlow0360

Thanks @TheFlow0360, no worries, this is useful. We're not actively developing the project this was for right now, but I'm bookmarking this for when we come back to it! ⭐

jalada avatar Nov 09 '21 09:11 jalada