SmartThingsCommunity / smartapp-sdk-nodejs

Javascript/NodeJS SDK to create SmartThings SmartApps
https://smartthings.developer.samsung.com/
Apache License 2.0
143 stars 80 forks source link

Signature verification failing with query parameters #255

Closed AmineI closed 8 months ago

AmineI commented 8 months ago

Describe the bug

When receiving a call from the SmartThings platform, the signature verification mechanism fails when the WebHook URL includes a query parameter. The actual url that is signed is the path without any query parameters.

This affects cases where WebHook URLs are like /api/smartapp?code=my-R4nd0mCode==, as implemented by hosts such as Azure Functions, which protects against unwanted calls via a "code" query parameter.

To Reproduce Steps to reproduce the behavior: Start a simple smartApp sample, on a server endpoint such as below :

server.post('/api/smartapp', async (req, res) => {
        //Do anything with a query parameter
    await smartApp.handleHttpCallback(req, res)
})

Add the[https://host]/api?code=my-R4nd0mCode== Webhook URL to SmartThings Developer Workspace. Attempt to validate the domain.

Expected behavior The requests from SmartThings to this endpoint should succeed.

Actual behavior The requests from SmartThings fail. The url [https://host]/api/smartapp?code=my-R4nd0mCode==, is properly called, but the signature verification from the SDK fails with an error "Forbidden - failed verifySignature" in the logs. The signature verification fails as signature contains the path /api/smartapp as the target url.

Here are the test cases and behavior I confirmed :

Additional context

I did not send a Pull Request, as I am unsure if this should be fixed in the module or in the SmartThings platform signature mechanism.

A workaround is to insert an assignment of req.originalUrl = req.path before the smartApp.handleHttpCallback call. A potential fix could be to include this statement here https://github.com/SmartThingsCommunity/smartapp-sdk-nodejs/blob/3725c0f5c6ff1f82a8219ba5399019e707526b49/lib/smart-app.js#L411-L415

Another workaround would be to use handleHttpCallbackUnverified in such cases, although this causes other issues (see #254).

AmineI commented 8 months ago

Merged in #259 🎉 .