Skip to content

Commit

Permalink
Don't redirect to login page on missing current account
Browse files Browse the repository at this point in the history
Currently, the `#current_account` method will redirect to the login page
if the user is not logged in or their account record has been
deleted/closed. This behaviour was originally added primarily to make
development smoother, where people can delete account records from the
console. Since `rodauth.require_login` and
`rodauth.require_authentication` don't check for existence of the
account record, doing this would result in errors.

However, now that `rodauth.require_account` is a public method, we
recommend using it so that the account record is immediately retrieved
and thus its existence is checked at the beginning of the request. This
works similarly to Devise's `authenticate_user!` controller helper.
`rodauth.require_login` and `rodauth.require_authentication` still have
their place if the developer wants to avoid the performance penalty of
retrieving the account record when it's not needed.

Since now one can easily check for existence of the account record while
checking for authentication, there is no need for `#current_account` to
redirect to the login page on missing account anymore, so we remove it.
This makes `current_account` behave more like Devise, and makes it more
versatile, allowing users to call it to check if the user is logged in
(which people used to Devise attempt to do). It also makes integration
with gems like Audited easier, which attempts to retrieve the current
account, so that it can associate it with audit logs.

This change will be backwards incompatible for applications relying on
graceful recovery when account record of a logged in session has been
closed/deleted. However, given that `#current_account` method logs the
session out when the account record was not found, that error will
disappear on next refresh. It think that's a good trade-off for removing
surprises.
  • Loading branch information
janko committed Sep 11, 2022
1 parent cd5a39c commit 4ec21ff
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 15 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class RodauthApp < Rodauth::Rails::App

# require authentication for /dashboard/* and /account/* routes
if r.path.start_with?("/dashboard") || r.path.start_with?("/account")
rodauth.require_authentication # redirect to login page if not authenticated
rodauth.require_account # redirect to login page if not authenticated
end
end
end
Expand All @@ -208,7 +208,7 @@ class ApplicationController < ActionController::Base
private

def authenticate
rodauth.require_authentication # redirect to login page if not authenticated
rodauth.require_account # redirect to login page if not authenticated
end
end
```
Expand Down Expand Up @@ -727,7 +727,7 @@ For controller tests, you can log in accounts by modifying the session:
```rb
# app/controllers/articles_controller.rb
class ArticlesController < ApplicationController
before_action -> { rodauth.require_authentication }
before_action -> { rodauth.require_account }

def index
# ...
Expand Down Expand Up @@ -951,7 +951,7 @@ end

In addition to Zeitwerk compatibility, this extra layer catches Rodauth redirects
that happen on the controller level (e.g. when calling
`rodauth.require_authentication` in a `before_action` filter).
`rodauth.require_account` in a `before_action` filter).

### Roda app

Expand Down
4 changes: 2 additions & 2 deletions lib/generators/rodauth/templates/app/misc/rodauth_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ class RodauthApp < Rodauth::Rails::App
r.rodauth # route rodauth requests

# ==> Authenticating requests
# Call `rodauth.require_authentication` for requests that you want to
# Call `rodauth.require_account` for requests that you want to
# require authentication for. For example:
#
# # authenticate /dashboard/* and /account/* requests
# if r.path.start_with?("/dashboard") || r.path.start_with?("/account")
# rodauth.require_authentication
# rodauth.require_account
# end

# ==> Secondary configurations
Expand Down
2 changes: 1 addition & 1 deletion lib/rodauth/rails/controller_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def self.included(controller)
end

def current_account(name = nil)
rodauth(name).rails_account || rodauth(name).login_required
rodauth(name).rails_account
end

def rodauth(name = nil)
Expand Down
6 changes: 0 additions & 6 deletions test/integration/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ class ControllerTest < IntegrationTest
test "current account" do
register(login: "user@example.com", verify: true)
assert_text "Authenticated as user@example.com"

capture_io { Account.last.destroy }

visit "/"
assert_text "Please login to continue"
assert_equal "/login", page.current_path
end

test "executing controller methods" do
Expand Down
2 changes: 1 addition & 1 deletion test/rails_app/app/controllers/test_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def auth1
end

def auth2
rodauth.require_authentication
rodauth.require_account

render :template
end
Expand Down
2 changes: 1 addition & 1 deletion test/rails_app/app/misc/rodauth_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class RodauthApp < Rodauth::Rails::App
r.on("assets") { "" }

if r.path == rails_routes.auth1_path
rodauth.require_authentication
rodauth.require_account
end
end
end

0 comments on commit 4ec21ff

Please sign in to comment.