Skip to content

feat: make auto signout possible in AshAuthentication.Phoenix #1070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: make auto signout possible in AshAuthentication.Phoenix #1070

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/ash_authentication/token_resource.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ defmodule AshAuthentication.TokenResource do
doc:
"The name of the action used to retrieve tokens from the store, if `require_tokens_for_authentication?` is enabled in your authentication resource.",
default: :get_token
],
endpoints: [
type: {:wrap_list, {:behaviour, Phoenix.Endpoint}},
doc:
"The list of the endpoints where we will propagate the disconnect notification, when the user logs out or triggers log out from all devices.",
default: []
],
live_socket_id_template: [
type: {:fun, 1},
Copy link
Collaborator

@jimsynz jimsynz Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, except I think we should support functions which take both the token record and the context, rather than just the token record.

Copy link
Author

@moutikabdessabour moutikabdessabour Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing I forgot to mention is this the function should handle both the token record and the claims map.

there should be a better way to do this but I am only 2 months into the Ash rabbit hole

  token do
    live_socket_id_template(fn
      %{jti: jti} -> "users_socket:#{jti}"
      %{"jti" => jti} -> "users_socket:#{jti}"
    end)

    endpoints(TaleedWeb.Endpoint)
  end

Copy link
Collaborator

@jimsynz jimsynz Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case maybe the type for this function should be (token_record :: Ash.Resource.record(), context ::%{:atom => any}, claims :: %{String.t => String.t} -> String.t()) that way the user can decide for themselves what information is relevant.

Copy link
Author

@moutikabdessabour moutikabdessabour Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is they should only use the information available in the claims and token record. [i.e the intersection or we could convert the token record into claims inside the notifier or the other way inside set_live_socket_id] this way the user has only to worry about one type of inputs and avoids repetition.

For the context, from where will we fetch it

Copy link
Author

@moutikabdessabour moutikabdessabour Sep 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimsynz Is there a way we can get the actual token record to success? I think this will be a major change to the API, and all the strategies will need to modified. I can just fetch the token from the db, this will be ran once on the signin success not a performance hit tbh

TokenResource.Actions.get_token(
                 token_resource,
                 %{
                   "jti" => jti,
                   "purpose" => "user"
                 }
               )

for the context do you mean :tenant?

Copy link
Collaborator

@jimsynz jimsynz Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so let's take a step back and ask what circumstances the user might need the actual token record and not just the claims?

Copy link
Author

@moutikabdessabour moutikabdessabour Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to make it easier for the user, instead of having him implement 2 functions, he will only have to handle the case where he gets the token record

Copy link
Collaborator

@jimsynz jimsynz Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zachdaniel do you have a take on this? I'm conflicted and need a grown up to hold my hand.

doc:
"Either a template string or a function that takes in the token resource and returns the livesocket id."
]
],
sections: [
Expand Down
Loading