jeremyevans / rodauth

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

Add `#account!` method #309

Closed janko closed 1 year ago

janko commented 1 year ago

I find this method very convenient in other authentication frameworks, and have always wished for it to exist in Rodauth 🙂

In rodauth-rails, I provided an equivalent based on #account_from_session, but have recently run into a scenario where it doesn't work as expected. I could solve it in rodauth-rails, but I thought it would be useful to get it into Rodauth, as the logic is not Rails-specific.

The #current_account method retrieves the account record that's currently logged in, without regard to its status. This allows it to work when the account is unverified without grace period, which can happen when create_account_autologin? is set to true, and only verify_account feature is enabled. I wanted this setup because I'm working on a screencast where I'm rebuilding GitHub authentication using Rodauth, and this is how GitHub's account verification flow seems to work.

I also wanted to handle the case when a Rodauth instance has been allocated without a Roda instance and has @account set, which is the scenario in rodauth-rails when creating emails in a background job, where you just want to grab information for creating an email and don't need the internal_request feature.

I found a few places that could be simplified with the new #current_account method, so I updated those as well 🙂

janko commented 1 year ago

I realized GitHub probably has a grace period for account verification, it just doesn't let you access most routes until you've verified your account. Thus enabling verify_account_grace_period feature should solve my issues that stemmed from using account_from_session. It's still a useful method to have, though 🙂

jeremyevans commented 1 year ago

When building rodauth, it was kind of deliberate that I didn't add a method for this. My idea was that the account should be set by the appropriate method (account_from_*) before being used, so that in a given Rodauth action, you always knew how the account was set. The problem with a method like this is there may be cases where you expect the account to be set by one method, but it isn't, and it falls back to using the session even if cases where you may not expect or want it to. While I think that design is a good defensive approach, obviously we already have code in Rodauth that violates the the approach, because the method is called in multiple contexts, at least one where the account is set and at least one where it isn't. A more defensive approach would be to separate those contexts and use different methods (passing through the appropriate account hash), but then configuration becomes more difficult, so the current situation is the result of a tradeoff between ease of configuration and defensive code.

I think if we did have a method like this, we would want it to use account_from_session, not access the dataset directly. The documentation for the method also would need to be updated, since it will return an already set record for an account not logged in, if the account was set other than by using account_from_session. I don't like the current_account name, mostly because of negative connotations with the way Rails uses Current, though I understand the reasons that rodauth-rails would want to use the name. I think account! may be an acceptable name, as the ! indicates that the user should think twice before using it.

janko commented 1 year ago

Thanks for the feedback 🙂 I agree that current_account could have the ActiveSupport::CurrentAttributes connotation, and it's also a bit vague. I also agree that calling account_from_session is better, especially since I realized I needed verify_account_grace_period for my screencast anyway. I renamed current_account to account! and updated the docs to be more accurate 👍🏻