-
Notifications
You must be signed in to change notification settings - Fork 26
WIP user representation #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,152 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import type { Oidc } from "./Oidc"; | ||||||||||||||||||||||||||||||||||||||||||
| import { decodeJwt } from "../tools/decodeJwt"; | ||||||||||||||||||||||||||||||||||||||||||
| import { id } from "../tools/tsafe/id"; | ||||||||||||||||||||||||||||||||||||||||||
| import { assert } from "../tools/tsafe/assert"; | ||||||||||||||||||||||||||||||||||||||||||
| import type { MaybeAsync } from "../tools/MaybeAsync"; | ||||||||||||||||||||||||||||||||||||||||||
| import { Deferred } from "../tools/Deferred"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| export function createGetUser<User>(params: { | ||||||||||||||||||||||||||||||||||||||||||
| issuerUri: string; | ||||||||||||||||||||||||||||||||||||||||||
| //createUser: ParamsOfCreateOidc["createUser"]; | ||||||||||||||||||||||||||||||||||||||||||
| createUser: | ||||||||||||||||||||||||||||||||||||||||||
| | ((params: { | ||||||||||||||||||||||||||||||||||||||||||
| decodedIdToken: Oidc.Tokens.DecodedIdToken_OidcCoreSpec; | ||||||||||||||||||||||||||||||||||||||||||
| accessToken: string; | ||||||||||||||||||||||||||||||||||||||||||
| fetchUserInfo: () => Promise<{ | ||||||||||||||||||||||||||||||||||||||||||
| [key: string]: unknown; | ||||||||||||||||||||||||||||||||||||||||||
| sub: string; | ||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||
| issuerUri: string; | ||||||||||||||||||||||||||||||||||||||||||
| }) => MaybeAsync<User>) | ||||||||||||||||||||||||||||||||||||||||||
| | undefined; | ||||||||||||||||||||||||||||||||||||||||||
| getTokens: () => Promise<Oidc.Tokens>; | ||||||||||||||||||||||||||||||||||||||||||
| subscribeToTokensChange: (onTokenChange: (tokens: Oidc.Tokens) => void) => void; | ||||||||||||||||||||||||||||||||||||||||||
| renewTokens(): Promise<void>; | ||||||||||||||||||||||||||||||||||||||||||
| oidcMetadata: { | ||||||||||||||||||||||||||||||||||||||||||
| userinfo_endpoint?: string; | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||||||||||||||||||||
| const { issuerUri, createUser, getTokens, subscribeToTokensChange, 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(); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| 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 hash_new = computeHash({ | ||||||||||||||||||||||||||||||||||||||||||
| accessToken: tokens.accessToken, | ||||||||||||||||||||||||||||||||||||||||||
| decodedIdToken: tokens.decodedIdToken | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const prUser_new = (async () => { | ||||||||||||||||||||||||||||||||||||||||||
| const tokens = await getTokens(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. forEach callback implicitly returns a value (Biome lint warning). The arrow function passed to Proposed fix- onUserChanges.forEach(onUserChange =>
- onUserChange({
+ onUserChanges.forEach(onUserChange => {
+ onUserChange({
user: user_new,
user_previous: user_current
- })
- );
+ });
+ });📝 Committable suggestion
Suggested change
🧰 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 |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return user_new; | ||||||||||||||||||||||||||||||||||||||||||
| })(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| state = { | ||||||||||||||||||||||||||||||||||||||||||
| hash: hash_new, | ||||||||||||||||||||||||||||||||||||||||||
| prUser: prUser_new | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+68
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Multiple critical bugs in
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 |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const refreshUser: R_GetUser["refreshUser"] = async () => { | ||||||||||||||||||||||||||||||||||||||||||
| if (state !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||
| state.hash = ""; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| renewTokens(); | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| subscribeToTokensChange(tokens => {}); | ||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const getUser: GetUser = async () => { | ||||||||||||||||||||||||||||||||||||||||||
| if (prUser === undefined) { | ||||||||||||||||||||||||||||||||||||||||||
| if (createUser === undefined) { | ||||||||||||||||||||||||||||||||||||||||||
| throw new Error("oidc-spa: createUser not provided"); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| __updatePrUser(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert(prUser !== undefined); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const user = await prUser; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return id<R_GetUser>({ | ||||||||||||||||||||||||||||||||||||||||||
| user, | ||||||||||||||||||||||||||||||||||||||||||
| refreshUser, | ||||||||||||||||||||||||||||||||||||||||||
| subscribeToUserChange | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return { getUser }; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| function computeHash(params: { | ||||||||||||||||||||||||||||||||||||||||||
| decodedIdToken: Oidc.Tokens.DecodedIdToken_OidcCoreSpec; | ||||||||||||||||||||||||||||||||||||||||||
| accessToken: string; | ||||||||||||||||||||||||||||||||||||||||||
| }): string {} | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,13 +65,25 @@ import type { Evt } from "../tools/Evt"; | |
| import type { ParamsOfCreateGetServerDateNow } from "../tools/getServerDateNow"; | ||
| import { SESSION_STORAGE_GLOBAL_PREFIX } from "../tools/lazySessionStorage"; | ||
|
|
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI failure: Prettier formatting. The pipeline reports this file isn't formatted per Prettier. Run 🤖 Prompt for AI Agents |
||
| // NOTE: Replaced at build time | ||
| const VERSION = "{{OIDC_SPA_VERSION}}"; | ||
|
|
||
| export type ParamsOfCreateOidc< | ||
| DecodedIdToken extends Record<string, unknown> = Oidc.Tokens.DecodedIdToken_OidcCoreSpec, | ||
| AutoLogin extends boolean = false | ||
| AutoLogin extends boolean = false, | ||
| User = never | ||
| > = { | ||
| createUser?: (params: { | ||
| decodedIdToken: Oidc.Tokens.DecodedIdToken_OidcCoreSpec; | ||
| accessToken: string; | ||
| fetchUserInfo: () => Promise<{ | ||
| [key: string]: unknown; | ||
| sub: string; | ||
| }>; | ||
| issuerUri: string; | ||
| }) => MaybeAsync<User>; | ||
|
Comment on lines
73
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Since this is a WIP PR, flagging for when the feature is completed: both 🤖 Prompt for AI Agents |
||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| /** | ||
| * See: https://docs.oidc-spa.dev/v/v10/providers-configuration/provider-configuration | ||
| */ | ||
|
|
@@ -449,7 +461,8 @@ export async function createOidc_nonMemoized< | |
| __metadata, | ||
| disableDPoP: disableDPoP_params = false, | ||
| sessionRestorationMethod: sessionRestorationMethod_params, | ||
| BASE_URL: BASE_URL_params | ||
| BASE_URL: BASE_URL_params, | ||
| createUser | ||
| } = params; | ||
|
|
||
| const exports_earlyInit = await (async () => { | ||
|
|
@@ -1925,7 +1938,22 @@ export async function createOidc_nonMemoized< | |
| log?.(`isNewBrowserSession: ${value}`); | ||
|
|
||
| return value; | ||
| })() | ||
| })(), | ||
| getUser: async () => { | ||
| if (createUser === undefined) { | ||
| throw new Error("oidc-spa: createUser wasn't provided"); | ||
| } | ||
|
|
||
| return { | ||
| user: null as any, | ||
| subscribeToUserChange: onUserChange => { | ||
| return { | ||
| unsubscribeFromUserChange: () => {} | ||
| }; | ||
| }, | ||
| refreshUser: () => {} | ||
| }; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| }); | ||
|
|
||
| if (resultOfLoginProcess.isRestoredFromSessionStorage) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,18 +18,24 @@ import { createStatefulEvt } from "../tools/StatefulEvt"; | |||||
| import { id } from "../tools/tsafe/id"; | ||||||
| import { toFullyQualifiedUrl } from "../tools/toFullyQualifiedUrl"; | ||||||
| import { setDesiredPostLoginRedirectUrl } from "../core/desiredPostLoginRedirectUrl"; | ||||||
| import type { MaybeAsync } from "../tools/MaybeAsync"; | ||||||
|
|
||||||
| export function createOidcSpaUtils< | ||||||
| AutoLogin extends boolean, | ||||||
| DecodedIdToken extends Record<string, unknown> | ||||||
| DecodedIdToken extends Record<string, unknown>, | ||||||
| User | ||||||
| >(params: { | ||||||
| autoLogin: AutoLogin; | ||||||
| decodedIdTokenSchema: | ||||||
| | ZodSchemaLike<Oidc_core.Tokens.DecodedIdToken_OidcCoreSpec, DecodedIdToken> | ||||||
| | undefined; | ||||||
| decodedIdToken_mock: DecodedIdToken | undefined; | ||||||
| }): OidcSpaUtils<AutoLogin, DecodedIdToken> { | ||||||
| const { autoLogin, decodedIdTokenSchema, decodedIdToken_mock } = params; | ||||||
| createUser: | ||||||
| | ((params: { decodedIdToken: DecodedIdToken; accessToken: string }) => MaybeAsync<User>) | ||||||
| | undefined; | ||||||
| user_mock: User | undefined; | ||||||
| }): OidcSpaUtils<AutoLogin, DecodedIdToken, User> { | ||||||
| const { autoLogin, decodedIdTokenSchema, decodedIdToken_mock, createUser, user_mock } = params; | ||||||
|
Comment on lines
+33
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These params are destructured but never passed to 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken>>(); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
- const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken>>();
+ const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken, User>>();📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
|
|
@@ -98,7 +104,7 @@ export function createOidcSpaUtils< | |||||
|
|
||||||
| function useOidc(params?: { | ||||||
| assert?: "user logged in" | "user not logged in"; | ||||||
| }): UseOidc.Oidc<DecodedIdToken> { | ||||||
| }): UseOidc.Oidc<DecodedIdToken, User> { | ||||||
| const { assert: assert_params } = params ?? {}; | ||||||
|
|
||||||
| if (!isBrowser) { | ||||||
|
|
@@ -272,7 +278,7 @@ export function createOidcSpaUtils< | |||||
|
|
||||||
| async function getOidc(params?: { | ||||||
| assert?: "user logged in" | "user not logged in"; | ||||||
| }): Promise<GetOidc.Oidc<DecodedIdToken>> { | ||||||
| }): Promise<GetOidc.Oidc<DecodedIdToken, User>> { | ||||||
| if (!isBrowser) { | ||||||
| throw new Error("oidc-spa: getOidc() can't be used on the server"); | ||||||
| } | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No HTTP error handling for the userinfo endpoint response.
fetchwill 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. Checkr.okbefore 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