Suggestion: 2FA - New credentials validation method | Warden strategy

[newfylox]

Hi guys,

I'm adding a 2FA with this gem but I have to completely copy the create action in the sessions_controller and it's dangerous because one may forget this in a future when updating devise_token_auth gem without updating our own copied code, and so a possibility of security vulnerabilities.

I read #1171 and #1280 but I think there could be another solution about implementing a 2FA with this gem.

Option 1

devise_token_auth/app/controllers/devise_token_auth/sessions_controller.rb

Lines 22 to 30 in eeed642

	if (@resource.respond_to?(:valid_for_authentication?) && !@resource.valid_for_authentication? { valid_password }) || !valid_password
	return render_create_error_bad_credentials
	end
	create_and_assign_token
	sign_in(@resource, scope: :user, store: false, bypass: false)
	yield @resource if block_given?

I think before create_and_assign_token, there should have a yield block with the @resource to add another layer of credentials validation, but we cannot have more then 1 yield block per method. So maybe there should have a new method being called with @resource returning true (or @resource) by default and then we have the possibility to override it in our sessions_controller without overriding the whole controller but just that part. And then, if we want, we can render à bad_credentials if it fails to our 2FA.

Option 2

devise_token_auth/app/controllers/devise_token_auth/sessions_controller.rb

Lines 26 to 28 in eeed642

	create_and_assign_token
	sign_in(@resource, scope: :user, store: false, bypass: false)

I don't know if it really makes sense but we could swap create_and_assign_token and sign_in(@resource, scope: :user, store: false, bypass: false) positions and so we have the possibility of adding our own Warden strategy to be effective when calling sign_in. So if it fails, no tokens are created and it's also compatible with devise gem. Because right now, if we do add our Warden strategy, tokens are created for nothing and if it fails and that we have reached of max count tokens (e.i. 10 tokens per session), then it will badly remove the older one.

What do you think?