-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(api-graphql): pass authToken via subprotocol #13727
Conversation
b11afc8
to
5665424
Compare
27bae13
to
fa09361
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just left one non-blocking comment!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving on bundle size increase
*/ | ||
convert( | ||
input, | ||
options: Base64EncoderConvertOptions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: options
should've been typed by the Base64Encoder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, I just wanted it to be more explicit, but I can follow up and remove in another PR if you prefer
urlSafe: boolean; | ||
skipPadding?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, should the urlSafe
be an optional property too, otherwise when you specify skipPadding
you'd need to pass in a value for urlSafe
as well. This is a internal tooling, I think it's safe to make a necessary change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentional. skipPadding
would only be valid if urlSafe
is set to true
.
Description of changes
AppSync Realtime is changing the way it accepts auth tokens when establishing subscription connections.
Today auth tokens are passed in the connection url as a query string param, e.g.
wss://api.example.com/graphql?header=<base64_encoded_token>
Going forward, the service expects the client to pass the auth token through as a WS subprotocol:
This change is an implementation detail that does not affect the behavior of the AppSyncRealtime client. AppSync will continue to accept the auth token in query params for existing APIs, so older versions of the Amplify library will work as expected.
Note
The
WebSocket
Web API prohibits separator characters in subprotocol values, e.g./
,=
, which are part of the standard base64 char set. With this change, the auth token is encoded as base64url, which lacks any prohibited characters in its alphabet. AppSync now supports this encoding for auth headers.The change is also being back-ported to v5 here: #13726
Description of how you validated changes
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.