自転車とプログラミング

自転車メーカーに勤める会社員がプログラミングを学ぶ中で感じたことを書きます。最近サービス作りました。

認可を設定するときは「タイミング」を気にしよう

はじめに

この記事は「フィヨルドブートキャンプ Part 2 Advent Calendar 2023」7日目の記事です。

adventar.org

Part1 はこちらです。

adventar.org

前回はオブレゴニアさんの「 黒魔術に入門している(Ruby メタプロ)」でした。 ※タイトルに誤りがあったので修正いたしました

この記事はRailsで作られたappに実装した管理者権限(認可)について、gemに頼らず自力で実装する際に押さえておくべきポイントをまとめています。

ここでいう管理者権限とは、一般ユーザーに対する管理者を指します。管理者は一般ユーザーの状態や内容を編集する権限を持ち、一般ユーザーを削除することもできます。この管理者と一般ユーザーを区別する認可(authorization)をきちんとやるには?、がこの記事の趣旨になります。

私が自作したものをbefore、レビューを受けて修正したものをafter、てな感じにしくじり先生スタイルで書きますので同じようなことをトライする人に届けばいいなと思います。

構成

結論

  • 認可の設定は重要度に応じて処理するタイミングを調整する
  • 権限管理は個別対応ではなくallow listで設定しよう
  • 処理を分離しよう。 ビバ単一責任原則

自分で作った管理者設定

# schema.rb
  create_table "users", force: :cascade do |t|
    # 不要な部分は省略
    t.boolean "admin", default: false, null: false
  end


# app/controller/application.rb
class ApplicationController < ActionController::Base
  def redirect_if_different_user
    return if current_user.admin?

    redirect_to root_path if current_user != User.find(params[:id])
  end
end

# app/controller/user_controller.rb
class UsersController < ApplicationController
  before_action :redirect_if_different_user, only: %i[show]
  # 不要な部分は省略
end

これが自作した管理者権限の構成です。 Userモデルの中でadminカラムを生成し、trueなら管理者、falseなら一般ユーザーに設定。

app/controller/application.rbで「adminカラムがtrueかどうか」「パラメーターで渡されたidは現在ログインしているユーザーかどうか」を確認する認可のメソッドredirect_if_different_userを作りました。 このredirect_if_different_userを各controllerで必要に応じてbefore_actionで呼び出すことでactionの処理が始まる前に管理者かどうかチェックしていました。

たとえばuser/:id/editにアクセスする場合はそのユーザー本人のほか、redirect_if_different_userを通過する管理者がアクセスできますし、ユーザーをupdateやdestoryする場合も同様にredirect_if_different_userを通過する管理者が実行できる権限をもちます。

他のControllerにおいても同様のことをしています。app/controller/user_controller.rbで基本的なactionは一般ユーザーと管理者とで共通に使う形を取ることには若干ながら構築の時短を目指した意図がありましたが。

全アクションに対して個別に認可メソッドを設定する方式になっていたので抜け漏れに気を使わないといけません。 「User自身と管理者は認可する」というように管理者だけを区切るわけではないので「何を対象にしているか」を一読では把握しづらい状態にもなってました。

認可そのものでなくてもredirect_if_different_userの良くない点としてparamsにそのユーザーのidが含まれていることが前提となっていることです。これが「ユーザーアイコンに使用している画像を削除する」という処理の場合は画像のidが渡されてしまっていました。

色々とOKとは言いづらいものではありますので案の定、レビュー※の際に指摘が入ります。

※本件はプログラミングスクール「フィヨルドブートキャンプ」でのお話。フィヨルドブートキャンプでは現役エンジニアのメンターからレビューをもらうことができます。

レビュー

以下、レビューの要約です。

管理者専用機能はroutes.rbでnamespaceを切って、/admins/usersみたいなURLにして、admins/appliction_controllerをつくり、そこで管理権限チェックを一括してやるようにするなどしたほうが安全かと思います。管理系の各controllerはadmins/application_controllerを継承する形をとりましょう。

レビューそのものは多岐に渡っていたものを該当部分のみ抜粋して要約しています。

レビューの前段階として別部分でbefore_actionのような前提的な処理は一括で適用する既定を作って例外を用いる「allow list方式(旧:ホワイトリスト)」が良いとのアドバイスがあり、管理者権限のような認可も同様に一括適用できるように構築すべきだ、との話になっています。

アフター

# routes.rb
  namespace :admin do
    resources :spots, only: [:update, :destroy]
    resources :users, only: [:index, :edit, :update, :destroy] do
      resources :avatars, only: [:destroy]
    end
  end

# admin/application_controller.rb
class Admin::ApplicationController < ApplicationController
  before_action :raise_permission_error

  def raise_permission_error
    raise(ActionController::RoutingError, 'Not Found') if !current_user.admin?
  end
end

ネームスペースでadminを区切ってroutesと controllerごと一般ユーザーと管理者で処理を分離させました。 これによって一般ユーザーと管理者に表示されるViewの段階から認可に基づいて別々の処理を使うことで、controllerに至るまでシンプルになります。

Controllerのディレクトリ、ファイルの構成はこちらの通り。

ディレクトリ構成

一般ユーザーと管理者とで同じ処理を使いつつ、その中で認可をするというぱっと見では理解しづらい状態が解消したうえ、認可メソッドを個別指定する必要がなくなりました。 controllerに関してはある程度一般ユーザーと管理者とで重複する処理を記述する必要もありますが、それ以上に分離したことでの保守性の向上の方がメリットはありそうですね。

まとめ

ビフォーからアフターで認可のタイミングが大きくかわって、admin/...のnamespaceであれば、抜け漏れのしづらい仕組みになりました ビフォーでは一般ユーザーも管理者も同じユーザーとしての処理の流れの中でcontrollerの段階で認可をしていましたが、アフターでは認可をベースにしてviewから一般ユーザーと管理者で表示を出し分けてバックエンドの処理も完全に分離しています。 認可の重要度により重要であればより早く分離して処理はするべきですが、一般ユーザーと遜色ない程度の権限であれば同一controllerに混在させても支障はないかと思います。