Open jawilson opened 5 months ago
token key name and token key value are only relevant to signed authorizers.
It looks like you're using the 311 client.
token key name and token key value are only relevant to signed authorizers.
@bretambrose that is not true. You can have a non-signed authorizer configured with a token so that IoT Core populates the event.token
value in the Lambda regardless of connection method (HTTPS, WebSocket, etc).
The only purpose for token key name and token key value is to communicate to a signing-enabled authorizer what the signature's unsigned value is. The fact that some field in the lambda context gets populated with it is not really relevant.
Username and password are the supported ways of passing auxiliary data into the authorizer lambda
Fair enough. The IoT dev guide isn't exactly clear on that, and all of the tools (cli, console, CDK, etc) let you create authorizers with a token key name configured and signing disabled. So why should this lib not allow you to pass a token to a custom authorizer without a signing key?
The 311 interface is old and umm, not ideal from a clarity standpoint. There was an existing function that was missing parameters, so the missing parameters were just glommed onto the end, leading to a situation where you can accidentally pass in invalid (from our perspective) combinations.
It probably would have been better to explicitly model signed and unsigned authorizer configurations as two separate data types, making it clear what's expected and allowed.
Thinking about it more, this is kind of bothering me. Custom auth is complicated enough as is and neither the 311 nor the 5 APIs are as clear as they should be. Tempted to deprecate the current API (still works of course) and add
interface UnsignedCustomAuthOptions { authorizer_name, username?, password? }
and
interface SignedCustomAuthOptions {authorizer_name, token_key_name, token_key_value, token_signature, username?, password?}
and new builder constructor APIs for
newSignedCustomAuth(...)
newUnsignedCustomAuth(...)
Let's leave this open as a reminder.
We'd been focusing on 311 since 5 had all of the "developer preview" warnings around it. I see that's finally gone, so we'll switch to MQTT 5 then.
Are you having any problems with using mqtt5? If not the we can close this issue
Per @bretambrose comment I went ahead and switched the custom authorizer to validate using the MQTT password unless a token
is present and the signatureVerified
field for the event was true as it sounds like this is more of the "correct" way. Eventually we plan to switch to signed tokens anyway. FWIW, mqtt5 seems to be working quite well.
So while I still think either the ticket is valid or the docs/cli tools should be updated to reflect the intention, this is no longer an issue for me.
Describe the bug
AwsIotMqttConnectionConfigBuilder.with_custom_authorizer()
is broken, signing isn't required for custom authorizers, yet it enforces it.Expected Behavior
Using
with_custom_authorizer()
with only theauthorizer_name
,token_key_name
, andtoken_value
successfully builds a configuration.Current Behavior
Using
with_custom_authorizer()
with only theauthorizer_name
,token_key_name
, andtoken_value
throws errorSigning-based custom authentication requires all token-related properties to be set
.Reproduction Steps
Possible Solution
Change https://github.com/awslabs/aws-crt-nodejs/blob/d1729bf773912f0b3e2dfbec9f18d5a3b973a8e5/lib/common/aws_iot_shared.ts#L85-L93 to
Additional Information/Context
No response
aws-crt-nodejs version used
1.21.0
nodejs version used
v20.10.0
Operating System and version
Ubuntu 22.04.3 LTS