Documentation and Cache is Confusing

[bpossolo]

Core Library

MSAL.js v2 (@azure/msal-browser), MSAL Node (@azure/msal-node)

Wrapper Library

MSAL Angular (@azure/msal-angular), MSAL React (@azure/msal-react), MSAL Node Extensions (@azure/msal-node-extensions)

Public or Confidential Client?

Public, Confidential

Documentation Location

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-node/docs/caching.md

Description

I've spun this issue off of an issue another person raised regarding confusion around library usage particularly the caching concepts.
I figured I should create a new issue since the other one is already closed and unlikely to be resurfaced.

I've been using msal for java/javascript/node for some time now and have experienced confusion around how to properly use the family of MSAL libraries.
I do like that the js/java/node sdks all have similar api/concepts/usage-pattern (makes it easy to work on apps using the various libs) but below are some issues/suggestions for them all as a whole.

Some of the ways that it's confusing

• it abstracts away refresh tokens behind a convoluted token cache concept
• the api/method names are strange and make it hard to understand how to use the library. for example, acquireTokenSilently would be better named something like:

function getAccessTokenFromCache(options: Options): string { ... }

interface Options {
  refreshTokenIfExpired: boolean;
}

// this makes it clear where the token is coming from (the cache) and that
// setting the option to true may result in a network request

• the concept of accounts has nothing to do with oauth spec and requires quite a bit of research to understand what it represents and how to use them to access tokens via the msal sdk
• the sdk hides refresh tokens behind account ids for "security reasons" even though the app developer can very easily call the oauth endpoints directly and get the id/access/refresh tokens directly
• in order to perform most sdk operations, the sdk-user must provide the account id which means the account id must be stored in a user-bound session. for server rendered web apps, the user-bound session must be stored in a cookie. for single-page web apps, the session identifier is stored in local storage right next to the other tokens. this isn't immediately obvious when you start using the sdk and, in the end, it feels like security through obfuscation. I realize that keeping the refresh token on the server is good practice when possible since a leaked refresh token can be abused whereas user web sessions can be easily invalidated.... however, this concept goes out the window for single page apps. there are a lot of mental hurdles to understand this model when picking up the sdk.
• the token cache is confusing. it's unclear if each entry in the cache is a distinct thing like an account, an id token, an access token, a refresh token, etc.... or if an entry represents a mapping from an account id to the respective bundle of tokens (id/access/refresh tokens) for that account.
• it's unclear if a serialized token cache contains everything for all accounts... or if a serialized token cache is a blob for one account. in other words, I don't understand when persisting this blob if it should be stored in a mysql table record for one user... or if the blob is a giant thing that must be serialized/deserialized any time a user anywhere logs in/out.
• sdk user must implement the DistributedCachePlugin.ts in all but the most trivial apps. this is kind of a big implementation detail that surprises developers after they've started to use the sdk
• the IPartitionManager.ts api doesn't make sense to me
  • it has two methods which are named almost the same thing.
  • the documentation doesn't say anything about key collisions, what will happen, how to avoid them, etc when coming up with an implementation
  • the DistributedCachePlugin (and associated IPartitionManager) must be specified when creating the confidential client. I assume the confidential client is intended to be created once and used as a singleton, yet the IPartitionManager.getKey() method takes no arguments... this suggests it's meant to be used on a per-request fashion. How is the partition manager supposed to generate a partition key for an account given it takes no inputs/arguments? I'd expect this interface to have a single method like getPartitionKey(accountId: string): Promise<string> | string which would be called when serializing and deserializing entries.
• the methods on IPartitionManger all return a promise of a string. it would be nice if they could return a simple string too.
• the home account id versus local account id concept makes little sense and is a plumbing detail that probably shouldn't be exposed to sdk users.

Some suggestions for improvements

• it could offer a "core" sdk which exposes the rudimentary oauth operations (i.e. authorization code grant url, code-to-token exchange which returns all tokens from the identity provider, refresh access token using refresh token, etc). this would leave management of the id/access/refresh tokens to the sdk-user
• the caching layer/logic could be a separate package on top of the core lib
• it would be good if scopes could be specified when creating the public/confidential client. the getAuthorizationUrl() and acquireTokenSilently() functions are usually called in completely different parts of an app codebase. the sdk could include scopes specified when creating the client with the calls to the various methods rather than requiring the sdk-user to specify them every time one of the methods is called. after a user is authenticated, the acquireTokenSilently method will be called a lot from various places. it should be as easy as possible to fetch a token from the sdk after authentication.
• sometimes an sdk-user wants to do something simple like check "is user authenticated" to render some ui element. there is no obvious way to do this via the sdk api. we need to call acquireTokenSilently() and wrap it in a try/catch block simply to determine if a user's access token is still valid and thereby "authenticated". it would be great if the sdk offered an obvious/clear way to do a simple isAuthenticated(accountId) check.
• most single-page apps only support one authenticated user at a time. the sdk forces the multi-account concept onto apps that don't need it. this functionality makes most sense for server-based web apps where the confidential client is used to service requests coming from many users simultaneously.
• the documentation for acquireTokenSilently should state what guarantees one can expect from a token returned by the method (for example, the token is guaranteed to be valid and not expired). this is important information to convey to the sdk user given that it's built on top of a cache

[jo-arroyo]

@bpossolo Thanks for your detailed feedback. Have you taken a look at any of the PRs linked on the previous issue to see whether those documentation upgrades addressed your concerns?

@sameerag @bgavrilMS @derisen for caching and documentation concerns, @EmLauber for visibility