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.

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

Session metadata #50

Closed
theangryangel opened this Issue Jan 5, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@theangryangel

theangryangel commented Jan 5, 2018

Hi,

Love the project and I'm considering ripping out a similar implementation in a project at work for phxauth as it's more complete.

One thing on our design document is some additional session metadata so we can display something akin to https://github.com/blog/1658-view-active-browser-sessions;

  • User agent + OS information (if present)
  • Original sign in source IP (for geo-lookup purposes)

If I put together a patch, would this be something you'd be interested in pulling in? Currently I'm thinking of morphing the sessions field into an embeds_many

@riverrun

This comment has been minimized.

Show comment
Hide comment
@riverrun

riverrun Jan 5, 2018

Owner

It would be great to see what ideas you have - about presenting the user device information and about using embeds_many to store the sessions.

Owner

riverrun commented Jan 5, 2018

It would be great to see what ideas you have - about presenting the user device information and about using embeds_many to store the sessions.

@theangryangel

This comment has been minimized.

Show comment
Hide comment
@theangryangel

theangryangel Jan 6, 2018

The initial ideas I had were on reflection perhaps quite naive. I assumed what people might want to store and was thinking of something very simple with fixed fields that I need at work.

The more I've thought about it the better it would be to perhaps slightly alter the sessions map and allow people to store arbitrary data? i.e.

# Adding a 4th optional field to 
Accounts.add_session(user, session_id, System.system_time(:second), %{os: "Linux", source_ip: "1.1.1.1"})

Then in the relevant functions in accounts.ex alter them along the lines of (I realise I'd need to modify more than just this, but I hope you get the idea)

  def add_session(%User{sessions: sessions} = user, session_id, timestamp, metadata) do
    session_data = put_in(metadata[:created_at], timestamp)
    change(user, sessions: put_in(sessions, [session_id], session_data))
    |> Repo.update
end

It's a relatively simple pair of patches, and if it's a concern I guess we could set it up so that it supports both the current sessions map and the proposed one.

Thoughts? :)

theangryangel commented Jan 6, 2018

The initial ideas I had were on reflection perhaps quite naive. I assumed what people might want to store and was thinking of something very simple with fixed fields that I need at work.

The more I've thought about it the better it would be to perhaps slightly alter the sessions map and allow people to store arbitrary data? i.e.

# Adding a 4th optional field to 
Accounts.add_session(user, session_id, System.system_time(:second), %{os: "Linux", source_ip: "1.1.1.1"})

Then in the relevant functions in accounts.ex alter them along the lines of (I realise I'd need to modify more than just this, but I hope you get the idea)

  def add_session(%User{sessions: sessions} = user, session_id, timestamp, metadata) do
    session_data = put_in(metadata[:created_at], timestamp)
    change(user, sessions: put_in(sessions, [session_id], session_data))
    |> Repo.update
end

It's a relatively simple pair of patches, and if it's a concern I guess we could set it up so that it supports both the current sessions map and the proposed one.

Thoughts? :)

@riverrun

This comment has been minimized.

Show comment
Hide comment
@riverrun

riverrun Jan 8, 2018

Owner

At the moment, I think it would be a good idea for you to experiment with what works for you at work, and then report back. At the very least, it can be something we add to the documentation / wiki.

You might also want to look at the elixir1.5 branch to see the changes (there are not many) that are coming soon (version 2.0).

Owner

riverrun commented Jan 8, 2018

At the moment, I think it would be a good idea for you to experiment with what works for you at work, and then report back. At the very least, it can be something we add to the documentation / wiki.

You might also want to look at the elixir1.5 branch to see the changes (there are not many) that are coming soon (version 2.0).

@riverrun

This comment has been minimized.

Show comment
Hide comment
@riverrun

riverrun Sep 2, 2018

Owner

I'm going to close this for now, but in version 2.0 - in the example app, I will probably use a separate table for Sessions, and that may provide you with some of the functionality you want.

Owner

riverrun commented Sep 2, 2018

I'm going to close this for now, but in version 2.0 - in the example app, I will probably use a separate table for Sessions, and that may provide you with some of the functionality you want.

@riverrun riverrun closed this Sep 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment