Skip to content

Commit dee8797

Browse files
authored
Fix spec bugs reported by togamid (#737)
* Fix spec bugs reported by togamid There are a few spec bugs in the `create an IdentityCredential` algorithm. Fixes #734 * Update note * more
1 parent 1e75d94 commit dee8797

File tree

1 file changed

+104
-80
lines changed

1 file changed

+104
-80
lines changed

spec/index.bs

Lines changed: 104 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -896,69 +896,37 @@ the exception immediately.
896896
since these are indepedent.
897897
1. Let |providerOrigin| be the [=/origin=] of |provider|'s {{IdentityProviderConfig/configURL}}.
898898
1. If |providerMap| [=map/contains=] |providerOrigin|, return (failure, true).
899+
1. If |mediation| is "{{CredentialMediationRequirement/silent}}" and there does not exist an
900+
account |account| such that [=connected accounts set=] [=list/contains=] the result of
901+
[=compute the connected account key=] given |account|, |provider|, and |globalObject|,
902+
[=continue=]. Note that this check can be performed by iterating over the
903+
[=connected accounts set=] or by keeping a separate data structure to make this lookup
904+
fast.
899905
1. Let |loginStatus| be the result of [=get the login status=] with |providerOrigin|.
900906
1. If |loginStatus| is [=unknown=], a user agent MAY set it to [=logged-out=].
901907
1. If |loginStatus| is [=logged-out=]:
902-
1. If |mode| is {{IdentityCredentialRequestOptionsMode/active}}:
908+
1. If |mode| is {{IdentityCredentialRequestOptionsMode/active}} and |mediation| is not
909+
"{{CredentialMediationRequirement/silent}}":
903910
1. Let |result| be the result of running
904911
[=fetch the config file and show an IDP login dialog=] with
905912
|provider| and |globalObject|.
906913
1. If |result| is failure, return (failure, true).
914+
1. Otherwise, set |loginStatus| to [=logged-in=].
907915
1. Otherwise, set |providerMap|[|providerOrigin|] to "logged-out" and [=continue=].
908916
1. Let |requiresUserMediation| be |providerOrigin|'s [=requires user mediation=].
909917
1. If |requiresUserMediation| is true and |mediation| is
910918
"{{CredentialMediationRequirement/silent}}", [=continue=].
911919
1. Let |config| be the result of running [=fetch the config file=] with
912920
|provider| and |globalObject|.
913921
1. If |config| is failure, [=continue=].
914-
1. <dfn>Fetch accounts step</dfn>: Let |accountsList| be the result of
915-
[=fetch the accounts=] with |config|, |provider|, and |globalObject|.
916-
1. If |accountsList| is failure, or the size of |accountsList| is 0:
917-
1. [=Set the login status=] of |providerOrigin| to [=logged-out=].
918-
A user agent may decide to skip this step if no credentials were
919-
sent to server.
920-
921-
Note: For example, if the fetch failed due to a DNS error, no
922-
credentials were sent and therefore the [=IDP=] did not learn
923-
the user's identity. In this situation, we do not know whether
924-
the user is signed in or not and so we may not want to reset
925-
the status.
926-
1. If |loginStatus| is [=logged-in=], set |providerMap|[|providerOrigin|] to "mismatch"
927-
and [=continue=].
928-
1. Assert: |accountsList| is not failure and the size of |accountsList| is not 0.
929-
1. [=Set the login status=] for |providerOrigin| to [=logged-in=].
930-
1. For each |acc| in |accountsList|:
931-
1. If |acc|["{{IdentityProviderAccount/picture}}"] is present, [=fetch the account picture=]
932-
with |acc| and |globalObject|. If the [=user agent=] displays this picture to
933-
the user at any point, it MUST reuse the result of this fetch instead of redownloading
934-
the picture.
935-
936-
Note: We require downloading the pictures here before we potentially filter the account
937-
list so that the identity provider cannot determine what hints were provided
938-
based on which fetches occurred.
939-
1. If |provider|'s {{IdentityProviderRequestOptions/loginHint}} is not empty:
940-
1. For every |account| in |accountList|, remove |account| from |accountList| if |account|'s
941-
{{IdentityProviderAccount/login_hints}} does not [=list/contain=] |provider|'s
942-
{{IdentityProviderRequestOptions/loginHint}}.
943-
1. If |accountList| is now empty, set |providerMap|[|providerOrigin|] to "mismatch" and
944-
continue.
945-
1. If |provider|'s {{IdentityProviderRequestOptions/domainHint}} is not empty:
946-
1. For every |account| in |accountList|:
947-
1. If {{IdentityProviderRequestOptions/domainHint}} is "any":
948-
1. If |account|'s {{IdentityProviderAccount/domain_hints}} is empty, remove
949-
|account| from |accountList|.
950-
1. Otherwise, remove |account| from |accountList| if |account|'s
951-
{{IdentityProviderAccount/domain_hints}} does not [=list/contain=] |provider|'s
952-
{{IdentityProviderRequestOptions/domainHint}}.
953-
1. If |accountList| is now empty, set |providerMap|[|providerOrigin|] to "mismatch" and
954-
continue.
955-
1. If |config|.{{IdentityProviderAPIConfig/account_label}} is present:
956-
1. For every |account| in |accountList|, remove |account| from |accountList| if |account|'s
957-
{{IdentityProviderAccount/label_hints}} does not [=list/contain=]
958-
|config|.{{IdentityProviderAPIConfig/account_label}}.
959-
1. If |accountList| is now empty, set |providerMap|[|providerOrigin|] to "mismatch" and
960-
continue.
961-
1. Set |providerMap|[|providerOrigin|] to |accountsList|.
922+
1. [=Fetch accounts=] given |config|, |provider|, and |globalObject|, and let |accountsList|
923+
be the result.
924+
1. If |accountsList| is empty:
925+
1. If |loginStatus| is [=logged-in=] and |mediation| is not
926+
{{CredentialMediationRequirement/silent}}, set |providerMap|[|providerOrigin|] to
927+
"mismatch".
928+
1. Otherwise, set |providerMap|[|providerOrigin|] to "logged-out".
929+
1. Otherwise, set |providerMap|[|providerOrigin|] to |accountsList|.
962930
1. If |providerMap| [=map/is empty=], return (failure, false).
963931
1. Let |registeredAccount| and |numRegisteredAccounts| be null and 0, respectively.
964932
1. Let |selectedAccount| be null.
@@ -968,31 +936,25 @@ the exception immediately.
968936
1. If |acc| is [=eligible for auto reauthentication=] given the relevant |provider|, and
969937
|globalObject|, set |registeredAccount| to |acc|, increase |numRegisteredAccounts| by 1,
970938
and set |requiresUserMediation| be |providerOrigin|'s [=requires user mediation=].
939+
1. Let |permission|, |permissionRequested|, and |isAutoSelected| be set to false.
971940
1. If |mediation| is not "{{CredentialMediationRequirement/required}}", |requiresUserMediation|
972941
is false, |numRegisteredAccounts| is equal to 1, and |providerMap|'s [=map/values=] do not
973942
[=map/contain=] "mismatch":
974-
1. Set |selectedAccount| to |registeredAccount| and |permission| to true. When doing this, the user
975-
agent MAY show some UI to the user indicating that they are being
976-
<dfn>auto-reauthenticated</dfn>.
977-
1. Set |isAutoSelected| to true.
978-
1. Otherwise, if |mediation| is "{{CredentialMediationRequirement/silent}}" and |providerMap|'s
979-
[=map/values=] do not [=map/contain=] "mismatch", return (failure, true).
980-
1. Let |permission|, |permissionRequested|, and |isAutoSelected| be set to false.
943+
1. Set |selectedAccount| to |registeredAccount|, and set |permission| and |isAutoSelected| to
944+
true. When doing this, the user agent MAY show some UI to the user indicating that they
945+
are being <dfn>auto-reauthenticated</dfn>. Go to the [=wait for user step=].
946+
1. Otherwise, if |mediation| is "{{CredentialMediationRequirement/silent}}":
947+
1. Assert that |providerMap|'s [=map/values=] do not [=map/contain=] "mismatch".
948+
1. Return (failure, false).
981949
1. Let |allAccounts| be an empty [=list=].
982950
1. Build UI by adding the following for each (|providerOrigin|, |value|) in |providerMap|:
983951
1. If |value| is "logged-out", the user agent adds one of the following:
984952
* Nothing: no UI is shown regarding this [=IDP=].
985-
* Prompt to continue with this [=IDP=]. If the user continues, the user
986-
agent SHOULD set the login status to [=unknown=]. This MAY include an
987953
* Return (failure, false).
988-
* Prompt the user whether to continue. If the user continues, the user
989-
agent SHOULD set |loginStatus| to [=unknown=]. This MAY include an
990-
affordance to [=show an IDP login dialog=].
991-
* If the user triggers this affordance:
992-
1. Let |result| be the result of running
993-
[=fetch the config file and show an IDP login dialog=]
994-
with the relevant |provider| and |globalObject|.
995-
1. If |result| is failure, return (failure, true).
954+
* Prompt the user whether to continue with this [=IDP=]. If the user continues:
955+
1. Let |result| be the result of running [=show an IDP login dialog=] with the
956+
relevant |config|, |provider|, and |globalObject|.
957+
1. If |result| is failure, return (failure, true).
996958
1. Otherwise, if |value| is "mismatch", add contents indicating this |providerOrigin|
997959
to the user. The contents SHOULD provide an affordance for the user to trigger
998960
the [=show an IDP login dialog=] algorithm with the relevant |config| and |provider|;
@@ -1002,36 +964,45 @@ the exception immediately.
1002964
to be signed in, but the accounts fetch indicated that the user
1003965
is signed out.
1004966

1005-
Note: This dialog ensures that silent tracking of the user is
1006-
impossible by always showing UI of some kind when credentials
1007-
were sent to the server.
967+
Note: This dialog ensures that silent tracking of the user is impossible by always
968+
showing UI of some kind when credentials were sent to the server and the login
969+
status was not [=unknown=].
1008970

1009971
* If the [=show an IDP login dialog=] algorithm was triggered:
1010972

1011973
1. Let |result| be the result of that algorithm.
1012974
1. If |result| is failure, return (failure, true). The user
1013975
agent MAY show a dialog to the user before or after
1014976
returning failure indicating this failure.
1015-
1. Otherwise, go back to the [=fetch accounts step=] to get an updated
1016-
value of |providerMap| for this [=IDP=].
977+
1. Otherwise, [=fetch accounts=] with the relevant |config|, |provider|, and
978+
|globalObject|, and let |accountsList| be the result.
979+
1. If |accountsList| is empty, show the [=confirm IDP login dialog=] again.
980+
1. Otherwise, [=list/Extend=] |allAccounts| with |accountsList| and
981+
[=show accounts=].
1017982
1. Otherwise, |value| is a [=list=] of accounts. [=list/Extend=] |allAccounts| with |value|.
1018983
1. If |options|.{{IdentityCredentialRequestOptions/mode}} is `"active"` and the provider's
1019984
|config|.{{IdentityProviderAPIConfig/supports_use_other_account}} is true, add an
1020985
affordance to trigger [=show an IDP login dialog=] to let the user sign in to another
1021986
account. If that affordance is triggered:
1022987
1. Let |result| be the result of that algorithm.
1023-
1. If |result| is failure, go to [=show accounts=].
1024-
1. Otherwise, go back to the [=fetch accounts step=] to get an updated
1025-
value of |providerMap| for this [=IDP=].
1026-
1. Also include a UI affordance to close the dialog. If the user closes this dialog, return (failure,
1027-
true).
988+
1. If |result| is failure, [=show accounts=].
989+
1. Otherwise:
990+
1. [=Fetch accounts=] given |config|, |provider|, and |globalObject|, and let
991+
|accountsList| be the result.
992+
1. If |accountsList| is empty, show the [=confirm IDP login dialog=].
993+
1. Otherwise, [=list/extend=] |allAccounts| with |accountsList| and
994+
[=show accounts=].
995+
1. Also include a UI affordance to close the dialog. If the user closes this dialog, return
996+
(failure, true).
1028997
1. <dfn for="create identity credential">Show accounts</dfn> step: if |allAccounts| is not
1029998
[=list/empty=], also add UI to present the account options to the user. If the user selects
1030999
an account, perform the following steps:
1031-
1. Set |selectedAccount| to the chosen {{IdentityProviderAccount}}.
1000+
1. Set |selectedAccount| to the chosen {{IdentityProviderAccount}} and |permission| to
1001+
true.
10321002
1. Close the dialog.
1033-
1. If the [=user agent=] created any dialogs requesting user choice or permission in the previous
1034-
steps, wait until they are closed.
1003+
1. <dfn for="create identity credential">Wait for user step</dfn>: if the [=user agent=] created
1004+
any dialogs requesting user choice or permission in the previous steps, wait until they are
1005+
closed.
10351006
1. If |permission| is false, then return (failure, true).
10361007
1. Assert: |selectedAccount| is not null.
10371008
1. Let |credential| be the result of running the [=fetch an identity assertion=] algorithm with
@@ -1055,6 +1026,54 @@ the exception immediately.
10551026
1. Return |credential|.
10561027
</div>
10571028

1029+
<div algorithm="fetch accounts">
1030+
When asked to <dfn for="create identity credential">fetch accounts</dfn> given an
1031+
{{IdentityProviderAPIConfig}} |config|, an {{IdentityProviderRequestOptions}} |provider|, and a
1032+
|globalObject|, run the following steps. This returns a possibly empty list of accounts.
1033+
1. Let |accountsList| be the result of [=fetch the accounts=] with |config|, |provider|, and
1034+
|globalObject|.
1035+
1. If |accountsList| is failure, or the size of |accountsList| is 0:
1036+
1. [=Set the login status=] of |providerOrigin| to [=logged-out=].
1037+
A user agent may decide to skip this step if no credentials were
1038+
sent to server.
1039+
1040+
Note: For example, if the fetch failed due to a DNS error, no
1041+
credentials were sent and therefore the [=IDP=] did not learn
1042+
the user's identity. In this situation, we do not know whether
1043+
the user is signed in or not and so we may not want to reset
1044+
the status.
1045+
1. Return an empty list.
1046+
1. Assert: |accountsList| is not failure and the size of |accountsList| is not 0.
1047+
1. [=Set the login status=] for |providerOrigin| to [=logged-in=].
1048+
1. For each |acc| in |accountsList|:
1049+
1. If |acc|["{{IdentityProviderAccount/picture}}"] is present, [=fetch the account picture=]
1050+
with |acc| and |globalObject|. If the [=user agent=] displays this picture to
1051+
the user at any point, it MUST reuse the result of this fetch instead of redownloading
1052+
the picture.
1053+
1054+
Note: We require downloading the pictures here before we potentially filter the account
1055+
list so that the identity provider cannot determine what hints were provided
1056+
based on which fetches occurred.
1057+
1. If |provider|'s {{IdentityProviderRequestOptions/loginHint}} is not empty:
1058+
1. For every |account| in |accountList|, if |account|'s
1059+
{{IdentityProviderAccount/login_hints}} does not [=list/contain=] |provider|'s
1060+
{{IdentityProviderRequestOptions/loginHint}}, remove |account| from |accountList| .
1061+
1. If |provider|'s {{IdentityProviderRequestOptions/domainHint}} is not empty:
1062+
1. For every |account| in |accountList|:
1063+
1. If {{IdentityProviderRequestOptions/domainHint}} is "any":
1064+
1. If |account|'s {{IdentityProviderAccount/domain_hints}} is empty, remove
1065+
|account| from |accountList|.
1066+
1. Otherwise, if |account|'s {{IdentityProviderAccount/domain_hints}} does not
1067+
[=list/contain=] |provider|'s {{IdentityProviderRequestOptions/domainHint}}, remove
1068+
|account| from |accountList|.
1069+
1. If |config|.{{IdentityProviderAPIConfig/account_label}} is present:
1070+
1. For every |account| in |accountList|, if |account|'s
1071+
{{IdentityProviderAccount/label_hints}} does not [=list/contain=]
1072+
|config|.{{IdentityProviderAPIConfig/account_label}}, remove |account| from |accountList|.
1073+
1. Return |accountsList|.
1074+
1075+
</div>
1076+
10581077
<div class="issue" heading="extension">
10591078
An extension may use the following instead of the [=create identity credential/show accounts=] step, where
10601079
|permissionRequested| is sometimes set:
@@ -1082,7 +1101,12 @@ An extension may use the following instead of the [=create identity credential/s
10821101
1. If |supportsUseOtherAccount| is true, the account chooser SHOULD provide
10831102
an affordance to use another account. If that affordance is triggered:
10841103
1. [=Show an IDP login dialog=] with |config|, |provider| and |globalObject|.
1085-
1. If that returned success, go back to the [=fetch accounts step=].
1104+
1. If that returned success:
1105+
1. [=Fetch accounts=] given |config|, |provider|, and |globalObject|, and let
1106+
|accountsList| be the result.
1107+
1. If |accountsList| is empty, show the [=confirm IDP login dialog=].
1108+
1. Otherwise, [=list/extend=] |allAccounts| with |accountsList| and
1109+
[=show accounts=] again.
10861110
1. Otherwise, go back to the [=UI to allow the user to select an account=].
10871111
1. If the user selects an account, perform the following steps:
10881112
1. Set |selectedAccount| to the chosen {{IdentityProviderAccount}}.

0 commit comments

Comments
 (0)