Add CSP headers #230
Add CSP headers #230
Conversation
|
We may even be able to allow them to self configure and overwrite the config if needed. The less config needed to be done by the end user, the better! |
|
@cstns now that you're back from holiday - what's the status on this? |
|
Not looking good. Spoke with @hardillb about it and it seems it's the oauth lib that we're using is causing the issues. I also stumbled across jaredhanson/passport#938 in which the author seems to take his hands of the wheel |
cstns
force-pushed
the
allow-the-editor-to-be-embedded-csp
branch
from
2d48f25 to
f9fab17
Compare
cstns
requested review from
knolleary and
hardillb
and removed request for
joepavitt
lib/runtimeSettings.js
Outdated
| }, | ||
| httpAdminCookieOptions: { | ||
| sameSite: 'None', | ||
| secure: true, |
There was a problem hiding this comment.
This breaks the editor on localfs where https is not setup. We should only be setting this flag if the core app is setup for https.
This can be tested for using settings.forgeURL as we do in the core app: https://github.com/FlowFuse/flowfuse/blob/3d7af2a8d4c1eb52ca4ef750d316251da45c57be/forge/routes/auth/index.js#L175
There was a problem hiding this comment.
Hmm, something else is going wrong - I cannot login to the editor at all, even with all three of the cookie options commented out.
There was a problem hiding this comment.
Hitting an unrelated issue introduced by #232 - will get that fixed first, then retest with these changes.
There was a problem hiding this comment.
I've fixed the other issue via #240.
Now, back to this one - I can confirm that having any of these three values set will break accessing the editor with localfs. So rather than just guarding the secure flag, all three should only be set if forgeURL starts https.
There was a problem hiding this comment.
That was a big oversight on my end, thank you for spotting it!
|
Argh - merged too quick - broken settings file. |
|
can we stop the deployment? |
Description
Take CSP into consideration and allows the editor to be embedded
Related Issue(s)
related to: #3801
part of: #3800
Checklist
flowforge.yml?FlowFuse/helmto update ConfigMap TemplateFlowFuse/CloudProjectto update values for Staging/ProductionLabels
backportlabelarea:migrationlabel