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