1

I'm using Devise to authenticate User in my Rails 7 app and Pundit for authorization. I would like to extend the standard login flow to check if the user has met the 2FA requirements. 2FA is simple and delivered by an external microservice - the user at each login enters the SMS (text message) code he got on his phone.

In a nutshell new login flow should be:

  • user authentication via Devise
  • allow to authorize if session[:_2fa] == 'success'
  • redirect to provide_code_path if session[:_2fa] == 'failed'

Because the user must go through the 2FA process every time he logs in, I've store such info inside the session params as session[:_2fa]. Now to make all the authorization and redirect dance I want to have access to that session to allow or not. I'm aware of this topic but it's 7y old so maybe today there is some modern approach instead of creating fake model just to get access to the session?

Code below:

# application_controller.rb

class ApplicationController < ActionController::Base
  before_action :authenticate_user!, :authorized_user

  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized

  def authorized_user
    authorize :global, :_2fa_passed?
  end

  private

  def user_not_authorized
    flash[:alert] = t('errors.user_not_authorized')
    redirect_to provide_code_path
  end
end

# global_policy.rb

class GlobalPolicy < ApplicationPolicy
  def _2fa_passed?
    session[:_2fa] == 'success'
  end
end
mr_muscle
  • 2,536
  • 18
  • 61
  • 1
    Two odd things here stand out to me: (1) Devise is for **authentication** ("do I know you?"); Pundit is for **authorisation** ("are you allowed to do this?"). Two-factor authentication should probably only be relevant to Devise/authentication; it's got nothing to do with Pundit/authorisation. (Unless you have the concept of "logged-in users, who didn't complete 2fa, should have restricted permissions"??!!) – Tom Lord Aug 08 '22 at 14:09
  • 1
    And (2) this does not look secure. One could just set the cookie: `_2fa = 'success'` and bypass two-factor authentication. – Tom Lord Aug 08 '22 at 14:11
  • 2
    There are existing/actively maintained libraries for 2fa in rails, such as [this](https://github.com/tinfoil/devise-two-factor); I'd advise against rolling your own potentially-insecure implementation of something like this, if possible. – Tom Lord Aug 08 '22 at 14:12
  • @TomLord Why did you explain me what I already described? lol, that's odd. Anyway, I have to use banking 2FA auth - that's the law, if you think it's better to use some gem instead of secure microservice then well... you're wrong. That's not the point where this `_2fa` will be set (probably it should be inside secure cookies), that's not the point of my question. The question is - is there a better way to pass session params to pundit rather than creating fake model. – mr_muscle Aug 08 '22 at 19:00
  • @TomLord And yes, "logged-in users, who didn't complete 2fa, should have restricted permissions" - they should be redirect to the page which they can re-enter the code they received by text message. It's all in my question. – mr_muscle Aug 08 '22 at 19:02
  • 1
    You did *not* say "my implementation has a big security hole" Setting a cookie as "secure" does not prevent a user from setting it themselves. – Tom Lord Aug 09 '22 at 18:50
  • 1
    My point wasn't "you shouldn't use [insert library here]", or "you should use [insert library here]". My point was "you should not add a back-door to bypass the security check, like `session[:_2fa] == 'success'`". This is like locking the door but leaving the window open. – Tom Lord Aug 09 '22 at 18:51
  • 1
    You can create a custom policy initialiser that takes the `session` variable, or even the `session[:_2fa]` value directly -- policies are just plain ruby objects: https://github.com/varvet/pundit#policies – Tom Lord Aug 09 '22 at 18:54
  • 1
    But besides, as I wrote above, this is really an *authentication* check not an *authorisation* check -- so you might as well forget about `pundit` and just add in `ApplicationController`: `before_action ... raise No2FAError unless session[:_2fa] == 'true'` – Tom Lord Aug 09 '22 at 18:58
  • 1
    But I'm reluctant to post that as a "solution", because it's trivial to bypass. For a **bank**, of all things, that's an unacceptable security vulnerability. – Tom Lord Aug 09 '22 at 18:58
  • 1
    @TomLord the question does not refer to use of `cookies[:_2fa]`, it refers to `session[:_2fa]` which is usually stored in encrypted cookie store and hence is not readable or modifiable without session encryption key. So it's not "trivial to bypass". – Tadas Sasnauskas Apr 19 '23 at 12:32

1 Answers1

0

Pundit documents this in Additional Context section of the readme.

Instead of user instance you can pass pundit user context object which would hold user instance and extra session context like 2FA authentication status.

class UserContext
  attr_reader :user, :second_factor_authenticated

  def initialize(user, second_factor_authenticated)
    @user = user
    @second_factor_authenticated = second_factor_authenticated
  end
end

class ApplicationController
  include Pundit::Authorization

  def pundit_user
    second_factor_authenticated = session[:_2fa] == 'success'
    UserContext.new(current_user, second_factor_authenticated)
  end
end

Then in policy classes rename your user attribute to context and refer to user instance through context.user and 2FA status through context.second_factor_authenticated.

So to answer the question - seven years later it is still perfectly acceptable solution.

Given you're using Devise an alternative could be adding ephemeral attribute second_factor_authenticated to user class and implementing your own .serialize_into_session / .serialize_from_session to store it as part of user "vector" (by default it's only user id + auth salt).

Tadas Sasnauskas
  • 2,183
  • 1
  • 22
  • 24