Merge pull request #113 from azegallo/master

Fix token refresh logic

Author: wiverson

Gotrue/Client.cs

@@ -488,7 +488,7 @@ public async Task<ResetPasswordForEmailState> ResetPasswordForEmail(ResetPasswor
 
 			await RefreshToken();
 
-			var user = await _api.GetUser(CurrentSession.AccessToken!);
+			var user = await _api.GetUser(CurrentSession.AccessToken);
 			CurrentSession.User = user;
 
 			return CurrentSession;
@@ -518,14 +518,18 @@ public async Task<Session> SetSession(string accessToken, string refreshToken, b
 				NotifyAuthStateChange(SignedIn);
 				return CurrentSession;
 			}
-
+			
+			var iat = payload.IssuedAt;
+			var exp = payload.ValidTo;
+			var expiresIn = (long)(exp - iat).TotalSeconds;
+			
 			CurrentSession = new Session
 			{
 				AccessToken = accessToken,
 				RefreshToken = refreshToken,
 				TokenType = "bearer",
-				ExpiresIn = payload.Expiration!.Value,
-				User = await _api.GetUser(accessToken)
+				ExpiresIn = expiresIn,
+				User = await _api.GetUser(accessToken),
 			};
 
 			NotifyAuthStateChange(SignedIn);
@@ -574,7 +578,7 @@ public async Task<Session> SetSession(string accessToken, string refreshToken, b
 				ExpiresIn = long.Parse(expiresIn),
 				RefreshToken = refreshToken,
 				TokenType = tokenType,
-				User = user
+				User = user,
 			};
 
 			if (storeSession)
@@ -595,14 +599,6 @@ public async Task<Session> SetSession(string accessToken, string refreshToken, b
 			if (CurrentSession == null)
 				return null;
 
-			// Check to see if the session has expired. If so go ahead and destroy it.
-			if (CurrentSession != null && CurrentSession.Expired())
-			{
-				_debugNotification?.Log($"Loaded session has expired");
-				DestroySession();
-				return null;
-			}
-
 			// If we aren't online, we can't refresh the token
 			if (!Online)
 			{
@@ -691,16 +687,28 @@ private void DestroySession()
 		/// <inheritdoc />
 		public async Task RefreshToken(string accessToken, string refreshToken)
 		{
+			if (!Online)
+				throw new GotrueException("Only supported when online", Offline);
+			
 			if (string.IsNullOrEmpty(accessToken) || string.IsNullOrEmpty(refreshToken))
 				throw new GotrueException("No token provided", NoSessionFound);
 
-			var result = await _api.RefreshAccessToken(accessToken, refreshToken);
-
-			if (result == null || string.IsNullOrEmpty(result.AccessToken))
-				throw new GotrueException("Could not refresh token from provided session.", NoSessionFound);
+			try
+			{
+				var result = await _api.RefreshAccessToken(accessToken, refreshToken);
 
-			CurrentSession = result;
-			NotifyAuthStateChange(TokenRefreshed);
+				if (result == null || string.IsNullOrEmpty(result.AccessToken))
+					throw new GotrueException("Could not refresh token from provided session.", NoSessionFound);
+				
+				CurrentSession = result;
+				NotifyAuthStateChange(TokenRefreshed);	
+			}
+			catch (GotrueException ex) when (ex.Reason is InvalidRefreshToken)
+			{
+				DestroySession();
+				NotifyAuthStateChange(SignedOut);
+				throw;
+			}
 		}
 
 		/// <inheritdoc />
@@ -712,17 +720,22 @@ public async Task RefreshToken()
 			if (CurrentSession == null || string.IsNullOrEmpty(CurrentSession?.AccessToken) || string.IsNullOrEmpty(CurrentSession?.RefreshToken))
 				throw new GotrueException("No current session.", NoSessionFound);
 
-			if (CurrentSession!.Expired())
-				throw new GotrueException("Session expired", ExpiredRefreshToken);
-
-			var result = await _api.RefreshAccessToken(CurrentSession.AccessToken!, CurrentSession.RefreshToken!);
-
-			if (result == null || string.IsNullOrEmpty(result.AccessToken))
-				throw new GotrueException("Could not refresh token from provided session.", NoSessionFound);
+			try
+			{
+				var result = await _api.RefreshAccessToken(CurrentSession.AccessToken!, CurrentSession.RefreshToken!);
+				if (result == null || string.IsNullOrEmpty(result.AccessToken))
+					throw new GotrueException("Could not refresh token from provided session.", NoSessionFound);
 
-			CurrentSession = result;
+				CurrentSession = result;
 
-			NotifyAuthStateChange(TokenRefreshed);
+				NotifyAuthStateChange(TokenRefreshed);	
+			}
+			catch (GotrueException ex) when (ex.Reason is InvalidRefreshToken)
+			{
+				DestroySession();
+				NotifyAuthStateChange(SignedOut);
+				throw;
+			}
 		}
 
 
@@ -791,7 +804,7 @@ public void Shutdown()
 
 			if (result == null || string.IsNullOrEmpty(result.AccessToken))
 				throw new GotrueException("Could not verify MFA.", MfaChallengeUnverified);
-
+			
 			var session = new Session
 			{
 				AccessToken = result.AccessToken,
@@ -835,14 +848,14 @@ public void Shutdown()
 
 			if (result == null || string.IsNullOrEmpty(result.AccessToken))
 				throw new GotrueException("Could not verify MFA.", MfaChallengeUnverified);
-
+			
 			var session = new Session
 			{
 				AccessToken = result.AccessToken,
 				RefreshToken = result.RefreshToken,
 				TokenType = "bearer",
 				ExpiresIn = result.ExpiresIn,
-				User = result.User
+				User = result.User,
 			};
 
 			UpdateSession(session);

Gotrue/Session.cs

@@ -17,7 +17,7 @@ public class Session
 		public string? AccessToken { get; set; }
 
 		/// <summary>
-		/// The number of seconds until the token expires (since it was issued). Returned when a login is confirmed.
+		/// The number of seconds until the access token expires (since it was issued). Returned when a login is confirmed.
 		/// </summary>
 		[JsonProperty("expires_in")]
 		public long ExpiresIn { get; set; }
@@ -49,17 +49,5 @@ public class Session
 
 		[JsonProperty("created_at")]
 		public DateTime CreatedAt { get; set; } = DateTime.UtcNow;
-
-		/// <summary>
-		/// The expiration date of this session, in UTC time.
-		/// </summary>
-		/// <returns></returns>
-		public DateTime ExpiresAt() => new DateTimeOffset(CreatedAt).AddSeconds(ExpiresIn).ToUniversalTime().DateTime;
-
-		/// <summary>
-		/// Returns true if the session has expired
-		/// </summary>
-		/// <returns></returns>
-		public bool Expired() => ExpiresAt() < DateTime.UtcNow;
 	}
 }

Gotrue/TokenRefresh.cs

@@ -1,5 +1,7 @@
 using System;
+using System.IdentityModel.Tokens.Jwt;
 using System.Threading;
+using Supabase.Gotrue.Exceptions;
 using Supabase.Gotrue.Interfaces;
 using static Supabase.Gotrue.Constants.AuthState;
 
@@ -46,10 +48,11 @@ public void ManageAutoRefresh(IGotrueClient<User, Session> sender, Constants.Aut
 				case SignedIn:
 					if (Debug)
 						_client.Debug("Refresh Timer started");
-					InitRefreshTimer();
+					CreateNewTimer();
 					// Turn on auto-refresh timer
 					break;
 				case SignedOut:
+				case Shutdown:
 					if (Debug)
 						_client.Debug("Refresh Timer stopped");
 					_refreshTimer?.Dispose();
@@ -58,32 +61,17 @@ public void ManageAutoRefresh(IGotrueClient<User, Session> sender, Constants.Aut
 				case UserUpdated:
 					if (Debug)
 						_client.Debug("Refresh Timer restarted");
-					InitRefreshTimer();
+					CreateNewTimer();
 					break;
 				case PasswordRecovery:
-					// Doesn't affect auto refresh
-					break;
 				case TokenRefreshed:
+				case MfaChallengeVerified:
 					// Doesn't affect auto refresh
 					break;
-				case Shutdown:
-					if (Debug)
-						_client.Debug("Refresh Timer stopped");
-					_refreshTimer?.Dispose();
-					// Turn off auto-refresh timer
-					break;
 				default: throw new ArgumentOutOfRangeException(nameof(stateChanged), stateChanged, null);
 			}
 		}
 
-		/// <summary>
-		/// Sets up the auto-refresh timer
-		/// </summary>
-		private void InitRefreshTimer()
-		{
-			CreateNewTimer();
-		}
-
 		/// <summary>
 		/// The timer calls this method at the configured interval to refresh the token.
 		///
@@ -119,29 +107,21 @@ private async void HandleRefreshTimerTick(object _)
 		/// </summary>
 		private void CreateNewTimer()
 		{
-			if (_client.CurrentSession == null || _client.CurrentSession.ExpiresIn == default)
+			if (_client.CurrentSession == null)
 			{
 				if (Debug)
 					_client.Debug($"No session, refresh timer not started");
 				return;
 			}
 
-			if (_client.CurrentSession.Expired())
-			{
-				if (Debug)
-					_client.Debug($"Token expired, signing out");
-				_client.NotifyAuthStateChange(SignedOut);
-				return;
-			}
-
 			try
 			{
-				TimeSpan interval = GetInterval();
+				TimeSpan refreshDueTime = GetSecondsUntilNextRefresh();
 				_refreshTimer?.Dispose();
-				_refreshTimer = new Timer(HandleRefreshTimerTick, null, interval, Timeout.InfiniteTimeSpan);
+				_refreshTimer = new Timer(HandleRefreshTimerTick, null, refreshDueTime, Timeout.InfiniteTimeSpan);
 
 				if (Debug)
-					_client.Debug($"Refresh timer scheduled {interval.TotalMinutes} minutes");
+					_client.Debug($"Refresh timer scheduled {refreshDueTime.TotalMinutes} minutes");
 			}
 			catch (Exception e)
 			{
@@ -151,23 +131,35 @@ private void CreateNewTimer()
 		}
 
 		/// <summary>
-		/// Interval should be t - (1/5(n)) (i.e. if session time (t) 3600s, attempt refresh at 2880s or 720s (1/5) seconds before expiration)
+		/// Returns remaining seconds until the access token should be refreshed.
+		/// Interval is calculated as:<code>t - (1/5(n))</code> (i.e. if session time (t) 3600s, attempt refresh at 2880s or 720s (1/5) seconds before expiration).
+		/// <remarks>
+		/// - The maximum refresh wait time is clamped to <see cref="ClientOptions.MaximumRefreshWaitTime"/>
+		/// </remarks>
+		/// <remarks>
+		/// - If the access token is expired it will refresh immediately.
+		/// </remarks>
 		/// </summary>
-		private TimeSpan GetInterval()
+		/// <returns>The remaining seconds until the token should be refreshed</returns>
+		private TimeSpan GetSecondsUntilNextRefresh()
 		{
-			if (_client.CurrentSession == null || _client.CurrentSession.ExpiresIn == default)
+			if (_client.CurrentSession is null || _client.CurrentSession.AccessToken == null)
 			{
 				return TimeSpan.Zero;
 			}
 
-			var interval = (long)Math.Floor(_client.CurrentSession.ExpiresIn * 4.0f / 5.0f);
-
-			var timeoutSeconds = Convert.ToInt64((_client.CurrentSession.CreatedAt.AddSeconds(interval) - DateTime.UtcNow).TotalSeconds);
+			var interval = (long)Math.Floor(_client.CurrentSession.ExpiresIn * 4.0 / 5.0);
+			var refreshAt = _client.CurrentSession.CreatedAt.AddSeconds(interval);
 
-			if (timeoutSeconds > _client.Options.MaximumRefreshWaitTime)
-				timeoutSeconds = _client.Options.MaximumRefreshWaitTime;
-
-			return TimeSpan.FromSeconds(timeoutSeconds);
+			var secondsUntilNextRefresh = Convert.ToInt64((refreshAt - DateTime.UtcNow).TotalSeconds);
+			
+			if (secondsUntilNextRefresh < 0)
+				return TimeSpan.Zero;
+			
+			if (secondsUntilNextRefresh > _client.Options.MaximumRefreshWaitTime)
+				secondsUntilNextRefresh = _client.Options.MaximumRefreshWaitTime;
+			
+			return TimeSpan.FromSeconds(secondsUntilNextRefresh);
 		}
 	}
 }

GotrueTests/AnonKeyClientFailureTests.cs

@@ -125,7 +125,9 @@ public async Task ClientTriggersTokenRefreshedEvent()
 			{
 				await _client.RefreshSession();
 			});
+			
 			AreEqual(InvalidRefreshToken, x.Reason);
+			IsNull(_client.CurrentSession);
 		}
 
 		[TestMethod("Client: expired token")]
@@ -138,18 +140,17 @@ public async Task ExpiredTokenTest()
 			IsNotNull(emailSession.RefreshToken);
 			IsNotNull(emailSession.User);
 
+			// Set CreatedAt to an old date - this should NOT prevent refresh from working
+			// Session "expiration" based on CreatedAt is about access token lifetime, not refresh token validity
+			_client.CurrentSession.CreatedAt = DateTime.UtcNow.AddDays(-10);
+
+			// Refresh should still succeed with a valid refresh token
 			await _client.RefreshSession();
-			
-			IsNotNull(emailSession.AccessToken);
-			IsNotNull(emailSession.RefreshToken);
-			IsNotNull(emailSession.User);
 
-			_client.CurrentSession.CreatedAt = DateTime.UtcNow.AddDays(-10);
-			var x = await ThrowsExceptionAsync<GotrueException>(async () =>
-			{
-				await _client.RefreshSession();
-			});
-			AreEqual(ExpiredRefreshToken, x.Reason);
+			IsNotNull(_client.CurrentSession);
+			IsNotNull(_client.CurrentSession.AccessToken);
+			IsNotNull(_client.CurrentSession.RefreshToken);
+			IsNotNull(_client.CurrentSession.User);
 		}
 
 		[TestMethod("Client: Send Reset Password Email for unknown email")]

GotrueTests/AnonKeyClientTests.cs

@@ -481,15 +481,5 @@ public async Task ClientCanSetSession()
 			// As this is being forced to regenerate, the original should be different than the cached.
 			AreNotEqual(refreshToken, _client.CurrentSession.RefreshToken);
 		}
-
-		[TestMethod("Session: `ExpiresAt` is Calculated Correctly.")]
-		public async Task SessionCalculatesExpiresAtCorrectly()
-		{
-			var email = $"{RandomString(12)}@supabase.io";
-			var session = await _client.SignUp(email, PASSWORD);
-
-			IsFalse(session.Expired());
-			AreEqual(session.ExpiresAt().Ticks, session.CreatedAt.ToUniversalTime().AddSeconds(session.ExpiresIn).Ticks);
-		}
 	}
 }