-
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 all 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,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(); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| 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
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 = ""; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| await renewTokens(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert(state !== undefined); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return state.prUser; | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
|
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 | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
|
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(""); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
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