feat(db): store user agent and last-access time in sessionTokens

[philbooth]

Partially addresses mozilla/fxa-auth-server#983.

Related to mozilla/fxa-auth-server#997.

Contains the following changes:

• Adds userAgent, os, deviceType and lastAccessTime fields to the sessionTokens entity.
• Adds updateSessionToken stored procedure.
• Updates the createSessionToken and sessionToken stored procedures.

Some things I was unsure about, so probably need changing:

• How to handle truncation of long UA strings? In the stored procedure or in the JS function or in fxa-auth-db-server? I'm not doing it anywhere yet.
• I haven't retired the old stored procedures yet. Is that right or wrong?

[philbooth]

• How to handle truncation of long UA strings? In the stored procedure or in the JS function or in fxa-auth-db-server? I'm not doing it anywhere yet.

Answered by this discussion. We should use ua-parser, store readable descriptions rather than raw UA strings and default to the empty string if the parser fails to return a match.

[rfk]

ISTM that lastAccessTime should always equal createdAt here, maybe we don't need to accept it as an additional parameter.

[rfk]

I haven't retired the old stored procedures yet. Is that right or wrong?

That's ok; we can't get rid of them until the next deployment cycle anyway because of backwards-compatibility concerns.

[philbooth]

@rfk, this one has now been updated to include the new fields.

[rfk]

Is userAgent here the full header or something parsed from it? If the later, let's call it something other than "userAgent" for clarity. Perhaps "application" or "browser"?

Is it worth naming these in a way that indicates where this information came from, e.g. "uaApplication", "uaOS", "uaDeviceType"? Or perhaps we should be factoring them out into a separate table somehow.

Naming is hard. @philbooth let's try to chat about this early next week before I break for PTO.

[philbooth]

Naming is hard. @philbooth let's try to chat about this early next week before I break for PTO.

That would be really good if we could, as there's some other things I'm uncertain about here too. For the record:

1. deviceType is not currently the same as what we discussed briefly last week. Instead of a generic mobile|desktop classification, it is a more specific iPhone|Blackberry kind of thing which would perhaps be better characterised as deviceName. However, we could still achieve the generic device type that we wanted by using a different user-agent parser. That would mean using a different dependency to the content server though, and I'm not sure whether that harmony between repos is important to us.
2. I'm not currently recording any version information because instinctively it didn't feel that useful, especially for browsers with rapid release cycles. Was that the right or wrong thing to do?
3. I briefly wondered about normalising this information in a separate table. On the one hand, I imagine we're going to end up with a lot of duplicates which could just be identical foreign keys. On the other hand, it's an extra JOIN when we fetch the data and conceptually they're not exactly related and it's not much data anyway so what's the point, really?

[rfk]

I'm not currently recording any version information because instinctively it didn't feel that useful,
especially for browsers with rapid release cycles.

I think it'll be useful. As a point of comparison, out kibana dashboards [1] keep coarse version information like "Windows 7" and "Firefox 39" which seems like a nice balance.

[1] e.g. https://kibana.fxa.us-west-2.prod.mozaws.net/index.html#/dashboard/file/auth_http_status.json

deviceType is not currently the same as what we discussed briefly last week.

From a brief look, it seems like this is almost entirely "mobile device type" and that desktop machines just get "Other". If so, maybe we can record it as such and just say "anything with a deviceType field is a mobile device". I don't know how well that maps onto the model provided by the lib though.

I briefly wondered about normalising this information in a separate table [...] so what's the point, really?

I slept on it, and I came to a similar conclusion, there's not really much point as it's only a handful of fields that I think are unlikely to grow.

[philbooth]

Okay, so we have:

• uaBrowser, e.g. Firefox.
• uaBrowserVersion, e.g. 42.
• uaOS, e.g. Android.
• uaOSVersion, e.g. 5.1.
• uaDeviceType, e.g. mobile.

Look good?

One more question: Where ua-parser returns Other, we want to replace that with the empty string right? Makes it easier to eyeball the data and matches up with the default value in SQL.

[rfk]

Okay, so we have: [...]

Looks good!

Where ua-parser returns Other, we want to replace that with the empty string right?

Yes, or perhaps even an explicit NULL - whichever you think seems to fit best

[rfk]

Since this is doing a DB migration already, what do you think about also adding a "device id" column while we're here? This would be in service of mozilla/fxa-auth-server#988 and I think it's by far the most important part of that bug - having the ability to more reliably distinguish between "this is a new device" and "this is an existing device that's reconnecting" feeds into a lot of short-term work including multi-device metrics, device status dashboard, and marketing email integration.

This would be an optional, opauqe string id provided by the client to identity itself.

[philbooth]

Since this is doing a DB migration already, what do you think about also adding a "device id" column while we're here?

Yep, happy to do that. Do you mean "just add it to this repo, don't worry about how it is conveyed to the auth server yet" or do you mean "also, make it work end-to-end"?

[rfk]

what do you think about also adding a "device id" column while we're here?
Do you mean "just add it to this repo, don't worry about how it is conveyed to the auth server yet"
or do you mean "also, make it work end-to-end"?

Actually I'm having second thoughts about this. In the short term it will be useful to be able to say "which sessionTokens belong to a device?". But longer term, it may make more sense for us to create a "devices" table that has device ID as primary key and sessionID as one of its fields. We're mostly going to want to do "lookup sessionToken given a deviceId" rather than the other way around, e.g. when the device is re-establishing its session or when we're revoking tokens for a device.

@philbooth what do you think? Perhaps we should just hold off on that one until we clarify how the device stuff is going to work on the client side.

[philbooth]

@dannycoates this one is ready to look at if you have the time.

[dannycoates]

Looks awesome! ❤️ the thorough documentation. r+ 👍