electerious / Ackee

Self-hosted, Node.js based analytics tool for those who care about privacy.
https://ackee.electerious.com
MIT License
4.27k stars 359 forks source link

Set Access-Control-Allow-Credentials for serverless mode #228

Closed elliottsj closed 3 years ago

elliottsj commented 3 years ago

This enables Ackee v3 to work on Vercel. Should work for Netlify too, though I've only tested Vercel.

Fixes #223

See https://github.com/apollographql/apollo-server/blob/d63afe1b03d8bd0d4c8a4c572c8f7bed23b6d270/packages/apollo-server-lambda/src/ApolloServer.ts#L106-L108

vercel[bot] commented 3 years ago

@elliottsj is attempting to deploy a commit to a Personal Account owned by @electerious on Vercel.

@electerious first needs to authorize it.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 546056388


Totals Coverage Status
Change from base Build 507221259: -2.0%
Covered Lines: 1186
Relevant Lines: 1300

💛 - Coveralls
electerious commented 3 years ago

Thanks for the PR!

It seems that apollo sets the credentials header when using this option (https://github.com/apollographql/apollo-server/blob/d63afe1b03d8bd0d4c8a4c572c8f7bed23b6d270/packages/apollo-server-lambda/src/ApolloServer.ts#L106-L108), but it won't mirror the origin when using a wildcard as the origin (https://github.com/apollographql/apollo-server/blob/main/packages/apollo-server-lambda/src/ApolloServer.ts#L132). This means that it would fix installations which are specifying multiple domains, but not those specifying a wildcard (*).

It's a bit confusing 😃 It currently misses this to fix #223 completely.

electerious commented 3 years ago

The bug is indeed fixed by the PR. It seems that settings cors: true reflects the request origin. I previously thought that we need to do that manually. 🎉