ueberauth / guardian

Elixir Authentication
MIT License
3.43k stars 382 forks source link

Revocation broken for normal use #606

Closed evadne closed 5 years ago

evadne commented 5 years ago

This is mainly a discussion. Custom server-side revocation (GuardianDb style) is broken when a custom module is used to track tokens and to revoke them remotely.

As per change made in https://github.com/ueberauth/guardian/pull/458 signing out of a specific key ensures that the “after” callback is called and the revoke callback is also called. But usually when the bundled Plug is used, the most obvious way to implement such behaviour will be to call sign_out(conn) with no further arguments, which by default is pattern-matched and picked up by do_sign_out(%{private: private} = conn, impl, :all, opts), which does not call any of the documented callbacks and simply wipes the token from session. This nullifies the benefit of having revocation callbacks as they are never really called under the most popular scenarios.

In an umbrella layout one application may vend the pipeline while another application implements the front-end which triggers sign out. It is not practical to force the knowledge of which Guardian key to empty out to be shared and so guiding people to not use the default form of sign_out without arguments is not practical.

From my view this implies that the do_sign_out function needs to be revised and the token/implementation callbacks always called even when all keys are being signed out.

Belatedly the GuardianDB test that has to do with testing revocations only verify that revocations work, but not that revocations are invoked when sign_out is called: https://github.com/ueberauth/guardian_db/blob/master/test/guardian/db_test.exs