19
17

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

SQLアンチパターン@Rails④アプリケーション開発

Posted at

パターン19:読み取り可能パスワード

パスワードは平文で保存するな。という話。Railsだと認証にはdeviseを使うのがデファクトスタンダードになってると思うので中でどういう扱いになってるかちょっと見てみたけど、

detabase_authenticatable.rb
module Devise
  # Digests the password using bcrypt.
  def self.bcrypt(klass, password)
    if klass.pepper.present?
      password = "#{password}#{klass.pepper}"
    end
    ::BCrypt::Password.create(password, cost: klass.stretches).to_s
  end
  
  def password=(new_password)
        @password = new_password
        self.encrypted_password = password_digest(@password) if @password.present?
  end

となってる。bcryptとは暗号化アルゴリズムでpepper,strechesというのは

    #   * +pepper+: a random string used to provide a more secure hash. Use
    #     `rake secret` to generate new keys.
    #
    #   * +stretches+: the cost given to bcrypt.

とある。何に使われてるか、ソルトみたいなものかな、と思ってググるといい記事が出てきたので参照されたし。
RailsはデフォルトでBasic認証とDigest認証が利用できるが、Railsガイドの例はソースコードに変数としてuser_name,passwordを平文で仕込んでいて、あんまりセキュアとは言えなさそう。基本的にまともなアプリケーションに対してこんなことはしないと思うけど(個人利用くらいを想定している)。環境変数に埋め込むという手法が提案されている。

パターン20:SQLインジェクション

ユーザー入力やクエリパラメータをそのままコードに反映させると、任意のSQLを実行されて情報を盗み取られてしまう、という脆弱性。以下に詳しい。

http://rails-sqli.org/
http://blog.ohgaki.net/activerecord-sql-injection-patterns
http://qiita.com/sutetotanuki/items/5eda6bbb5532dd64529a
実質的にplace holderを使うとどう安全なのかについては、引数がquoteされるかどうかの差になっている。
http://babiy3104.hateblo.jp/entry/2014/03/05/224525

もしplaceholderを使わないと、文字列がそのまま渡ってしまう。

パターン21:擬似キー潔癖性(シュードキー・ニートフリーク)

idが歯抜けになるのは仕方ないので我慢しようという話。

パターン22:臭いものに蓋(シー・ノー・エビル)

DBが出すエラーを無視しないということ。Railsにおいてはnilを返すメソッドなのか例外を発生させるメソッドなのかの使い分けで対応すればいいと考えられる。

パターン23:外交特権(デイプロマティック・イミュニティ)

DBもちゃんと管理しようという話。バージョン管理とER図の生成がオススメされている。ER図に関しては自動生成してくれるgemがあるようなのでこれを使うと良さそう。バージョン管理は適当にgitとか。migrationを使っていると、roll backが便利だけど、結局migration fileもgit管理してるならあんまり意味ないなあ、と思ってridgepoleを使ってりしている。こちらのほうが可読性があるのと、冪等性が保障されているのが大きい。(ただし、db:migrateを前提としている一部rails moutable engine等との相性が少し悪いので難しい。exportも可能なので対応はできるのだが。)

パターン24:魔法の豆(マジックビーンズ)

ActiveRecordパターンへの警告。テーブルとクラスを1:1対応させているだけだけとゆくゆく辛くなっていく、という話。
この章は難しくてちゃんと理解できている自信がないのだけど、単純にデータコンテナとしてだけのActiveRecordパターンだとロジックがモデルの外に出てしまい、fat controllerになる、という主張のようだ。それについては意識的にモデルのロジックを書くように努めればいいのでは、と思った。それが解決策の*「モデルがActiveRecordを持つ」ようにする*という方針だと思う。

それはそれとして、fat modelという問題が生まれる。コード量が増えて行くにつれてどこかしらが太るのは当然の理のように思えるんだけど、単一責任の法則という視点があって、要するに一つのモデルに色々仕事させるのはよろしくない。しかしながらコントローラーにロジックが流出するのはもっとまずい。というわけで、それぞれの役割ごとに小さいモデルを作って、責任を分散させる、というのが推奨されている方針っぽい。このロジックを担う用のモデルについてはActiveRecordパターンに則っていなくても良い。つまり、データとしてのモデルとロジックを担当するモデルに分割する。

例えば、この記事が参考になる。moduleにしてconcerns/以下に置くのは最悪っぽくて、実質的に同じモデルなのにファイルだけ分散して見通しだけ悪くなっているというその場しのぎですらないと感じる。Railsは一般的な操作を行うような基底クラスを(validator,callbackなどがわかりやすいと思う)用意してくれてるので、まずは自分のモデルの責任をその辺りに分散できないか考えてみるのが良いのではないか。

パターン25:砂の城

意訳:俺たちの戦いはこれからだ

19
17
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
19
17

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?