GET verify_authy_installation should redirect if authy_id is nil

[DannyBen]

This is probably an opinionated request, but since we (authy-devise users) have no control over the controller, I am putting it here for consideration.

The route GET /users/verify_authy_installation can be accessed by users manually, without them first visiting GET /users/enable_authy

https://github.com/twilio/authy-devise/blob/cc38aa4bb2018e5f0f193604b14bae96009c9eb7/app/controllers/devise/devise_authy_controller.rb#L92-L94

Shouldn't this code redirect to /users/enable_authy if there is no authy user?

Maybe something along these lines:

if @authy_user.ok?
  render :verify_authy_installation
else
  redirect_to [resource_name, :user_enable_authy]
end

Otherwise, the only way I know about to protect this route from this misuse, is to add such logic in the view, which is undesired (but works):

# app/views/devise/devise_authy/verify_authy_installation.html.slim
- controller.redirect_to :user_enable_authy unless current_user.authy_id

[philnash]

This is an interesting point. If a user does send themselves to the verify page without enabling authy all they can do is submit a verification that will fail.

I guess you're right that we could happily redirect them if they don't have an authy_id. Same for the POST_verify_authy_installation endpoint too.

[DannyBen]

Yup. Although it is not a big deal, since a) they have to intentionally go to that page and b) no real harm is done, we always prefer to not allow URLs to be accessed, unless they can fulfill their purpose, and they are allowed - this is especially true in all things that are auth(entication|orization) related.

[philnash]

Agreed. There's no reason not to fix it though!

[DannyBen]

Thanks for the fixes blitz! Cant wait for the new version.