Class: RuboCop::Cop::DevDoc::Auth::CurrentUserBranching

Inherits:
Base
  • Object
show all
Defined in:
lib/rubocop/cop/dev_doc/auth/current_user_branching.rb

Overview

Forbid branching on authentication state in page-specific code.

## Rationale In a Rails app using Pundit + Devise, ‘current_user` is guaranteed non-nil inside any controller action or view that requires auth —the policy has already denied anonymous visitors. Branching on `if current_user` or `if user_signed_in?` inside that code is therefore either dead code (the branch for nil can never fire) or a signal that the developer was unsure whether the page requires auth.

If the page genuinely serves both anonymous and signed-in visitors, the branching should be explicit and kept in shared/layout code, not sprinkled through action bodies and page views.

❌ Authenticated page branching on auth state (branch is dead code)
# app/views/posts/show.json.jbuilder
if current_user
  json.actions [:edit, :delete]
end

✔️ Shared layout — branching here is the right place
# app/views/layouts/_nav_bar.html.erb
<% if user_signed_in? %>
  <%= render 'profile_menu' %>
<% else %>
  <%= link_to 'Log in', new_user_session_path %>
<% end %>

✔️ Genuinely dual-state page — suppress the cop with a reason comment
def new
  # rubocop:disable DevDoc/Auth/CurrentUserBranching
  # Reason: contact form is intentionally dual-state; pre-fills for signed-in users.
  if current_user
    @form.name = current_user.full_name
  end
  # rubocop:enable DevDoc/Auth/CurrentUserBranching
end

## Patterns flagged

  • ‘if current_user` / `unless current_user` (block or modifier) where the body is non-empty and is not a bare `return` (bare returns are the nil-guard pattern covered by LoadResourceCurrentUserGuard).

  • ‘if user_signed_in?` / `unless user_signed_in?` (any form).

  • ‘if current_user&.foo` and other safe-nav uses of `current_user` as a condition. The `&.` is itself a confession that the dev expects `current_user` to be nil sometimes — i.e., it’s the same anti-pattern as ‘if current_user`, just in disguise: when `current_user` is nil the csend returns nil → branch is dead for anonymous visitors, exactly what the cop prevents.

  • Ternaries: ‘current_user ? a : b`, `user_signed_in? ? a : b`.

  • Hash/argument values: ‘authenticated: user_signed_in?`.

## Allowed paths (Exclude:) By default the cop is silent in:

app/policies/**/*.rb          ← auth-dependent checks belong here
app/helpers/**/*.rb
app/controllers/concerns/**/*.rb
app/views/layouts/**/*        ← display branching (nav bars, etc.)

Override via ‘Exclude:` in your `.rubocop.yml`. Note: literal file paths (e.g. `app/controllers/application_controller.rb`) in `.rubocop.yml` `Exclude:` lists are flagged by the `DevDoc::Test::Lints::NoFileExcludes` lint — they hide the suppression from readers of that file. If `ApplicationController` needs an exception, use an inline `# rubocop:disable` at the specific line with a reason.

## Before disabling inline, consider the Policy The Policy exclusion is not accidental — it’s the canonical Rails home for auth-dependent branching. If the flagged line is an authorization check (‘return if @record.accessible_by?(current_user)`, `if current_user.admin?`, etc.), the right move is almost always to push that check into a Pundit policy method, not disable inline. The controller becomes `return unless policy(@record).access?` — same behaviour, no `current_user` reference in the controller, and the auth logic is reusable + testable in isolation.

Even better: declare ‘glib_authorize_resource` at the controller level. glib-web runs the appropriate policy method before each action automatically, so the per-action `verify_access` / `authorize @record` line is usually not needed at all — the controller body no longer references `current_user` because the auth check has moved out of the action entirely. See `best_practices/backend/en/05_controller.md` item #7 for the canonical pattern.

❌ Auth check in the controller — flagged
def verify_access
  return if @support_question.accessible_by?(current_user)
  # ... token fallback ...
end

✔️ Same check in the Policy — silently allowed
# app/policies/support_question_policy.rb
def access?
  user.present? && record.accessible_by?(user)
end

# in the controller:
def verify_access
  return if policy(@support_question).access?
  # ... token fallback ...
end

NOTE: The cop does not autocorrect — there is no mechanical fix. The right response depends on developer intent: drop the branch (if auth is required), restructure into a shared layout (if dual-state), or add an inline disable with a reason.

Examples:

# bad — inside an authenticated action
def show
  if current_user
    @data = current_user.private_data
  end
end

# bad — safe-nav predicate, same anti-pattern in disguise
def show
  if current_user&.admin?
    admin_thing
  end
end

# bad — inside a page view
# user_signed_in? ? render_private : render_public

# good — bare nil-guard return (LoadResourceCurrentUserGuard rule)
return unless current_user

# good — inside a shared layout file (excluded by default)
# app/views/layouts/_nav.html.erb
if user_signed_in?
  ...
end

Constant Summary collapse

MSG = 
'Avoid branching on auth state in page code. ' \
'If this page serves both anonymous and signed-in users, ' \
'add an inline disable with a reason.'.freeze
AUTH_METHODS = 
%i[current_user user_signed_in?].freeze

Instance Method Summary collapse

Instance Method Details

#on_if(node) ⇒ Object 



148
149
150
151
152
153
 
# File 'lib/rubocop/cop/dev_doc/auth/current_user_branching.rb', line 148

def on_if(node)
  return unless auth_branch?(node)
  return if bare_return_guard?(node)

  add_offense(if_offense_location(node))
end

#on_send(node) ⇒ Object

‘value: user_signed_in?` — condition passed as a value.



156
157
158
159
160
161
 
# File 'lib/rubocop/cop/dev_doc/auth/current_user_branching.rb', line 156

def on_send(node)
  return unless auth_method_call?(node)
  return unless used_as_value?(node)

  add_offense(node.loc.selector)
end