fix: Ensure basic auth headers are set on extra target requests #28387
Conversation
| }) | ||
|
|
||
| // NOTE: webkit opens a new window and doesn't download the file | ||
| it('downloads basic auth protected file that opens in a new tab', { browser: '!webkit' }, () => { |
There was a problem hiding this comment.
can this assert that a new window was opened successfully rather than skipping it in webkit?
There was a problem hiding this comment.
Having a new window open isn't the desired behavior, so I would hesitate to codify that behavior. It's supposed to download the file, but it seems that doesn't work in webkit for some reason. Might be something we fix in the future, but I'd say it's out of scope for this bug fix.
| @@ -52,6 +52,7 @@ const ExtractCypressMetadataHeaders: RequestMiddleware = function () { | |||
| delete this.req.headers['x-cypress-is-from-extra-target'] | |||
|
|
|||
| this.onlyRunMiddleware([ | |||
| 'MaybeSetBasicAuthHeaders', | |||
There was a problem hiding this comment.
Do we really want to attach basic auth to all requests in the new target? The download use-case seems like the exception not the default.
There was a problem hiding this comment.
I guess we would only attach if the origins match, correct? So, that's probably fine.
There was a problem hiding this comment.
Yeah, they have to cy.visit() the origin with the credentials first
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Fixes a regression caused by #28188, which cut down the request middleware a bit too much. The authorization header for basic auth set through
cy.visit()was not being carried through for extra tabs.PR Tasks
cypress-documentation?type definitions?