Fix token refresh after unauthorized response

[julius-rennmann]

Hello Sebastian,

Squidex access tokens expire after 30 days. The SDK kept a resolved getToken() promise after a normal cache hit. When Squidex later returned 401, the retry cleared the token store but still reused that old promise and sent the expired token again.

Token handling is now simpler: the token store is the only cache. getToken() returns a valid stored token or requests and stores a new one. After a 401, the retry requests a fresh token as expected.

Added a regression test for cached token -> 401 -> new token -> successful retry. Full SDK test suite passes against the local Squidex Compose setup.

Hope this works for you.
Julius

[SebastianStehle]

I see the problem, but the solution is also not so good. The promise was used to prevent too many tokens from being requested. I agree thaet the tokenapi is weird as it has a dependency to the token store, but the solution is also not correct.

I think we should have a token-manager or so, which ensures that only one token is requested at any time, he could just unset the internal promise then.

[julius-rennmann]

Hey thanks for the quick answer, I see the problem now and understand your design. I reverted to you original code and just pulled the cache test out of the async function, so that only function calls with real "await" happen, so that the promise always gets written before the "finally" runs.

I added two regression test to make sure the re-authentication works and the multiple requests do not start multiple auth requests.

[SebastianStehle]

I am not sure if the implementation correct. Lets say the token becomes invalid, would it not resolve to the one token retrieved before?

[julius-rennmann]

That is what happened before. That is what I fixed.

The Bug is that when getToken() get executed and the token is still valid then inside the async function no await is triggered and it gets executed till the finally() block sets this.tokenPromise to undefined and only afterwards this.tokenPromise gets set to the token. It then stays this way after everything has run. From that point on this.tokenPromise is filled with a value when it should not be.

That is the original bug.

What i did is move the check from the async function outside so that each execution of the async function has an "await" call inside, that way this.tokenPromise gets set through ||= before the finally() block resolves to reset the this.tokenPromise to undefined.

---

Another thing: While looking through the code again to write the lines above I came across another potential for unnecessary requests. If several requests get started with an expired token and a new token is requested and set before the other requests with the invalid token are completed then they would again request a new token. I made a new commit with a regression test for this and a fix that adds an argument to clearToken(accessToken) that allows us to only clear the token in a retried request if the current token is the same one we made the original request with. So if we have an old token and it has already been refreshed, we just use the new/current one.