fix: Disallow `same-superdomain-origin` cy.origin blocks #24569

mjhenkes · 2022-11-07T17:54:21Z

Closes cy.origin should throw an error when block is declared within same-origin context #24169

User facing changelog

cy.origin will now prevent users from setting up same-superdomain-origin cy.origin blocks. In these cases cy.origin is not required and users would be better served by not using the command.

Additional details

There are several changes included in this pr

Creating a cy.origin block with the same-superdomain-origin as top will throw an error.
Added a list of domains which are exceptions to the above rule and will only throw an error if the urls are strictly same-origin. Google is the only domain that is currently excepted.
Correspondingly we now use the same list of domains to know when to inject 'fullCrossOrigin' in requests for the excepted domains that violate same-origin but not same-superdomain-origin.
I noticed when top is in a secure context (https), we fail to create a spec bridge for an insecure context (http). I added a timeout for cy.origin to handle cases when the spec bridge isn't created. Previously this would cause a hang in the test.

Steps to test

Run the tests added in the PR.

How has the user experience changed?

Same domain error cy cy.origin:

Failed to create spec bridge error:

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?

…igin as top.

…-origin-blocks

cypress-bot · 2022-11-07T17:54:23Z

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.

…-origin-blocks

packages/driver/cypress/e2e/e2e/origin/validation.cy.ts

mjhenkes · 2022-11-07T17:58:15Z

packages/driver/src/cy/commands/origin/index.ts


      const origin = location.origin

      // This is set while IN the cy.origin command.
      cy.state('currentActiveOrigin', origin)

      return new Bluebird((resolve, reject, onCancel) => {
-        const cleanup = ({ readyForOriginFailed }: {readyForOriginFailed?: boolean} = {}): void => {


readyForOriginFailed was dead code

mjhenkes · 2022-11-07T18:27:22Z

packages/network/lib/cors.ts

  })
 }

-export function urlMatchesOriginProps (url, props) {


I removed a couple of functions that were not used anywhere:

urlMatchesOriginProps

urlMatchesPolicyProps

urlMatchesSameSiteProps

urlsSuperDomainOriginMatch and urlMatchesSuperDomainOriginProps are replaced by urlMatchesPolicyBasedOnDomain and urlMatchesPolicyBasedOnDomainProps

mjhenkes · 2022-11-07T18:28:15Z

packages/server/lib/controllers/files.js

@@ -39,13 +39,14 @@ module.exports = {
    })
  },

-  handleCrossOriginIframe (req, res) {
+  handleCrossOriginIframe (req, res, namespace) {


These are changes to make cypress in cypress work better (not totally fixed yet)

mjhenkes · 2022-11-07T18:28:48Z

packages/server/lib/routes-e2e.ts

+  // TODO: The below route is not technically correct for cypress in cypress tests.
+  // We should be using 'config.namespace' to provide the namespace instead of hard coding __cypress, however,
+  // In the runner when we create the spec bridge we have no knowledge of the namespace used by the server so
+  // we create a spec bridge for the namespace of the server specified in the config, but that server hasn't been created.
+  // To fix this I think we need to find a way to listen in the cypress in cypress server for routes from the server the
+  // cypress instance thinks should exist, but that's outside the current scope.


Comment says it all

mjhenkes · 2022-11-07T18:52:26Z

packages/server/lib/remote_states.ts

-      const { subdomain: _unused, ...remoteProps } = cors.parseUrlIntoHostProtocolDomainTldPort(remoteOrigin)
+      const remoteProps = cors.parseUrlIntoHostProtocolDomainTldPort(remoteOrigin)


Without this change, checking same-origin of something like www.google.com would always fail since the www was getting dropped. We need to keep subdomain.

…-origin-blocks

cypress · 2022-11-07T19:38:05Z

Test summary

4179 • 0 • 1095 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`03d7842`
Started	Nov 8, 2022 4:37 PM
Ended	Nov 8, 2022 4:54 PM
Duration	16:35 💡
OS	Linux Debian -
Browser	WebKit 16

View run in Cypress Dashboard ➡️

Flakiness

	cypress/e2e/commands/xhr.cy.js		1
1	src/cy/commands/xhr > abort > aborts xhrs currently in flight

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

I wonder if we need to change the messaging on whether the user provides same origin vs cross origin but is same super domain origin to help point them to the right place

packages/driver/cypress/e2e/e2e/origin/origin.cy.ts

packages/driver/cypress/e2e/e2e/origin/validation.cy.ts

packages/driver/src/cypress/error_messages.ts

packages/network/lib/cors.ts

packages/server/lib/server-e2e.ts

packages/driver/cypress/e2e/e2e/origin/validation.cy.ts

rnovosad · 2022-11-22T17:41:29Z

packages/network/lib/cors.ts

@@ -11,6 +11,8 @@ const debug = debugModule('cypress:network:cors')
 // match IP addresses or anything following the last .
 const customTldsRe = /(^[\d\.]+$|\.[^\.]+$)/

+const strictSameOriginDomains = Object.freeze(['google.com'])


@mjhenkes can we allow somehow parametrizing this? we have mobile tests that are on the same domain, but they require different user-agent. we were trying to override it with cy.origin() and it worked perfectly well till this PR change...

@rnovosad would you be able to share an example?

@AtofStryker I provided it in the comment #2100 (comment)

@rnovosad got it. I will reply to the thread there!

mjhenkes added 8 commits November 1, 2022 11:32

fix: throw error if the cy.origin origin is in the same superDomainOr…

57635c0

…igin as top.

testing test tweaks

f0e7f41

'fix' cypress in cypress tests

e4be3d3

Merge branch 'develop' into matth/fix/disallow-same-superDomainOrigin…

0bc579d

…-origin-blocks

Inject cross origin in google subdomains when not same-origin

45a6474

style tweaks

a0fd562

Ensure strict same-origin check works for google.

7fb829c

test fixes

581bb07

mjhenkes self-assigned this Nov 7, 2022

mjhenkes changed the title ~~Disallow same-superdomain-origin cy.origin blocks~~ Fix: Disallow same-superdomain-origin cy.origin blocks Nov 7, 2022

Merge branch 'develop' into matth/fix/disallow-same-superDomainOrigin…

46cac95

…-origin-blocks

mjhenkes commented Nov 7, 2022

View reviewed changes

packages/driver/cypress/e2e/e2e/origin/validation.cy.ts Outdated Show resolved Hide resolved

mjhenkes commented Nov 7, 2022

View reviewed changes

mjhenkes added 2 commits November 7, 2022 12:08

we don't need the location object when we just want the href.

e7cbf8b

what is in a name?

9d8c9c6

mjhenkes commented Nov 7, 2022

View reviewed changes

Merge branch 'develop' into matth/fix/disallow-same-superDomainOrigin…

2d8501d

…-origin-blocks

mjhenkes marked this pull request as ready for review November 7, 2022 19:08

mjhenkes requested a review from AtofStryker November 7, 2022 19:46

AtofStryker reviewed Nov 7, 2022

View reviewed changes

chrisbreiding changed the title ~~Fix: Disallow same-superdomain-origin cy.origin blocks~~ fix: Disallow same-superdomain-origin cy.origin blocks Nov 7, 2022

chrisbreiding approved these changes Nov 7, 2022

View reviewed changes

packages/driver/cypress/e2e/e2e/origin/validation.cy.ts Outdated Show resolved Hide resolved

Address PR Comments

03d7842

AtofStryker self-requested a review November 8, 2022 14:55

AtofStryker approved these changes Nov 8, 2022

View reviewed changes

mjhenkes merged commit 23299ac into develop Nov 9, 2022

mjhenkes deleted the matth/fix/disallow-same-superDomainOrigin-origin-blocks branch November 9, 2022 14:29

cypress-bot bot mentioned this pull request Nov 9, 2022

cy.origin should throw an error when block is declared within same-origin context #24169

Closed

rnovosad reviewed Nov 22, 2022

View reviewed changes

nagash77 mentioned this pull request Nov 28, 2022

.origin() doesn't differentiate subdomains as individual origins #24820

Closed

mmonteiroc mentioned this pull request Feb 14, 2023

Azure AD B2C bad request when redirecting / using cy.origin() #25806

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Disallow `same-superdomain-origin` cy.origin blocks #24569

fix: Disallow `same-superdomain-origin` cy.origin blocks #24569

mjhenkes commented Nov 7, 2022 •

edited by chrisbreiding

cypress-bot bot commented Nov 7, 2022

mjhenkes Nov 7, 2022

mjhenkes Nov 7, 2022

mjhenkes Nov 7, 2022

mjhenkes Nov 7, 2022

mjhenkes Nov 7, 2022

cypress bot commented Nov 7, 2022 •

edited

AtofStryker left a comment

rnovosad Nov 22, 2022

AtofStryker Nov 23, 2022

rnovosad Nov 23, 2022

AtofStryker Nov 28, 2022

		const { subdomain: _unused, ...remoteProps } = cors.parseUrlIntoHostProtocolDomainTldPort(remoteOrigin)
		const remoteProps = cors.parseUrlIntoHostProtocolDomainTldPort(remoteOrigin)

fix: Disallow same-superdomain-origin cy.origin blocks #24569

fix: Disallow same-superdomain-origin cy.origin blocks #24569

Conversation

mjhenkes commented Nov 7, 2022 • edited by chrisbreiding

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Nov 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Nov 7, 2022 • edited

Test summary

Run details

Flakiness

AtofStryker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fix: Disallow `same-superdomain-origin` cy.origin blocks #24569

fix: Disallow `same-superdomain-origin` cy.origin blocks #24569

mjhenkes commented Nov 7, 2022 •

edited by chrisbreiding

cypress bot commented Nov 7, 2022 •

edited