Skip to content

Conversation

@tammoippen
Copy link

Description of changes

In the connect call of events, the authToken is not passed to the subsequent connect calls, resulting in no Authorization header being added, and an error 'No auth token specified' thrown.

const channel = await events.connect('/test/channel', {
      authToken: "TOKEN"
});

This PR makes sure, the authToken is properly dispatched to the subsequent connect calls.

Issue #14124

Description of how you validated changes

  • created a sample vuejs project: yarn create vue

  • added amplify: yarn add aws-amplify.

  • follow the contribution guide to build and link the changes from core and api-graphql

  • setup a aws AppSync Event, lambda function and connect them in the aws console

    def lambda_handler(event, context):
        token: str | None = event.get("authorizationToken")
        headers: dict[str, str] = event.get("requestHeaders", {})
        print(f"{token=} {headers=}")
        return {"isAuthorized": True}
  • get amplify_outputs.json from the console, make sure to have "defaultAuthMode": "lambda" set

  • in the vue project, add to main.ts:

    import { Amplify } from 'aws-amplify';
    import config from './amplify_outputs.json';
    Amplify.configure(config);
  • in the vue project, add to App.vue:

    import { events } from "aws-amplify/data";
    
    async function connect() {
      const channel = await events.connect("/default/*", { authToken: "TOKEN" });
      channel.subscribe({
        next: (d) => {
          console.log("received", d);
        },
        error: (err) => console.error("error", err),
      });
    
    }
  • running the vue app and observe console

Checklist

  • PR description included
  • yarn test passes
  • Unit Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

Checklist for repo maintainers

  • Verify E2E tests for existing workflows are working as expected or add E2E tests for newly added workflows
  • New source file paths included in this PR have been added to CODEOWNERS, if appropriate

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@tammoippen tammoippen requested a review from a team as a code owner February 1, 2025 13:05
options?: EventsOptions,
): Promise<EventsChannel> {
const providerOptions = configure();
const configureOptions = configure();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the _connectWebSocket (it is called deeper in the await eventProvider.connect(providerOptions);) method still won't work with non-apiKey auth with this fix. Wouldn't it be better to fix the configure(); helper right away?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think configure(); is called anywhere down the line of await eventProvider.connect(providerOptions);. It always uses the provided providerOptions, also for the _connectWebSocket. All non-apiKey auths add their authentifiation via additionalHeadersFromOptions(options); and here.

I have a (temporary) package published with the changes of this PR. In our internal project, the socket connection is established, we can subscribe to channels and receive events.

@madhavInnoloft
Copy link

Facing the same problem with lambda based calls. Would be great to the the solution live!

@stocaaro
Copy link
Contributor

stocaaro commented Apr 7, 2025

I believe this issue was fixed by another PR. That change is built to the unstable tag and will be released with the next release process. Can we close this PR? Does this change capture any behavior not covered by the merged fix?

Thanks!
Aaron

@tammoippen
Copy link
Author

Hi @stocaaro, yes the changes are covered in #14278 . Closing

@tammoippen tammoippen closed this Apr 8, 2025
@stocaaro
Copy link
Contributor

stocaaro commented Apr 8, 2025

Thank you and much appreciation for the work you put into this. Glad to have it fixed and if you encounter any issues with this behavior after the next release to latest, please ping me.

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.

4 participants