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

Convert required email field in user account to optional #433

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bvandersloot-mozilla
Copy link
Collaborator

@bvandersloot-mozilla bvandersloot-mozilla commented Feb 13, 2023

This makes the email field of the IdentityProviderAccount optional. This can give a little flexibility to the IDP where accounts may not have an email address tied to them or be a useful way for the user to identify the account.


Preview | Diff

@npm1
Copy link
Collaborator

npm1 commented Feb 13, 2023

Is there an issue associated with this PR? Also what use-case is this trying to solve? In Chrome we currently do require the email. If it was not required, I guess we could show the name plus account ID or just the name when the email is not present. But it's worth thinking about how the user agent would be able to present enough information to the user so they can pick their account correctly.

@samuelgoto
Copy link
Collaborator

Is there an issue associated with this PR? Also what use-case is this trying to solve? In Chrome we currently do require the email. If it was not required, I guess we could show the name plus account ID or just the name when the email is not present. But it's worth thinking about how the user agent would be able to present enough information to the user so they can pick their account correctly.

Yeah, this is indeed normative, in the sense that it is, by choice, that we wrote email as a required field.

We are certainly interested in making this not the case, but we should think this through.

@bvandersloot-mozilla
Copy link
Collaborator Author

Is there an issue associated with this PR?

Not currently. I'll file one and we can have the discussion there. (#435)

Also what use-case is this trying to solve?

per the PR, "This can give a little flexibility to the IDP where accounts may not have an email address tied to them or be a useful way for the user to identify the account."

@samuelgoto
Copy link
Collaborator

Is there an issue associated with this PR?

There are at least two issues that are somewhat related.

One of the them, came up in our TAG reviews:

w3ctag/design-reviews#718 (comment)

The other is this one here:

#242 (comment)

In both of these issues, my intuition / suggestion is that we should give RPs the ability to select what they need to be disclosed (e.g. optionality of email) and have the browser be able to honor that (the idp also has a say in what can be optional and what can't).

I don't know exactly what the API looks like, but here is what I responded to the TAG when that came up:

await navigator.credentials.get({
      federated: {
        claims: {
          name: {essential: true}, // required
          email: null, // optional
          emailVerified: null, // optional
          picture: {essential: true}, // required
        },
        providers: [{
          url: "https://idp.example",
          clientId: "123"
        }]
      }
});

WDYT?

@cbiesinger
Copy link
Collaborator

I wrote a response in the issue Ben filed (#435), could we keep discussion in one place?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants