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

Acceptable UV caching time affordance extension added #2021

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -7580,6 +7580,61 @@ To <dfn abstract-op>Create a new supplemental public key record</dfn>, perform t
[=set/append=] this [=supplemental public key record=] to |credentialRecord|.[$credential record/supplementalPubKeys$].


### User verification caching extension (<dfn>userVerificationCaching</dfn>) ### {#sctn-user-verification-caching-extension}

In some cases it is good enough for the [=[RP]=] to know whether the user was verified by the authenticator "recently".

This extension allows the [=[RP]=] to specify such [=user verification=] caching time, i.e. the time for which the [=user verification=] status can be "cached" by the [=authenticator=].

For example: Do not ask the user for a fresh [=user verification=] for sign-in if the user was verified by this authenticator within the past 300 seconds.

: Extension identifier
:: `userVerificationCaching`

: Operation applicability
Copy link
Member

Choose a reason for hiding this comment

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

This is where the user verification selection criteria should go IMO.

Also shouldn't this apply to both creation and authentication?

If so, suggest the following:

:   Operation applicability
::  [=registration extension|Registration=] and [=authentication extension|Authentication=] when {{AuthenticatorSelectionCriteria/userVerification}} is set to {{UserVerificationRequirement/preferred}}.

:: [=authentication extension|authentication=]

: Client extension input
:: The maxTimeSinceLastUV denotes the maximum acceptable number of milliseconds elapsed since the last time the user was successfully verified.
Copy link
Member

Choose a reason for hiding this comment

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

Should we bound this with a min and max? (#2034 (comment))

<xmp class="idl">
partial dictionary AuthenticationExtensionsClientInputs {
unsigned long long maxTimeSinceLastUV;
Copy link
Member

Choose a reason for hiding this comment

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

This member name needs to be the same as the extension identifier.

Suggested change
unsigned long long maxTimeSinceLastUV;
unsigned long long userVerificationCaching;

Copy link
Member

@timcappalli timcappalli Mar 27, 2024

Choose a reason for hiding this comment

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

should long be there twice? I guess it needs to be if we're not going to bound it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, unsigned long long is the WebIDL type for a 64-bit integer.

...oh, I was about to say we use the same type for timeout, but it turns out it's just unsigned long (32-bit). So yeah, let's make it just unsigned long here too for consistency. 264 milliseconds is almost 50 days, I can't imagine anyone needing more range than that.

};
</xmp>

: Client extension processing
:: None, except creating the [=authenticator extension input=] from the client extension input.

: Client extension output
:: Returns the number of milliseconds elapsed since the last time the user was successfully verified as returned by the [=authenticator=].
Copy link
Member

Choose a reason for hiding this comment

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

s/verified as/verified, as

<xmp class="idl">
partial dictionary AuthenticationExtensionsClientOutputs {
unsigned long long timeSinceLastUV;
Copy link
Member

Choose a reason for hiding this comment

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

This member name needs to be the same as the extension identifier.

Suggested change
unsigned long long timeSinceLastUV;
unsigned long long userVerificationCaching;

};
</xmp>

: Authenticator extension input
:: The maximum acceptable time in milliseconds elapsed since last user verification, encoded in CBOR.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's missing here is the negative case - if this extension is not present, do not allow caching at all. This needs to be clearly stated in the spec.


```
$$extensionInput //= (
mtslUV: uint .size 4
Copy link
Member

Choose a reason for hiding this comment

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

This member name needs to be the same as the extension identifier.

Suggested change
mtslUV: uint .size 4
userVerificationCaching: uint .size 4

)
```

: Authenticator extension processing
:: When user verification is requested, the [=authenticator=] triggers user verification only if more milliseconds have elapsed since the last time the user was verified than indicated by the maxTimeSinceLastUV value in the extension.
Copy link
Member

Choose a reason for hiding this comment

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

"If more milliseconds have elapsed since the last time the user was verified than indicated by the maxTimeSinceLastUV value in the extension, request [=user verification=]."


: Authenticator extension output
:: If no fresh user verification needed to be triggered triggered, the authenticator reports the time last last user verification time back to the [=[RP]=] to ensure the [=[RP]=] is aware that no fresh user verification was triggered. It is up to the authenticator to decide whether to return the real elapsed time, or a "rounded" value. If user verification was requested, this value SHALL not exceed the value originally provided in the extension input.
Copy link
Contributor

@sbweeden sbweeden Mar 20, 2024

Choose a reason for hiding this comment

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

What is the extension output if UV is done as part of the current ceremony? I know that the UV bit should be set in authenticatorData, but should the extension output also be set with a value of 0 perhaps? The purpose of returning that is to at least indicate to the RP that the extension was acknowledge and processed. The RP could infer that UV was done, but might otherwise not have been if the maxTimeSinceLastUV had been shorter. This would differentiate it from an authenticator that doesn't support the extension and just did UV because it always does UV when preferred is set for userVerification in the initial authentication request.

Copy link
Contributor

@MasterKale MasterKale Mar 27, 2024

Choose a reason for hiding this comment

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

To be clear, the suggestion here is that extension output should be set to 0 if UV takes place during the ceremony? Which naturally means the extension output should never be greater than the duration specified as maxTimeSinceLastUV. That sounds sensible to me.


```
$$extensionOutput //= (
tslUV: uint .size 4
Copy link
Member

Choose a reason for hiding this comment

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

This member name needs to be the same as the extension identifier.

Suggested change
tslUV: uint .size 4
userVerificationCaching: uint .size 4

)
```


# User Agent Automation # {#sctn-automation}

For the purposes of user agent automation and [=web application=] testing, this document defines a number of [[WebDriver]] [=extension commands=].
Expand Down
Loading