Allow for the unique Device identifier to be scoped to Orgs

[joshk]

This change allows for Device identifiers to be reused across Orgs.

Device identifiers are unique across the entire platform, and this will remain the default.

To allow for device identifiers to be reused, the env var PLATFORM_UNIQUE_DEVICE_IDENTIFIERS must be set to any other value than true, TRUE, or 1 (default is true).

Additionally, you will need to run the following SQL in your DB
DROP INDEX devices_identifier_index;
and then CREATE INDEX devices_identifier_org_id_index ON devices(identifier, org_id);

There are two issues which require discussion:

• the API routes /api/devices/:identifier
• the DeviceEventsStreamChannel websocket channel

The API routes /api/devices/:identifier

The 'short' API routes are only enabled when PLATFORM_UNIQUE_DEVICE_IDENTIFIERS is enabled. I've also hidden them from the Swagger UI when PLATFORM_UNIQUE_DEVICE_IDENTIFIERS is false.

The DeviceEventsStreamChannel websocket channel

I've kept support for device:[device_identifier] if PLATFORM_UNIQUE_DEVICE_IDENTIFIERS is true, otherwise users will need to use org:[org_name]:device:[device_identifier]

[lawik]

Yeah this is probably the reasonable approach.

for the endpoints, I think supporting the id primarily and only supporting the identifier if using unique platform identifiers makes sense.

We should also add an endpoint and channel for: including the org and device identifier.

I don't think we expose the internal device_id almost anywhere (if anywhere).

We should also do the plumbing to return better errors on connection failures due to duplicate device ids. Right now they are opaque, 401 I believe. This error would live on when a device ends up on multiple products using shared secrets for example.

[nshoes]

I'd suggest we either remove these endpoints, or to change the paths to /api/devices/:id

I really don't want to surface internal device ids here. How does a user even find an id to then query for? Is it too much to return multiple if they're found, and/or a nice API message to explain multiple were found because of a product setting?

The DeviceEventsStreamChannel websocket channel

I vote we support "device:" <> device_identifier, then "device:" <> device_id as well if the setting is enabled.

The more I think on this feature, the more I'm confused. I read #2495's comments, but isn't it easier to understand why unique device identifiers are the less evil of the two options?

[jjcarstens]

I haven't gotten very far, but before it gets too deep, we should not be considering using an enumerating database ID in any public facing situation. That has been a common security anti-pattern as it greatly increases the likelihood of guess based attacks and interceptions. We are actively moving away from them in many situations.

We also should be careful in error responses. While saying "this api request has multiple returns for the identifier" is very helpful, it may provide too much information from a security stance.

One could argue that the "internal" topics should not apply, but they are still discoverable to an extent through browser and web tooling quite easily, so I would be hesitant to include enumerating IDs there too.

[joshk]

Had a little async chat with @jjcarstens, he requested

"don't expose or use any incremental id anywhere they aren't already used"

which is reasonable.

I will take a look at the two issues I've mentioned, with a possibility of disabling, and hiding from the OpenAPI spec, the APIs used for interacting with a device at the root level.

[joshk]

@nshoes @jjcarstens I've addressed the two issues/points I called out in my PR description, and have updated the description to include how it now works.

Essentially, no change for self-hosted installs, only NervesCloud (and NervesCloud users) are affected.

[lawik]

Yeah, this looks good to me :)

Will let the others chime in.

[nshoes]

Sweet, this looks good to me now. Thanks for the putting in the work after the feedback.