-
Notifications
You must be signed in to change notification settings - Fork 167
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
Make makeCredential() more precise. #347
Changes from all commits
b69aefa
c172b17
7a0d27a
7ff341d
1ca3523
788ec72
99b6ec5
62a5ac0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,10 @@ Markup Shorthands: css off, markdown on | |
|
||
<pre class="anchors"> | ||
|
||
spec: ECMAScript; urlPrefix: https://tc39.github.io/ecma262/# | ||
type: method | ||
for: JSON; text: stringify; url: sec-json.stringify | ||
|
||
<!-- spec: HTML; urlPrefix: https://html.spec.whatwg.org/multipage/ --> | ||
spec: HTML52; urlPrefix: https://w3c.github.io/html/ | ||
type: dfn | ||
|
@@ -53,6 +57,10 @@ spec: HTML52; urlPrefix: https://w3c.github.io/html/ | |
type: interface | ||
text: Navigator | ||
|
||
spec: TokenBinding; urlPrefix: https://tools.ietf.org/html/draft-ietf-tokbind-protocol-13# | ||
type: dfn | ||
text: Token Binding ID; url: section-3.2 | ||
|
||
spec: WebCryptoAPI; urlPrefix: https://www.w3.org/TR/WebCryptoAPI/ | ||
type: dfn | ||
text: normalizing an algorithm; url: dfn-normalize-an-algorithm | ||
|
@@ -61,6 +69,7 @@ spec: WebCryptoAPI; urlPrefix: https://www.w3.org/TR/WebCryptoAPI/ | |
</pre> <!-- class=anchors --> | ||
|
||
<pre class="link-defaults"> | ||
spec:infra; type:dfn; text:list | ||
spec:webidl; type:interface; text:Promise | ||
</pre> | ||
|
||
|
@@ -161,7 +170,8 @@ or a combination of both. | |
|
||
## Dependencies ## {#dependencies} | ||
|
||
This specification relies on several other underlying specifications. | ||
This specification relies on several other underlying specifications, listed | ||
below and in [[#index-defined-elsewhere]]. | ||
|
||
: Base64url encoding | ||
:: The term <dfn>Base64url Encoding</dfn> refers to the base64 encoding using the URL- and filename-safe character set defined | ||
|
@@ -372,6 +382,7 @@ underlying platform, which may involve data storage managed by the browser or th | |
approve this operation. On success, the promise will be resolved with a {{ScopedCredentialInfo}} object describing the newly | ||
created credential. | ||
|
||
<div class="note"> | ||
This method takes the following parameters: | ||
|
||
<ul dfn-type="argument" dfn-for="WebAuthentication/makeCredential(accountInformation, cryptoParameters, attestationChallenge, options)"> | ||
|
@@ -397,6 +408,7 @@ This method takes the following parameters: | |
[[#credential-options]]. | ||
|
||
</ul> | ||
</div> | ||
|
||
When this method is invoked, the user agent MUST execute the following algorithm: | ||
|
||
|
@@ -405,13 +417,10 @@ When this method is invoked, the user agent MUST execute the following algorithm | |
value. If {{ScopedCredentialOptions/timeout}} was not specified, then set |adjustedTimeout| to a platform-specific | ||
default. | ||
|
||
1. Let |promise| be [=a new Promise=]. Return |promise| and start a timer for |adjustedTimeout| milliseconds. | ||
Then asynchronously continue executing the following steps. If any fatal error is encountered in this process other than the | ||
ones enumerated below, cancel the timer, reject |promise| with a {{DOMException}} whose name is "{{UnknownError}}", and terminate | ||
this algorithm. | ||
Issue: Put some constraints on the "reasonable range". | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use Github to track issues and not put them in the document? Over time it becomes painful to track these inline issues. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see thread here: #347 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Straw man: I chose [15,120] seconds as the range for timeouts in the first Firefox implementation. Under 15 seconds seems too short to dig out a security key, and making things blink for more than two minutes seemed cruel. |
||
|
||
1. Set |callerOrigin| to the <a>current settings object</a>'s <a>origin</a>. If |callerOrigin| is | ||
an <a>opaque origin</a>, reject |promise| with a {{DOMException}} whose name is "{{NotAllowedError}}", and | ||
an <a>opaque origin</a>, [=reject=] |promise| with a {{DOMException}} whose name is "{{NotAllowedError}}", and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But you deleted the previous step where |promise| was instantiated, so at this point it is undefined. I suspect what you want to do is keep the part about creating |promise| in the step above this one, and only go async by returning |promise| later on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, just get rid of |promise| entirely, use [=a promise rejected with=] here and in other error cases, and use [=a new promise=] when going async below. |
||
terminate this algorithm. | ||
|
||
1. If the {{ScopedCredentialOptions/rpId}} member of {{options}} is not <a>present</a>, then set |rpId| to | ||
|
@@ -424,58 +433,150 @@ When this method is invoked, the user agent MUST execute the following algorithm | |
terminate this algorithm. | ||
1. Set |rpId| to the {{ScopedCredentialOptions/rpId}}. | ||
|
||
1. Process each element of {{cryptoParameters}} using the following steps, to produce a new sequence |normalizedParameters|. | ||
- Let |current| be the currently selected element of {{cryptoParameters}}. | ||
- If `current.type` does not contain a {{ScopedCredentialType}} supported by this implementation, then stop processing | ||
|current| and move on to the next element in {{cryptoParameters}}. | ||
- Let |normalizedAlgorithm| be the result of <a>normalizing an algorithm</a> [[!WebCryptoAPI]], | ||
with |alg| set to `current.algorithm` and |op| set to 'generateKey'. If an error occurs during this | ||
procedure, then stop processing |current| and move on to the next element in {{cryptoParameters}}. | ||
- Add a new object of type {{ScopedCredentialParameters}} to |normalizedParameters|, with |type| set to `current.type` and | ||
|algorithm| set to |normalizedAlgorithm|. | ||
|
||
1. If |normalizedAlgorithm| is empty and {{cryptoParameters}} was not empty, cancel the timer started in step 2, reject | ||
|promise| with a DOMException whose name is "{{NotSupportedError}}", and terminate this algorithm. | ||
|
||
1. If the {{ScopedCredentialOptions/extensions}} member of {{options}} is <a>present</a>, process any extensions supported by | ||
this client platform, to produce the extension data that needs to be sent to the authenticator. If an error is encountered | ||
while processing an extension, skip that extension and do not produce any extension data for it. Call the result of this | ||
processing |clientExtensions|. | ||
Issue(w3c/webauthn#259): The rest of this algorithm assumes |rpId| is an | ||
[=origin=], but the above step sometimes produces a string. | ||
|
||
1. Let |normalizedParameters| be a new [=list=] whose [=list/items=] are pairs of | ||
ScopedCredentialType and a [=dictionary=] type (as returned by [=normalizing | ||
an algorithm=]). | ||
1. [=list/For each=] |current| of {{cryptoParameters}}: | ||
1. If <code>|current|.{{ScopedCredentialParameters/type}}</code> does not | ||
contain a {{ScopedCredentialType}} supported by this implementation, | ||
then [=continue=]. | ||
1. Let |normalizedAlgorithm| be the result of <a>normalizing an algorithm</a> | ||
[[!WebCryptoAPI]], with |alg| set to | ||
<code>|current|.{{algorithm}}</code> and |op| set to `"generateKey"`. If | ||
an error occurs during this procedure, then [=continue=]. | ||
1. [=list/Append=] the pair of | ||
<code>|current|.{{ScopedCredentialParameters/type}}</code> and | ||
|normalizedAlgorithm| to |normalizedParameters|. | ||
|
||
1. If |normalizedParameters| is empty and {{cryptoParameters}} was not empty, | ||
cancel the timer started in step 2, return [=a promise rejected with=] with | ||
a {{DOMException}} whose name is "{{NotSupportedError}}", and terminate this | ||
algorithm. | ||
|
||
1. Let |clientExtensions| be a new [=list=]. | ||
1. If the {{ScopedCredentialOptions/extensions}} member of {{options}} is | ||
<a>present</a>, then [=map/for each=] |extension| → |argument| of | ||
<code>{{options}}.{{ScopedCredentialOptions/extensions}}</code>: | ||
1. If |extension| is not supported by this client platform, then either: | ||
* [=Continue=], or | ||
* Let |result| be a CBOR ([[!RFC7049]]) encoding of |extension|. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The consensus among implementers was that we do not want to blindly pass extensions along since this may break a contract the UA has with the user, e.g. you may blindly pass along a location extension when the user has asked to block location info. So maybe we should have [=Continue=] as the only option. |
||
|
||
Issue(w3c/webauthn#363): Define this encoding more precisely. | ||
1. Otherwise, let |result| be the result of running |extension|'s [=client | ||
processing=] algorithm on |argument|. If the algorithm returned an | ||
error, [=continue=]. | ||
|
||
Issue(w3c/webauthn#363): Ensure all extensions define a client | ||
processing algorithm. | ||
1. [=list/Append=] |result| to |clientExtensions|. | ||
|
||
1. Let |clientData| be a new {{ClientData}} instance whose fields are: | ||
: {{challenge}} | ||
:: The [=base64url encoding=] of {{attestationChallenge}} | ||
: {{origin}} | ||
:: The [=unicode serialization of an origin|unicode serialization=] of |rpId| | ||
: {{hashAlg}} | ||
:: UA-chosen | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've intentionally avoided using "UA" anywhere in this spec. Suggest "Hash algorithm for generating clientDataHash, by the client". Maybe we could recommend SHA-256 for maximum interop. |
||
|
||
Issue(w3c/webauthn#362): We need *some* constraints on the possible hash | ||
algorithms, or else sites will fail on unusual UAs. | ||
: {{tokenBinding}} | ||
:: The [=Token Binding ID=] associated with |callerOrigin| (if any) | ||
|
||
Issue(w3c/webauthn#360): Make sure this association was set up properly. | ||
: {{ClientData/extensions}} | ||
:: |clientExtensions| | ||
|
||
1. Let |clientDataJSON| be the [=UTF-8 encoding=] of the result of calling the | ||
initial value of {{JSON/stringify|JSON.stringify}} on |clientData|. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this fixes #274 by precisely specifying the JSON format. Do UAs need to use the flexibility that was here before? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we don't want to allow for multiple JSON implementations (and then later have that end up being required due to some silliness/webness). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to make this change centrally in https://w3c.github.io/webauthn/#clientdata-clientdatajson and reference that here. Maybe we should also change the variable name so that we can say something like "Generate the [=clientDataJSON=] and call it |J|." |
||
|
||
Issue: Some extensions contain ArrayBuffers, which don't stringify well. | ||
What's the intent here? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should base64url encode those just as we do with the challenge. Need to update client processing sections of extensions, maybe this should be part of #363. |
||
1. Let |clientDataHash| be the hash of |clientDataJSON| using | ||
<code>|clientData|.{{hashAlg}}</code>. | ||
|
||
1. Let |issuedRequests| and |currentlyAvailableAuthenticators| be new [=ordered | ||
sets=]. | ||
|
||
1. For each |authenticator| currently available on this platform, if | ||
<code>{{options}}.{{ScopedCredentialOptions/attachment}}</code> is not | ||
[=present=] or its value matches |authenticator|'s attachment modality, | ||
[=set/append=] |authenticator| to |currentlyAvailableAuthenticators|. | ||
|
||
1. [=set/For each=] |authenticator| in |currentlyAvailableAuthenticators|: | ||
1. Let |excludeList| be a new [=list=]. | ||
1. [=list/For each=] credential |C| in <code>{{options}}.{{ScopedCredentialOptions/excludeList}}</code>: | ||
1. If |C| has an empty {{transports}} list, [=list/append=] |C| to | ||
|excludeList| and [=continue=]. | ||
1. If |authenticator| is connected over a transport not mentioned in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an "else" from the previous point, right? This seems a bit unwieldy. Perhaps we could replace all these three sub-bullets with two:
|
||
<code>|C|.{{transports}}</code>, the client MAY [=continue=]. | ||
|
||
Issue: I'm not sure this captures the intent of the original wording. | ||
1. [=list/Append=] |C| to |excludeList|. | ||
1. [=In parallel=], invoke the <a>authenticatorMakeCredential</a> operation | ||
on |authenticator| with |rpId|, |clientDataHash|, | ||
{{accountInformation}}, |normalizedParameters|, |excludeList| and | ||
|clientExtensions| as parameters. | ||
1. [=set/Append=] |authenticator| to |issuedRequests|. | ||
|
||
1. Let |promise| be [=a new promise=]. Return |promise| and start a timer for |adjustedTimeout| milliseconds. | ||
Then execute the following steps [=in parallel=]. If any fatal error is encountered in this algorithm other than the | ||
ones enumerated below, cancel the timer, [=reject=] |promise| with a DOMException whose name is "{{UnknownError}}", and terminate | ||
this algorithm. | ||
|
||
1. Use {{attestationChallenge}}, |callerOrigin| and |rpId|, along with the token binding key associated with |callerOrigin| (if | ||
any), to create a {{ClientData}} structure representing this request. Choose a hash algorithm for {{ClientData/hashAlg}} and | ||
compute the {{ScopedCredentialInfo/clientDataJSON}} and its <a>clientDataHash</a>. | ||
Issue: What kinds of fatal errors are you worried about? I suggest we just | ||
remove that sentence. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. The initial intention was to guard against things like low-level communication breakdowns between client and authenticator, but maybe that becomes part of "any authenticator returns an error" now. |
||
|
||
1. Initialize |issuedRequests| and |currentlyAvailableAuthenticators| to empty lists. | ||
1. While |issuedRequests| is not empty, perform the following actions depending upon the |adjustedTimeout| timer and responses | ||
from the authenticators: | ||
<dl class="switch"> | ||
|
||
1. For each authenticator currently available on this platform, add the authenticator to |currentlyAvailableAuthenticators| | ||
unless the {{ScopedCredentialOptions/attachment}} member of {{options}} is <a>present</a>. In that case, let |attachment| | ||
be {{ScopedCredentialOptions/attachment}}, and add the authenticator to |currentlyAvailableAuthenticators| if its attachment | ||
modality matches |attachment|. | ||
<dt>If the |adjustedTimeout| timer expires,</dt> | ||
|
||
1. For each authenticator in |currentlyAvailableAuthenticators|: asynchronously invoke the <a>authenticatorMakeCredential</a> | ||
operation on that authenticator with |rpId|, <a>clientDataHash</a>, {{accountInformation}}, |normalizedParameters|, | ||
{{ScopedCredentialOptions/excludeList}} and |clientExtensions| as parameters. Add a corresponding entry to |issuedRequests|. | ||
- For each credential |C| in the {{ScopedCredentialOptions/excludeList}} member of {{options}} that has a non-empty | ||
|transports| list, optionally use only the specified transports to test for the existence of |C|. | ||
<dd>[=set/For each=] | ||
|authenticator| in |issuedRequests| invoke the | ||
<a>authenticatorCancel</a> operation on |authenticator| and | ||
[=set/remove=] |authenticator| from |issuedRequests|.</dd> | ||
|
||
1. While |issuedRequests| is not empty, perform the following actions depending upon the |adjustedTimeout| timer and responses | ||
from the authenticators: | ||
- If the |adjustedTimeout| timer expires, then for each entry in |issuedRequests| invoke the <a>authenticatorCancel</a> | ||
operation on that authenticator and remove its entry from the list. | ||
- If any authenticator returns a status indicating that the user cancelled the operation, delete that authenticator's | ||
entry from |issuedRequests|. For each remaining entry in |issuedRequests| invoke the <a>authenticatorCancel</a> | ||
operation on that authenticator and remove its entry from the list. | ||
- If any authenticator returns an error status, delete the corresponding entry from |issuedRequests|. | ||
- If any authenticator indicates success: | ||
- Remove this authenticator's entry from |issuedRequests|. | ||
- Create a new {{ScopedCredentialInfo}} object named |value| and populate its fields with the values returned from the | ||
authenticator as well as the {{ScopedCredentialInfo/clientDataJSON}} computed earlier. | ||
- For each remaining entry in |issuedRequests| invoke the <a>authenticatorCancel</a> operation on that authenticator and | ||
remove its entry from the list. | ||
- Resolve |promise| with |value| and terminate this algorithm. | ||
<dt>If any |authenticator| returns a status indicating that the user cancelled the operation,</dt> | ||
|
||
1. Reject |promise| with a {{DOMException}} whose name is "{{NotAllowedError}}", and terminate this algorithm. | ||
<dd> | ||
1. [=set/Remove=] |authenticator| from |issuedRequests|. | ||
2. [=set/For each=] remaining |authenticator| in |issuedRequests| invoke | ||
the <a>authenticatorCancel</a> operation on |authenticator| and | ||
[=set/remove=] it from |issuedRequests|. | ||
|
||
</dd> | ||
|
||
<dt>If any |authenticator| returns an error status,</dt> | ||
|
||
<dd>[=set/Remove=] |authenticator| from |issuedRequests|.</dd> | ||
|
||
<dt>If any |authenticator| indicates success,</dt> | ||
|
||
<dd> | ||
1. [=set/Remove=] |authenticator| from |issuedRequests|. | ||
2. Let |value| be a new {{ScopedCredentialInfo}} object whose fields are: | ||
: {{ScopedCredentialInfo/clientDataJSON}} | ||
:: A new {{ArrayBuffer}} containing the bytes of |clientDataJSON|. | ||
: {{ScopedCredentialInfo/attestationObject}} | ||
:: A new {{ArrayBuffer}} containing the bytes of the value returned | ||
from the successful [=authenticatorMakeCredential=] operation | ||
3. [=set/For each=] remaining |authenticator| in |issuedRequests| invoke the | ||
<a>authenticatorCancel</a> operation on |authenticator| and | ||
[=set/remove=] it from |issuedRequests|. | ||
4. [=Resolve=] |promise| with |value| and terminate this algorithm. | ||
|
||
</dd> | ||
|
||
1. [=Reject=] |promise| with a {{DOMException}} whose name is | ||
"{{NotAllowedError}}". | ||
|
||
Issue: {{NotAllowedError}} seems incorrect for at least the timeout and | ||
cancelled exit conditions above. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggested alternative? If you have one we can just put it in with this PR or open a new issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've filed #376 for this. |
||
|
||
During the above process, the user agent SHOULD show some UI to the user to guide them in the process of selecting and | ||
authorizing an authenticator. | ||
|
@@ -542,7 +643,7 @@ When this method is invoked, the user agent MUST execute the following algorithm | |
execute a platform-specific procedure to determine which, if any, credentials listed in {{AssertionOptions/allowList}} | ||
might be present on this authenticator, and set |credentialList| to this filtered list. If no such filtering is | ||
possible, set |credentialList| to an empty list. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should that be [=list=]? Or were you planning to make getAssertion more precise along similar lines in a new PR? |
||
- For each credential C within the |credentialList| that has a non-empty |transports| list, optionally use only the | ||
- For each credential C within the |credentialList| that has a non-empty {{transports}} list, optionally use only the | ||
specified transports to get assertions using credential C. | ||
- If the above filtering process concludes that none of the credentials on the {{AssertionOptions/allowList}} can possibly | ||
be on this authenticator, do not perform any of the following steps for this authenticator, and proceed to the next | ||
|
@@ -2115,7 +2216,7 @@ Note: Extensions should aim to define authenticator arguments that are as small | |
over low-bandwidth links such as Bluetooth Low-Energy or NFC. | ||
|
||
|
||
## Extending client processing ## {#extension-client-processing} | ||
## Extending <dfn>client processing</dfn> ## {#extension-client-processing} | ||
|
||
Extensions may define additional processing requirements on the client platform during the creation of credentials or the | ||
generation of an assertion. In order for the <a>[RP]</a> to verify the processing took place, or if the processing has a result | ||
|
@@ -2932,6 +3033,14 @@ Brad Hill, Jing Jin, Anne van Kesteren, Giridhar Mandyam, Axel Nennker, Yaron Sh | |
"href": "https://tools.ietf.org/html/draft-greevenbosch-appsawg-cbor-cddl", | ||
"status": "Internet Draft (work in progress)", | ||
"date": "21 September 2016" | ||
}, | ||
|
||
"TokenBinding": { | ||
"authors": ["A. Popov", "M. Nystroem", "D. Balfanz", "A. Langley", "J. Hodges"], | ||
"title": "The Token Binding Protocol Version 1.0", | ||
"href": "https://tools.ietf.org/html/draft-ietf-tokbind-protocol-13", | ||
"status": "Internet-Draft", | ||
"date": "February 16, 2017" | ||
} | ||
} | ||
</pre> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add "infra" to the list of dependencies in
{#dependencies}
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bikeshed does provide an [INFRA] in https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/jyasskin/webauthn/clean-make-credential/index.bs#normative with the used terms listed in https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/jyasskin/webauthn/clean-make-credential/index.bs#index-defined-elsewhere. I can add it redundantly to
#dependencies
, or we could plan to remove that whole section in favor of the automated sections.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually state them upfront so readers know what kind of things to expect, but maybe we should stop doing that? Probably worth revisiting at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annevk said:
yeah, something to think about. of course i was lame and didn't search further down in doc for mentions/links to [INFRA]. [ perhaps also bikeshed could eventually construct a
{#dependencies}
section nearer front of specs. ]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would y'all file an issue in https://github.com/tabatkins/bikeshed/issues about generating a dependencies section earlier in the document? For now ... I'm going to take this thread as an indication not to add "infra" to
#dependencies
unless y'all say otherwise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jyasskin
done: speced/bikeshed#937
ok, tho perhaps Dependencies section ought to have link to
#index-defined-elsewhere
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.