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

Check Feature Policy before attaching Client Hints #811

Closed
wants to merge 2 commits into from

Conversation

eeeps
Copy link

@eeeps eeeps commented Sep 24, 2018

This PR is part of an attempt to address w3c/webappsec-permissions-policy#129 – using Feature Policy to let authors opt-into sending specific Client Hints to specific origins.

The overall, basic idea is:

  1. A new set of policy-controlled features – one for each Client Hint header field – is defined in the Client Hints spec.
  2. A new algorithm which determines whether a request is able to use a policy-controlled feature is defined in the Feature Policy spec.
  3. The step in Fetch that appends Client Hint headers calls the new algorithm, checking if a request is allowed to use a Client Hint’s policy-controlled feature before appending the Client Hint header. That change is contained in this PR.

Preview | Diff

policy-controlled feature</a>, returns <code>true</code>, the user agent should
<a for="header list">append</a> <var>hintName</var>/<var>value</var> to <var>request</var>'s
<a for=request>header list</a>.
[[!FEATURE-POLICY]] [[!CLIENT-HINTS]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some existing plumbing that we need to refactor:

I think the behavior we want here is..

  1. We need to define the Accept-CH opt-in processing in HTML spec
  2. For nav request, update Fetch to reference (1) instead of current "client hints tokens" ref to IETF spec
  3. For subresources, execute algorithm defined in FP, checking for delegation

@eeeps @annevk WDYT, does that make sense?

Copy link
Collaborator

@yoavweiss yoavweiss Oct 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) and (2) are implemented (but not yet landed) in whatwg/html#3774 and #773

@eeeps & @tarunban - maybe we should merge all the different CH related PRs, to make it easier to discuss them in a holistic manner. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd much prefer we don't add feature policy until the architecture is more correctly described.

@tarunban
Copy link

tarunban commented Oct 2, 2018

I agree that it's best to merge related PRs. However, I really do not have a pending PR. So, I am not sure if I can help much here.

@eeeps
Copy link
Author

eeeps commented Jan 17, 2019

Closing, as I've moved this PR at over to yoavweiss#1 in order to consolidate CH/Fetch work.

@eeeps eeeps closed this Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants