Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Session tokens are not correctly renewed when using AWS Cognito #45

Closed
iarenaza opened this issue Jul 5, 2021 · 1 comment
Closed

Comments

@iarenaza
Copy link
Contributor

iarenaza commented Jul 5, 2021

Our ClojureScript session management system should auto-refresh the session token (ID Token in our case) before it gets expired. But what happens now is that the session expires once the token expires, denying any call to back end REST endpoints.

I think it's broken because of this: https://github.com/magnetcoop/hydrogen.duct-template/blob/master/resources/session/cognito/cljs/session.cljs#L28-L30. There we return the value of calling getSession(). Which fine as long as the current token hasn't expired, as we return the locally cached session (I have console.log calls all over the AWS Cognito JS library that proves this).

But when the current token has expired, the cached session is no longer valid and it is destroyed. And a HTTP call is made to Cognito service to refresh the token. But that call is made using fetch, which uses JavaScript promises. When the promise is resolved (or rejected), our callback is called with either an error or a result.

But, here's the interesting tidbit, while the fetch promise is still ongoing, getSession() returns. In other words, getSession() returns without waiting for the promise to be resolved. As it has nothing to return (the promise is still not resolved!), it returns undefined (which is equivalent to ClojuresScript nil).

And there is where our problem begins. We use the return value of getSession() as it was the currently valid session. But in this case it is not! It is empty (nil) because the promise is still running. So we return nil from get-user-session and our session management code thinks that the user is no longer logged in. And as a result it redirects the user to the login page. This happens so fast that the redirection happens before the fetch promise is finished. And because we are leaving the current location to go to another one (remember, we are redirecting the user to the login page), the browser cancels the fetch promise and returns a TypeError of type NetworkError.

iarenaza added a commit that referenced this issue Jul 5, 2021
Our ClojureScript session management system should auto-refresh the
session token (ID Token in our case) before it gets expired. But what
happens now is that the session expires once the token expires,
denying any call to back end REST endpoints.

It is broken because of this:
https://github.com/magnetcoop/hydrogen.duct-template/blob/master/resources/session/cognito/cljs/session.cljs#L28-L30.
There we return the value of calling getSession(). Which fine as long
as the current token hasn't expired, as we return the locally cached
session.

But when the current token has expired, the cached session is no
longer valid and it is destroyed. And a HTTP call is made to Cognito
service to refresh the token. But that call is made using fetch, which
uses JavaScript promises. When the promise is resolved (or rejected),
our callback is called with either an error or a result.

But, here's the interesting tidbit, while the fetch promise is still
ongoing, getSession() returns. In other words, getSession() returns
without waiting for the promise to be resolved. As it has nothing to
return (the promise is still not resolved!), it returns
undefined (which is equivalent to ClojuresScript nil).

And there is where our problem begins. We use the return value of
getSession() as it was the currently valid session. But in this case
it is not! It is empty (nil) because the promise is still running. So
we return nil from get-user-session and our session management code
thinks that the user is no longer logged in. And as a result it
redirects the user to the login page. This happens so fast that the
redirection happens before the fetch promise is finished. And because
we are leaving the current location to go to another one (remember, we
are redirecting the user to the login page), the browser cancels the
fetch promise and returns a TypeError of type NetworkError.

So in order to fix this we need to:

- Stop assuming that get-user-session always returns a valid, active
  session. The session may have just expired and getSession may have
  not refreshed it yet, so it may return nil. We need to deal with
  this scenario.

- We need to dispatch the ::set-token-and-schedule-refresh event only
  when we know the session has been successfully refreshed and we have
  a valid, fresh token. Given that the getSession() doesn't force a
  refresh of a session/token until it has expired (which is too late
  for us!) we need to trigger the refresh ourselves. And dispatch the
  above event only when we are sure the refresh was performed
  successfully.

[Re: #45]
@iarenaza
Copy link
Contributor Author

iarenaza commented Jul 5, 2021

Fixed in commit 5c03913

@iarenaza iarenaza closed this as completed Jul 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant