Deprecate nonce parameter #768
Conversation
spec/index.bs
Outdated
| hint. If provided, the user agent will not show accounts which do not match the domain hint | ||
| value. | ||
|
|
||
| Note: "nonce" to be passed with-in {{IdentityProviderRequestOptions/params}} |
There was a problem hiding this comment.
nit: typo here, and also add period at the end of the sentence
There was a problem hiding this comment.
@npm1 — What's the typo you see? Can you make a suggestion of your edit? Or confirm that I see the same thing you did?
| Note: "nonce" to be passed with-in {{IdentityProviderRequestOptions/params}} | |
| Note: "nonce" is to be passed within {{IdentityProviderRequestOptions/params}}. |
| providers: [{ // sequence<IdentityCredentialRequestOptions> | ||
| configURL: "https://idp.example/manifest.json", // IdentityProviderConfig.configURL | ||
| clientId: "123", // IdentityProviderConfig.clientId | ||
| nonce: "nonce" // IdentityProviderConfig.nonce |
There was a problem hiding this comment.
I'd rewrite this as:
params: {
nonce: "nonce"
}So that a reader can tell where they'd put the nonce.
There was a problem hiding this comment.
Well actually I thought we discussed intentionally not putting nonce here since it is now not something the browser needs to handle. We do have a note about it later on. I would reject this suggestion, eg remove this.
| value. | ||
|
|
||
| Note: "nonce" to be passed with-in {{IdentityProviderRequestOptions/params}} | ||
| Note: "nonce" is to be passed within {{IdentityProviderRequestOptions/params}}. |
There was a problem hiding this comment.
I do not see myself nor @npm1 credited/blamed for this change, which was made based on our review comments. I think neither the "commit suggestion" nor "add to batch" buttons were used.
There was a problem hiding this comment.
Ah yes that's fair @TallTed. I think sometimes it is easier to just make the changes locally when addressing multiple comments, so the 'add to batch' button is not used. But you have done a lot of reviews to help improve FedCM, so perhaps we can populate https://w3c-fedid.github.io/FedCM/#acknowledgements and include you there?
| providers: [{ // sequence<IdentityCredentialRequestOptions> | ||
| configURL: "https://idp.example/manifest.json", // IdentityProviderConfig.configURL | ||
| clientId: "123", // IdentityProviderConfig.clientId | ||
| nonce: "nonce" // IdentityProviderConfig.nonce |
There was a problem hiding this comment.
Well actually I thought we discussed intentionally not putting nonce here since it is now not something the browser needs to handle. We do have a note about it later on. I would reject this suggestion, eg remove this.
| value. | ||
|
|
||
| Note: "nonce" to be passed with-in {{IdentityProviderRequestOptions/params}} | ||
| Note: "nonce" is to be passed within {{IdentityProviderRequestOptions/params}}. |
There was a problem hiding this comment.
Ah yes that's fair @TallTed. I think sometimes it is easier to just make the changes locally when addressing multiple comments, so the 'add to batch' button is not used. But you have done a lot of reviews to help improve FedCM, so perhaps we can populate https://w3c-fedid.github.io/FedCM/#acknowledgements and include you there?
|
We got consensus on this in the call. CC @bvandersloot-mozilla but I'll merge this now |
SHA: b8e3ce8 Reason: push, by npm1 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: b8e3ce8 Reason: push, by pull[bot] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
4 participants
The Federated Credential Management (FedCM) API currently allows Identity Providers (IdPs) to specify a
nonceparameter as a top-level field in the provider configuration object. This proposal proposes deprecating this top-level parameter and moving it to theparamsobject, which is the intended location for all IdP-specific parameters.Preview | Diff