jeremyevans / rodauth

Ruby's Most Advanced Authentication Framework
http://rodauth.jeremyevans.net
MIT License
1.67k stars 95 forks source link

Extract request halting into `#return_response` method #232

Closed janko closed 2 years ago

janko commented 2 years ago

This change allows libraries integrating with Rodauth to react on thrown responses by overriding #return_response and #redirect. It also makes the code a bit DRYer.

I would like to use this in rodauth-rails. Currently, when a Rodauth method that redirects or otherwise returns a response is called from within a Rails controller, the response status is missing from Action Controller's instrumentation output. With this change, rodauth-rails' rails feature can override #redirect and #return_response to save the response status, and assign it for instrumentation.

rodauth-rails change ```diff diff --git a/lib/rodauth/rails/controller_methods.rb b/lib/rodauth/rails/controller_methods.rb index 0d80fc0..9114aa3 100644 --- a/lib/rodauth/rails/controller_methods.rb +++ b/lib/rodauth/rails/controller_methods.rb @@ -16,6 +16,13 @@ module Rodauth request.env.fetch ["rodauth", *name].join(".") end + def append_info_to_payload(payload) + super + if request.env["rodauth.rails.status"] + payload[:status] = request.env["rodauth.rails.status"] + end + end + private def rodauth_response diff --git a/lib/rodauth/rails/feature/instrumentation.rb b/lib/rodauth/rails/feature/instrumentation.rb index beba6b9..9d86268 100644 --- a/lib/rodauth/rails/feature/instrumentation.rb +++ b/lib/rodauth/rails/feature/instrumentation.rb @@ -10,6 +10,14 @@ module Rodauth def redirect(*) rails_instrument_redirection { super } + ensure + request.env["rodauth.rails.status"] = response.status + end + + def return_response(*) + super + ensure + request.env["rodauth.rails.status"] = response.status end def rails_render(*) ``` Before: ``` Started GET "/posts" for ::1 at 2022-03-25 14:16:27 +0100 Processing by PostsController#index as HTML Redirected to /login Completed in 4ms (ActiveRecord: 0.0ms | Allocations: 5568) ``` After: ``` Started GET "/posts" for ::1 at 2022-03-25 14:16:39 +0100 Processing by PostsController#index as HTML Redirected to /login Completed 302 Found in 1ms (ActiveRecord: 0.0ms | Allocations: 338) ```
jeremyevans commented 2 years ago

I'm OK with the approach. Should we add a configuration method so that users can override the behavior and call super?

janko commented 2 years ago

Hmm, I consider this more of an internal method. If we provided the return_response configuration method, I feel it might communicate to the developer that it's called every time Rodauth returns a response, which is not the case (it's not called on redirects & Roda routing matchers).

jeremyevans commented 2 years ago

That's a good point. I'll try testing and merging this later today.

jeremyevans commented 2 years ago

Merged at 12f911c75fd3d5e34a842c9991eabbb93adbaca8