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

feat: setup CORS to allow all origins #7

Closed
wants to merge 1 commit into from
Closed

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Jan 17, 2024

This allows single-page applications to query our stats. I thought I would need this for our public Grafana Dashboard, so that it can fetch data from the browser. At the end, I found a different option, so this is not strictly needed. But it can be useful as it allows our community to build their own visualisations from our stats.

@bajtos bajtos requested a review from juliangruber January 17, 2024 08:35
@@ -86,6 +86,8 @@ export const today = () => getDayAsISOString(new Date())
* @param {(import('pg').Pool, import('./typings').Filter) => Promise<object[]>} fetchStatsFn
*/
const getStatsWithFilterAndCaching = async (pathname, searchParams, res, pgPool, fetchStatsFn) => {
res.setHeader('access-control-allow-origin', '*')
Copy link
Member

Choose a reason for hiding this comment

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

I believe browsers disallow *, and we need to return a hostname instead. I propose to ditch this feature until we can do it proper, with tests etc

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC the docs in MDN, * is allowed only for requests with no credentials. I think we should be fine here.

I propose to ditch this feature until we can do it proper, with tests etc

Agreed.

@bajtos bajtos closed this Jan 17, 2024
@bajtos bajtos deleted the cors-allow-all branch January 17, 2024 10:48
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.

2 participants