Store singing keys as fallback #1047
Conversation
|
Hi @dome4 thanks for the PR Thinking out loud now. Need to think about this a bit. I think that this is cached anyway by the browser as we use a HTTP GET request. One problem would be with key rotation. If we read the keys first from storage , we might be using old keys and validation will fail. This would probably never happen because the session storage gets clean up once we close the tab, browser etc., so not really a problem. If the HTTP request fail this is awesome. So maybe it good. @FabianGosebrink @dome4 Would love your thoughts on this Greetings Damien |
|
I totally agree with the concerns about key rotation. That's the reason why I tried to implement a network-first strategy: the idea is to try the request for three times ( Furthermore, caching get requests through the browser is not suitable for me, as I am trying to implement a PWA that stores the keys in local storage to be able to launch the app event when offline. In this case the session could be already destroyed when restarting the app. |
|
@dome4 thanks, I think this is good and we add the feature. I'll test this once the code is good. @FabianGosebrink can you do a code review as well, LGTM, needs some tests. Greetings Damien |
| @@ -88,4 +100,12 @@ export class HistoryJwtKeysCallbackHandlerService { | |||
| private resetBrowserHistory() { | |||
| window.history.replaceState({}, window.document.title, window.location.origin + window.location.pathname); | |||
| } | |||
|
|
|||
| private storeSigningKeys(jwtKeys: JwtKeys) { | |||
| this.storagePersistanceService.write('jwtKeys', jwtKeys); | |||
There was a problem hiding this comment.
Make this a constant in the class to avoid having different keys on read/write
|
Code looks absolutely fine, just one small comment. coverage dropped a bit, would be nice to have this staying at least the same. Good Job. UPDATE: Just checked the coverage and it went down in a completely different place. IMHO we can merge this. |
I was thinking about how to cache the JWKS in
callbackHistoryAndResetJwtKeys()of theHistoryJwtKeysCallbackHandlerServicebecause the method is used in every refresh cycle. In production, the token refresh might succeed, but if the connection is lost before requesting the JWKS, the entire pipeline will break after two retries. However, storing the result like ingetAuthWellKnownEndPoints()of theAuthWellKnownServiceis not feasible because of possible key rotations by the identity server. Therefore, I decided on a network-first strategy that tries the request for three times but falls back to the stored keys in case of failure.In addition, the retrieved JWKS always replace the stored keys, which minimizes the risk of validating the tokens against expired keys.
Any thoughts?