HackToTech

Hack To Technology

APIの権限チェックに@PreAuthorizeを使うのをやめた話

tl;dr

@PreAuthorize@Valid を同時に使用した場合に、評価の順番に対して直感的にはわかりづらい問題がある
Issueとしては↓があたるが、
要はコントローラーのリクエストハンドラーの変数の評価よりもあとに @PreAuthorize の評価がされる為、
本来であれば権限的に叩けないようなAPIに対してもリクエストボディなどの検証が先に動き、403で返すべきところを400で返す問題がある github.com

Issueにコメントしている人もいる通り、 BindingResultをハンドラー側で受け取れば回避も出来るだろうからそうするのも勿論ありではあると思う
ただ既に @Valid を大半に使っていて作りを変える気がない場合などは、 @PreAuthorize をそのまま使うことは控えたほうが個人的には良いと思う

@PreAuthorizeをやめてどうすることにしたか

自前でInterceptor書いてリクエストハンドラーの前に割り込むことにした
とりあえず対象のパスに来たらInterceptorを呼ばせる

@Configuration
class WebMcvConfig(
    private val authorizationInterceptor: AuthorizationInterceptor,
): WebMvcConfigurer {
    override fun addInterceptors(register: InterceptorRegistry) {
        registry.addInterceptor(authorizationInterceptor).addPathPatterns("<対象とするパス>")
    }
}

詳細はドキュメントを見たほうが早いが、
これでControllerのリクエストハンドラーより先に評価させることが出来る
なぜ HandlerMethod かどうかを確認しているかというと、 ResourceHttpRequestHandler が入るケースがあって、今回はその場合は評価させたくなかったのでこうしている

@Component
class AuthorizationInterceptor(
    private val authorizationEvaluator: AuthorizationEvaluator,
): HandlerInterceptor {
    override fun preHandle(
        request: HttpServletRequest,
        response: HttpServletResponse,
        handler: Any,
    ): Boolean {
        if (handler is HandlerMethod) authorizationEvaluator.evaluate(handler.method)
        return true
    }
}
@Component
class AuthorizationEvaluator {
    fun evaluate(method: Method): Boolean {
        // MethodからAnnotationを取得するなりして、権限が足りていればTrueを返して足りなければFalseを返す
    }
}

あとは自前でアノテーションを作って好きに評価することにした

今のところ特に問題がなくうまくいっているが、
もちろん @PreAuthorize のほうが便利だし、
今回はあくまで扱いたいケースが単純だったので楽に実装出来ただけなので、その辺もちゃんと見極めた上でAPIの権限チェックに何を使うかは考えたほうが良いと思う
ただ、ライブラリを使うのに比べれば自前で後で捨てやすいように作るほうが、個人的には好きなので今回はこうしてみた