-
Notifications
You must be signed in to change notification settings - Fork 2
Feat: make embeddable into openMFP #175
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two typos (didn't mark all changes), but apart from that, it looks great! I also tested the embedding in HSP, and it works for me 👍
…frontend into feat/make-embeddable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables the service to be embedded in openMFP by updating cookie settings for cross-site use, introducing a CSP that restricts framing, and exposing a new FRAME_ANCESTORS
environment variable. It also renames the dev flag to --local-dev
and wires in @fastify/helmet
for CSP enforcement.
- Adjust session cookies to
SameSite=None
,secure: true
, andpartitioned: true
for cross-site embedding. - Add
FRAME_ANCESTORS
env var, register it, and enforce it in a CSPframe-ancestors
directive via Helmet. - Rename local dev flag, update scripts and plugin registrations accordingly.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
server/encrypted-session.js | Set cookies to cross-site mode (None ), always secure, partitioned |
server/config/env.js | Added required FRAME_ANCESTORS variable to env schema |
server/app.js | Removed env plugin registration before session setup (regression) |
server.js | Registered envPlugin , added Helmet with CSP including frame-ancestors |
package.json | Changed dev script to use --local-dev , added Helmet dependency |
index.html | Removed legacy window.global shim |
.env.template | Documented new FRAME_ANCESTORS variable |
Comments suppressed due to low confidence (2)
.env.template:25
- [nitpick] Consider adding example values (e.g.
https://app.example.com https://admin.example.com
) to clarify how to format multiple origins forFRAME_ANCESTORS
.
FRAME_ANCESTORS=
server/app.js:12
- The environment plugin registration (
envPlugin
) was removed, butencryptedSession
relies onfastify.config
. Re-addawait fastify.register(envPlugin);
before registeringencryptedSession
to ensure config values are available.
fastify.register(encryptedSession, {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
What this PR does / why we need it:
None
to allow cookies and therefore session when embedded in iFrameframe-anchestors
to only be embeddable in website we trustWe can not allow CORS since every browser initiated request will contain the session cookie (as defined by
SameSite
). Without CORS and with the frame-anchestors we allow this to only happen within our domain/site.This change allow us to be used in openMFP.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: