Skip to content

Commit

Permalink
fix: session refresh loop if cookies writes are disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
anku255 committed Jun 26, 2024
1 parent cafd795 commit 2a8264a
Show file tree
Hide file tree
Showing 14 changed files with 157 additions and 35 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [unreleased]

## [20.1.1] - 2024-07-13
## [20.1.2] - 2024-06-26

### Changes

- Fixed a session refresh loop caused by blocked cookie writes. The SDK would throw/log a helpful error message when this happens.

## [20.1.1] - 2024-06-13

### Changes

Expand Down
2 changes: 1 addition & 1 deletion bundle/bundle.js

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions lib/build/axios.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 27 additions & 6 deletions lib/build/fetch.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/build/version.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/build/version.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 17 additions & 8 deletions lib/build/xmlhttprequest.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions lib/ts/axios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,9 @@ export function responseInterceptor(axiosInstance: any) {
!doNotDoInterception &&
// we do not call doesSessionExist here cause the user might override that
// function here and then it may break the logic of our original implementation.
!((await getLocalSessionState(true)).status === "EXISTS")

// Calling getLocalSessionState with tryRefresh: false, since the session would have been refreshed in the try block if expired.
(await getLocalSessionState(false)).status === "NOT_EXISTS"
) {
logDebugMessage(
"responseInterceptor: local session doesn't exist, so removing anti-csrf and sFrontToken"
Expand Down Expand Up @@ -597,7 +599,10 @@ async function saveTokensFromHeaders(response: AxiosResponse) {

const antiCsrfToken = response.headers["anti-csrf"];
if (antiCsrfToken !== undefined) {
const tok = await getLocalSessionState(true);
// Call getLocalSessionState with tryRefresh: false as the session was just refreshed.
// If the local session doesn't exist, it means we failed to write to cookies.
// Using tryRefresh: true would cause an infinite refresh loop.
const tok = await getLocalSessionState(false);
if (tok.status === "EXISTS") {
logDebugMessage("doRequest: Setting anti-csrf token");
await AntiCsrfToken.setItem(tok.lastAccessTokenUpdate, antiCsrfToken);
Expand Down
25 changes: 22 additions & 3 deletions lib/ts/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,24 @@ export async function getLocalSessionState(tryRefresh: boolean): Promise<LocalSe
status: "NOT_EXISTS"
};
}
logDebugMessage("getLocalSessionState: Retrying post refresh");
return await getLocalSessionState(tryRefresh);

const lastAccessTokenUpdate = await getFromCookies(LAST_ACCESS_TOKEN_UPDATE);
const frontTokenExists = await FrontToken.doesTokenExists();

// If we fail to retrieve the local session state after a successful refresh,
// it indicates an issue with writing to cookies. Without the FrontToken,
// session refresh may work but other SDK functionalities won't work as expected.
// Therefore, we throw an error here instead of retrying.
if (!frontTokenExists || lastAccessTokenUpdate === undefined) {
const errorMessage = `Failed to retrieve local session state from cookies after a successful session refresh. This indicates a configuration error or that the browser is preventing cookie writes (e.g., incognito mode).`;
console.error(errorMessage);
throw new Error(errorMessage);
}

logDebugMessage(
"getLocalSessionState: returning EXISTS since both frontToken and lastAccessTokenUpdate exists post refresh"
);
return { status: "EXISTS", lastAccessTokenUpdate };
} else {
logDebugMessage("getLocalSessionState: returning: " + response.status);
return response;
Expand Down Expand Up @@ -797,7 +813,10 @@ async function saveTokensFromHeaders(response: Response) {
}
const antiCsrfToken = response.headers.get("anti-csrf");
if (antiCsrfToken !== null) {
const tok = await getLocalSessionState(true);
// Call getLocalSessionState with tryRefresh: false as the session was just refreshed.
// If the local session doesn't exist, it means we failed to write to cookies.
// Using tryRefresh: true would cause an infinite refresh loop.
const tok = await getLocalSessionState(false);
if (tok.status === "EXISTS") {
logDebugMessage("saveTokensFromHeaders: Setting anti-csrf token");
await AntiCsrfToken.setItem(tok.lastAccessTokenUpdate, antiCsrfToken);
Expand Down
2 changes: 1 addition & 1 deletion lib/ts/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
* License for the specific language governing permissions and limitations
* under the License.
*/
export const package_version = "20.1.1";
export const package_version = "20.1.2";

export const supported_fdi = ["1.16", "1.17", "1.18", "1.19", "2.0", "3.0"];
22 changes: 17 additions & 5 deletions lib/ts/xmlhttprequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ export function addInterceptorsToXMLHttpRequest() {
// define constructor for my proxy object
XMLHttpRequest = function (this: XMLHttpRequestType) {
const actual: XMLHttpRequestType = new oldXMLHttpRequest();
let delayedQueue = firstEventLoopDone;
function delayIfNecessary(cb: () => void | Promise<void>) {
delayedQueue = delayedQueue.finally(() => cb()?.catch(console.error));
}

const self = this;
const listOfFunctionCallsInProxy: { (xhr: XMLHttpRequestType): void }[] = [];
Expand All @@ -70,6 +66,21 @@ export function addInterceptorsToXMLHttpRequest() {

const eventHandlers: Map<keyof XMLHttpRequestEventMap, Set<XHREventListener<any>>> = new Map();

let delayedQueue = firstEventLoopDone;
function delayIfNecessary(cb: () => void | Promise<void>) {
delayedQueue = delayedQueue.finally(() =>
cb()?.catch(err => {
const ev = new ProgressEvent("error");
(ev as any).error = err;
if (self.onerror !== undefined && self.onerror !== null) {
self.onerror(ev);
}

redispatchEvent("error", ev);
})
);
}

// We define these during open
// let method: string = "";
let url: string | URL = "";
Expand Down Expand Up @@ -215,7 +226,8 @@ export function addInterceptorsToXMLHttpRequest() {
return true;
} finally {
logDebugMessage("XHRInterceptor.handleResponse: doFinallyCheck running");
if (!((await getLocalSessionState(false)).status === "EXISTS")) {
// Calling getLocalSessionState with tryRefresh: false, since the session would have been refreshed in the try block if expired.
if ((await getLocalSessionState(false)).status === "NOT_EXISTS") {
logDebugMessage(
"XHRInterceptor.handleResponse: local session doesn't exist, so removing anti-csrf and sFrontToken"
);
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "supertokens-website",
"version": "20.1.1",
"version": "20.1.2",
"description": "frontend sdk for website to be used for auth solution.",
"main": "index.js",
"dependencies": {
Expand Down
49 changes: 49 additions & 0 deletions test/cross.auto_refresh.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -856,5 +856,54 @@ addTestCases((name, transferMethod, setupFunc, setupArgs = []) => {

await page.setRequestInterception(false);
});

it("should break out of session refresh loop if cookie writes are disabled", async function () {
await startST(300, false);
await setup({
disableCookies: true
});

let consoleLogs = [];
page.on("console", message => {
consoleLogs.push(message.text());
});

await page.evaluate(async () => {
supertokens.init({
apiDomain: BASE_URL,
maxRetryAttemptsForSessionRefresh: 5
});

const userId = "testing-supertokens-website";

const loginResponse = await toTest({
url: `${BASE_URL}/login`,
method: "post",
headers: {
Accept: "application/json",
"Content-Type": "application/json"
},
body: JSON.stringify({ userId })
});

assert.strictEqual(loginResponse.statusCode, 200);
assert.strictEqual(loginResponse.responseText, userId);

await assert.rejects(async () => {
await toTest({ url: `${BASE_URL}/` });
});
});

assert(
consoleLogs.includes(
"Saving to cookies was not successful, this indicates a configuration error or the browser preventing us from writing the cookies (e.g.: incognito mode)."
)
);
assert(
consoleLogs.includes(
"Failed to retrieve local session state from cookies after a successful session refresh. This indicates a configuration error or that the browser is preventing cookie writes (e.g., incognito mode)."
)
);
});
});
});

0 comments on commit 2a8264a

Please sign in to comment.