mgomes / api_auth

HMAC authentication for Rails and HTTP Clients
MIT License
480 stars 147 forks source link

Request too old is true when the parameter cannot be parsed #103

Open nwittstruck opened 8 years ago

nwittstruck commented 8 years ago

I've just noticed that if the timestamp sent from the client is invalid (i.e. it cannot be parsed), the request will be denied without any information / log entry. In my opinion, it should at least be logged that the format was invalid. Otherwise, it is quite hard to determine why the request has been denied. What do you think? I'll create a PR if you agree.

def request_too_old?(headers)
  #900 seconds is 15 minutes
  begin
    Time.httpdate(headers.timestamp).utc < (Time.now.utc - 900)
  rescue ArgumentError
    true
  end
end
kjg commented 8 years ago

I agree that it is extremely difficult to know WHY a particular request was not authentic.

I'd love for ApiAuth to provide a way to know exactly what caused a request to fail the check.

I personally would prefer that ApiAuth provide some sort of methods to determine the failure reason as opposed to logging directly. Then it would up to the consumer of the ApiAuth gem to decide exactly what they want to do with the info, weather it be log, or provide a more detailed error response back to the consumer of their API, or whatever else they might want to do with it.