feat: adding device information to the session

[kelkarajay]

This PR introduces saving device information where the session is in use - IP Address, UA, Location (if available)

The priority sequence for Client IP derivation is -

• Cloudflare injected header - True-Client-IP
• Real IP header injected by reverse proxies like nginx X-Real-IP
• X-Forwarded-For header that's a comma separated list with each node's host IP in the request chain
• If none of the above work, fallback to Go's request RemoteAddr

Related issue(s)

• https://github.com/ory-corp/cloud/issues/3011
• Store additional metadata with sessions #2091

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change introduces a new feature.
• I am following the contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security vulnerability.
  If this pull request addresses a security. vulnerability,
  I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
• I have added tests that prove my fix is effective or that my feature works.
• I have added or changed the documentation.

[codecov]

Codecov Report

Merging #2715 (1d6d74e) into master (f002649) will increase coverage by 0.09%.
The diff coverage is 86.22%.

@@            Coverage Diff             @@
##           master    #2715      +/-   ##
==========================================
+ Coverage   75.56%   75.65%   +0.09%     
==========================================
  Files         293      294       +1     
  Lines       17059    17160     +101     
==========================================
+ Hits        12891    12983      +92     
- Misses       3181     3187       +6     
- Partials      987      990       +3     

Impacted Files	Coverage Δ
selfservice/flow/login/hook.go	87.50% <0.00%> (ø)
corpx/faker.go	40.90% <30.76%> (-1.76%)	⬇️
session/handler.go	70.00% <66.66%> (ø)
persistence/sql/persister_session.go	84.88% <76.47%> (-0.16%)	⬇️
selfservice/flow/registration/hook.go	79.62% <100.00%> (ø)
selfservice/strategy/link/strategy_recovery.go	62.60% <100.00%> (ø)
session/expand.go	100.00% <100.00%> (ø)
session/manager_http.go	73.18% <100.00%> (ø)
session/session.go	77.58% <100.00%> (+6.47%)	⬆️
session/test/persistence.go	100.00% <100.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[aeneasr]

Thank you, it's an improvement to the previous iteration! Unfortunately I still have doubts that the data model will work at scale / long-term. Can you please take a look at the comments?

Before doing any additional coding work, please post a comment with the proposed data layout (feel free to ping me in Slack) that you want to implement so that we can get it right before spending a lot of coding time. Thanks! :)

In the future, we should also stick to our design document process ( https://github.com/ory/kratos/issues/new?assignees=&labels=rfc&template=DESIGN-DOC.yml ) when foundational API and data design is being done.

[kelkarajay]

@aeneasr First the minor stuff,

What about devices or clients like in the original proposal? Metadata is very generic and I expect that we will have user-specified metadata in the future, so I don't think that this key name is a good fit for the functionality.

I'll rename the metadata struct to Client. The initial idea was to call it a Log and the DB table session_logs (5bb40d8). This however could cause confusion in terms of how people interpreted it and I renamed it after discussion with @zepatrik.

The model looks like below which is then embedded as a list in the Session.

type Metadata struct {
  ID uuid.UUID `json:"id" faker:"-" db:"id"`
  SessionID uuid.UUID `json:"-" faker:"-" db:"session_id"`
  IPAddress *string `json:"ip_address" db:"ip_address"`
  UserAgent *string `json:"user_agent" db:"user_agent"`
  Location *string `json:"location" db:"location"`
  CreatedAt time.Time `json:"seen_at" faker:"-" db:"created_at"`
  LastSeen time.Time `json:"last_seen" faker:"-" db:"last_seen"`
  NID uuid.UUID `json:"-"  faker:"-" db:"nid"`
}

  // Metadata has history of all clients where the session was used
  Metadata []Metadata `json:"metadata" faker:"-" db:"-"`

The metadata records are inserted in a new table.

The core idea is that it's a 1-X relationship. One session can have multiple Client/logs based on IP address assuming its a machine that is probably switching between a VPN or direct connection or perhaps different locations like coffee shops/home. The unique index for records would be SessionID + IP. (The User Agent is an unreliable piece of information to uniquely identify a device since it changes when the browser is updated for example.)

Last seen would implicate that we need to update the session on every whoami call. That makes the calls very write heavy and can clog up the DB at scale. I think we should avoid this for now!

The last seen field field could be updated only if it exceeds a previous duration threshold. This could be done to save on the number of writes to the database. For example, Github doesn't give you the exact timestamp, rather that the token was used within the last week or two weeks.

The current model helps represent all the IP's the session has been active at. An example below is what could be represented using the current model -

1. Session being used from 2 IP's. Could be VPN that disconnects on a machine.
2. Another session probably used months ago

Session	IP Address	User Agent	Last Seen
ABCSession1	54.12.145.12 (Munich, Germany)	Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36	5 July 2022
ABCSession1	25.12.145.12 (Dresden, Germany)	Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36	1 September 2022
WERSession3	86.123.211.123 (Leipzig, Germany)	Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36	5 April 2021
RTESession2	86.123.211.123 (FFM, Germany)	Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36	5 December 2020

---

Data cleanup

A cascade delete would help keep the records clean with history being removed along with an expired session record.

Thoughts?

[aeneasr]

Perfect, just two things that could be improved in the SQL logic!

[fdelucchijr]

It's exiting seeing this PR progress so close to finish!

[aeneasr]

minor changes to match Ory's code architecture, then it's good to go

[aeneasr]

Awesome! If we move https://github.com/ory/kratos/pull/2715/files#diff-b496b7e4f89db676e41b419d69c98c5b1336cf9b6029e0d819e07be0a79cea28R124 to a separate function (can be private for now!) we can merge this :)