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

Feature Request: Removing app tokens #640

Closed
rayluo opened this issue Dec 18, 2023 · 6 comments · Fixed by #666
Closed

Feature Request: Removing app tokens #640

rayluo opened this issue Dec 18, 2023 · 6 comments · Fixed by #666

Comments

@rayluo
Copy link
Collaborator

rayluo commented Dec 18, 2023

Problem description: Historically, there is no api in MSAL to remove tokens obtained from AcquireTokenForClient(). Therefore, there is no way for an app to "log out" a service principal.

Proposal: MSALs add a new API ConfidentialClientApplication.RemoveAppTokens() ConfidentialClientApplication.remove_tokens_for_client().

  • It will purge app tokens that match the current app's authority (host + tenant) and client_id.
  • It does not accept a scope parameter, because our intention is to purge all app tokens for the current app, regardless of their scopes.
  • It will NOT purge user tokens obtained by this app.
@bgavrilMS
Copy link
Member

CC @jmprieur @localden @pmaytak @gladjohn @trwalke - let's discuss about this next time we meet.

@localden
Copy link
Collaborator

@rayluo @bgavrilMS if the app tokens are purged and the user tokens are not, does it have any implications on the end-user experience?

We probably should spec this out.

@rayluo
Copy link
Collaborator Author

rayluo commented Jan 18, 2024

@rayluo @bgavrilMS if the app tokens are purged and the user tokens are not, does it have any implications on the end-user experience?

We probably should spec this out.

I believe app tokens and user tokens are independent with each other. This was also called out in the 3rd bullet point in this issue's description.

BTW, there was another github issue came in yesterday, which was actually the same topic. I was proposing a different function name remove_tokens_for_client() there.

@bgavrilMS
Copy link
Member

@localden - to remove user tokens, the pattern is

var app = PublicClient or ConfidentialClient;
var accounts = await app.GetAccounts(); 

foreach (var acc in accounts)
    app.Remove(acc);

So the API is very much focused on the concept of "Account" which is a materialization of a user in a tenant. If you fetch app tokens, GetAccounts returns null.

To me it makes sense to not try to use the user token removal pattern for service principals, but would like to know your opinion.

@localden
Copy link
Collaborator

@localden - to remove user tokens, the pattern is

var app = PublicClient or ConfidentialClient;
var accounts = await app.GetAccounts(); 

foreach (var acc in accounts)
    app.Remove(acc);

So the API is very much focused on the concept of "Account" which is a materialization of a user in a tenant. If you fetch app tokens, GetAccounts returns null.

To me it makes sense to not try to use the user token removal pattern for service principals, but would like to know your opinion.

Makes sense to keep the user and app token patterns somewhat separate. If we treat "Account" as materialization of users and tenants, what is the same alternative for apps?

@rayluo
Copy link
Collaborator Author

rayluo commented Jan 24, 2024

@localden - to remove user tokens, the pattern is

var app = PublicClient or ConfidentialClient;
var accounts = await app.GetAccounts(); 

foreach (var acc in accounts)
    app.Remove(acc);

So the API is very much focused on the concept of "Account" which is a materialization of a user in a tenant. If you fetch app tokens, GetAccounts returns null.
To me it makes sense to not try to use the user token removal pattern for service principals, but would like to know your opinion.

Makes sense to keep the user and app token patterns somewhat separate. If we treat "Account" as materialization of users and tenants, what is the same alternative for apps?

I think that "alternative" has long been implicitly established by its acquisition method's naming, AcquireTokenForClient(). So, they are "tokens for client", as opposite to "tokens for a user/account". That's also why I would suggest NOT bother inventing a new concept/noun, we can just have a new API following the same naming pattern as "RemoveTokensForClient()`.

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

Successfully merging a pull request may close this issue.

3 participants