sinatra

Comments and changes to this ticket

• 

  Blake Mizerany April 30th, 2008 @ 11:36 AM

  oh snap. nice. thank you. I'll get this in a soon as I can.

• 

  Ryan Tomayko May 2nd, 2008 @ 03:13 PM

  Wow. This is a lot more simple than I thought it would be. Good stuff, Chrononaut. Here's some food for thought:

  It might be worth investigating the use of Rack::Auth::Basic::Request. It's not well documented so it might be best to dive into the Rack sources: here's a link to lib/rack/auth on Rack's master. There's quite a bit of good, solid functionality in there.

  All you need to use it is to pass the Rack environment hash into a new Rack::Auth::Basic::Request object. From there, you have access to a few useful convenience methods:

  auth = Rack::Auth::Basic::Request.new(@env)
  auth.provided? #=> true when authentication headers present
  auth.basic? #=> true when authentication type is basic
  auth.credentials #=> [ 'username', 'password' ]
  auth.username #=> 'username'
  

  Like I said, the implementation you've come up with is quite simple so using the Rack APIs isn't really buying much in this case. However, it's always a good idea to use what Rack gives us when possible, IMO.

• 

  Ryan Tomayko May 2nd, 2008 @ 03:39 PM

  One thing that worries me a bit about this patch is that it adds some fairly generic method names to EventContext: #authenticate, #authorization, #username_and_password, #decode_credentials, etc. Seems like there's a good chance these are going to clash with app-specific helper methods or future authentication related code. I suppose we can deal with the latter when it comes up but the former seems worth preventing.

  For example, suppose someone has implemented form/session-based auth using a helper method named "authenticate":

  helpers do
    def authenticate
      if session[:user].nil?
        ...
      end
    end
  end
  
  get '/admin' do
    authenticate
    ...
  end
  

  Wouldn't this cause problems? Seems like this type of thing could be avoided with some simple method renaming or pulling the code from within those methods up into the very-specifically named public methods.

• 

  Chrononaut May 3rd, 2008 @ 02:48 AM

  Thanks for your feedback Ryan! Yes, I suspected Rack might have utility methods for this, but as you said the implementation turned out okay without them, so I didn't investigate further at that point. I agree with your view on using the Rack API as much as possible though, so I'll try to make them to use.

  On your second point: Rails solves this by placing the public and private methods into two separate modules, and only includes the public one into ActionController. I had something in mind here - I just forgot to do it. I'll fix this right now.

• 

  Chrononaut May 3rd, 2008 @ 07:20 AM

  Rack::Auth::Basic::Request made the implementation a whole lot cleaner. :) I suspect this could be taken even further, by utilizing Rack::Auth::Basic.new(app) during the application build. I think it will require storing the login procedure and realm name in Sinatra.options (or similar) though. I dropped it for now since I'm not sure if it's worth it.

• 

  Ryan Tomayko May 3rd, 2008 @ 01:04 PM

  Awesome.

  Regarding Rack::Auth::Basic -- we'll be able to do this very easily once we get our pseudo builder support in. You'll be able to do something like the following right at the top-level of your sinatra file to get basic auth for the entire app:

  use Rack::Auth::Basic do |username,password|
    password == 'secret'
  end
  

• 

  Chrononaut May 12th, 2008 @ 03:12 AM

  Just wondering if there's anything else missing before this can be pulled upstream?

• 

  Blake Mizerany May 12th, 2008 @ 12:02 PM

  Chrononaut,

  My free time. ;) I'm wrapping a project at work and will pull soon. Thank you for your work!

• 

  Chrononaut May 16th, 2008 @ 02:22 AM

  Ah, no problem. :) Btw I tried to clean up the history by rebasing all the small commits into a large one, but I got into some trouble (my git-fu is not that strong). Maybe you can clean it up on your part when you pull. It's not a big deal of course.

• 

  Ryan Tomayko September 7th, 2008 @ 06:29 PM

  • → Tag changed from “” to “auth http patch”
  • → Assigned user changed from “Blake Mizerany” to “Ryan Tomayko”
• 

  Ryan Tomayko September 14th, 2008 @ 03:17 AM

  • → Milestone cleared.
  • → State changed from “new” to “open”

  Pushing to 0.4. I'd like to keep new features in 0.3.x to a minimum so that we're not burdening 0.4 with even more work for backward compatibility.

• 

  Ryan Tomayko December 30th, 2008 @ 02:45 AM

  • → Milestone changed from “” to “0.9.0 Hoboken”
  • → State changed from “open” to “hold”

  Sorry, moving to hoboken and putting on hold for now.

• 

  Ryan Tomayko December 30th, 2008 @ 03:00 AM

  • → State changed from “hold” to “new”
• 

  Ryan Tomayko January 7th, 2009 @ 11:10 AM

  • → Milestone changed from “0.9.0 Hoboken” to “0.9.1”