Skip to content
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

[Feature] Trace viewer: allow fetching traces from origins that require credentials #28501

Closed
pmattj opened this issue Dec 5, 2023 · 3 comments · Fixed by #28502
Closed

[Feature] Trace viewer: allow fetching traces from origins that require credentials #28501

pmattj opened this issue Dec 5, 2023 · 3 comments · Fixed by #28502

Comments

@pmattj
Copy link
Contributor

pmattj commented Dec 5, 2023

My team is working to allow our developers to load traces directly in the online trace viewer. Our ideal scenario is that a developer could open a test result document and jump directly to the trace in the viewer with a simple click of a link.

Our CI infrastructure uploads traces to a location protected by credentials — to fetch a trace, the request needs to contain valid authentication cookies. These cookies are sent if we open the trace URL in the browser directly, but we've found that the online trace viewer does the trace fetch without cookies, even though the origin sends Access-Control-Allow-Credentials: true.[1]

We suspected this could be due to the trace viewer not applying the credentials option on the fetch request. We tested this theory by deploying the trace viewer locally and adding a credentials: 'include' option on this line, and this allowed our local trace viewer to load the trace.

Our request is for this option to be added upstream. This would allow us to use the Playwright-hosted trace viewer rather than hosting it ourselves, and I suspect it would unblock other customers whose traces are credential-protected (assuming their origins also send the proper Access-Control-Allow-Origin and Access-Control-Allow-Credentials headers).

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

@pmattj pmattj changed the title [Feature] Trace viewer: allow fetching traces from origins with credentials required [Feature] Trace viewer: allow fetching traces from origins that require credentials Dec 5, 2023
pmattj added a commit to pmattj/playwright that referenced this issue Dec 5, 2023
…traces cross-origin

Add the "'credentials': include" option on the trace fetch so the browser can include
cookies when fetching from a different origin, assuming the origin returns the correct
Access-Control-Allow-Origin and Access-Control-Allow-Credentials headers.

Fixes microsoft#28501
@dgozman dgozman self-assigned this Dec 5, 2023
@dgozman dgozman added the v1.41 label Dec 5, 2023
dgozman pushed a commit that referenced this issue Dec 14, 2023
…traces cross-origin (#28502)

Add the `'credentials': include` option on the trace fetch so the
browser can include cookies when fetching from a different origin,
assuming the origin returns the correct Access-Control-Allow-Origin and
Access-Control-Allow-Credentials headers.

Fixes #28501
@dgozman
Copy link
Contributor

dgozman commented Jan 17, 2024

Reopening since the fix was reverted in #29024.

@mxschmitt
Copy link
Member

Note: For the next time we implement it, we should add a URL parameter like credentials=include which only then adds this specific property to Fetch. By default it should not do that.

@pavelfeldman
Copy link
Member

Why was this issue closed?

Thank you for your involvement. This issue was closed due to limited engagement (upvotes/activity), lack of recent activity, and insufficient actionability. To maintain a manageable database, we prioritize issues based on these factors.

If you disagree with this closure, please open a new issue and reference this one. More support or clarity on its necessity may prompt a review. Your understanding and cooperation are appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants