Add API to show error messages from failed token fetches

[npm1]

Adds the capability for the FedCM API to show error dialogs in certain scenarios after the user has chosen to perform federated login with an account. For this purpose:

• An IdentityCredentialError is created, which may contain an error which is a string that contains the specific error, and the user agent may use to customize the UI). It may also contain url, in case the IDP wants to show a url where the user could get more information.
• Shows error UI when there are network errors or when the IDP returns that an error has occurred.

Fixes #488

---

Preview | Diff

[npm1]

Ready for another look @yi-gu

[yi-gu]

Thanks. LGTM!

[npm1]

Hey @bvandersloot-mozilla @martinthomson @cboozar requesting Mozilla to take a look.

[cbiesinger]

Thanks, lgtm

[samuelgoto]

@npm1 just checking: is there anything else we need to act on before we merge this?

[npm1]

@npm1 just checking: is there anything else we need to act on before we merge this?

Yes, I need to rebase and change the code to whatever we agree on (awaiting @bvandersloot-mozilla's confirmation about whether error is acceptable)

[bvandersloot-mozilla]

I don't think we need to make this web-observable and doing so risks leaking information to the RP about shared browser state.

[npm1]

I don't think we need to make this web-observable and doing so risks leaking information to the RP about shared browser state.

What is 'this'? And can you elaborate on the leak you are thinking about?

[bvandersloot-mozilla]

"this" is the IdentityCredentialError itself, particularly the code variable which contains an unspecified string (presumably one of "invalid_request", "unauthorized_client", "access_denied", "server_error", and "temporarily_unavailable"). If I understand this patch correctly, that ends up returned from navigator.credentials.get and reveals internal information about the credential discovery to the RP.

[npm1]

"this" is the IdentityCredentialError itself, particularly the code variable which contains an unspecified string (presumably one of "invalid_request", "unauthorized_client", "access_denied", "server_error", and "temporarily_unavailable"). If I understand this patch correctly, that ends up returned from navigator.credentials.get and reveals internal information about the credential discovery to the RP.

That is correct, but the IdP needs to set this value itself in the response in order for it to be passed to the RP (it is not 'leaked'). Also perhaps worth noting that the error is passed from the ID assertion fetch, so the user must have tried to use FedCM in the RP.

[bvandersloot-mozilla]

Ah, I didn't realize this is happening on the id assertion endpoint only. In that case, this is reasonable, but we should probably enumerate the possible values for code and specify when they are returned.

[npm1]

Ah, I didn't realize this is happening on the id assertion endpoint only. In that case, this is reasonable, but we should probably enumerate the possible values for code and specify when they are returned.

I was hoping to leave the values for error (previously code) unspecified similar to how we do not specify the format/return of token. That way the IdP can send the information needed for the RP to process the error, and the user agent may choose to show generic UI or show more specific UI if it chooses to do so based on the error string. What do you think? I did mention the strings Chrome uses to show more specific UI in case it is of interest for an implementor to match.

[TallTed]

Small tweaks for clarity

[npm1]

Putting this on hold to get some discussion with other WGs on error handling, hopefully to occur during TPAC.

[npm1]

Rebased this PR in case people want to take another look ahead of TPAC discussion