fix: PR changes

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     -   now clears the access token cookie if it was called without a refresh token (if an access token cookie exists and if using cookie-based sessions).
     -   now clears cookies from the old domain if `OlderCookieDomain` is specified and multiple refresh/access token cookies exist, without updating the front-token or any of the tokens.
     -   now a 200 response may not include new session tokens.
+- Fixed a bug in the `normaliseSessionScopeOrThrowError` util function that caused it to remove leading dots from the scope string.
 
 ### Rationale
 

recipe/session/apiImplementation.go

@@ -24,11 +24,6 @@ import (
 
 func MakeAPIImplementation() sessmodels.APIInterface {
 	refreshPOST := func(options sessmodels.APIOptions, userContext supertokens.UserContext) (sessmodels.SessionContainer, error) {
-		err := ClearSessionCookiesFromOlderCookieDomain(options.Req, options.Res, options.Config, userContext)
-		if err != nil {
-			return nil, err
-		}
-
 		return RefreshSessionInRequest(options.Req, options.Res, options.Config, options.RecipeImplementation, userContext)
 	}
 

recipe/session/sessionRequestFunctions.go

@@ -322,6 +322,11 @@ func GetSessionFromRequest(req *http.Request, res http.ResponseWriter, config se
 func RefreshSessionInRequest(req *http.Request, res http.ResponseWriter, config sessmodels.TypeNormalisedInput, recipeImpl sessmodels.RecipeInterface, userContext supertokens.UserContext) (sessmodels.SessionContainer, error) {
 	supertokens.LogDebugMessage("refreshSession: Started")
 
+	err := ClearSessionCookiesFromOlderCookieDomain(req, res, config, userContext)
+	if err != nil {
+		return nil, err
+	}
+
 	refreshTokens := map[sessmodels.TokenTransferMethod]*string{}
 	// We check all token transfer methods for available refresh tokens
 	// We do this so that we can later clear all we are not overwriting
@@ -366,8 +371,16 @@ func RefreshSessionInRequest(req *http.Request, res http.ResponseWriter, config
 			return nil, err
 		}
 		if (allowedTokenTransferMethod == sessmodels.AnyTransferMethod || allowedTokenTransferMethod == sessmodels.CookieTransferMethod) && token != nil {
-			setCookie(config, res, accessTokenCookieKey, "", 0, "accessTokenPath", req, userContext)
-			supertokens.LogDebugMessage("refreshSession: cleared access token and returning UNAUTHORISED because refresh token in request is undefined")
+			supertokens.LogDebugMessage("refreshSession: cleared all session tokens and returning UNAUTHORISED because refresh token in request is undefined")
+
+			// We're clearing all session tokens instead of just the access token and then throwing an UNAUTHORISED
+			// error with `ClearTokens: True`. This approach avoids confusion and we don't want to retain session
+			// tokens on the client in any case if the refresh API is called without a refresh token but with an access token.
+			True := true
+			return nil, errors.UnauthorizedError{
+				Msg:         "Refresh token not found but access token is present. Clearing all tokens.",
+				ClearTokens: &True,
+			}
 		}
 
 		False := false

recipe/session/sessionUtils_test.go

@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2024, VRAI Labs and/or its affiliates. All rights reserved.
+ *
+ * This software is licensed under the Apache License, Version 2.0 (the
+ * "License") as published by the Apache Software Foundation.
+ *
+ * You may not use this file except in compliance with the License. You may
+ * obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package session
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNormaliseSessionScope(t *testing.T) {
+	t.Run("test with leading dot", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError(".example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, ".example.com", *result)
+	})
+
+	t.Run("test without leading dot", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "example.com", *result)
+	})
+
+	t.Run("test with http prefix", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("http://example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "example.com", *result)
+	})
+
+	t.Run("test with https prefix", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("https://example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "example.com", *result)
+	})
+
+	t.Run("test with IP address", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("192.168.1.1")
+		assert.NoError(t, err)
+		assert.Equal(t, "192.168.1.1", *result)
+	})
+
+	t.Run("test with localhost", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("localhost")
+		assert.NoError(t, err)
+		assert.Equal(t, "localhost", *result)
+	})
+
+	t.Run("test with leading and trailing whitespace", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("  example.com  ")
+		assert.NoError(t, err)
+		assert.Equal(t, "example.com", *result)
+	})
+
+	t.Run("test with subdomain", func(t *testing.T) {
+		result, err := normaliseSessionScopeOrThrowError("sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError("http://sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError("https://sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError(".sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, ".sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError("a.sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "a.sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError("http://a.sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "a.sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError("https://a.sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, "a.sub.example.com", *result)
+
+		result, err = normaliseSessionScopeOrThrowError(".a.sub.example.com")
+		assert.NoError(t, err)
+		assert.Equal(t, ".a.sub.example.com", *result)
+	})
+}

recipe/session/session_test.go

@@ -134,7 +134,7 @@ func TestCookieBasedAuth(t *testing.T) {
 		assert.Equal(t, "The request contains multiple session cookies. This may happen if you've changed the 'cookieDomain' value in your configuration. To clear tokens from the previous domain, set 'olderCookieDomain' in your config.\n", string(content))
 	})
 
-	t.Run("access token cookie is cleared if refresh token api is called without the refresh token", func(t *testing.T) {
+	t.Run("all session tokens are cleared if refresh token api is called without the refresh token but with the access token", func(t *testing.T) {
 		req, err = http.NewRequest(http.MethodPost, testServer.URL+"/auth/session/refresh", nil)
 		assert.NoError(t, err)
 		req.Header.Add("Cookie", "sAccessToken="+cookieData["sAccessToken"])
@@ -147,6 +147,7 @@ func TestCookieBasedAuth(t *testing.T) {
 		assert.Empty(t, cookieData["sAccessToken"])
 		assert.Equal(t, cookieData["accessTokenExpiry"], "Thu, 01 Jan 1970 00:00:00 GMT")
 		assert.Empty(t, cookieData["sRefreshToken"])
+		assert.Equal(t, cookieData["refreshTokenExpiry"], "Thu, 01 Jan 1970 00:00:00 GMT")
 	})
 
 	resetAll()

recipe/session/utils.go

@@ -271,35 +271,45 @@ func GetURLScheme(URL string) (string, error) {
 }
 
 func normaliseSessionScopeOrThrowError(sessionScope string) (*string, error) {
-	sessionScope = strings.TrimSpace(sessionScope)
-	sessionScope = strings.ToLower(sessionScope)
+	helper := func(scope string) (string, error) {
+		scope = strings.TrimSpace(scope)
+		scope = strings.ToLower(scope)
 
-	sessionScope = strings.TrimPrefix(sessionScope, ".")
+		scope = strings.TrimPrefix(scope, ".")
 
-	if !strings.HasPrefix(sessionScope, "http://") && !strings.HasPrefix(sessionScope, "https://") {
-		sessionScope = "http://" + sessionScope
+		if !strings.HasPrefix(scope, "http://") && !strings.HasPrefix(scope, "https://") {
+			scope = "http://" + scope
+		}
+
+		parsedURL, err := url.Parse(scope)
+		if err != nil {
+			return "", errors.New("please provide a valid sessionScope")
+		}
+
+		hostname := parsedURL.Hostname()
+
+		return hostname, nil
 	}
 
-	urlObj, err := url.Parse(sessionScope)
+	noDotNormalised, err := helper(sessionScope)
 	if err != nil {
-		return nil, errors.New("Please provide a valid sessionScope")
+		return nil, err
 	}
 
-	sessionScope = urlObj.Hostname()
-	sessionScope = strings.TrimPrefix(sessionScope, ".")
-
-	noDotNormalised := sessionScope
-
 	isAnIP, err := supertokens.IsAnIPAddress(sessionScope)
 	if err != nil {
 		return nil, err
 	}
-	if sessionScope == "localhost" || isAnIP {
-		noDotNormalised = sessionScope
+
+	if noDotNormalised == "localhost" || isAnIP {
+		return &noDotNormalised, nil
 	}
+
 	if strings.HasPrefix(sessionScope, ".") {
-		noDotNormalised = "." + sessionScope
+		noDotNormalised = "." + noDotNormalised
+		return &noDotNormalised, nil
 	}
+
 	return &noDotNormalised, nil
 }