Skip to content

fixup algs contd 2#495

Merged
jcjones merged 5 commits into
masterfrom
jeffh-fixup-algs-contd-2
Jul 5, 2017
Merged

fixup algs contd 2#495
jcjones merged 5 commits into
masterfrom
jeffh-fixup-algs-contd-2

Conversation

@equalsJeffH

@equalsJeffH equalsJeffH commented Jun 16, 2017

Copy link
Copy Markdown
Contributor

Presently, this (hopefully) fixes #480.

This also fixes #475 ("Level 1" editorial addition).

update 28-Jun-2017: this also (re-) fixes #268


Preview | Diff

@AngeloKai AngeloKai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This would be a step closer to fix the algorithm. We should merge this to get us on track soon.

@AngeloKai AngeloKai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I looked through too quickly. PR 495 and 498 are actually two approaches instead of improving on top of the other. I will comment on 498.

@equalsJeffH

Copy link
Copy Markdown
Contributor Author

@AngeloKai

PR 495 and 498 are actually two approaches instead of improving on top of the other

actually they are not two approaches to the same thing -- they address different problems. pr #495 is more simple and ready-to-merge AFAIK. PR #498 needs more work.

@rlin1 rlin1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread index.bs Outdated
Note: In this case, the [=[RP]=] did not supply a list of acceptable credential descriptors. Thus the
authenticator is being asked to exercise any credential it may possess that is bound to the [=[RP]=].
authenticator is being asked to exercise any credential it may possess that is bound to
the [=[RP]=] identified by |rpId|.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

s/identified/as identified/

@equalsJeffH

equalsJeffH commented Jun 27, 2017

Copy link
Copy Markdown
Contributor Author

@rlin1 -- thanks. I have minor comment on the clarification you added -- i can fix it if you like.
update: I fixed it in below commit 9ff5bbf

@AngeloKai AngeloKai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thx Jeff for the clarification. If this is considered a standalone PR, I'd consider it approved.

@equalsJeffH

Copy link
Copy Markdown
Contributor Author

can we go ahead and merge this given @rlin1's and @AngeloKai's reviews? thx.

@jcjones jcjones merged commit 49da4d6 into master Jul 5, 2017
WebAuthnBot pushed a commit that referenced this pull request Jul 5, 2017
@emlun emlun deleted the jeffh-fixup-algs-contd-2 branch April 23, 2019 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants