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

Fix CSP issue and API status in dashboard #2845

Merged
merged 1 commit into from
Feb 6, 2022

Conversation

JerryHue
Copy link
Contributor

@JerryHue JerryHue commented Feb 6, 2022

Issue This PR Addresses

Fixes #2511

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

There are two CSP-related issues that the PR addresses.

  • The first one is related to a script-src rule violation. This is due to the onclick handlers written right into the HTML. This was done by the Material UI template. Since there were of no use, they were removed.
  • The second one is related to a connect-src rule violation. This is due to the redirection when requesting the statuses of the APIs, the redirection would slightly change the protocol from https to http, violating the rule. To avoid redirection, we tighten the regex used to trigger this redirection by traefik.

Steps to test the PR

  • Get the changes from the branch
  • Install dependencies (pnpm install)
  • Start running the status service (pnpm run services:start status)

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR does not include tests as it does not modify implementation details
  • Screenshots: image The warnings are gone!
  • Documentation: This PR does not include documentation as it does not add user exposed functionality.

@JerryHue JerryHue added the area: dashboard Related to Telescope's dashboard (the page that has stats) label Feb 6, 2022
@JerryHue JerryHue added this to the 2.7 Release milestone Feb 6, 2022
@JerryHue JerryHue requested a review from humphd February 6, 2022 05:24
@JerryHue JerryHue self-assigned this Feb 6, 2022
@gitpod-io
Copy link

gitpod-io bot commented Feb 6, 2022

@JerryHue JerryHue marked this pull request as ready for review February 6, 2022 05:27
@Kevan-Y Kevan-Y added the type: bug Something isn't working label Feb 6, 2022
Copy link
Contributor

@Andrewnt219 Andrewnt219 left a comment

Choose a reason for hiding this comment

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

I read the description in the issue, great catch.

Copy link
Contributor

@Kevan-Y Kevan-Y left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -35,7 +35,7 @@ services:
# Specify the status service port
- 'traefik.http.services.status.loadbalancer.server.port=${STATUS_PORT}'
# Define redirect middleware for any requests to /v1/status -> /v1/status/ (trailing slash)
- 'traefik.http.middlewares.status_redirect.redirectregex.regex=(^.*\/status$$)'
- 'traefik.http.middlewares.status_redirect.redirectregex.regex=(^.*\/[^status]+\/status$$)'
Copy link
Contributor

Choose a reason for hiding this comment

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

Clever!

@JerryHue JerryHue merged commit 28235d9 into Seneca-CDOT:master Feb 6, 2022
@JerryHue JerryHue linked an issue Feb 10, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dashboard Related to Telescope's dashboard (the page that has stats) type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deal with console errors/warnings in Dashboard CSP error in Dashboard
5 participants