supertokens/supertokens-flutter

Shortcomings and issues that prevent easy migration from Firebase

Opened this issue · 7 comments

I am porting our Flutter multiplatform applications (linux, web, android) from firebase to supertokens based authentication. During the process of porting I found several problems and was forced to sacrifice some of the existing user account management functionality. Here I have tried to summarize the difficulties I encountered in the hope that some of them will be addressed by the supertokens_flutter team in the future.

  1. The supertokens_flutter package is built on the assumption that all applications use HTTP to communicate not only with the supertokens server, but also with their own business logic backends. Since our apps use grpc to communicate with our backends, the supertokens_flutter cannot be used as intended, e.g. as a magical wrapper around the http communication package that hides details such as session refresh, access token insertion, etc. Initially, I had doubts whether I should use this package at all, but currently I came up with a solution in which I reuse few of the publicly available properties and functions like init(), attemptRefreshingSession(), etc. We cannot use the supertokens frontend either, because it would create discrepancies in the look and feel, and also damage apps support for theming, internationalization, etc.

In order for the supertokens_flutter package to be useful for use cases similar to ours, the current API needs to be extended to allow subscribing to authentication state changes as well as user profile and access token changes, similar to what the Firebase Auth API for Flutter provides.

  1. The supertokens_flutter package uses shared preferences to persist some sensitive data, but our applications use a different storage implementation. It would be nice if a storage API was defined so that people could interface their own storage implementation. That way, their applications can store their preferences in a single store, which could eventually implement ecryption.

  2. The timeJoined field is only included in responses to signup or signin calls. If the applications just resume the authentication session on startup instead of forcing the user to re-signin, then there seems to be no way to display this value on the user profile page. The result of the getAccessTokenPayloadSecurely() function call does not contain timeJoined, and I have not been able to find any other Supertokens API call besides signup and signin that would allow me to get the user's timeJoined property.

  3. Unlike Firebase authentication, there is no out-of-the-box support for the displayName of the user account. see firebase get user data API call

  4. Unlike Firebase authentication, there is no out-of-the-box support for the passwordUpdatedAt of the user account. see firebase get user data API call

  5. Unlike Firebase authentication, there is no out-of-the-box support for the lastLoginAt of the user account. see firebase get user data API call

  6. Unlike Firebase authentication, there is no out-of-the-box support for the photoUrl of the user account. see firebase get user data API call

Screenshot_User_Profile_2

As you can see from the screenshot of the Firebase powered user profile page of our apps, most of the information and functionality cannot be easily implemented with Supertokens.

Thanks for the detailed feedback @coderaper. I will address them below:

The supertokens_flutter package is built on the assumption that all applications use HTTP to communicate not only with the supertokens server, but also with their own business logic backends. Since our apps use grpc to communicate with our backends, the supertokens_flutter cannot be used as intended, e.g. as a magical wrapper around the http communication package that hides details such as session refresh, access token insertion, etc. Initially, I had doubts whether I should use this package at all, but currently I came up with a solution in which I reuse few of the publicly available properties and functions like init(), attemptRefreshingSession(), etc. We cannot use the supertokens frontend either, because it would create discrepancies in the look and feel, and also damage apps support for theming, internationalization, etc.

For HTTP requests, you can use our interceptors. For non http request, you can fetch the access token using the getAccessToken function in our SDK, and adding that to the grpc request however you like. This access token is already refreshed so you don't need to handle refreshing yourself.

The supertokens_flutter package uses shared preferences to persist some sensitive data, but our applications use a different storage implementation. It would be nice if a storage API was defined so that people could interface their own storage implementation. That way, their applications can store their preferences in a single store, which could eventually implement ecryption.

Fair point. We will work on this when we have the time.

For points 3, 4, 5, 6, 7

You have two options here:

  • Call an API from the frontend which does session verification and returns the user's data to the frontend. You can get the timeJoined by fetching the user object via our backend SDK. As per other fields, you will have to store them yourself somewhere, either in the user metadata recipe we have, or in your own db. For example, you can store hte passwordUpdatedAt time in the password reset API override on the backend.
  • Instead of making an API, you can put this info in the session's access token payload via the create_new_session override on the backend (in the session recipe). Then on the frontend, you can read these values via the getAccessTokenPayloadSecurely function.

For non http request, you can fetch the access token using the getAccessToken function in our SDK, and adding that to the grpc request however you like. This access token is already refreshed so you don't need to handle refreshing yourself.

The Firebase based applications I ported were built around a globally accessible, automatically updated User (account) object. If this user object was in a signed-in state, then all the tokens it contained were valid and could be passed to the backends. If the User object was not in a sign-in state, for example due to an initiated sign-out, a communication failure with the authentication server, etc., the application user was automatically redirected from its current location to the sign-in page. This User object was also visualised on the profile management pages, meaning that it acted as a cache and therefore no calls were made to the authentication backend unless account referesn or some account management operations were initiated. I have found that many other applications follow a similar architecture.

I wanted to keep this architecture to minimise the changes when porting to Supertokens and even to make the authentication backends interchangeable. That's why my initial thought was to replace the access to the access tokens of the User object with a call to getAccessToken() which is exactly what you suggest. After analyzing what getAccessToken() does, I have decided not to go down this road. There are several reasons for this:

  1. getAccessToken() can return null and it doesn't provide any means such as throwing an exception to give information about what is the exact reason for the failure.

  2. Even if it did provide meaningful information about why it failed by returning an error code or throwing an exception it would be very inconvenient to handle this error at every point in the application where we call the backend and need to pass an access token. This means that I would have to change every place in our applications where we call the backend API call.

  3. As far as I can follow your code, the Supertokens package does not perform an access token refresh in the background. This means that if the access token has expired and I call getAccessToken() in preparation for calling the backend API, I'll trigger an access token renewal at that very moment. The delay of this access token renewal may not be desirable in some use cases such as time critical processing, bidding applications, etc.

  4. Even for the use cases where the delay in access token renewal is not so important, we would have an undesirable side effect in the case of a lost connection. Shortly after the user performs some action that requires a backend API call, a getAccessToken() call would be made and it would return null. If I do not want to change the application architecture too much, I will have to respond to this event by changing the state of the user to not logged in, which would end up redirecting the user to the sign-in page. I do not think that such behaviour can be considered good UIX. That's because no one wants to press a button only to be suddenly logged out.

That's why I finally came up with the solution of not using getAccessToken(), but instead implementing a wrapper that takes care of providing and maintaining an automatically updated user (account) object. This also motivated me to give you my feedback. There may be other Supertoken users who use non-HTTP requests to their backends and would benefit from a more flexible API.

Appreciate the detailed feedback here. Here are my thoughts:

getAccessToken() can return null and it doesn't provide any means such as throwing an exception to give information about what is the exact reason for the failure.

If you are using cookie based auth (which is default for web), there is no way that the frontend has access to the access token, which is why it returns undefined. Instead, you can switch to using header based auth. Usually, before calling this function, you should check if a session exists by calling await Session.doesSessionExist()

As far as I can follow your code, the Supertokens package does not perform an access token refresh in the background. This means that if the access token has expired and I call getAccessToken() in preparation for calling the backend API, I'll trigger an access token renewal at that very moment. The delay of this access token renewal may not be desirable in some use cases such as time critical processing, bidding applications, etc.

Well, in this case, you can extend the access token lifetime. Or, if you want, you can have your own setInterval somewhere which calls await Session.attemptRefreshingSession() function, which effectively does refresh in the background.

Even for the use cases where the delay in access token renewal is not so important, we would have an undesirable side effect in the case of a lost connection. Shortly after the user performs some action that requires a backend API call, a getAccessToken() call would be made and it would return null

I believe that a failure in refreshing the session cause of network issues should cause the getAccessToken function call to throw an error. Are you observing some of other behaviour?

That's why I finally came up with the solution of not using getAccessToken(), but instead implementing a wrapper that takes care of providing and maintaining an automatically updated user (account) object. This also motivated me to give you my feedback. There may be other Supertoken users who use non-HTTP requests to their backends and would benefit from a more flexible API.

Im glad you got a solution working. Do you mind sharing the relevant code snippet so I can see how we can improve our SDK? Thanks.

I assume you have a typo in your answer and you mean SuperTokens.doesSessionExist() and SuperTokens.attemptRefreshingSession() instead of Session.doesSessionExist() and Session.attemptRefreshingSession().

I do not see much sense in calling SuperTokens.doesSessionExist() right before calling getAccessToken(), first because I can still get null due to race conditioning, and second because it does not solve 2, 3, and 4.

What I am currently doing is to periodically call SuperTokens.attemptRefreshingSession() just before the access token expires and then call SuperTokens.getAccessToken() to get the refreshed token.

What I am missing are public API functions that can give me information and control over the session state.

What I mean here is that if I call SuperTokens.doesSessionExist() after I restarting an application with an active session, it would return false even if there was a session at the moment the application was closed and there is a refresh token stored in the Shared Preferences. In fact, if I call SuperTokens.attemptRefreshingSession() anyway, the session would be restored.

So I think that doesSessionExist() should probably be renamed to doesActiveSessionExist() to better reflect it's purpose. We also need an additional function which I'll tentatively call doesPersistedRefreshTokenExist(), which should return true if there is a refresh token stored in the Shared Preferences that can be used to resume/refresh the session and false otherwise.

I also think that unconditionally storing the refresh token in the Shared Preferences is a bad idea from a security point of view, especially considering that this storage is not encrypted. It should only be done if the user explicitly agrees.

In most of the applications including ours there is an option in the user profile typically called "Keep me signed in on this device". What we expect is that if keepMeSignedIn is off than the app would not persist the refresh tokens and when restarted it would prompt the user to sign in. Of course, this does not imply that the app would not refresh the session unless the app was not closed. Whether the application would refresh the session or let it expire (if the user does not respond to the notification) depends on the particular use case.

The current SuperTokens implementation does not have a keepMeSignedIn like input argument, and it always stores both the refresh and access tokens on the device. Although I have managed to implement keepMeSignedIn so that it works as expected from the point of view of the unsuspecting observer, in reality this creates a potential vulnerability. This is a huge step back from the Firebase version where I had control over whether and where to store the refresh token and where the access token was never persisted on the device.

Is it there really any reason to store the access token in the Shared Preferences? It would be nice if you stopped persisting it at all and just kept it only in the memory.

Another session control function we need is a one that deletes the currently persisted refresh token from the Shared Preferences. I'll call it deletePersistedRefreshToken() for now. The need for this function arises, for example, when you change the value of keepMeSignedIn from On to Off, or select a different SuperTokens server. Such use case arises when we have a development, staging and production implementation of our servers, each with its own dedicated authentication server.

If we summarise my main wishes, they are:

  • implement doesPersistedRefreshTokenExist()
  • implement deletePersistedRefreshToken()
  • add a boolean persistRefreshToken parameter to SuperTokens.init(). If persistRefreshToken is false do not store the refresh token into the Shared Preferences.
  • stop persisting the the access token in Shared Preferences. If app is restarted and there is refresh token in Shared Preferences than attemptRefreshingSession() should be able to create a new access token

Meanwhile you can only call SuperTokens.init() once. This is a problem if we want to implement a dialogue in the application that allows us to change the endpoint of the SuperTokens server without restarting. I have managed to overcome this problem by abusing the existing API -- my init() wrapper resets the SuperTokens.isInitCalled flag before calling SuperTokens.init(). Of course, there is no guarantee that my solution will work after the next update of the supertokens_flutter package. It could be that someone decides to increase the encapsulation and changes SuperTokens.isInitCalled from a public variable to a getter, or hides it altogether.

I believe that a failure in refreshing the session cause of network issues should cause the getAccessToken function call to throw an error. Are you observing some of other behaviour?

No, because I always refresh the session just before it expires. If the refresh fails, then the user enters Unauthenticated or Reauthenticable state and he is redirected to the corresponding pages, therefore he is not able to call the backend API and he does not need to prepare access token.

calling getAccessToken(), first because I can still get null due to race conditioning

What's the race condition?

What I mean here is that if I call SuperTokens.doesSessionExist() after I restarting an application with an active session, it would return false even if there was a session at the moment the application was closed

I don't think that's true. This function returns true if the front-token exists in storage, and that exists as long as the refresh token exists.

I also think that unconditionally storing the refresh token in the Shared Preferences is a bad idea from a security point of view, especially considering that this storage is not encrypted. It should only be done if the user explicitly agrees.

I agree. We can consider allowing users to add a storage plugin for now which will allow them to store the tokens wherever they like.

About the keepMeSignIn feature

It makes sense. I believe this would be solved when we allow users to specify their own storage for tokens. You could, for example, keep the session tokens in memory and that way on app close, the user would be logged out.

Another session control function we need is a one that deletes the currently persisted refresh token from the Shared Preferences. I'll call it deletePersistedRefreshToken() for now. The need for this function arises, for example, when you change the value of keepMeSignedIn from On to Off, or select a different SuperTokens server. Such use case arises when we have a development, staging and production implementation of our servers, each with its own dedicated authentication server.

How would this be different from the current signOut function we have? That calls an API to delete the session from the server, as well as from the device storage.

No, because I always refresh the session just before it expires. If the refresh fails, then the user enters Unauthenticated or Reauthenticable state and he is redirected to the corresponding pages, therefore he is not able to call the backend API and he does not need to prepare access token.

Right. But if you did not do this background refresh and let our sdk do a refresh if needed when doesSessionExist is called, does it throw in case the refresh API fails? If not, this is a bug and we will fix it.

calling getAccessToken(), first because I can still get null due to race conditioning

What's the race condition?

What I mean here is this. Suppose I have already lost the connection to the authentication server. I first call doesSessionExist(), which fetches the cached FrontToken, checks that it has not expired and returns true. Encouraged, I call getAccessToken(), which internally calls doesSessionExist() again. Now there is a non-zero probability that this time the session expiration check

    ...
    if (accessTokenExpiry != null && accessTokenExpiry < now) {
     ...
    }
    ...

will fail (because some time has elapsed), but due to the lost connection the session cannot be renewed. In the end, I may still get null from the getAccessToken() call, even though I have just checked that a session exists.

What I mean here is that if I call SuperTokens.doesSessionExist() after I restarting an application with an active session, it would return false even if there was a session at the moment the application was closed

I don't think that's true. This function returns true if the front-token exists in storage, and that exists as long as the refresh token exists.

My bad here. I have already forgotten what the actual reason was that made me decide to stop using the doesSessionExist() function. It was actually the other way round. From the name, I initially assumed that this was a purely informational function that would return true if there was an active session or a persisted refresh token that could be used to resume the session. What I didn't expect is that this function has a side effect, and that side effect is to restore a non-active but persisted session. This turned out to be quite an obstacle because I lost control over when and on which page I wanted to resume the inactive but persisted session. That's because I want to show the user what's going on, display appropriate text, show a busy indicator, etc.

I hope this explains why I need a pure getter function that has no side effects. In my previous comment I tentatively called this function doesPersistedRefreshTokenExist().

From my point of view, doesSessionExist() is actually equivalent to calling attemptRefreshingSession() while catching any exceptions and finally returning a simple boolean result indicating success or failure.

About the keepMeSignIn feature

It makes sense. I believe this would be solved when we allow users to specify their own storage for tokens. You could, for example, keep the session tokens in memory and that way on app close, the user would be logged out.

I agree. If the application developer has control on this whether and where to store the tokens than there is no need to provide any support for keepMeSignedIn on the SuperTokens side.

Another session control function we need is a one that deletes the currently persisted refresh token from the Shared Preferences. I'll call it deletePersistedRefreshToken() for now.

How would this be different from the current signOut function we have? That calls an API to delete the session from the server, as well as from the device storage.

The difference is that in the case of sign out, the user is immediately kicked out of the program. In the case of setting keepMeSignedIn to Off (i.e. do not persist the refresh tokens anymore) and deleting the last persisted refresh token, the user can continue working until he closes the program. At least this was the behavior of our applications when using Firebase based authentication. Now I am looking for a way to achieve equivalent behavior with SuperTokens based authentication, which seems difficult if I as a developer do not have control over whether the tokens are persisted.

My current workaround is to maintain additional flags in the application's local DB that would indicate whether or not to ignore the always-persisted refresh tokens. I am not happy about this, because it gives the users the false impression that their tokens are not stored on the particular device, when in fact they are stored in a plain text format. (Of course, the tokens are deleted if the user signs out before closing the application.)

No, because I always refresh the session just before it expires. If the refresh fails, then the user enters Unauthenticated or Reauthenticable state and he is redirected to the corresponding pages, therefore he is not able to call the backend API and he does not need to prepare access token.

Right. But if you did not do this background refresh and let our sdk do a refresh if needed when doesSessionExist is called, does it throw in case the refresh API fails? If not, this is a bug and we will fix it.

My test showed that doesSessionExist() it doesn't throw when there is no connection to the authentication server, but this doesn't matter to me because, as I have already wrote above, I have stopped using it because of the side effect it exhibits.

Thanks for the feedback. We will investigate some of the issues you pointed out when we get the time :)