Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent resending of confirmation mail before invitation is accepted #703

Open
TheFlow0360 opened this issue Mar 8, 2017 · 9 comments
Open

Comments

@TheFlow0360
Copy link

TheFlow0360 commented Mar 8, 2017

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 TheFlow0360 changed the title Prevent resending of confirmation mail before invite is accepted Prevent resending of confirmation mail before invitation is accepted Mar 8, 2017
@TheFlow0360
Copy link
Author

TheFlow0360 commented Mar 8, 2017

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?

@scambra
Copy link
Owner

scambra commented Mar 8, 2017

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.

@TheFlow0360
Copy link
Author

TheFlow0360 commented Mar 9, 2017

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.

@scambra
Copy link
Owner

scambra commented Mar 9, 2017

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

@TheFlow0360
Copy link
Author

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
Copy link
Author

TheFlow0360 commented Mar 10, 2017

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.

@jalada
Copy link

jalada commented Sep 21, 2021

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

@TheFlow0360
Copy link
Author

TheFlow0360 commented Nov 9, 2021

@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).

@jalada
Copy link

jalada commented Nov 9, 2021

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! ⭐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants