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

FF123 updates for permission publickey-credentials-create #32135

Merged
merged 13 commits into from
Feb 15, 2024

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Feb 6, 2024

FF123 supports Feature policy publickey-credentials-create in https://bugzilla.mozilla.org/show_bug.cgi?id=1870863
As per usual for FF, this can't be set on the header, but only via allow on the frame.

This attempts to update the docs with some corrections according to advice from engineers and my interpretation of the spec. Note that I think there are probably more relevant exceptions, but I don't propose to go through all the specs right now.

The specific issues fixed are:

  1. Docs say creating/getting a credential is a SecurityError. Spec says NotAllowedError in step 5 here
  2. If allow is declared in an iframe then transient activation is required on the iframe as well, or you get the same error.
  3. This explicitly said you can't use create() in an iframe, but that was changed in WebAuthn spec in Allow for credential creation in a cross-origin iframe w3c/webauthn#1801 and to the Credential Management spec in Add support for publickey-credentials-create permission policy w3c/webappsec-credential-management#209. I added some examples.

I'm seeing confirmation from engineers too in https://bugzilla.mozilla.org/show_bug.cgi?id=1870863#c7 but the spec seems fairly clear on the NotAllowedError and transient activation.

Other docs work can be tracked in #31890

@hamishwillee hamishwillee requested review from a team as code owners February 6, 2024 04:25
@hamishwillee hamishwillee requested review from wbamberg, teoli2003 and chrisdavidmills and removed request for a team February 6, 2024 04:25
@github-actions github-actions bot added Content:WebAPI Web API docs Content:HTTP HTTP docs size/s [PR only] 6-50 LoC changed labels Feb 6, 2024
@hamishwillee
Copy link
Collaborator Author

Thanks @dasJ and @jschanck. I've fixed up the issues and also clarified the cases where Transient activation are required.

@hamishwillee hamishwillee requested a review from a team as a code owner February 9, 2024 06:05
@github-actions github-actions bot added the Content:HTML Hypertext Markup Language docs label Feb 9, 2024
@hamishwillee hamishwillee requested review from wbamberg and dasJ February 9, 2024 06:11
@github-actions github-actions bot added size/m [PR only] 51-500 LoC changed and removed size/s [PR only] 6-50 LoC changed labels Feb 11, 2024
@hamishwillee hamishwillee force-pushed the ff123_credentials_create branch from 56eef41 to 2959b46 Compare February 12, 2024 20:47
@hamishwillee
Copy link
Collaborator Author

@wbamberg Anything else needed here (when you have a moment)

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Just a couple of nits.

files/en-us/web/api/web_authentication_api/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/web_authentication_api/index.md Outdated Show resolved Hide resolved
Co-authored-by: wbamberg <will@bootbonnet.ca>
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

👍 thank you Hamish!

@wbamberg wbamberg merged commit 9db533c into mdn:main Feb 15, 2024
9 checks passed
@hamishwillee hamishwillee deleted the ff123_credentials_create branch February 15, 2024 22:23
@hamishwillee
Copy link
Collaborator Author

No, thanks for your patience and help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants