fnando / coupons

Coupons is a Rails engine for creating discount coupons.
MIT License
141 stars 113 forks source link

HTTP BASIC AUTHENTICATION SECURITY CONCERN #17

Open kranthi1027 opened 8 years ago

kranthi1027 commented 8 years ago

Hi. I've started using this gem to generate coupons and its so easy and works like a charm. Saved me a hell lot of time.

The only Issue is I have is using basic authentication to authorize the person who creates the coupons.

I've read in the following links that http basic authentication is not that safe.

http://security.stackexchange.com/questions/988/is-basic-auth-secure-if-done-over-https

http://stackoverflow.com/questions/3323245/is-basic-access-authentication-secure

I was thinking is there a way where I can use my devise authentication to give access to admin(person who creates the coupons) so that it would become much secure.

I'm not even sure if that works or is a viable option. Its just a thought I have.

If we can use devise authentication and serves the purpose of security then how do we do it?

Thanks.

cabgfx commented 8 years ago

I've solved this in my use of the gem, perhaps my solution can help you. I don't use Devise, so your mileage may vary slightly.

Essentially, you want to allow the Coupon engine to know about your app’s controller(s). This certainly crosses an otherwise clear boundary, but it's the only way I could get it working.

At least it's a simple fix: Here's what I did:

Update /initializers/coupons.rb:

  config.authorizer = proc do |controller|
    # You most likely want to skip authentication on AJAX requests to the /apply endpoint,
    # so you can let the user “check” their coupon, and possibly update price in the UI, before proceeding
    unless controller.action_name == 'apply' && controller.request.xhr?
      # Not sure if this exact approach would work for Devise, but you hopefully get the idea:
      # Do the authentication here, and send the user on their way
      controller.before_filter :authenticate_user!
    end
  end

Finally, you'll most likely have to use the “special” helper main_app to send the correct *_path|url in any redirects you do from your authentication logic. Click for details in the Rails API

As an example, here's my authenticate_user! method:

  def authenticate_user!
    store_location!

    if !session[:access_token]
      session[:access_token] = nil
      redirect_to main_app.signin_path # Need to use `main_app` to accommodate Engines
    end
  end

Again, this is not for Devise — I use a homegrown solution, due to my users being authenticated via OAuth, and I don't save any user info/credentials in my app.

Hope this can at least steer you in the right direction!

kranthi1027 commented 8 years ago

Thanks. Will give it a try.

jwilsjustin commented 8 years ago

You could also use a constraints declaration in your routes:

mount Coupons::Engine => '/', as: 'coupons_engine', constraints: WhitelistConstraint.new

Docs: http://guides.rubyonrails.org/routing.html#advanced-constraints

megetron commented 8 years ago

@kranthi1027 , Did you solve this with devise? I could do authentication with devise with @cabgfx advise of editing gem's application controller and add this lines: ` before_action :authenticate_user!

def authenticate_user! unless current_user.admin? flash[:alert] = "You are not authorized to perform this action." redirect_to(request.referrer || "/") end super
end

` Works great!

But I do prefer not editing application controller and do it in the initialiser configuration which is what it meant for to be more clear coding. The problem is that in coupon initialiser it should user request object as devise using it to authenticate user, and such request doesn't exist in initialisers (as expected).

Please let me know if solution found except for the "dirty" one. Thanks,

megetron commented 8 years ago

OK Cool. I keep the application_controller as is without changes and set the following

  config.authorizer = proc do |controller|
    # You most likely want to skip authentication on AJAX requests to the /apply endpoint,
    # so you can let the user “check” their coupon, and possibly update price in the UI, before proceeding
    unless controller.action_name == 'apply' && controller.request.xhr?
      unless controller.current_user.admin?
        controller.flash[:alert] = "You are not authorized to perform this action."
        controller.redirect_to(controller.request.referrer || "/")
      end
    end
  end