Skip to content
Closed
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
117 changes: 63 additions & 54 deletions src/core/Oidc.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { OidcInitializationError } from "./OidcInitializationError";

export declare type Oidc<
DecodedIdToken extends Record<string, unknown> = Oidc.Tokens.DecodedIdToken_OidcCoreSpec
> = Oidc.LoggedIn<DecodedIdToken> | Oidc.NotLoggedIn;
DecodedIdToken extends Record<string, unknown> = Oidc.Tokens.DecodedIdToken_OidcCoreSpec,
User = never
> = Oidc.LoggedIn<DecodedIdToken, User> | Oidc.NotLoggedIn;

export declare namespace Oidc {
export type Common = {
Expand Down Expand Up @@ -36,59 +37,67 @@ export declare namespace Oidc {
initializationError: OidcInitializationError | undefined;
};

export type LoggedIn<DecodedIdToken extends Record<string, unknown> = Record<string, unknown>> =
Common & {
isUserLoggedIn: true;
renewTokens(params?: {
extraTokenParams?: Record<string, string | undefined>;
}): Promise<void>;
getTokens: () => Promise<Tokens<DecodedIdToken>>;
subscribeToTokensChange: (onTokenChange: (tokens: Tokens<DecodedIdToken>) => void) => {
unsubscribeFromTokensChange: () => void;
};
getDecodedIdToken: () => DecodedIdToken;
logout: (
params:
| { redirectTo: "home" | "current page" }
| { redirectTo: "specific url"; url: string }
) => Promise<never>;
goToAuthServer: (params: {
extraQueryParams?: Record<string, string | undefined>;
redirectUrl?: string;
transformUrlBeforeRedirect?: (url: string) => string;
}) => Promise<never>;
subscribeToAutoLogoutCountdown: (
tickCallback: (params: { secondsLeft: number | undefined }) => void
) => { unsubscribeFromAutoLogoutCountdown: () => void };
/**
* If you called `goToAuthServer` or `login` with extraQueryParams, this object let you know the outcome of the
* of the action that was intended.
*
* For example, on a Keycloak server, if you called `goToAuthServer({ extraQueryParams: { kc_action: "UPDATE_PASSWORD" } })`
* you'll get back: `{ extraQueryParams: { kc_action: "UPDATE_PASSWORD" }, result: { kc_action_status: "success" } }` (or "cancelled")
*/
backFromAuthServer:
| {
extraQueryParams: Record<string, string>;
result: Record<string, string>;
}
| undefined;
/**
* This is true when the user has just returned from the login pages.
* This is also true when the user navigate to your app and was able to be silently signed in because there was still a valid session.
* This false however when the use just reload the page.
*
* This can be used to perform some action related to session initialization
* but avoiding doing it repeatedly every time the user reload the page.
*
* Note that this is referring to the browser session and not the OIDC session
* on the server side.
*
* If you want to perform an action only when a new OIDC session is created
* you can test oidc.isNewBrowserSession && oidc.backFromAuthServer !== undefined
*/
isNewBrowserSession: boolean;
export type LoggedIn<
DecodedIdToken extends Record<string, unknown> = Record<string, unknown>,
User = never
> = Common & {
isUserLoggedIn: true;
renewTokens(params?: { extraTokenParams?: Record<string, string | undefined> }): Promise<void>;
getTokens: () => Promise<Tokens<DecodedIdToken>>;
subscribeToTokensChange: (onTokenChange: (tokens: Tokens<DecodedIdToken>) => void) => {
unsubscribeFromTokensChange: () => void;
};
getDecodedIdToken: () => DecodedIdToken;
logout: (
params: { redirectTo: "home" | "current page" } | { redirectTo: "specific url"; url: string }
) => Promise<never>;
goToAuthServer: (params: {
extraQueryParams?: Record<string, string | undefined>;
redirectUrl?: string;
transformUrlBeforeRedirect?: (url: string) => string;
}) => Promise<never>;
subscribeToAutoLogoutCountdown: (
tickCallback: (params: { secondsLeft: number | undefined }) => void
) => { unsubscribeFromAutoLogoutCountdown: () => void };
/**
* If you called `goToAuthServer` or `login` with extraQueryParams, this object let you know the outcome of the
* of the action that was intended.
*
* For example, on a Keycloak server, if you called `goToAuthServer({ extraQueryParams: { kc_action: "UPDATE_PASSWORD" } })`
* you'll get back: `{ extraQueryParams: { kc_action: "UPDATE_PASSWORD" }, result: { kc_action_status: "success" } }` (or "cancelled")
*/
backFromAuthServer:
| {
extraQueryParams: Record<string, string>;
result: Record<string, string>;
}
| undefined;
/**
* This is true when the user has just returned from the login pages.
* This is also true when the user navigate to your app and was able to be silently signed in because there was still a valid session.
* This false however when the use just reload the page.
*
* This can be used to perform some action related to session initialization
* but avoiding doing it repeatedly every time the user reload the page.
*
* Note that this is referring to the browser session and not the OIDC session
* on the server side.
*
* If you want to perform an action only when a new OIDC session is created
* you can test oidc.isNewBrowserSession && oidc.backFromAuthServer !== undefined
*/
isNewBrowserSession: boolean;

getUser: () => Promise<{
user: User;
subscribeToUserChange: (
onUserChange: (params: { user: User; user_previous: User | undefined }) => void
) => {
unsubscribeFromUserChange: () => void;
};
refreshUser: () => Promise<User>;
}>;
};

export type Tokens<
DecodedIdToken extends Record<string, unknown> = Tokens.DecodedIdToken_OidcCoreSpec
Expand Down
188 changes: 188 additions & 0 deletions src/core/createGetUser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
import type { Oidc } from "./Oidc";
import { id } from "../tools/tsafe/id";
import { assert } from "../tools/tsafe/assert";
import type { MaybeAsync } from "../tools/MaybeAsync";
import type { NonPostableEvt } from "../tools/Evt";
import { decodeJwt } from "../tools/decodeJwt";

export function createGetUser<User>(params: {
issuerUri: string;
createUser:
| ((params: {
decodedIdToken: Oidc.Tokens.DecodedIdToken_OidcCoreSpec;
accessToken: string;
fetchUserInfo: () => Promise<{
[key: string]: unknown;
sub: string;
}>;
issuerUri: string;
}) => MaybeAsync<User>)
| undefined;
getCurrentTokens: () => Oidc.Tokens<any>;
evtTokensChange: NonPostableEvt<void>;
renewTokens(): Promise<void>;
oidcMetadata: {
userinfo_endpoint?: string;
};
}) {
const { issuerUri, createUser, getCurrentTokens, evtTokensChange, renewTokens, oidcMetadata } =
params;

type GetUser = Oidc.LoggedIn<any, User>["getUser"];

type R_GetUser = Awaited<ReturnType<GetUser>>;

async function fetchUserInfo(params: { accessToken: string }) {
const { accessToken } = params;

const { userinfo_endpoint } = oidcMetadata;

if (!userinfo_endpoint) {
// TODO: Make a class for this error
throw new Error("oidc-spa: AS does not expose a userinfo endpoint");
}

const r = await fetch(userinfo_endpoint, {
headers: {
Authorization: `Bearer ${accessToken}`
}
});

return r.json();
}
Comment on lines +45 to +52

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

No HTTP error handling for the userinfo endpoint response.

fetch will not throw on 4xx/5xx responses. If the authorization server returns a 401 (expired/invalid token) or 500, r.json() will parse the error body (or fail if the body isn't JSON), producing misleading results. Check r.ok before parsing.

🛡️ Proposed fix
     const r = await fetch(userinfo_endpoint, {
         headers: {
             Authorization: `Bearer ${accessToken}`
         }
     });

+    if (!r.ok) {
+        throw new Error(
+            `oidc-spa: userinfo endpoint responded with ${r.status} ${r.statusText}`
+        );
+    }
+
     return r.json();
🤖 Prompt for AI Agents
In `@src/core/createGetUser.ts` around lines 46 - 53, The userinfo fetch in
createGetUser (the request to userinfo_endpoint using accessToken) doesn't check
HTTP status; update the logic to verify r.ok before calling r.json(): if !r.ok,
attempt to read/parse the error body safely (fallback to r.statusText) and throw
or return a structured error including status and message, otherwise return the
parsed JSON; ensure the Authorization header and existing callsites of
createGetUser remain unchanged but now receive a thrown error or explicit error
object on non-2xx responses.


let state: { prUser: Promise<User>; hash: string } | undefined = undefined;

const onUserChanges = new Set<(params: { user: User; user_previous: User | undefined }) => void>();

const subscribeToUserChange: R_GetUser["subscribeToUserChange"] = onUserChange => {
onUserChanges.add(onUserChange);

return {
unsubscribeFromUserChange: () => {
onUserChanges.delete(onUserChange);
}
};
};

function __updatePrUserIfHashChanged() {
assert(createUser !== undefined, "94302");

const hash_current = state?.hash;

const tokens = getCurrentTokens();

const hash_new = computeHash({
accessToken: tokens.accessToken,
decodedIdToken: tokens.decodedIdToken_original
});

const prUser_new = (async () => {
const prUser_current = state?.prUser;

if (hash_current === hash_new) {
assert(prUser_current !== undefined);
return prUser_current;
}

const user_new = await createUser({
accessToken: tokens.accessToken,
decodedIdToken: tokens.decodedIdToken_original,
issuerUri,
fetchUserInfo: () => fetchUserInfo({ accessToken: tokens.accessToken })
});

{
const user_current = await prUser_current;

onUserChanges.forEach(onUserChange =>
onUserChange({
user: user_new,
user_previous: user_current
})
);
}
Comment on lines +95 to +104

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

forEach callback implicitly returns a value (Biome lint warning).

The arrow function passed to forEach returns the result of onUserChange(...). While harmless at runtime, it triggers the Biome useIterableCallbackReturn rule. Wrap the call in braces to make it a void statement.

Proposed fix
-                onUserChanges.forEach(onUserChange =>
-                    onUserChange({
+                onUserChanges.forEach(onUserChange => {
+                    onUserChange({
                         user: user_new,
                         user_previous: user_current
-                    })
-                );
+                    });
+                });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
const user_current = await prUser_current;
onUserChanges.forEach(onUserChange =>
onUserChange({
user: user_new,
user_previous: user_current
})
);
}
{
const user_current = await prUser_current;
onUserChanges.forEach(onUserChange => {
onUserChange({
user: user_new,
user_previous: user_current
});
});
}
🧰 Tools
🪛 Biome (2.3.13)

[error] 99-99: This callback passed to forEach() iterable method should not return a value.

Either remove this return or remove the returned value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
In `@src/core/createGetUser.ts` around lines 96 - 105, The forEach callback
currently returns the call expression which triggers the Biome lint rule; in the
block where you await prUser_current and iterate on onUserChanges, change the
arrow callback passed to onUserChanges.forEach from an expression body to a
block body that calls onUserChange({...}); (i.e., wrap the call in braces so it
becomes a void statement) so functions like onUserChange({ user: user_new,
user_previous: user_current }) do not implicitly return a value.


return user_new;
})();

state = {
hash: hash_new,
prUser: prUser_new
};
}
Comment on lines +68 to +113

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Multiple critical bugs in __updatePrUserIfHashChanged.

  1. tokens is not in scope at lines 74-76. The variable tokens is only declared inside the async IIFE at line 80 (const tokens = await getTokens()), but computeHash at line 74 references tokens.accessToken and tokens.decodedIdToken in the outer function scope where no such variable exists. This is a ReferenceError at runtime.

  2. computeHash (line 149-152) has an empty body — it declares a return type of string but returns nothing (undefined). Every hash comparison will compare undefined === undefined, so the "hash changed" logic will never work correctly.

These two issues together mean user creation/refresh will never work as intended.

🧰 Tools
🪛 Biome (2.3.13)

[error] 99-99: This callback passed to forEach() iterable method should not return a value.

Either remove this return or remove the returned value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
In `@src/core/createGetUser.ts` around lines 69 - 114, __updatePrUserIfHashChanged
currently references tokens before they are fetched and relies on computeHash
which returns nothing; fix by first awaiting getTokens() at the top of
__updatePrUserIfHashChanged and use that tokens object when calling computeHash
and later when calling createUser (keep passing tokens.decodedIdToken_original
to createUser as before), and implement computeHash to return a deterministic
string (e.g., compute a stable SHA-256 or other stable digest over the
accessToken and decodedIdToken payload) so hash comparisons work correctly;
ensure references to computeHash, getTokens, __updatePrUserIfHashChanged, and
createUser are updated to use the same tokens source.


const refreshUser: R_GetUser["refreshUser"] = async () => {
if (state !== undefined) {
state.hash = "";
}

await renewTokens();

assert(state !== undefined);

return state.prUser;
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

evtTokensChange.subscribe(() => {
__updatePrUserIfHashChanged();
});

const getUser: GetUser = async () => {
if (createUser === undefined) {
throw new Error("oidc-spa: createUser not provided");
}

if (state === undefined) {
__updatePrUserIfHashChanged();

assert(state !== undefined);
}

const user = await state.prUser;

return id<R_GetUser>({
user,
refreshUser,
subscribeToUserChange
});
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return { getUser };
}

function computeHash(params: {
decodedIdToken: Oidc.Tokens.DecodedIdToken_OidcCoreSpec;
accessToken: string;
}): string {
const { decodedIdToken, accessToken } = params;

const decodedIdToken_stableish = (() => {
const { exp, iat, nonce, auth_time, amr, acr, ...rest } = decodedIdToken;

return rest;
})();

const decodedAccessToken_stableish = (() => {
let decodedAccessToken: Record<string, unknown>;

try {
decodedAccessToken = decodeJwt(accessToken);
} catch {
return undefined;
}

const { exp, iat, jti, nbf, cnf, ...rest } = decodedAccessToken;

return rest;
})();

const stringify = (obj: Record<string, unknown>) =>
JSON.stringify(Object.entries(obj).sort(([a], [b]) => a.localeCompare(b)));

return [
stringify(decodedIdToken_stableish),
"|",
decodedAccessToken_stableish === undefined ? "" : stringify(decodedAccessToken_stableish)
].join("");
}
Loading
Loading