[PIWEB-21318] Improve access token caching #269
Conversation
* If we obtain a valid access token, we should always store that in the local cache, no matter what * When we don't store that we can never explicitly trigger the creation of a new token from calling code that is reused (stored in cache) * We either trigger the creation of a token that is not stored or we get a possibly outdated token from cache - neither of these 2 conditions is great
|
|
||
| if( result == null ) | ||
| return null; | ||
|
|
||
| if( !bypassLocalCache ) |
There was a problem hiding this comment.
Removing this line is the actual bugfix.
There was a problem hiding this comment.
The whole bypass option exists so that we can store, test and use credential information only from our import plan specific credential storage in AutoImporter. The idea is that the authentication of these automations should be independent from any actions you do in other clients. This will keep working as long as the option still prevents reading the cache. But I'm not sure we want to pollute the cache of other applications with connection configuration in AutoImporter as import automation will most likely use another user. So instead of changing the behavior of the bypass flag, why is the bypass flag used for imports from planner in the first place?
There was a problem hiding this comment.
Oh and if we decide this change should go through, you need to update the parameter documentation.
There was a problem hiding this comment.
Also keep in mind that the token cache is using the URL as the key, meaning each "cache skipping" login may overwrite a previously saved and valid token. I also wonder why the import in Planner does this, wouldn't this mean the user has to login everytime? If there is a valid reason, it may be an option to add it more as an opt-in feature, so you have to specify that you want to skip the cache with "bypassLocalCache" and that you want to force-store a new token with something like "forceTokenCacheUpdate", with false as default. So for existing usages there is no change but you can adapt the usage where you need it. This way we also don't need to introduce a breaking change for this logic change. The other changes regarding cancellation are optional parameters and would not require this, a feature increase of the version would be enough.
daniel-flemming
deleted the
feature/PIWEB-21318_Improve_AccessTokenCaching
branch
[PIWEB-21318] fix: Always store valid access tokens in local cache