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

move the credentialId uniqueness handling to the formal alg steps. #709

Merged
merged 3 commits into from Dec 21, 2017

Conversation

rlin1
Copy link
Contributor

@rlin1 rlin1 commented Dec 6, 2017

Close #579


Preview | Diff

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, thx @rlin1 ! one minor suggested edit below.

index.bs Outdated
13. If the attestation statement |attStmt| verified successfully and is found to be trustworthy, then register the new
1. Check that the <code>[=credentialId=]</code> is not yet registered to any other user. If registration
is requested for a credential that is already registered to a different user, the [=[RP]=] SHOULD
fail this ceremony, or it MAY decide to accept the registration, e.g. while deleting the older registration.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/this ceremony /this [=registration=] ceremony /

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@equalsJeffH equalsJeffH added this to the CR milestone Dec 6, 2017
@equalsJeffH
Copy link
Contributor

reviewed: #709 (review)
LGTM
assigned to CR milestone.

Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@rlin1
Copy link
Contributor Author

rlin1 commented Dec 13, 2017

Just waiting for feedback from @AngeloKai

@rlin1
Copy link
Contributor Author

rlin1 commented Dec 20, 2017

Waiting for feedback from @akshayku instead.

@rlin1 rlin1 removed the request for review from AngeloKai December 20, 2017 18:43
Copy link
Contributor

@akshayku akshayku left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@rlin1 rlin1 merged commit a6c0da2 into master Dec 21, 2017
@rlin1 rlin1 deleted the cred-id-uniqueness-579 branch December 21, 2017 07:19
WebAuthnBot pushed a commit that referenced this pull request Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants