fix: error on creating Cognito authorizers for V1 AWS APIs

[greenlynx]

Fixes the error discussed in sst/sst#4058

[jayair]

Thanks

[jamesleech]

I've got this same issue in V1,

This is causing an issue when trying to create a Cognito Authoriser.
When setting up a Cognito Authoriser we do not required a function, and in fact errors before this line if you try to add a token/request Function.

fn? will be rightly undefined in this case, and should not be force to be non null.

If I revert this change, and do the below, my authoriser is setup correctly and works.

api.addAuthorizer({
    name: "my-authorizer",
    userPools: [myUserPoolArn],
  });

[jamesleech]

it does look like this issue could also be in V2 (http) of the apigateway function also.

[greenlynx]

I haven't tested it, but I don't think this is an issue in V2 API Gateway - in there, as the code currently stands, it looks like fn will only be undefined if lamb is undefined, and the fn! line is only hit if lamb is truthy. I think there's an argument to be made for avoiding the use of the non-null assertion operator as far as possible, to avoid bugs creeping in inadvertantly in future if the logic changes - but I'd rather keep this PR focused on fixing the specific bug we're hitting.

Maintainers - are you happy with this PR? Do I need to do anything further to get it merged?

[jamesleech]

I haven't tested it, but I don't think this is an issue in V2 API Gateway - in there, as the code currently stands, it looks like fn will only be undefined if lamb is undefined, and the fn! line is only hit if lamb is truthy. I think there's an argument to be made for avoiding the use of the non-null assertion operator as far as possible, to avoid bugs creeping in inadvertantly in future if the logic changes - but I'd rather keep this PR focused on fixing the specific bug we're hitting.

Maintainers - are you happy with this PR? Do I need to do anything further to get it merged?

"argument to be made for avoiding the use of the non-null assertion operator as far as possible" 100% Agree.

Would appreciate this PR being released, as I've currently out of scoped auth for a specific API route because of this bug for a project I'm working on to carry on to meet project deadlines, we'll loop back once this is merged and released.

[greenlynx]

Would appreciate this PR being released, as I've currently out of scoped auth for a specific API route because of this bug for a project I'm working on to carry on to meet project deadlines, we'll loop back once this is merged and released.

Me too, as it's blocking me as well. I don't know how to proceed though - I've never contributed to this project before and am not sure of the process.

[fwang]

Will be in v3.2.29 - sorry about the delay!

[jamesleech]

thanks @fwang ! great news