fix(addFrameOptions): strict csp domain now matches #704
Conversation
|
why is this needed? |
📊 Bundle Size Report
|
| @@ -6,7 +6,7 @@ | |||
| "packages": { | |||
| "": { | |||
| "name": "@americanexpress/one-app", | |||
| "version": "5.13.0", | |||
| "version": "5.13.1", | |||
There was a problem hiding this comment.
its changed to match the actual current version.
| @@ -21,8 +21,8 @@ export default function addFrameOptionsHeader(req, res, next) { | |||
| const referer = req.get('Referer'); | |||
|
|
|||
| const frameAncestorDomains = getCSP()['frame-ancestors']; | |||
|
|
|||
| const matchedDomain = frameAncestorDomains && frameAncestorDomains.find((domain) => matcher.isMatch(referer, `${domain}/*`) | |||
| const trimmedReferrer = referer && referer.replace('https://', ''); | |||
There was a problem hiding this comment.
Why was https:// causing issues?
There was a problem hiding this comment.
because if the referrer is https://some.domain.com it will always fail to match against some.domain.com . i think it is safer to strip https:// instead of putting a wildcard in front of the csp domain.
| @@ -18,7 +18,7 @@ import addFrameOptionsHeader from '../../../src/server/middleware/addFrameOption | |||
|
|
|||
| jest.mock('../../../src/server/middleware/csp', () => ({ | |||
| getCSP: () => ({ | |||
| 'frame-ancestors': ['*.example.com'], | |||
| 'frame-ancestors': ['valid.example.com'], | |||
There was a problem hiding this comment.
How will these tests protect against regression?
There was a problem hiding this comment.
this was the previously failing scenario.
Because only * domains from the CSP frame-ancestors were being added to the frame options header which results in weaker security. |
Description
remove "https://" from referrer when looking for frame-ancestors match
Motivation and Context
fix
How Has This Been Tested?
test suide
Types of Changes
Checklist:
What is the Impact to Developers Using One App?