Token auth happening on sign_in
james-caresnap opened this issue · 1 comments
If a client sends a valid JWT to the sessions/sign_in controller action, the client can log in even if they post bogus user/password credentials.
Truthfully, this is really a programmer error. The client shouldn't be sending a valid JWT along with a sign in request. However, the current behavior is super confusing and took some time to track down. Raising an error (or really anything else) seems like it would be helpful. In this case the developer was using a networking library which (unbeknownst to the dev) had automatic cookie handling. They reported a bug to me where they could log in with with bogus credentials... and assured me they weren't setting an Authorization header. It was a huge waste of both of our time.
I'd be happy to submit a patch if this seems like a useful addition.
Expected behavior
JWT decoding and login would not execute for a sign_in path, or the token would be revoked as a pre-processing step for the login route.
Actual behavior
If a valid JWT is passed (in a header or cookie) then the jwt_authenticatable strategy kicks in and succeeds regardless of the credentials passed.
Steps to Reproduce the Problem
- hit the sign_in endpoint with valid credentials.
- use the new JWT in a subsequent request to the sign_in endpoint with invalid credentials.
Debugging information
- Version of
devise-jwtin use: 0.8.0 - Version of
railsin use: 6.0.3.3 - Version of
warden-jwt_authin use: 0.5.0 - Output of
Devise::JWT.config:
{
:secret => "[redacted]",
:expiration_time => 3600,
:dispatch_requests => [
[0] [
[0] "POST",
[1] /^\/api\/v1\/users\/sign_in$/
],
[1] [
[0] "POST",
[1] /^\/users\/sign_in$/
],
[2] [
[0] "POST",
[1] /^\/api\/v1\/users\/two_factor\/verify$/
],
[3] [
[0] "POST",
[1] /^\/users\/two_factor\/verify$/
],
[4] [
[0] "POST",
[1] /^\/users\/sign_in$/
],
[5] [
[0] "POST",
[1] /^\/users$/
]
],
:revocation_requests => [
[0] [
[0] "DELETE",
[1] /^\/api\/v1\/users\/sign_out$/
],
[1] [
[0] "DELETE",
[1] /^\/users\/sign_out$/
]
],
:aud_header => "JWT_AUD",
:request_formats => {}
}
- Output of
Warden::JWTAuth.config
{
:secret => "[redacted]"",
:algorithm => "HS256",
:expiration_time => 3600,
:aud_header => "JWT_AUD",
:mappings => {
:user => class User < ApplicationRecord {
:id => :integer,
:email => :string,
:phone => :string,
:encrypted_password => :string,
:reset_password_token => :string,
:reset_password_sent_at => :datetime,
:remember_created_at => :datetime,
:sign_in_count => :integer,
:current_sign_in_at => :datetime,
:last_sign_in_at => :datetime,
:current_sign_in_ip => :inet,
:last_sign_in_ip => :inet,
:confirmation_token => :string,
:confirmed_at => :datetime,
:confirmation_sent_at => :datetime,
:unconfirmed_email => :string,
:failed_attempts => :integer,
:unlock_token => :string,
:locked_at => :datetime,
:created_at => :datetime,
:updated_at => :datetime,
:user_profile_id => :integer,
:encrypted_otp_secret => :string,
:encrypted_otp_secret_iv => :string,
:encrypted_otp_secret_salt => :string,
:consumed_timestep => :integer,
:otp_required_for_login => :boolean
}
},
:dispatch_requests => [
[0] [
[0] "POST",
[1] /^\/api\/v1\/users\/sign_in$/
],
[1] [
[0] "POST",
[1] /^\/users\/sign_in$/
],
[2] [
[0] "POST",
[1] /^\/api\/v1\/users\/two_factor\/verify$/
],
[3] [
[0] "POST",
[1] /^\/users\/two_factor\/verify$/
],
[4] [
[0] "POST",
[1] /^\/users\/sign_in$/
],
[5] [
[0] "POST",
[1] /^\/users$/
]
],
:revocation_requests => [
[0] [
[0] "DELETE",
[1] /^\/api\/v1\/users\/sign_out$/
],
[1] [
[0] "DELETE",
[1] /^\/users\/sign_out$/
]
],
:revocation_strategies => {
:user => class User < ApplicationRecord {
:id => :integer,
:email => :string,
:phone => :string,
:encrypted_password => :string,
:reset_password_token => :string,
:reset_password_sent_at => :datetime,
:remember_created_at => :datetime,
:sign_in_count => :integer,
:current_sign_in_at => :datetime,
:last_sign_in_at => :datetime,
:current_sign_in_ip => :inet,
:last_sign_in_ip => :inet,
:confirmation_token => :string,
:confirmed_at => :datetime,
:confirmation_sent_at => :datetime,
:unconfirmed_email => :string,
:failed_attempts => :integer,
:unlock_token => :string,
:locked_at => :datetime,
:created_at => :datetime,
:updated_at => :datetime,
:user_profile_id => :integer,
:encrypted_otp_secret => :string,
:encrypted_otp_secret_iv => :string,
:encrypted_otp_secret_salt => :string,
:consumed_timestep => :integer,
:otp_required_for_login => :boolean
}
}
}
- Output of
Devise.mappings
{
:user => #<Devise::Mapping:0x00007f914116cd60 @scoped_path="users", @singular=:user, @class_name="User", @klass=#<Devise::Getter:0x00007f914116c9c8 @name="User">, @path="users", @path_prefix=nil, @sign_out_via=:delete, @format=nil, @router_name=nil, @failure_app=Devise::FailureApp, @controllers={:confirmations=>"users/confirmations", :passwords=>"users/passwords", :registrations=>"users/registrations", :sessions=>"users/sessions", :unlocks=>"users/unlocks"}, @path_names={:registration=>"", :new=>"new", :edit=>"edit", :sign_in=>"sign_in", :sign_out=>"sign_out", :password=>"password", :sign_up=>"sign_up", :cancel=>"cancel", :confirmation=>"confirmation", :unlock=>"unlock"}, @modules=[:database_authenticatable, :rememberable, :recoverable, :registerable, :validatable, :confirmable, :lockable, :trackable, :jwt_authenticatable], @routes=[:session, :password, :registration, :confirmation, :unlock], @used_routes=[:session, :password, :registration, :confirmation, :unlock], @used_helpers=[:session, :password, :registration, :confirmation, :unlock]>
}
Cookies have nothing to do with the Authorization header, so probably your problem was related to the Session storage caveat.
Other than that, cascading through all available strategies is inherent in how warden works. Definitely, we don't want to blindly refuse token for sign-in, as it could break other people's workflow. We could add a configurable deny list for paths that we don't want to authenticate and use it in the #valid? method of the strategy, but sincerely I think there's no need as you do need to be very intentional to send the authorization header for a sign in. As I said, your problem probably wasn't related to that as it has nothing to do with cookies.
If you really want to go down that road, you can use a warden hook.
Hope all of this makes sense.