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

IDP Revocation should account for deleting their account with the IDP as well #313

Closed
kdenhartog opened this issue Jul 14, 2022 · 12 comments

Comments

@kdenhartog
Copy link
Contributor

Right now the way the language reads for IDP revocation makes it seem like only the account is deleted with the RP when the IDP revocation flow occurs. This flow should also account for handling the automatic revocation of all accounts with the RP when the user chooses to delete their account from the IDP.

@npm1
Copy link
Collaborator

npm1 commented Jul 15, 2022

Not sure what's the request here? If the user chooses to delete their account from the IDP, this is something between the user and the IDP. I agree that the IDP should notify all relevant RPs about logout (and no longer provide valid sign in tokens). This sounds like something relevant to IDP best practices, but unclear how it is related to this API.

@kdenhartog
Copy link
Contributor Author

This is essentially trying to figure out the proper way to handle the case where the user deletes their FB account and all the sudden loses access to all the RP websites as well. This can be handled in a variety of ways.

  1. The IDP doesn't notify the RP and the account becomes dormant (simple, but frustrating UX)
  2. The RP is notified of the deletion and deletes the users account/data as well. (more complicated, but frustrating UX. potential advantages for improving data retention practices for RPs)
  3. The RP does some sort of account/user data account migration path (complex, better UX)
  4. The user needs to update their RP to migrate login solutions first before deleting the IDP account (simple, but frustrating UX)

This sounds like something relevant to IDP best practices, but unclear how it is related to this API.

Yeah, that's one way to handle it but that would likely lead to number 1 becoming the most common case. Essentially what I had in mind is that this spec could define option 2 or 3 where the IDP communicates directly to the RP or via a browser API to trigger account deletion on the RP side of things.

So the request here is to purposefully account for how this case should be handled in the spec since it's common enough of a problem that something should be noted about the recommended ways to handle this.

@timcappalli
Copy link

I agree that these use cases are important, but they don't belong in a browser API spec.

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Jul 18, 2022

I agree that these use cases are important, but they don't belong in a browser API spec.

Is that the only thing that's being defined here? The spec looks like it's defining more than that since section 5 seems to be normatively define IDP APIs and section 6 looks like it's heading in a similar direction where it's normatively defining RP APIs.

@npm1
Copy link
Collaborator

npm1 commented Jul 18, 2022

Well this is a browser API that enables communication between IDP and RP, so it's true it needs to define some endpoints so that the browser can communicate with both. It sounds like this issue is basically a feature request to expose a 'IDP account has been deleted' so that the RP can be notified when this happens and handle it gracefully, right? I can investigate how this works nowadays...

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Jul 18, 2022

Yup, exactly just a feature request for this makes sense and can be dependent on dev demand on whether to bring it in now or make it extensible later

@kdenhartog
Copy link
Contributor Author

@npm1 you can assign this issue to me and I'll open a PR for it

@samuelgoto
Copy link
Collaborator

expose a 'IDP account has been deleted' so that the RP can be notified

Ah, got it, yeah, that makes sense to extend if needed.

Wondering: how does OIDC handle this? I'm guessing that that would be through top-level navigations (rather than through third party cookies and iframes?).

On a related note, this seems somewhat similar to the front-channel logout endpoint (but as opposed to logging out, the IDP wants to propagate account deletion)?

@kdenhartog
Copy link
Contributor Author

I'm pretty sure it's option 1 so this is a new feature. For example, in the past when I temporary disabled my FB account I found it stopped allowing me to login with FB, but when I brought my FB account I was able to log back in.

As you point out we could do this similarly to the front-channel logout endpoint to achieve option 2.

@samuelgoto
Copy link
Collaborator

samuelgoto commented Aug 23, 2022

Is that the only thing that's being defined here?

Yes.

I'm pretty sure it's option 1 so this is a new feature.

Ah, ok.

Can we start by making a proposal to OIDC first then? And once/if that settles, we'll enable that through browsers?

@kdenhartog
Copy link
Contributor Author

+1 that makes sense as the right way to handle this and we can just plan for this to be an extension added later than

@samuelgoto
Copy link
Collaborator

+1 that makes sense as the right way to handle this and we can just plan for this to be an extension added later than

SGTM!

I'll close this issue for now and we can follow up in the OIDC bug tracker! We can re-visit / re-open this when that settles, but for now, nothing to act on.

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

No branches or pull requests

4 participants