feat: After user sign out all other tabs will disconnect

[moutikabdessabour]

Modified the igniter generated auth_controller.ex to auto disconnect user once he logs out

[jimsynz]

Thank you for the contribution @moutikabdessabour

This is great work and definitely something I wanted to add. I'd like to make it a little more generic, however. Do you think we can make the code that works with the live_socket_id into something that is configurable by the Log Out Everywhere Add-on? For example it could be something like:

def success(conn, # ...
  conn 
  |> # ...
  |> set_live_socket_id(user)
  # ...
end
  
def sign_out(conn, # ...
  conn
  |> # ...
  |> disconnect_sessions(user)
end

That way we can add a new disconnect_live_sessions? option to the log out everywhere DSL and the two helper functions can choose between setting a live_socket_id that contains the user resource's primary key so that a broadcast signs out across all connected sessions or just the session's JTI for local-session sign out.

Also, do we need to test it?

[zachdaniel]

@jimsynz its a good point that we should do this on log_out_everywhere as well, and its actually a bit confusing how we'd accomplish it 😅If we use the user's id as the socket id, then signing out anywhere would disconnect all sessions which isn't quite ideal but is manageable. If we use the jti, then we'd have to look up all active sessions and broadcast a log out to those sessions for this to disconnect all sessions on password change. Also, log out everywhere only happens on password change right now, not on log out IIRC also.

[jimsynz]

Oh good point. I think we need to go back and revisit the log out everywhere add on because we should provide a plug helper that triggers it so that folks can opt into it (for example a "log me out everywhere" checkbox on their logout screen).

[jimsynz]

Oh, wait, if we use the JTI to generate the live_socket_id we can add a destroy notifier to publish these events.

[moutikabdessabour]

I think it should be done on the revoke_all_stored_for_subject change, but again the user could change the way the livesocket id is generated, this is why we changed the generated code only. Maybe if we modify the TokenResource DSL to take a livesocket id template function that takes a token and returns a string. @jimsynz @zachdaniel

We would also need to provide an option to specify the endpoint where the reconnect notification will be sent.

Also maybe we want to extend this to if the token expires and the user still has a websocket connection open, i.e he didn't explicitly logout.

I have sth that resembles a PoC for this, using notifiers [as james proposed] but I still need to generalize it + the livesocket_id template thing. Acutally we don't even need to change the generator with this method because revoke_token and revoke_jti would trigger the notifier.

[jimsynz]

We would also need to provide an option to specify the endpoint where the reconnect notification will be sent.

We also need to be able to support multiple endpoints, so a {:wrap_list, {:behaviour, Phoenix.Endpoint}} typed option I guess.

Also maybe we want to extend this to if the token expires and the user still has a websocket connection open, i.e he didn't explicitly logout.

I agree. To manage this we may want to extend the expunger genserver to keep track of any JTI's expiring "soon" and publish disconnects for them at the appropriate time.

[moutikabdessabour]

this is the notifier I implemented after merging #666 this would actually trigger the disconnect the user [:revoke_token, :revoke_jti, :revoke_all_stored_by_subject]

defmodule TokenRevokationNotifier do
  use Ash.Notifier
  require Logger

  def notify(%Ash.Notifier.Notification{
        changeset: %{attributes: "user"}
      }),
      do: :ok

  def notify(%Ash.Notifier.Notification{
        changeset: %{data: %{subject: "user"}}
      }),
      do: :ok

  # data = notification.changeset.data
  def notify(%Ash.Notifier.Notification{
        changeset: %{
          attributes: %{purpose: "revocation", jti: _} = data,
          context: %{changed?: true}
        }
      }),
      do: notify_all(data)

  def notify(%Ash.Notifier.Notification{
        changeset: %{
          attributes: %{purpose: "revocation"},
          data: data,
          context: %{changed?: true}
        }
      }),
      do: notify_all(data)

  def notify(%Ash.Notifier.Notification{
        changeset: %{data: %{purpose: "revocation"} = data, context: %{changed?: true}}
      }),
      do: notify_all(data)

  defp notify_all(%{jti: jti} = data) do
    TaleedWeb.Endpoint.broadcast("users_socket:#{jti}", "disconnect", %{})
  end

  def notify(n) do
    :noop
  end
end

[zachdaniel]

We also need to be able to support multiple endpoints, so a {:wrap_list, {:behaviour, Phoenix.Endpoint}} typed option I guess.

This is generated code right? So they can modify it as needed.

[moutikabdessabour]

I think with the notifier we don't even need to expose this as generated code, basically they can set it on the Dsl

[moutikabdessabour]

• PR feat: make auto signout possible in AshAuthentication.Phoenix  ash_authentication#1070 must be merged before this can go through.
• Update tests. 2 tests now fail both are igniter ones.

Implementation:

• Added 2 Options to the TokenResource token Diesel block AshAuthentication PR:
  • allow for specifying the endpoints type should be {:wrap_list, {:behaviour, Phoenix.Endpoint}}
  • define the livesocket_id template {:fn, 1}
• Created AshAuthentication.Phoenix.TokenRevocationNotifier, that will send disconnects to the specified endpoints on the TokenResource.
• Update the generated TokenResource to have the notifier included in the actions

[jimsynz]

Do we need to handle failure here? We're not verifying the function, it's arity or the return value.

[moutikabdessabour]

I think we should return the Plug.Conn, this is why I added the else. but are you speaking about whether the error path will ever be hit, it would happen because there is no default live_socket_id_template.

I think if I will need to modify the AshAuthentication.TokenResource.Transformer.transform if I want to set one by default

[jimsynz]

Right but what if a user provides a function that raises an exception or has the wrong arity or returns a nonsense value?

[moutikabdessabour]

We could specify that the function should return a tuple, but this still doesn't solve the default value issue.

But if the user like on purpose returns the same livesocket I'd for all users then that's not our issue. As it will just reconnect the websockets [the token would still be valid] so it will pass through