TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
979 stars 308 forks source link

`version_ids.serial_number` field is ignored in the device creation API call #5702

Closed ymgupta closed 1 year ago

ymgupta commented 2 years ago

Summary

While creating the device using APIs, we see that the version_ids.serial_number field is ignored. We have included the version_ids values (brand_id, model_id, serial_number) in the request body of Identity Server API call and field mask. But, we see that only brand_id, and model_id are stored for the device. The API response doesn't contain the serial_number field. However, as per the API reference documentation, we see that serial_number is part of the version identifiers. And, it is stored in Entity Registry, Network Server and Application Server.

Steps to Reproduce

  1. Create a device in all components of TTS using APIs.
  2. In the request body of the API call to IS, NS and AS, set the version_ids as follows:
    "version_ids": {
            "brand_id": "the-things-products",
            "model_id": "the-things-uno",
            "serial_number": "some-sample-serial-number"
        }

    Also, set the following values in the field mask of the API call:

    "version_ids.brand_id",
    "version_ids.model_id",
    "version_ids.serial_number"

Current Result

  1. brand_id and model_id are stored successfully for the device with the IS API call, but the serial_number is ignored.
  2. The IS API response doesn't contain the serial_number.
  3. Also, the NS and AS doesn't accept serial_number in the field mask. Including it in the API calls raises forbidden path error.
  4. Additionally, we don't see the flag to create the version_ids.serial_number in CLI as well.

    $ ttn-lw-cli device create --help | grep version-ids
    
      --version-ids.band-id string                                                            
      --version-ids.brand-id string                                                           
      --version-ids.firmware-version string                                                   
      --version-ids.hardware-version string                                                   
      --version-ids.model-id string    

Expected Result

The IS, AS and NS should store the serial_number value from the API requests.

Relevant Logs

#### 1. Identity Server:

**Request:**

curl --location --request POST 'https://<tenant-id-redacted>.eu1.cloud.thethings.industries/api/v3/applications/<app-id-redacted>/devices' \
--header 'authority: <tenant-id-redacted>.eu1.cloud.thethings.industries' \
--header 'accept: application/json, text/plain, */*' \
--header 'accept-language: en-GB,en-US;q=0.9,en;q=0.8' \
--header 'authorization: Bearer XXXXXXXXXX' \
--header 'content-type: application/json' \
--data-raw '{
    "end_device": {
        "ids": {
            "device_id": "things-uno-1",
            "dev_eui": "<dev-eui-redacted>",
            "join_eui": "0000000000000000",
            "application_ids": {
                "application_id": "<app-id-redacted>"
            }
        },
        "join_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
        "network_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
        "application_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
        "version_ids": {
            "brand_id": "the-things-products",
            "model_id": "the-things-uno",
            "hardware_version": "1.0",
            "firmware_version": "quickstart",
            "band_id": "EU_863_870",
            "serial_number": "0000000000000000"
        }
    },
    "field_mask": {
        "paths": [
            "join_server_address",
            "network_server_address",
            "application_server_address",
            "version_ids.brand_id",
            "version_ids.model_id",
            "version_ids.hardware_version",
            "version_ids.firmware_version",
            "version_ids.band_id",
            "version_ids.serial_number"
        ]
    }
}'

Response:

{
    "ids": {
        "device_id": "things-uno-1",
        "application_ids": {
            "application_id": "<app-id-redacted>"
        },
        "dev_eui": "<dev-eui-redacted>",
        "join_eui": "0000000000000000"
    },
    "created_at": "2022-08-04T09:54:42.345Z",
    "updated_at": "2022-08-04T09:54:42.345Z",
    "version_ids": {
        "brand_id": "the-things-products",
        "model_id": "the-things-uno",
        "hardware_version": "1.0",
        "firmware_version": "quickstart",
        "band_id": "EU_863_870"
    },
    "network_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
    "application_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
    "join_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries"
}

2. Network Server

Request:(Didn't mention the serial_number in the field mask as it is not supported in this call)

curl --location --request PUT 'https://<tenant-id-redacted>.eu1.cloud.thethings.industries/api/v3/ns/applications/<app-id-redacted>/devices/things-uno-1' \
--header 'authority: <tenant-id-redacted>.eu1.cloud.thethings.industries' \
--header 'accept: application/json, text/plain, */*' \
--header 'accept-language: en-GB,en-US;q=0.9,en;q=0.8' \
--header 'authorization: Bearer XXXXXXXXXX' \
--header 'content-type: application/json' \
--data-raw '{
    "end_device": {
        "supports_join": true,
        "lorawan_version": "MAC_V1_0_2",
        "ids": {
            "device_id": "things-uno-1",
            "dev_eui": "<dev-eui-redacted>",
            "join_eui": "0000000000000000",
            "application_ids": {
                "application_id": "<app-id-redacted>"
            }
        },
        "frequency_plan_id": "EU_863_870",
        "version_ids": {
            "brand_id": "the-things-products",
            "model_id": "the-things-uno",
            "hardware_version": "1.0",
            "firmware_version": "quickstart",
            "band_id": "EU_863_870",
            "serial_number": "0000000000000000"
        },
        "lorawan_phy_version": "PHY_V1_0_2_REV_B",
        "mac_settings": {
            "supports_32_bit_f_cnt": true
        }
    },
    "field_mask": {
        "paths": [
            "supports_join",
            "lorawan_version",
            "ids.device_id",
            "ids.dev_eui",
            "ids.join_eui",
            "ids.application_ids.application_id",
            "frequency_plan_id",
            "version_ids.brand_id",
            "version_ids.model_id",
            "version_ids.hardware_version",
            "version_ids.firmware_version",
            "version_ids.band_id",
            "lorawan_phy_version",
            "mac_settings.supports_32_bit_f_cnt"
        ]
    }
}'

Response:

{
    "ids": {
        "device_id": "things-uno-1",
        "application_ids": {
            "application_id": "<app-id-redacted>"
        },
        "dev_eui": "<dev-eui-redacted>",
        "join_eui": "0000000000000000"
    },
    "created_at": "2022-08-04T09:56:36.140211903Z",
    "updated_at": "2022-08-04T09:56:36.140211903Z",
    "version_ids": {
        "brand_id": "the-things-products",
        "model_id": "the-things-uno",
        "hardware_version": "1.0",
        "firmware_version": "quickstart",
        "band_id": "EU_863_870"
    },
    "lorawan_version": "MAC_V1_0_2",
    "lorawan_phy_version": "PHY_V1_0_2_REV_B",
    "frequency_plan_id": "EU_863_870",
    "supports_join": true,
    "mac_settings": {
        "supports_32_bit_f_cnt": true
    }
}

3. Application Server

Request: (Didn't mention the serial_number in the field mask as it is not supported in this call)

curl --location --request PUT 'https://<tenant-id-redacted>.eu1.cloud.thethings.industries/api/v3/as/applications/<app-id-redacted>/devices/things-uno-1' \
--header 'authority: <tenant-id-redacted>.eu1.cloud.thethings.industries' \
--header 'accept: application/json, text/plain, */*' \
--header 'accept-language: en-GB,en-US;q=0.9,en;q=0.8' \
--header 'authorization: Bearer XXXXXXXXXX' \
--header 'content-type: application/json' \
--data-raw '{
    "end_device": {
        "ids": {
            "device_id": "things-uno-1",
            "dev_eui": "<dev-eui-redacted>",
            "join_eui": "0000000000000000",
            "application_ids": {
                "application_id": "<app-id-redacted>"
            }
        },
        "version_ids": {
            "brand_id": "the-things-products",
            "model_id": "the-things-uno",
            "hardware_version": "1.0",
            "firmware_version": "quickstart",
            "band_id": "EU_863_870",
            "serial_number": "0000000000000000"
        },
        "formatters": {
            "up_formatter": "FORMATTER_REPOSITORY",
            "down_formatter": "FORMATTER_REPOSITORY"
        }
    },
    "field_mask": {
        "paths": [
            "ids.device_id",
            "ids.dev_eui",
            "ids.join_eui",
            "ids.application_ids.application_id",
            "version_ids.brand_id",
            "version_ids.model_id",
            "version_ids.hardware_version",
            "version_ids.firmware_version",
            "version_ids.band_id",
            "formatters.up_formatter",
            "formatters.down_formatter"
        ]
    }
}'

Response:

{
    "ids": {
        "device_id": "things-uno-1",
        "application_ids": {
            "application_id": "<app-id-redacted>"
        },
        "dev_eui": "<dev-eui-redacted>",
        "join_eui": "0000000000000000"
    },
    "created_at": "2022-08-04T09:58:58.093324743Z",
    "updated_at": "2022-08-04T09:58:58.093324743Z",
    "version_ids": {
        "brand_id": "the-things-products",
        "model_id": "the-things-uno",
        "hardware_version": "1.0",
        "firmware_version": "quickstart",
        "band_id": "EU_863_870"
    },
    "formatters": {
        "up_formatter": "FORMATTER_REPOSITORY",
        "down_formatter": "FORMATTER_REPOSITORY"
    }
}

4. Join Server

Request:

curl --location --request PUT 'https://<tenant-id-redacted>.eu1.cloud.thethings.industries/api/v3/js/applications/<app-id-redacted>/devices/things-uno-1' \
--header 'authority: <tenant-id-redacted>.eu1.cloud.thethings.industries' \
--header 'accept: application/json, text/plain, */*' \
--header 'accept-language: en-GB,en-US;q=0.9,en;q=0.8' \
--header 'authorization: Bearer XXXXXXXXXX' \
--header 'content-type: application/json' \
--data-raw '{
    "end_device": {
        "ids": {
            "device_id": "things-uno-1",
            "dev_eui": "<dev-eui-redacted>",
            "join_eui": "0000000000000000",
            "application_ids": {
                "application_id": "<app-id-redacted>"
            }
        },
        "network_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
        "application_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
        "root_keys": {
            "app_key": {
                "key": "<app-key-redacted>"
            }
        }
    },
    "field_mask": {
        "paths": [
            "network_server_address",
            "application_server_address",
            "root_keys.app_key.key",
            "ids.device_id",
            "ids.dev_eui",
            "ids.join_eui",
            "ids.application_ids.application_id"
        ]
    }
}'

Response:

{
    "ids": {
        "device_id": "things-uno-1",
        "application_ids": {
            "application_id": "<app-id-redacted>"
        },
        "dev_eui": "<dev-eui-redacted>",
        "join_eui": "0000000000000000"
    },
    "created_at": "2022-08-04T09:59:42.646225071Z",
    "updated_at": "2022-08-04T09:59:42.646225071Z",
    "network_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
    "application_server_address": "<tenant-id-redacted>.eu1.cloud.thethings.industries",
    "root_keys": {
        "app_key": {
            "key": "<app-key-redacted>"
        }
    }
}

URL

No response

Deployment

The Things Stack Cloud

The Things Stack Version

v3.21.0

Client Name and Version

Using the CLI: ttn-lw-cli 3.21.0 on Ubuntu 18.04.6 LTS
Using cURL: curl 7.58.0 (x86_64-pc-linux-gnu)

Other Information

Currently, the details such as brand_id and model_id are available in the version_ids object of uplink messages. They are used to route the messages in the user's application based on those values. However, the user wants to route the messages based on the serial_number value as well.

Ref: https://github.com/TheThingsIndustries/lorawan-stack-support/issues/821

Proposed Fix

No response

Contributing

Code of Conduct

adriansmares commented 2 years ago

Implementation wise this is not complex. On the NS/AS/IS side we just need to add it to the allowed field mask, and on the IS side we need to add another column to the {gorm|bun}store.

The question is - do we want to implement this ? My understanding is that serial_number ended up in EndDeviceVersionIdentifiers because it is part of the QR code specification, but it does not necessarily have anything to do with the end device version identifiers themselves.

The intent here is to basically inject a form of metadata into the uplink message, and this pushes the sliding window for what we include in the uplink messages. I'm inclined to not implement this.

Any thoughts @johanstokking ?

johanstokking commented 2 years ago

Honestly I think that we made a mistake by introducing https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.21/api/identifiers.proto#L163-L167 to EndDeviceVersionIdentifiers. As you say, these aren't version identifiers.

The serial number is metadata of the end device. It should be a toplevel field of EndDevice, stored in IS. And shouldn't go in upstream messages.

The Vendor and Profile IDs are something typical TR005.

@KrishnaIyer can we deprecate these fields based on progressive insight? The required changes are here: https://github.com/TheThingsNetwork/lorawan-stack/pull/5134/files

KrishnaIyer commented 2 years ago

Ok I agree indeed, we can deprecate it and add it to EndDevice. I'll add a quick PR