feat: add experimentalModifyObstructiveThirdPartyCode flag for regex rewriter #22568

AtofStryker · 2022-06-28T18:50:23Z

Closes Write cy.origin integration tests for Azure Active Directory #21476
Closes Microsoft login removes the Cypress test frame when it redirects, causing tests to fail #21307

with the completion of #22479

User facing changelog

Introduces a new experimental flag, called experimentalModifyObstructiveThirdPartyCode. When enabled, experimentalModifyObstructiveThirdPartyCode will turn on additional modifyObstructiveCode options to prevent frame busting, as well as strips integrity tags out of <link> and <script> elements. In the current state of this experimental flag, SRI is not supported.

Additional details

As a team, we felt it be best that changes to the regex-rewriter for modifyObstructiveCode be wrapped in its own experimental flag to reduce the potential impact of the rewriter modify code that breaks users who are not leveraging cy.origin or wish to preserve SRI within their application. The impact of this flag will change as we continue to modify the regex-rewritter to handle different websites/authentication platforms. Additionally, the team plans to support server-side integrity matching to verify <link> and <script> elements that have had their integrity stripped be revalidated on the server through hash comparison.

When the experimental flag is enabled:
- third party html/scripts are run through the rewriter
- regex rewriter will look for patterns similar to e.top === e.self
- subresource integrity hashes removed from script and link tags
- calling formElement.submit() correctly handles unsupported targets

Steps to test

additional unit tests to the regex-rewriter have been added to help guarantee the expected rewriting behavior for newly added modifications to the strip/stripStream function, as well as adding the integrity.cy.ts test inside the origin folder. For this test suite to run correctly, both the experimentalSessionAndOrigin and experimentalExpandedModifyObstructiveCode MUST be set to true. This has been modified in the package.json to pass in CI.

How has the user experience changed?

Users should now be able to log into Microsoft Online SSO and Login Live SSO (not pictured) via Cypress

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

…ritter integrity implementation against

… can't without lookback).update stripped integrity tag to make jquery happy

… order, legitimacy, and include link tags

…o regex-rewriter to allow users to opt in to additional modify obstructiver code and SRI stripping

…nded moc

cypress-bot · 2022-06-28T18:50:26Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

…expanded-modify-obstructive-code

…ode is enabled

cypress · 2022-06-28T19:29:30Z

Test summary

4379 • 0 • 49 • 0 • 0

Run details


Project	cypress
Status	Passed
Commit	`97bb6e0`
Started	Jul 22, 2022 5:18 AM
Ended	Jul 22, 2022 5:30 AM
Duration	11:51 💡
OS	Linux Debian - 11.3
Browser	Electron 102

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

AtofStryker · 2022-06-28T19:36:39Z

packages/proxy/lib/http/response-middleware.ts

-    || resContentTypeIsJavaScript(this.incomingRes)
-  )
+  this.res.wantsSecurityRemoved = this.config.modifyObstructiveCode &&
+    (this.res.wantsInjection === 'full' || this.res.wantsInjection === 'fullCrossOrigin' || (resContentTypeIsJavaScript(this.incomingRes) && this.config.experimentalExpandedModifyObstructiveCode))


Modify the content if 1st party html full injection, or html that is being injected for cy.origin. Currently, we don't have a good mechanism for modifying general 3rd party javascript and has been a considered a bug in the past #8983, so for now we will make sure the experimentalExpandedModifyObstructiveCode is enabled before doing so. This is subject to change, and we might even be able to determine if we can modify the code based on the initiator/requestor in the future.

I am also wondering if we want to do the same thing for html with this flag. Right now we run into weird edge casesd where HTML is returned from a redirect after the cy.origin block has exited, which turns the rewriting off for that html and framebusts, which is going to make it difficult for users to test

I added more aggressive behavior in 5597dfa to remove security for all html/javascript resources, regardless of context, if the flag is enabled. This should prevent the weird gap use case where remote state changes at the end of a cy.origin block and html is still being fed to page from that origin, even though we have left that context and the rewriter no longer attempts to rewrite that html.

packages/proxy/lib/http/util/regex-rewriter.ts

AtofStryker · 2022-06-28T19:42:13Z

packages/rewriter/lib/html-rules.ts

@@ -29,7 +29,7 @@ export function install (url: string, rewriter: RewritingStream, deferSourceMapR
    const sriAttr = find(startTag.attrs, { name: 'integrity' })

    if (sriAttr) {
-      sriAttr.name = 'cypress:stripped-integrity'
+      sriAttr.name = 'cypress-stripped-integrity'


I believe this is used by the AST rewriter. For some reason, jQuery has a fit when an html attribute contains a : when doing a generic selector (guessing a collision on stand html attribute characters?) I am wondering if we want to standardize this attribute and go with data-cypress-stripped-integrity

mschile · 2022-07-18T17:16:04Z

packages/driver/src/cy/top_attr_guards.ts

@@ -5,21 +5,44 @@ const invalidTargets = new Set(['_parent', '_top'])
 export type GuardedEvent = Event & {target: HTMLFormElement | HTMLAnchorElement}


There aren't really any changes to this file. I just created the handleInvalidTarget function and updated the handleInvalidEventTarget and handleInvalidAnchorTarget functions to call it.

mjhenkes · 2022-07-20T21:08:21Z

packages/driver/src/cross-origin/patches/cookies.js

@@ -15,7 +13,7 @@
 // - On an interval, get the browser's cookies for the given domain, so that
 //   updates to the cookie jar (via http requests, cy.setCookie, etc) are
 //   reflected in the document.cookie value.
-export const patchDocumentCookie = (Cypress) => {
+export const patchDocumentCookie = (Cypress, window) => {


these patches are pretty straight forward and a good way to keep the injected code small 👍

frnc07 · 2022-07-27T14:59:05Z

Waiting anxiously for this feature to be released. Any timeline on when would this be released?

mschile · 2022-07-27T16:34:09Z

@frnc07, this change should go in our next release which is scheduled for next week.

ChrystelCy · 2022-08-08T09:45:03Z

@frnc07, this change should go in our next release which is scheduled for next week.

I can't see it on the 10.4 changelog. Do you have a release date for this feature please?

emilyrohrbough · 2022-08-08T12:53:10Z

@ChrystelCy It's listed as the 3rd bullet under the features section: https://docs.cypress.io/guides/references/changelog#10-4-0

ChrystelCy · 2022-08-08T13:58:40Z

@ChrystelCy It's listed as the 3rd bullet under the features section: https://docs.cypress.io/guides/references/changelog#10-4-0

Thanks, I was looking for "22568" ...

emilyrohrbough · 2022-08-08T14:01:19Z

@ChrystelCy no worries at all. Generally we link to the issue(s) associated to a change in the changelog unless there was not an original issue to link to.

mschile and others added 14 commits June 23, 2022 16:04

temp changes for microsoft auth

dc11094

add top changing event handler to cross origin cypress

60bb1ac

login.microsoftonline changes

04363f8

add basic battery of integrity tests to eventually test our regex rew…

0d0540d

…ritter integrity implementation against

add ejs disclaimer to fixture

b9ddf62

strip integrity from scripts and link tags. validation to come

addbd03

update regex rules for microsoft to be broader (might need spaces but…

035d7f5

… can't without lookback).update stripped integrity tag to make jquery happy

add additional rewritting to regex-rewritter spec to change integrity…

21957ab

… order, legitimacy, and include link tags

wire in experimental flag experimentalExpandedModifyObstructiveCode t…

8dc5be8

…o regex-rewriter to allow users to opt in to additional modify obstructiver code and SRI stripping

remove redunant code and uncomment out integrity stripping

7e95aef

remove document.cookie patch

90b031e

fix gaps in the regex rewriter

4d6cbea

move expressions for regex rewritter into groups by experimental expa…

ee100fa

…nded moc

wrap injection MOC setAttribute to make sure expanded MOC is on

5ac724c

AtofStryker added the topic: cy.origin Problems or enhancements related to cy.origin command label Jun 28, 2022

AtofStryker self-assigned this Jun 28, 2022

AtofStryker changed the title ~~Feature/expanded modify obstructive code~~ Feat: add experimentalExpandedModifyObstructiveCode flag for regex rewriter Jun 28, 2022

update cy.task test to include new events for integrity

97d03e3

AtofStryker changed the title ~~Feat: add experimentalExpandedModifyObstructiveCode flag for regex rewriter~~ feat: add experimentalExpandedModifyObstructiveCode flag for regex rewriter Jun 28, 2022

AtofStryker added 2 commits June 28, 2022 14:57

Merge branch 'develop' of github.com:cypress-io/cypress into feature/…

0bb97f5

…expanded-modify-obstructive-code

only rewrite javascript if the experimentalExpandedModifyObstructiveC…

5f2813f

…ode is enabled

AtofStryker commented Jun 28, 2022

View reviewed changes

add additional rules for modifying JS for stripping security

c804760

AtofStryker commented Jun 28, 2022

View reviewed changes

packages/proxy/lib/http/util/regex-rewriter.ts Outdated Show resolved Hide resolved

AtofStryker commented Jun 28, 2022

View reviewed changes

AtofStryker assigned mschile Jun 28, 2022

AtofStryker added 2 commits June 28, 2022 17:16

add new experimental config to config spec

a8f4b37

fix lint types

a4deaee

mschile marked this pull request as ready for review July 15, 2022 20:45

Merge branch 'develop' into feature/expanded-modify-obstructive-code

791a3e0

mschile changed the title ~~feat: add experimentalExpandedModifyObstructiveCode flag for regex rewriter~~ feat: add experimentalModifyObstructiveThirdPartyCode flag for regex rewriter Jul 15, 2022

mschile and others added 3 commits July 15, 2022 23:58

fixing integrity with script

2c267ba

fixing link integrity

50ca2ac

Merge branch 'develop' into feature/expanded-modify-obstructive-code

0594a45

mschile reviewed Jul 18, 2022

View reviewed changes

AtofStryker and others added 5 commits July 19, 2022 09:12

Merge branch 'develop' into feature/expanded-modify-obstructive-code

35f85ec

Merge branch 'develop' into feature/expanded-modify-obstructive-code

ebd157f

Merge branch 'develop' into feature/expanded-modify-obstructive-code

639fead

Merge branch 'develop' into feature/expanded-modify-obstructive-code

1b1e3fc

Merge branch 'develop' into feature/expanded-modify-obstructive-code

98c2c2f

mjhenkes reviewed Jul 20, 2022

View reviewed changes

mjhenkes approved these changes Jul 20, 2022

View reviewed changes

chrisbreiding approved these changes Jul 21, 2022

View reviewed changes

mschile added 2 commits July 21, 2022 11:40

Merge branch 'develop' into feature/expanded-modify-obstructive-code

e895651

Merge branch 'develop' into feature/expanded-modify-obstructive-code

97bb6e0

mschile merged commit 5ff1504 into develop Jul 22, 2022

mschile deleted the feature/expanded-modify-obstructive-code branch July 22, 2022 14:30

cypress-bot bot mentioned this pull request Jul 22, 2022

Write cy.origin integration tests for Azure Active Directory #21476

Closed

mschile mentioned this pull request Jul 22, 2022

Microsoft login removes the Cypress test frame when it redirects, causing tests to fail #21307

Closed

mjhenkes mentioned this pull request Jul 25, 2022

Webpage doesn't load in Cypress where as Page loads perfectly in chrome browser #22684

Closed

AtofStryker mentioned this pull request Jul 25, 2022

Unable to complete a test Paypal payment from a commercial website #22186

Closed

AtofStryker mentioned this pull request Aug 3, 2022

chore: add experimentalModifyObstructiveThirdPartyCode documentation … #23088

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add experimentalModifyObstructiveThirdPartyCode flag for regex rewriter #22568

feat: add experimentalModifyObstructiveThirdPartyCode flag for regex rewriter #22568

AtofStryker commented Jun 28, 2022 •

edited by mschile

Loading

cypress-bot bot commented Jun 28, 2022

cypress bot commented Jun 28, 2022 •

edited

Loading

AtofStryker Jun 28, 2022

AtofStryker Jun 30, 2022

AtofStryker Jun 30, 2022

AtofStryker Jun 28, 2022

mschile Jul 18, 2022 •

edited

Loading

mjhenkes Jul 20, 2022

frnc07 commented Jul 27, 2022

mschile commented Jul 27, 2022

ChrystelCy commented Aug 8, 2022

emilyrohrbough commented Aug 8, 2022

ChrystelCy commented Aug 8, 2022

emilyrohrbough commented Aug 8, 2022

		@@ -5,21 +5,44 @@ const invalidTargets = new Set(['_parent', '_top'])
		export type GuardedEvent = Event & {target: HTMLFormElement \| HTMLAnchorElement}

feat: add experimentalModifyObstructiveThirdPartyCode flag for regex rewriter #22568

feat: add experimentalModifyObstructiveThirdPartyCode flag for regex rewriter #22568

Conversation

AtofStryker commented Jun 28, 2022 • edited by mschile Loading

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jun 28, 2022

cypress bot commented Jun 28, 2022 • edited Loading

Test summary

Run details

AtofStryker Jun 28, 2022

Choose a reason for hiding this comment

AtofStryker Jun 30, 2022

Choose a reason for hiding this comment

AtofStryker Jun 30, 2022

Choose a reason for hiding this comment

AtofStryker Jun 28, 2022

Choose a reason for hiding this comment

mschile Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

mjhenkes Jul 20, 2022

Choose a reason for hiding this comment

frnc07 commented Jul 27, 2022

mschile commented Jul 27, 2022

ChrystelCy commented Aug 8, 2022

emilyrohrbough commented Aug 8, 2022

ChrystelCy commented Aug 8, 2022

emilyrohrbough commented Aug 8, 2022

AtofStryker commented Jun 28, 2022 •

edited by mschile

Loading

cypress bot commented Jun 28, 2022 •

edited

Loading

mschile Jul 18, 2022 •

edited

Loading