Open2

user.current_otp の例外について

hwatjphwatjp
$ rails c
Loading development environment (Rails 8.0.2)
[1] pry(main)> user = User.new
=> #<User id: nil, email: "", created_at: nil, updated_at: nil, admin: false, username: nil, otp_secret: nil, consumed_timestep: nil, otp_required_for_login: nil, otp_backup_codes: []>
[2] pry(main)> user.current_otp
NoMethodError: undefined method `tr' for nil (NoMethodError)

        str = str.tr('=', '').upcase
                 ^^^
Did you mean?  try
from $HOME/.rbenv/versions/3.3.6/lib/ruby/gems/3.3.0/gems/rotp-6.3.0/lib/rotp/base32.rb:16:in `decode'
[3] pry(main)> 

これは devise に関連して 2FA のための rotp を実装している User モデルの例外を目にしたところです。

この例外の原因は単に、インスタンスの otp_secretnil の場合に起こるものなのですが、エラーメッセージが適当ではないので気になりました。

hwatjphwatjp

例外発生現場はここです:

https://github.com/mdp/rotp/blob/v6.3.0/lib/rotp/base32.rb#L16

インスタンスの otp_secretnil である場合は、ここの strnil になっていて、 nil について tr メソッドを呼んでいるので、純粋な例外メッセージとしては真っ当ですが、 User#current_otp を呼んだ時の例外としては不適当に思えます。

そこで、どんな例外ならば適当なのかと言うと、どうでしょう? この現場である ROPT::Base32 クラスでは独自に例外クラスを作って使っているようなので、それにあやかるのが良い気もしましたが、それだと「User#current_otp を呼んだ時の例外」としては、やはり適当ではないままに変わりはないと思います。

#current_otp メソッドは devise-two-factor の由来です:

https://github.com/devise-two-factor/devise-two-factor/blob/v6.1.0/lib/devise_two_factor/models/two_factor_authenticatable.rb#L58-L60

したがって例外は devise-two-factor のほうで捕捉して、対応してほしい気もします。原因となる otp_secretnil であることを知っているのもそちら側なのですから。そもそも #current_otp を呼び出すときに先に otp_secret チェックして、例外なり nil なり false なりを返してくれてもいい気もします。少なくとも NoMethodError をそのまま投げっぱなしにするのは、不親切でよくないと思います。