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

Recommend duration of challenge validity #1855

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Conversation

emlun
Copy link
Member

@emlun emlun commented Feb 20, 2023

Fixes #1848.

Additions I also considered but decided against:

  • Mentioning that the RP MAY enforce a shorter timeout if a shorter options.timeout is requested. I think the recommendation of a "similar" timeout probably communicates that this is a vague limit with a lot of wiggle room for such things.
  • Recommending that challenges SHOULD NOT be valid past the recommended duration. This is softly implied, and doesn't seem necessary as this section comes from the angle of how long the RP needs to store the challenge, so the RP probably prefers a short time already.
  • Recommending allowing only a single verification attempt per challenge/ceremony. While probably a good idea, it shouldn't matter much in practice, and there might be reasons to allow retrying in some cases. It's also perhaps not entirely clear what "once per ceremony" means - once per navigator.credentials.{create,get}() call or once per session? - so it might be more confusing than helpful.

Preview | Diff

@emlun emlun self-assigned this Feb 20, 2023
Copy link
Contributor

@MasterKale MasterKale left a comment

Choose a reason for hiding this comment

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

I thought to suggest we frame "discouraged" vs "preferred"/"required" as "second-factor" vs "passwordless"/"passkeys", but maybe that framing is too "practical" to be included in the spec?

Anyway I think this looks good so I'm going ahead and approving.

@emlun
Copy link
Member Author

emlun commented Feb 24, 2023

Yes, I think that's a bit too much indirection. The reader has to do more work to relate those terms to the UserVerificationRequirement values, when we can just refer to them directly instead. Especially since this new section is also referenced in the client algorithms, I already think that "the [=[RP]=]'s [=user verification=] preference for the ceremony" is almost too informal as it doesn't explicitly and unambiguously say what defines that preference. But I think it's probably clear enough with the reference to the UserVerificationRequirement values.

@nadalin nadalin added this to the L3-WD-01 milestone Mar 8, 2023
@emlun emlun marked this pull request as draft March 22, 2023 19:16
@emlun
Copy link
Member Author

emlun commented Mar 22, 2023

2023-03-22 WG call: conditional mediation makes this a bit complicated. We might change the API a bit to better support conditional mediation, holding off on this one until that's decided.

@emlun emlun added the stat:Blocked Prerequisites are not yet satisfied label Apr 5, 2023
index.bs Outdated

If the [=[RP]=]'s [=user verification=] preference for the ceremony is

<dl class="switch">
Copy link
Contributor

Choose a reason for hiding this comment

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

See also #1885 where I am proposing adjusting these ranges.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to account for the changes in #1885.

@emlun emlun removed the stat:Blocked Prerequisites are not yet satisfied label Jun 14, 2023
@emlun emlun marked this pull request as ready for review June 28, 2023 10:11
@emlun emlun requested a review from akshayku June 28, 2023 10:12
@emlun
Copy link
Member Author

emlun commented Jun 28, 2023

@akshayku @sbweeden This is now updated to account for the changes in #1885, and ready for re-review.

@Kieun
Copy link
Member

Kieun commented Jun 28, 2023

I was thinking that this issue is blocked until #1856 is resolved.
Regarding original questions for conditional-mediation, the RP client needs to abort request and only allow modal request if the user click some buttons?

@emlun
Copy link
Member Author

emlun commented Jun 28, 2023

Oh right, I forgot. This should probably stay blocked then. This issue is on the agenda for today's WG meeting, we'll decide on that then.

@plehegar plehegar modified the milestones: L3-WD-01, L3-WD-02 Oct 4, 2023
Copy link
Contributor

@ve7jtb ve7jtb 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 d4510f8 into main Nov 15, 2023
2 checks passed
@emlun emlun deleted the issue-1848-challenge-timeout branch November 15, 2023 20:11
github-actions bot added a commit that referenced this pull request Nov 15, 2023
SHA: d4510f8
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How long the relying party should maintain the challenge and related information?
8 participants