Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for externally managed websocket event authorizers #12344

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alanmun
Copy link

@alanmun alanmun commented Jan 31, 2024

Externally managed authorizers (like cross account) for a Websocket $connect route seems to create an invalid permission, causing deployment to fail. I found out that this is the case because although the documentation seems to suggest that cross-account lambda authorizers are supported with websocket, they currently aren't. The code always creates permissions instead of skipping them as is done with http api.

This small PR adds code to not generate a warning anymore when you use "managedExternally" in "authorizers" since it exists now as well as code to detect that property and skip creating permissions. It also adds a documentation update to reflect that this is supported now more clearly.

I tried my best to follow CONTRIBUTING.md. If I did something wrong please let me know. I thought that this bug fix was simple enough to not warrant an issue. I consider it a bug fix because the documentation seems to imply that this is normally possible, but it wasn't:
image

I ran npm run prettify:update to only prettify the changed code, and npm run lint.

Oh, btw, I ran npm test and saw a bunch of output saying that certain tests were skipped due to not being able to create symlinks because no admin privileges. I actually work on a Win11 machine, and so admin privileges are no longer needed with Windows 11. This article also claims this to be true: https://www.howtogeek.com/16226/complete-guide-to-symbolic-links-symlinks-on-windows-or-linux/

So I'm guessing those tests do something like if ANY_WINDOWS && NO_ADMIN_PRIVS and just give up when those are true at the same time.

@alanmun
Copy link
Author

alanmun commented Mar 18, 2024

@ac360 Hello, any chance this could get looked at, anything else I need to do? My team and I depend on this functionality for our project. @ashwinsaval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant