`around_*` hooks, in particular `around_rodauth` (proof-of-concept included)

35 views
Skip to first unread message

Bo Jeanes

unread,
Nov 6, 2020, 6:51:25 AM11/6/20
to Rodauth
Hi Jeremy,

I've recently completed the weeks-long project of moving our app from Devise to Rodauth. So far, pretty seamless! I have some notes and hope to write a blog post as well as publish a lot of the custom features I wrote for the integration, but will depend on time!

One thing that came up, in the context of rodauth-rails, is that there is no good way (i.e. that isn't brittle) to participate in Rails' own action hooks. This is chiefly because Rails' hook code is quite complicated and works by taking the action to run as a block-parameter but Rodauth has no `around_rodauth`.

To discuss possibilities, I've taken the liberty of whipping up a proof-of-concept of what `around` hooks might look like in Rodauth and implemented the hook for the Base feature (`around_rodauth`) based on some back-and-forth with Janko.


The idea would be that rodauth-rails could then do something like:

  def around_rodauth
    rails_controller_instance.run_callbacks(:process_action) do
      # TBD: handle potential response from Rails `before_action` here
      yield
    end
  end

In my actual PoC, I was undecided as to whether `around 'feature'` should also define `before 'feature'` and `after 'feature'` and even whether it should own calling those hooks (so one might only use `around_feature { the_action }` and have all three hooks covered). Presently, I opted not to define them, but to call them if they have been defined.

There is obviously some consideration to be had here around whether `after_<feature>` should be run unconditionally, or only when no failure has happened.

If you are generally supportive of moving in this direction for the benefit of an improved rodauth-rails experience, then the specifics can be ironed out with your design guidance.

Cheers,
Bo


Bo Jeanes

unread,
Nov 6, 2020, 6:52:27 AM11/6/20
to Rodauth
Whoops I forgot to include a link to the discussion on rodauth-rails that prompted this proof-of-concept and discussion:

Jeremy Evans

unread,
Nov 6, 2020, 8:27:11 AM11/6/20
to [email protected]
Bo,

I've looked at the implementation, and other than integration with Rails, I'm not sure what it adds to Rodauth.  The spec example can be handled by a before hook.

I'm not totally opposed to the idea, but I think a simpler approach is warranted, such as:

  # in rodauth.rb
  around_rodauth do
    before_rodauth # keep this so it is always called
    send(internal_handle_meth, request)
  end

  # in base.rb
  auth_private_methods :around_rodauth
  private
  def around_rodauth
    yield
  end

Then you can do:

  around_rodauth do
    rails_controller_instance.run_callbacks(:process_action) do
      super() # super is better than yield here, as features could theoretically add their own hooks.
    end
  end

Basically, I don't see a need for a generic around hook when the hook is only used for the around_rodauth action.  I also don't see a justification for adding an after_rodauth hook, considering that around_rodauth would be flexible enough to handle the need via ensure.  Without the addition of after_rodauth, I don't think the audit_logging change is needed.

Thanks,
Jeremy

Bo Jeanes

unread,
Nov 6, 2020, 11:13:11 AM11/6/20
to [email protected]
Hey Jeremy,

Thanks for taking a look. If I understand correctly you're supportive of adding an `around_rodauth` but don't see the need for a generic `around_*`? That seems super sensible and I believe works for the driving use-case in my mind, so seems fine.

I think the only thing that your simpler proposal forgoes (and mind you, this may be totally fine) is that you can't conditionally skip calling `super()` without also skipping the `before_rodauth` hook. In my mind, it is likely important that, in the rodauth-rails case, a Rails `before_action` which renders/redirects causes the Rodauth route to be skipped. Perhaps in your mind it is in fact more suitable for `before_rodauth` to be skipped here too, but I think that could be surprising.

Is this something you're prepared to add? Would you like a PR? Do you feel there is anything left to discuss first?

Cheers,
Bo

--
You received this message because you are subscribed to a topic in the Google Groups "Rodauth" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/rodauth/kZfPJdVdQI8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/rodauth/CADGZSSd32p8wX-g5ERUY3fS-iDXShPXc9XbCSwgcEohaqqkfuw%40mail.gmail.com.

Jeremy Evans

unread,
Nov 7, 2020, 12:04:19 AM11/7/20
to [email protected]
On Thu, Nov 5, 2020 at 6:13 PM Bo Jeanes <[email protected]> wrote:
Hey Jeremy,

Thanks for taking a look. If I understand correctly you're supportive of adding an `around_rodauth` but don't see the need for a generic `around_*`? That seems super sensible and I believe works for the driving use-case in my mind, so seems fine.

I think the only thing that your simpler proposal forgoes (and mind you, this may be totally fine) is that you can't conditionally skip calling `super()` without also skipping the `before_rodauth` hook. In my mind, it is likely important that, in the rodauth-rails case, a Rails `before_action` which renders/redirects causes the Rodauth route to be skipped. Perhaps in your mind it is in fact more suitable for `before_rodauth` to be skipped here too, but I think that could be surprising.

I think that's fine.  If the around hook decides to skip, the before hook is not called.  That's how Sequel hooks work and it makes the most sense to me.
 
Is this something you're prepared to add? Would you like a PR? Do you feel there is anything left to discuss first?

You did the majority of work on this.  If you think you can make the changes, please submit a PR.  Otherwise, I can probably handle it before the next release.

Thanks,
Jeremy

Bo Jeanes

unread,
Nov 9, 2020, 1:33:07 PM11/9/20
to Rodauth
Sure thing. I'll prepare a PR today or tomorrow.

Janko Marohnić

unread,
Nov 10, 2020, 2:43:40 AM11/10/20
to Rodauth
Thank you both for your work on this.

I'm assuming that, if another Rodauth feature were to utilize the new around_rodauth hook (in my case the one that ships with rodauth-rails), that feature would then override the #_around_rodauth method, and call super? That seems like it would work, I'll give it a shot.

In our case, the goal is for rodauth-rails to automatically handle the Rails controller action callbacks Bo mentioned, so that's why I need to use the new hook on the feature level.

Kind regards,
Janko

Jeremy Evans

unread,
Nov 10, 2020, 4:01:59 AM11/10/20
to [email protected]
On Mon, Nov 9, 2020 at 9:43 AM Janko Marohnić <[email protected]> wrote:
Thank you both for your work on this.

I'm assuming that, if another Rodauth feature were to utilize the new around_rodauth hook (in my case the one that ships with rodauth-rails), that feature would then override the #_around_rodauth method, and call super? That seems like it would work, I'll give it a shot.

In our case, the goal is for rodauth-rails to automatically handle the Rails controller action callbacks Bo mentioned, so that's why I need to use the new hook on the feature level.

Overriding _around_rodauth on a feature level should work fine.  The only time it would cause an issue is if someone uses around_rodauth in their configuration and yields directly instead of calling super.  I doubt that is a significant enough problem to worry about it, but if you think it is a significant issue, I can reconsider the around_rodauth/_around_rodauth split that Bo had previously.

Thanks,
Jeremy

Bo Jeanes

unread,
Nov 10, 2020, 5:53:38 AM11/10/20
to [email protected]
Yeah that's what was on my mind with that split. Let's see how far this more restrained API takes us first :). Thanks everyone!

--
You received this message because you are subscribed to a topic in the Google Groups "Rodauth" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/rodauth/kZfPJdVdQI8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/rodauth/CADGZSSfC8JDxezbECpCWvHs1tmkysfqASB4%3DhhUjZSmyGpLN-Q%40mail.gmail.com.
Reply all
Reply to author
Forward
0 new messages