Finalized Content Security Policy Fix#1567
Finalized Content Security Policy Fix#1567BrianRaymond800 wants to merge 11 commits intoOpenEnergyDashboard:developmentfrom
Conversation
…d to any emotion based styles. Fixed majority of the CSP style tag problems but a few still remain. (Still in Progress)
and better clarification.
huss
left a comment
huss
left a comment
There was a problem hiding this comment.
Thanks to @BrianRaymond800 for updating this work.
- There is a merge conflict in a file. I also think the branch may be out of date. Please carefully merge in development.
- When I run OED and look in the web browser console, I see a number of security content msgs. These need to be figured out. I don't know if it relates to the commented out code that was removed as noted in a comment.
- I've added some comments to consider.
src/client/app/emotionCache.ts
Outdated
| @@ -0,0 +1,13 @@ | |||
| //imports the createSche function from React's Emotion library | |||
There was a problem hiding this comment.
Is it createSche or createCache? However, OED does not usually put in import comments like this so okay for it to be removed.
src/client/app/emotionCache.ts
Outdated
| import createCache from '@emotion/cache'; | ||
|
|
||
| //creates the nonce for the script being run | ||
| //he nonce works alongside the webpack_nonce and plotly_nonce to protect against unwated scripts |
src/client/app/index.tsx
Outdated
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| //these lines take the nonce, and create the webpack and plotly nonces from it | ||
| //these are additional nonces that contribute to styling with webpacvk and plotly |
src/client/app/index.tsx
Outdated
| throw err; | ||
| } | ||
| }; | ||
| import 'bootstrap/dist/css/bootstrap.css'; |
There was a problem hiding this comment.
OED normally puts all imports at the top of the file. What about moving these and the two below to the top?
| <title>Open Energy Dashboard</title> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet"> | ||
|
|
There was a problem hiding this comment.
There are extra spaces at the start of this line and some others. VSC command to format document should fix this up.
src/server/app.js
Outdated
| const conversions = require('./routes/conversions'); | ||
| const ciks = require('./routes/ciks'); | ||
|
|
||
| const crypto = require('node:crypto') |
There was a problem hiding this comment.
Please remove blank link above this line and put a semicolon at the end of the line.
src/server/app.js
Outdated
| let htmlPlusData = html.toString().replace('SUBDIR', subdir); | ||
|
|
||
| //assigns a value to the nonce in order to check for authenticity | ||
| const nonce = crypto.randomBytes(16).toString('base64url') |
There was a problem hiding this comment.
This and following lines should end with a semicolon.
| rules: [ | ||
| // All TypeScript ('.ts' or '.tsx') will be handled by 'awesome-typescript-loader'. | ||
| { test: /\.[jt]sx?$/, exclude: /node_modules/, use: 'ts-loader' }, | ||
| { test: /\.[jt]sx?$/, exclude: /node_modules/, use: 'ts-loader'}, |
There was a problem hiding this comment.
The space before the final } was removed but should be there. There are some other formatting issues so doing format document to fix (even though there before this wor) would be nice.
src/client/app/index.tsx
Outdated
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| //these lines take the nonce, and create the webpack and plotly nonces from it |
There was a problem hiding this comment.
This and next line have an extra space at the start.
| multiple sites as an exceptions would be : img-src 'self' http://example.com https://site_example.net; becomes img-src 'self' | ||
| http://example.com https://site_example.net https://newException.com; | ||
| --> | ||
|
|
There was a problem hiding this comment.
A number of commented out lines were removed here from the original PR. I wanted to check if they had any value or were examples.
There was a problem hiding this comment.
From what I can tell, those lines were an alternate implementation of the CSP that was rejected and replaced with the current one, but never fully removed. That is why I chose to remove it.
BrianRaymond800
force-pushed
the
pr-update
branch
from
589f9b4 to
28c2cd9
Compare
3 participants
Description
This PR contains work completed by @pogoco26 and @CamClendenon, which added a Content Security Policy to OED. Information about their work can be found in PR #1484. Since then, the code from that PR has been updated by @BrianRaymond800 to contain more comments, clarification, and general cleanup.
Type of change
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)
Limitations
I am not aware of any limitations, besides those that were listed in the original PR.