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.

Sign up for GitHub

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

Jump to bottom

Note that largeBlob requires discoverable key on CTAP authenticators #1622

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Note that largeBlob requires discoverable key on CTAP authenticators #1622

merged 3 commits into from
Aug 18, 2021

Conversation

emlun
Copy link
Member

@emlun emlun commented Jun 14, 2021 •
edited by pr-preview bot
Loading

I did some interoperability tests with the largeBlob extension, and was confused because Chrome indicated "Your security key can't be used with this site" with largeBlob: { support: "required" } even though my security key should support it. It turned out to be because CTAP only supports the underlying extensions for discoverable keys. This is not a general requirement, but CTAP seems like an important enough class of authenticators to point this out.

Preview | Diff

@emlun emlun requested a review from agl June 14, 2021 00:45
@emlun emlun self-assigned this Jun 14, 2021
agl
agl approved these changes Jun 14, 2021
View reviewed changes
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
equalsJeffH
equalsJeffH reviewed Jun 16, 2021
View reviewed changes
Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

The first sentence seems clear to me, but I don't understand the second sentence.

index.bs Outdated Show resolved Hide resolved
@nadalin nadalin added this to the L3-WD-01 milestone Jun 16, 2021
equalsJeffH
equalsJeffH suggested changes Jun 30, 2021
View reviewed changes
Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

Now that I more fully understand, please see comment below. I tried to get it to apply to the entire set of lines but github would not cooperate, sorry :(

index.bs Outdated Show resolved Hide resolved
emlun
emlun commented Jul 14, 2021
View reviewed changes
index.bs Outdated Show resolved Hide resolved
@equalsJeffH equalsJeffH self-requested a review August 12, 2021 09:33
equalsJeffH
equalsJeffH approved these changes Aug 12, 2021
View reviewed changes
Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

LGTM

@emlun emlun merged commit 38f9f72 into main Aug 18, 2021
@emlun emlun deleted the largeblob-ctap-requires-rk branch August 18, 2021 17:14
github-actions bot added a commit that referenced this pull request Aug 18, 2021
@emlun @github-actions
Merge pull request #1622 from w3c/largeblob-ctap-requires-rk
d35285c 
SHA: 38f9f72
Reason: push, by @emlun

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agl agl agl approved these changes

@equalsJeffH equalsJeffH equalsJeffH approved these changes

Assignees

@emlun emlun

Labels
subtype:extensions type:editorial
Projects
None yet
Milestone
L3-WD-01
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@emlun @agl @equalsJeffH @nadalin