Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions apps/cyberstorm-remix/app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
} from "@thunderstore/ts-api-react/src/SessionContext";
import {
getPublicEnvVariables,
getSessionTools,
type publicEnvVariablesType,
} from "cyberstorm/security/publicEnvVariables";
import { StorageManager } from "@thunderstore/ts-api-react/src/storage";
Expand Down Expand Up @@ -580,12 +581,19 @@ const TooltipProvider = memo(function TooltipProvider({

function App() {
const data = useLoaderData<RootLoadersType>();
const dapper = new DapperTs(() => {
return {
apiHost: data?.publicEnvVariables.VITE_API_URL,
sessionId: data?.config.sessionId,
};
});
const sessionTools = getSessionTools();
const dapper = new DapperTs(
() => {
return {
apiHost: data?.publicEnvVariables.VITE_API_URL,
sessionId: data?.config.sessionId,
};
},
() =>
sessionTools.clearInvalidSession(
data?.publicEnvVariables.VITE_COOKIE_DOMAIN
)
);

return (
<Outlet
Expand Down
13 changes: 8 additions & 5 deletions apps/cyberstorm-remix/cyberstorm/utils/dapperClientLoaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ export function makeTeamSettingsTabLoader<T>(

const setupDapper = () => {
const tools = getSessionTools();
const config = tools?.getConfig();
return new DapperTs(() => ({
apiHost: config?.apiHost,
sessionId: config?.sessionId,
}));
const config = tools.getConfig();
return new DapperTs(
() => ({
apiHost: config.apiHost,
sessionId: config.sessionId,
}),
() => tools.clearInvalidSession()
);
};
5 changes: 4 additions & 1 deletion apps/cyberstorm-remix/cyberstorm/utils/dapperSingleton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ export function initializeClientDapper(factory?: ConfigFactory) {

if (!window.Dapper) {
const resolvedFactory = resolveConfigFactory();
window.Dapper = new DapperTs(resolvedFactory);
const tools = getSessionTools();
window.Dapper = new DapperTs(resolvedFactory, () =>
tools.clearInvalidSession()
);
}
}

Expand Down
35 changes: 30 additions & 5 deletions packages/dapper-ts/src/methods/currentUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,31 @@ import {

import { DapperTsInterface } from "../index";

function isInvalidTokenError(error: ApiError): boolean {
const detail = extractErrorDetail(error.responseJson);
return (
typeof detail === "string" && detail.toLowerCase().includes("invalid token")
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string matching logic using .includes("invalid token") is fragile and could fail to match:

  1. Variations in casing (though .toLowerCase() helps)
  2. Variations in wording (e.g., "token is invalid", "invalid session token", "bad token")
  3. Localized error messages in different languages
  4. Changes to the API error message format

Consider using a more robust approach, such as:

  • Checking for specific error codes if the API provides them
  • Using a regex pattern to match variations
  • Documenting the expected API error message format and adding a test to ensure it matches
Suggested change
typeof detail === "string" && detail.toLowerCase().includes("invalid token")
typeof detail === "string" &&
/invalid\s+token|token\s+is\s+invalid|invalid\s+session\s+token|bad\s+token/i.test(detail)

Copilot uses AI. Check for mistakes.
);
}

function extractErrorDetail(payload: unknown): string | undefined {
if (!payload) {
return undefined;
}
if (typeof payload === "string") {
return payload;
}
if (
typeof payload === "object" &&
payload !== null &&
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable 'payload' is of type date, object or regular expression, but it is compared to an expression of type null.

Copilot uses AI. Check for mistakes.
"detail" in payload &&
typeof (payload as { detail?: unknown }).detail === "string"
) {
return (payload as { detail: string }).detail;
}
return undefined;
}

export async function getCurrentUser(this: DapperTsInterface) {
try {
const data = await fetchCurrentUser({
Expand All @@ -17,13 +42,13 @@ export async function getCurrentUser(this: DapperTsInterface) {
return data;
} catch (error) {
if (error instanceof ApiError && error.response.status === 401) {
// If the user is not authenticated, we remove the session hook
this.removeSessionHook?.();
if (isInvalidTokenError(error)) {
// If the token is invalid, clear any persisted session data
this.removeSessionHook?.();
}
Comment on lines +45 to +48
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional session cleanup logic creates an inconsistency where only 401 errors with the exact text "invalid token" trigger session cleanup, while other 401 responses (e.g., expired tokens, revoked tokens, or authentication failures with different error messages) return null without clearing the session. This could leave stale session data in storage.

Consider either:

  1. Clearing the session for all 401 errors (restoring the previous behavior), or
  2. If the distinction is intentional, add a comment explaining why some 401s should preserve session data while others should clear it.
Suggested change
if (isInvalidTokenError(error)) {
// If the token is invalid, clear any persisted session data
this.removeSessionHook?.();
}
// Clear any persisted session data for all 401 errors
this.removeSessionHook?.();

Copilot uses AI. Check for mistakes.
return null;
} else {
// If it's another error, we throw it
throw error;
}
throw error;
}
}

Expand Down
37 changes: 33 additions & 4 deletions packages/ts-api-react/src/SessionContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export interface ContextInterface {
clearSession: (clearApiHost?: boolean) => void;
/** Remove session cookies. */
clearCookies: (domain: string) => void;
/** Clear all persisted session data and flag as stale. */
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The JSDoc comment should be more descriptive about what "clear all persisted session data" means. Consider specifying:

  • Clears current user data
  • Clears API host
  • Clears session cookies
  • Sets session as stale

Example: /** Clear all persisted session data (current user, API host, cookies) and mark session as stale. */

Suggested change
/** Clear all persisted session data and flag as stale. */
/**
* Clear all persisted session data (current user, API host, cookies) and mark session as stale.
*/

Copilot uses AI. Check for mistakes.
clearInvalidSession: (cookieDomainOverride?: string) => void;
/** Set SessionData in storage */
setSession: (sessionData: SessionData) => void;
/** Set session stale state */
Expand Down Expand Up @@ -99,6 +101,28 @@ export const clearCookies = (domain: string) => {
deleteCookie("sessionid", domain);
};

export const clearInvalidSession = (
_storage: StorageManager,
cookieDomainOverride?: string
) => {
if (typeof window === "undefined") {
return;
}
try {
clearSession(_storage, true);
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new clearInvalidSession function calls clearSession(_storage, true), which clears the API host, while the previous implementation at line 205 called clearSession(_storage, false), which preserved the API host. This is a subtle behavior change that could affect session recovery flows.

Verify this is intentional. If the API host should be preserved during invalid session cleanup (to allow the user to re-authenticate with the same API), consider passing false instead.

Suggested change
clearSession(_storage, true);
clearSession(_storage, false);

Copilot uses AI. Check for mistakes.
const cookieDomain =
cookieDomainOverride ||
_storage.safeGetValue(COOKIE_DOMAIN_KEY) ||
undefined;
if (cookieDomain) {
clearCookies(cookieDomain);
}
setSessionStale(_storage, true);
} catch (error) {
console.error("Failed to clear invalid session", error);
}
};

export const getConfig = (
_storage: StorageManager,
domain?: string
Expand Down Expand Up @@ -178,10 +202,7 @@ export const updateCurrentUser = async (
customClearSession
? customClearSession
: () => {
// This function gets called when the dapper getCurrentUser gets 401 as a response
clearSession(_storage, false);
// We want to clear the sessionid cookie if it's invalid.
clearCookies(_storage.safeGetValue(COOKIE_DOMAIN_KEY) || "");
clearInvalidSession(_storage);
}
);
const currentUser = await dapper.getCurrentUser();
Expand Down Expand Up @@ -247,6 +268,13 @@ export const getSessionContext = (
clearCookies(domain);
};

const _clearInvalidSession = (cookieDomainOverride?: string) => {
clearInvalidSession(
_storage,
cookieDomainOverride || cookieDomain || undefined
);
};

const _getConfig = (domain?: string): RequestConfig => {
return getConfig(_storage, domain);
};
Expand Down Expand Up @@ -289,6 +317,7 @@ export const getSessionContext = (
return {
clearSession: _clearSession,
clearCookies: _clearCookies,
clearInvalidSession: _clearInvalidSession,
getConfig: _getConfig,
runSessionValidationCheck: _runSessionValidationCheck,
updateCurrentUser: _updateCurrentUser,
Expand Down
1 change: 1 addition & 0 deletions packages/ts-api-react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export {
CURRENT_USER_KEY,
setSession,
clearSession,
clearInvalidSession,
getConfig,
runSessionValidationCheck,
storeCurrentUser,
Expand Down
Loading