feat(docs): first draft requirements for the device registration API

[philbooth]

I cobbled this together for discussion, based on the approach taken in #66 and the information in Danny's gist. Hopefully it's a starting point from which to nail down the details of the device registration API.

@shane-tomlinson @rfk r?

[shane-tomlinson]

What do you have in mind for these?

[philbooth]

Breaking the stories down one per endpoint seems like a good approach to me but I wasn't sure whether you guys would agree, which is why I asked the question.

[rfk]

"User stories" for a backend-end API is an interesting challenge, but doing one per endpoint seems like a reasonable approach. Also if it doesn't seem like a useful way to frame things, feel free to not have "stories" here. The point isn't to conform to any particular structure, it's just to capture things in a single location.

[rfk]

It might be interesting to lead with descriptions of behavior from the device's point of view, in order to clarify why we have each of these endpoints and how they fit together into an API "flow". Things like:

• User creates an account on a brand new device
• User signs in to a brand new device
• User signs in to something on the web
• User signs in to a legacy device with no device-api support
• User resets their password
• Users signs in after resetting their password
• User explicitly signs out from a device
• User disconnects a device through the web view
• probably others that we should think through as well...

[philbooth]

I've pushed a first stab at the stories.

I started to come at them from the point of view of the device, then changed tack a bit and came at them from the point of view of the API consumer, whether that be the browser or the content server. Not sure how that sits with you guys, let me know what you think.

[shane-tomlinson]

What sort of data is this?

[philbooth]

Not 100% sure I understand the question but it's just supposed to be a note about what is stored in the database. I've added an explanatory sentence before to try and clarify that.

[shane-tomlinson]

The URL in New device and Registered device sections reference https://api-accounts.dev.lcip.org/v1/account/device

[philbooth]

What should they reference instead?

[shane-tomlinson]

Does this API give the ability to register a device as part of login? If so, my guess is the /v1/account/login would both accept email, authPW and an optional device field, which would be the same as the current POST data.

curl -v \
-X POST \
-H "Content-Type: application/json" \
"https://api-accounts.dev.lcip.org/v1/account/login" \
-d '{
  "email": "me@example.com",
  "authPW": "996bc6b1aa63cd69856a2ec81cbf19d5c8a604713362df9ee15c2bf07128efab",
  "device": {
    "name": "My Phone",
    "type": "mobile",
    "callback": "https://updates.push.services.mozilla.com/update/abcdef01234567890abcdefabcdef01234567890abcdef"
  }
}'

[philbooth]

My bad, there were copy/paste errors here and in some of the other code blocks. Have pushed fixes.

[shane-tomlinson]

Can login really take an OAuth token? If so, I had no idea.

[rfk]

Using an OAuth token here doesn't sound right to me.

[rfk]

Actually it's not authenticated with either of those things, you have to provide the password. Copy-paste from other device-related APIs that are authenticated in that manner?

[philbooth]

Yep, sorry, more copy-paste insanity. Have removed.

[shane-tomlinson]

How come device ids are not globally unique? I was not part of that discussion, that context would be useful to have in this document somewhere.

[philbooth]

@dannycoates?

[philbooth]

As per Ryan's comment, this is not the case any more. Have updated.

[shane-tomlinson]

It seems strange to register a device with an already existing device ID. When would that happen?

[philbooth]

See the question How are shared devices handled? below.

[shane-tomlinson]

Can we add the notion of current, which is a boolean? Since no sessionToken is returned, the front end won't be able to determine which item in the list it is.

[philbooth]

Assuming nobody else can see a problem with that, it sounds good to me.

[rfk]

@shane-tomlinson we may actually need to return the sessionToken here, depending on the logic that will be used by the devices view. If it wants to display things that are sessions but don't have a device record, then it will need to be able to match each device record back to its current session token.

[philbooth]

I've changed this endpoint to return the session token and added a clarifying note pointing out that when the sessionToken field is null it means the device is disconnected.

[shane-tomlinson]

Is there any restriction on two devices having the same name, if owned by the same user?

[philbooth]

It's not prohibited by the database schema. Do you think it should be?

[rfk]

I don't think we should enforce this, although we should try to generate unique names when we suggest defaults in the UI

[shane-tomlinson]

What does disconnected mean? Does that mean the user has not connected the device in a while, or it's not available via push, the user has signed out, or that the device has been destroyed?

[philbooth]

@dannycoates?

[rfk]

It means that the device does not have a session token, e.g. as a result of password reset zapping all the session tokens but leaving the device records alone. (Unless we make password reset also zap all your device records).

[philbooth]

Fwiw, I've added an extra question to the Details section that covers this.

[shane-tomlinson]

How did this requirement come into being? What are we guarding against and why?

[philbooth]

@dannycoates, can you field this one?

[philbooth]

As per Ryan's comment, this requirement no longer exists. Have deleted this section.

[rfk]

I think in the latest drafts these are now full-size uuids, see e.g. mozilla/fxa-auth-db-mysql#110 (comment) and related discussion in that PR.

[philbooth]

Cool, have updated.

[rfk]

If we want to send message bodies in our push messages, I think we're going to also need to store a key of some kind here, see e.g. the getKey() method described in [1] and the related encryption spec [2]

[1] https://developer.mozilla.org/en-US/docs/Web/API/Push_API/Using_the_Push_API#Technology_overview
[2] https://tools.ietf.org/html/draft-ietf-webpush-encryption-01

[philbooth]

I finally got round to reading these links, thanks @rfk.

I'm still a bit sketchy on the details of all the ways that we're going to use the push endpoint. I know one usage is to notify a device that it's been disconnected. Are there others and, if so, what are they?

Assuming there's more than one then, yep, we'll need to send body data and, yep, we'll need to store the public key and, yep, I'll need to update the database.

[philbooth]

I realise that none of the push use-cases are covered by stories yet. So I've added a story to cover the disconnected push notification and will also add any others as they come up here.

[philbooth]

I'm still a bit sketchy on the details of all the ways that we're going to use the push endpoint. I know one usage is to notify a device that it's been disconnected. Are there others and, if so, what are they?

Just chatted about this with @shane-tomlinson and he also came up with:

• When a new device is connected.
• When the user changes the device name.
• When the user changes their password.
• When the user resets their password.
• Maybe when there is a profile change.

[philbooth]

I've updated the readme to include the public key and raised mozilla/fxa-auth-db-mysql#114 to add it to the database.

[rfk]

I think we should explicitly de-scope device recovery from the initial version of this feature, and revisit it in mozilla/fxa-auth-server#1077

[philbooth]

Cool, have deleted this section.

[vladikoff]

for next 2 weeks...

[philbooth]

Marking this as in review again.

[shane-tomlinson]

Is this under the device namespace like in /account/create?

[philbooth]

Yes. That appears to be yet another sodding copy/paste error, sorry. Fixing.

[rfk]

I'm going to nitpick the names here, because it's not at all obvious that "callback" and "publicKey" are related and should be used as a pair. Perhaps we can put "push" in the name of both somehow, to make it more obvious?

[philbooth]

Good point, well made. Fixed.

[rfk]

IIUC, when you're creating a new account, you can't possibly have an existing device id, because device ids are local to the account.

[philbooth]

Oh yeah, I think I saw that when I looked at the schema but failed to think it through. Have removed this and all the shared device gubbins.

[rfk]

In our implementation, the device id is subsidiary to the account, so I don't think there's anything useful we can do with the notion of "shared devices". If Bob logs in, the device now belongs to Bob as far as we're concerned. We may be able to just remove this whole section.

[rfk]

Would it be better to return null, or to generate and return a new device id? The device clearly wanted to be registered as such.

[philbooth]

Good point. Just so I understand precisely: if the login request arrives with a new device id and none of the other device info, should that request fail?

[rfk]

TBH, I don't know what the best behaviour would be in that case. It may be a case of "try it out and see what feels right".

[philbooth]

Okey-doke, I've updated this section to specify that a new device record is created and the newly-generated device id is returned in the response. We can cross/burn the other bridge as seems appropriate when we get to it.

[philbooth]

@rfk, I've pushed some new "As a user..." stories to try and capture some of the missing flow that happens outside the auth server.

I'm reluctant to replace the "As an API consumer..." stories with these entirely because I like the granularity that those stories bring. The ones below are a bit fluffier. Am I on the right track with them?

[rfk]

One tiny nit we be that the user stories should come first, but at that point we're well off into the process-over-product weeds :-)

I think this document completely fulls its purported purpose of "first draft" and we should merge it, and continue to iterate on it concurrently with the implementation. Thanks for pushing this through @philbooth!

[philbooth]

Ha, I realised this too and had it on my list of things to get round to. Will do it in a follow-up PR.

[shane-tomlinson]

What happens if a 2nd device is registered for a given sessionToken? Is the existing device removed? Does the endpoint throw a 400/500?

[shane-tomlinson]

If the existing device is removed, is its sessionToken destroyed?

[philbooth]

What happens if a 2nd device is registered for a given sessionToken?

I think it should be a 400. Will update the doc to clarify.

If the existing device is removed, is its sessionToken destroyed?

Yep, both are deleted by the stored procedure.

[shane-tomlinson]

If the ID is unknown, is a 400 error returned?

[philbooth]

I think the handling should be consistent across endpoints. As we've already said it generates and returns a new device id in this scenario on /v1/account/login, I think the behaviour should be the same here too.

But happy to be over-ruled if you guys think differently.

[rfk]

Consitency++. However this case should be pretty rare on both endpoints, perhaps there's value in using a 400 error in both places.

We can also revisit this decision as implementation progresses, so let's not get too caught up on it right now.

[shane-tomlinson]

Are the only two valid types mobile and desktop?

[philbooth]

Actually, the two possibilities at the moment are "mobile" and null. See:

https://github.com/mozilla/fxa-auth-server/blob/master/lib/userAgent.js#L58-L62

[shane-tomlinson]

Looking at the linked code, it looks like type is determined based on the UA string, whereas here, we are passing in an explicit type as determined by the browser.

As an aside, null for desktop seems weird to me, can we have the endpoint accept desktop and it can use whatever representation it pleases internally?

[philbooth]

You're right of course, clearly I was feeling tired when I responded. 😕

If there's a plan to seed legacy device info from the sessionTokens table, this is where it will be seeded from but, of course, there's nothing to stop us translating null into "desktop" as part of that process.

So I'm happy to accept whatever strings you think are suitable, although "desktop" seems weirder than null to me. As a word to describe common computing habits it's becoming an anachronism I think.

[philbooth]

I'm happy to accept whatever strings you think are suitable

Just to be crystal clear, max length in the database is 16 characters.

[rfk]

If there's a plan to seed legacy device info from the sessionTokens table

I'm less and less excited about this idea, I feel like having a device record that's not actively maintained by the device is a recipe for weirdness.

[shane-tomlinson]

The auth-server api.md document specifies a list of possible errors for each endpoint, we should probably spec those out here too.

[shane-tomlinson]

Are we allowing OAuth tokens to be used to interact with these endpoints?

[rfk]

I think we should eventually, but maybe we can drop this req from the initial version since I think all initial consumers will have a sessionToken.

[shane-tomlinson]

If a device is destroyed, does that destroy its sessionToken as well?

[rfk]

Yes. The lifetime of a devices sessionToken should be bounded by the lifetime of the device itself.

[rfk]

I'm going to merge this and file follow-up bugs for a couple of outstanding items.