diff --git a/.github/workflows/downstream-verify.yml b/.github/workflows/downstream-verify.yml index ce3439b4c810..30dc1f236665 100644 --- a/.github/workflows/downstream-verify.yml +++ b/.github/workflows/downstream-verify.yml @@ -155,6 +155,7 @@ jobs: src/composables/__tests__/useModelWhitelist.spec.ts \ src/components/account/__tests__/BulkEditAccountModal.spec.ts \ src/views/admin/__tests__/AccountsView.console.spec.ts \ + src/utils/__tests__/accountKeySearch.spec.ts \ src/views/admin/__tests__/AccountsView.priorityColumn.spec.ts \ src/features/image-studio/__tests__/api.spec.ts \ src/components/layout/__tests__/AppSidebar.spec.ts \ diff --git a/backend/internal/repository/account_repo.go b/backend/internal/repository/account_repo.go index 72d07b6695bf..4ced339dda1f 100644 --- a/backend/internal/repository/account_repo.go +++ b/backend/internal/repository/account_repo.go @@ -1142,7 +1142,7 @@ func (r *accountRepository) accountListFilteredQuery(platform, accountType, stat } } if search != "" { - q = q.Where(dbaccount.NameContainsFold(search)) + q = q.Where(service.AccountSearchPredicate(search)) } if groupID == service.AccountListGroupUngrouped { q = q.Where(dbaccount.Not(dbaccount.HasAccountGroups())) diff --git a/backend/internal/repository/account_repo_integration_test.go b/backend/internal/repository/account_repo_integration_test.go index 2b24cbde3760..509a0aafd2ce 100644 --- a/backend/internal/repository/account_repo_integration_test.go +++ b/backend/internal/repository/account_repo_integration_test.go @@ -4,7 +4,9 @@ package repository import ( "context" + "crypto/sha256" "database/sql" + "encoding/hex" "strings" "testing" "time" @@ -638,6 +640,15 @@ func (s *AccountRepoSuite) TestListWithFilters() { } } +func (s *AccountRepoSuite) TestListWithFilters_SearchByAPIKeyDigest() { + requireAccountSearchByAPIKeyDigest(s.T(), s.client, "key-search-", func(search string) []int64 { + accounts, page, err := s.repo.ListWithFilters(s.ctx, pagination.PaginationParams{Page: 1, PageSize: 10}, "", "", "", search, 0, "") + s.Require().NoError(err) + s.Require().Equal(int64(len(accounts)), page.Total) + return idsOfAccounts(accounts) + }) +} + // --- ListByGroup / ListActive / ListByPlatform --- func (s *AccountRepoSuite) TestListByGroup() { @@ -1838,3 +1849,33 @@ func idsOfAccounts(accounts []service.Account) []int64 { } return out } + +// accountSearchKeyDigestTerm is the search value the admin page sends in place +// of an API key. +func accountSearchKeyDigestTerm(key string) string { + sum := sha256.Sum256([]byte(key)) + return service.AccountSearchKeyDigestPrefix + hex.EncodeToString(sum[:]) +} + +// requireAccountSearchByAPIKeyDigest seeds accounts named with namePrefix and +// checks one account search path against them; search returns the matched IDs. +func requireAccountSearchByAPIKeyDigest(t *testing.T, client *dbent.Client, namePrefix string, search func(term string) []int64) { + t.Helper() + key := namePrefix + "key" + create := func(name string, credentials map[string]any) int64 { + return mustCreateAccount(t, client, &service.Account{ + Name: namePrefix + name, Type: service.AccountTypeAPIKey, Credentials: credentials, + }).ID + } + sharedA := create("shared-a", map[string]any{"api_key": key}) + sharedB := create("shared-b", map[string]any{"api_key": key}) + padded := create("padded", map[string]any{"api_key": "\t" + key + " \r\n"}) + // Named after the key, while its own key merely starts with it. + namedAfterKey := create("key-in-name", map[string]any{"api_key": key + "-other"}) + create("number", map[string]any{"api_key": 1234567890}) + create("missing", map[string]any{}) + + require.ElementsMatch(t, []int64{sharedA, sharedB, padded}, search(accountSearchKeyDigestTerm(key))) + require.Empty(t, search(accountSearchKeyDigestTerm("1234567890")), "a non-string api_key is not a key") + require.Equal(t, []int64{namedAfterKey}, search(key), "plain text matches names only") +} diff --git a/backend/internal/repository/account_taxonomy_integration_test.go b/backend/internal/repository/account_taxonomy_integration_test.go index f881adca5c14..f19edc74bddf 100644 --- a/backend/internal/repository/account_taxonomy_integration_test.go +++ b/backend/internal/repository/account_taxonomy_integration_test.go @@ -404,6 +404,19 @@ func TestAccountTaxonomyCombinedFiltersAndFacetsIntegration(t *testing.T) { require.Equal(t, 1, integrationTagCount(facets.Tags, tagBlue.ID)) } +func TestAccountTaxonomyConsoleSearchByAPIKeyDigestIntegration(t *testing.T) { + ctx := context.Background() + prefix := taxonomyIntegrationPrefix("key-search") + client, admin := newAccountTaxonomyIntegrationAdmin(t, prefix) + + requireAccountSearchByAPIKeyDigest(t, client, prefix, func(search string) []int64 { + accounts, total, err := admin.ListAccountsConsole(ctx, 1, 20, service.AccountConsoleFilters{Search: search}) + require.NoError(t, err) + require.Equal(t, int64(len(accounts)), total) + return idsOfAccounts(accounts) + }) +} + func TestAccountTaxonomyBulkUpdateAndOrderingIntegration(t *testing.T) { ctx := context.Background() prefix := taxonomyIntegrationPrefix("bulk") diff --git a/backend/internal/service/account_search_predicate.go b/backend/internal/service/account_search_predicate.go new file mode 100644 index 000000000000..b319a9d926dc --- /dev/null +++ b/backend/internal/service/account_search_predicate.go @@ -0,0 +1,64 @@ +package service + +import ( + "crypto/sha256" + "strings" + + dbaccount "github.com/Wei-Shaw/sub2api/ent/account" + dbpredicate "github.com/Wei-Shaw/sub2api/ent/predicate" + + entsql "entgo.io/ent/dialect/sql" +) + +// AccountSearchKeyDigestPrefix marks an account search value as an API key +// digest term: the prefix followed by the lowercase hex SHA-256 of the key's +// UTF-8 bytes. The admin page sends this term instead of the key, so the +// plaintext key never reaches the server. +const AccountSearchKeyDigestPrefix = "sha256:" + +// AccountSearchPredicate is the account search condition shared by every admin +// account query builder, so list, facets, export and bulk-by-filter select the +// same accounts. A key digest term matches the accounts whose +// credentials.api_key hashes to it and never matches names; any other value is +// a case-insensitive substring of the account name. +func AccountSearchPredicate(search string) dbpredicate.Account { + digest, ok := accountSearchKeyDigest(search) + if !ok { + return dbaccount.NameContainsFold(search) + } + return dbpredicate.Account(func(s *entsql.Selector) { + credentials := s.C(dbaccount.FieldCredentials) + s.Where(entsql.And( + // ->> renders any JSON value as text; only a JSON string is a key. + entsql.P(func(b *entsql.Builder) { + b.WriteString("jsonb_typeof("). + Ident(credentials). + WriteString(" -> 'api_key') = 'string'") + }), + // Stored keys are used with surrounding whitespace trimmed, so the + // digest is taken over the trimmed value as well. + entsql.P(func(b *entsql.Builder) { + b.WriteString("encode(sha256(convert_to(btrim("). + Ident(credentials). + WriteString(` ->> 'api_key', E' \t\n\r'), 'UTF8')), 'hex') = `). + Arg(digest) + }), + )) + }) +} + +// accountSearchKeyDigest returns the hex digest carried by a key digest term. +// Only the exact form is a digest term: the prefix followed by 64 lowercase +// hex characters and nothing else. +func accountSearchKeyDigest(search string) (string, bool) { + digest, ok := strings.CutPrefix(search, AccountSearchKeyDigestPrefix) + if !ok || len(digest) != 2*sha256.Size { + return "", false + } + for i := 0; i < len(digest); i++ { + if c := digest[i]; (c < '0' || c > '9') && (c < 'a' || c > 'f') { + return "", false + } + } + return digest, true +} diff --git a/backend/internal/service/account_search_predicate_test.go b/backend/internal/service/account_search_predicate_test.go new file mode 100644 index 000000000000..7bdc572879da --- /dev/null +++ b/backend/internal/service/account_search_predicate_test.go @@ -0,0 +1,70 @@ +package service + +import ( + "crypto/sha256" + "encoding/hex" + "strings" + "testing" + + "entgo.io/ent/dialect" + entsql "entgo.io/ent/dialect/sql" + dbaccount "github.com/Wei-Shaw/sub2api/ent/account" + dbpredicate "github.com/Wei-Shaw/sub2api/ent/predicate" + "github.com/stretchr/testify/require" +) + +// SHA-256 of the UTF-8 bytes of "sk-test-key", as the admin page computes it. +const accountSearchTestKeyDigest = "0d62f396c1317066f55a96086517047c737087c61eb2bf016b72e6298927b15b" + +func accountSearchPredicateSQL(predicates ...dbpredicate.Account) (string, []any) { + selector := entsql.Dialect(dialect.Postgres).Select().From(entsql.Table(dbaccount.Table)) + for _, predicate := range predicates { + predicate(selector) + } + return selector.Query() +} + +func TestAccountSearchPredicateKeyDigestTermMatchesAPIKeyDigestOnly(t *testing.T) { + sum := sha256.Sum256([]byte("sk-test-key")) + term := AccountSearchKeyDigestPrefix + hex.EncodeToString(sum[:]) + require.Equal(t, "sha256:"+accountSearchTestKeyDigest, term) + + const keyDigestSQL = `jsonb_typeof("accounts"."credentials" -> 'api_key') = 'string'` + + ` AND encode(sha256(convert_to(btrim("accounts"."credentials" ->> 'api_key', E' \t\n\r'), 'UTF8')), 'hex') = ` + + query, args := accountSearchPredicateSQL(AccountSearchPredicate(term)) + require.Equal(t, `SELECT * FROM "accounts" WHERE `+keyDigestSQL+`$1`, query) + require.Equal(t, []any{accountSearchTestKeyDigest}, args) + + // Next to another filter the condition stays one AND-ed unit and the digest + // takes the next placeholder. + query, args = accountSearchPredicateSQL(dbaccount.PlatformEQ(PlatformOpenAI), AccountSearchPredicate(term)) + require.Equal(t, `SELECT * FROM "accounts" WHERE "accounts"."platform" = $1 AND (`+keyDigestSQL+`$2)`, query) + require.Equal(t, []any{PlatformOpenAI, accountSearchTestKeyDigest}, args) +} + +func TestAccountSearchPredicateOtherTermsSearchNameOnly(t *testing.T) { + for _, tc := range []struct { + name string + search string + }{ + {"plain text", "Alpha"}, + {"plaintext key", "sk-test-key"}, + {"uppercase hex", AccountSearchKeyDigestPrefix + strings.ToUpper(accountSearchTestKeyDigest)}, + {"63 hex characters", AccountSearchKeyDigestPrefix + accountSearchTestKeyDigest[:63]}, + {"65 hex characters", AccountSearchKeyDigestPrefix + accountSearchTestKeyDigest + "0"}, + {"non-hex character", AccountSearchKeyDigestPrefix + accountSearchTestKeyDigest[:63] + "g"}, + {"uppercase prefix", "SHA256:" + accountSearchTestKeyDigest}, + {"leading text", "key " + AccountSearchKeyDigestPrefix + accountSearchTestKeyDigest}, + {"trailing text", AccountSearchKeyDigestPrefix + accountSearchTestKeyDigest + " key"}, + } { + t.Run(tc.name, func(t *testing.T) { + query, args := accountSearchPredicateSQL(AccountSearchPredicate(tc.search)) + require.Equal(t, `SELECT * FROM "accounts" WHERE "accounts"."name" ILIKE $1`, query) + + wantQuery, wantArgs := accountSearchPredicateSQL(dbaccount.NameContainsFold(tc.search)) + require.Equal(t, wantQuery, query) + require.Equal(t, wantArgs, args) + }) + } +} diff --git a/backend/internal/service/admin_account_taxonomy.go b/backend/internal/service/admin_account_taxonomy.go index 94e606f721d5..19494e79ec7f 100644 --- a/backend/internal/service/admin_account_taxonomy.go +++ b/backend/internal/service/admin_account_taxonomy.go @@ -698,7 +698,7 @@ func (s *adminServiceImpl) accountConsoleQuery(filters AccountConsoleFilters) *d query = query.Where(dbaccount.TypeIn(values...)) } if search := strings.TrimSpace(filters.Search); search != "" { - query = query.Where(dbaccount.NameContainsFold(search)) + query = query.Where(AccountSearchPredicate(search)) } if len(filters.AccountIDs) > 0 { query = query.Where(dbaccount.IDIn(uniquePositiveIDs(filters.AccountIDs)...)) diff --git a/frontend/src/api/admin/accounts.ts b/frontend/src/api/admin/accounts.ts index 7311573c9f05..f3edca31b891 100644 --- a/frontend/src/api/admin/accounts.ts +++ b/frontend/src/api/admin/accounts.ts @@ -92,6 +92,16 @@ export async function list( return data } +/** + * Check whether any account uses the API key behind a digest term + * @param digestTerm - "sha256:" search term of the key (utils/accountKeySearch); the key itself is not sent + * @returns Whether at least one account has that key, whatever the list is filtered by + */ +export async function hasAPIKeyDigest(digestTerm: string, view?: CapturedAccountView): Promise { + const { total } = await list(1, 1, { search: digestTerm, lite: '1' }, undefined, view) + return total > 0 +} + export interface AccountListWithEtagResult { notModified: boolean etag: string | null @@ -1392,6 +1402,7 @@ export async function refreshOpenCodeGoUsage(id: number): Promise> = { list: 4, + hasAPIKeyDigest: 1, getUpstreamBillingRatesWithEtag: 4, listWithEtag: 4, getById: 1, diff --git a/frontend/src/i18n/locales/en/admin/accounts.ts b/frontend/src/i18n/locales/en/admin/accounts.ts index 5461d6f5f1d3..b1e232d6cbff 100644 --- a/frontend/src/i18n/locales/en/admin/accounts.ts +++ b/frontend/src/i18n/locales/en/admin/accounts.ts @@ -204,7 +204,7 @@ export default {accounts: { crsBack: 'Back', editAccount: 'Edit Account', deleteAccount: 'Delete Account', - searchAccounts: 'Search accounts...', + searchAccounts: 'Search name or API key', notes: 'Notes', notesPlaceholder: 'Enter notes', notesHint: 'Notes are optional', diff --git a/frontend/src/i18n/locales/zh/admin/accounts.ts b/frontend/src/i18n/locales/zh/admin/accounts.ts index e5e5d0cc07fd..733b9b4b51c0 100644 --- a/frontend/src/i18n/locales/zh/admin/accounts.ts +++ b/frontend/src/i18n/locales/zh/admin/accounts.ts @@ -203,7 +203,7 @@ export default {accounts: { crsBack: '返回', editAccount: '编辑账号', deleteAccount: '删除账号', - searchAccounts: '搜索账号...', + searchAccounts: '搜索名称或 API Key', notes: '备注', notesPlaceholder: '请输入备注', notesHint: '备注可选', diff --git a/frontend/src/styles/console/pages/accounts.css b/frontend/src/styles/console/pages/accounts.css index 47abc2ac42d8..72f8358b1187 100644 --- a/frontend/src/styles/console/pages/accounts.css +++ b/frontend/src/styles/console/pages/accounts.css @@ -72,6 +72,12 @@ html.flat-theme [data-ui="accounts-toolbar"] > [data-ui="account-search"] { max-width: 288px; } +/* At its narrowest (about 155px at 1280) the search leaves its text 111px with the field's 10px right inset, and the + Chinese placeholder is 113px wide: 6px on the right keeps the placeholder whole there. */ +html.flat-theme [data-ui="accounts-toolbar"] > [data-ui="account-search"] > input.input[data-ui~="search-field"] { + padding-right: 0.375rem; +} + html.flat-theme [data-ui="accounts-toolbar"] > [data-ui="account-view-switcher"] { flex: none; } diff --git a/frontend/src/utils/__tests__/accountKeySearch.spec.ts b/frontend/src/utils/__tests__/accountKeySearch.spec.ts new file mode 100644 index 000000000000..3951af329dd7 --- /dev/null +++ b/frontend/src/utils/__tests__/accountKeySearch.spec.ts @@ -0,0 +1,30 @@ +import { webcrypto } from 'node:crypto' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { accountKeyDigestTerm, isAccountKeyDigestTerm } from '../accountKeySearch' + +afterEach(() => { vi.unstubAllGlobals() }) + +describe('account key search terms', () => { + it('builds the digest term of the trimmed text', async () => { + vi.stubGlobal('crypto', webcrypto) + // SHA-256("abc"), the FIPS 180 example + await expect(accountKeyDigestTerm(' \tabc\r\n')).resolves.toBe( + 'sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad' + ) + }) + + it('has no digest term for an empty text or without crypto.subtle', async () => { + vi.stubGlobal('crypto', webcrypto) + await expect(accountKeyDigestTerm(' ')).resolves.toBeNull() + vi.stubGlobal('crypto', {}) + await expect(accountKeyDigestTerm('abc')).resolves.toBeNull() + }) + + it('recognises only a complete lowercase digest term', () => { + const term = `sha256:${'0f'.repeat(32)}` + expect(isAccountKeyDigestTerm(term)).toBe(true) + for (const value of [term.toUpperCase(), `sha256:${'0F'.repeat(32)}`, term.slice(0, -1), `${term}0`, ` ${term}`, 'abc', '']) { + expect(isAccountKeyDigestTerm(value)).toBe(false) + } + }) +}) diff --git a/frontend/src/utils/accountKeySearch.ts b/frontend/src/utils/accountKeySearch.ts new file mode 100644 index 000000000000..5ccb0e8ef8cb --- /dev/null +++ b/frontend/src/utils/accountKeySearch.ts @@ -0,0 +1,19 @@ +// The account search names an upstream API key by a digest term instead of carrying the key: this prefix followed by +// the lowercase hex SHA-256 of the key's UTF-8 bytes. +export const ACCOUNT_KEY_DIGEST_PREFIX = 'sha256:' + +const ACCOUNT_KEY_DIGEST_TERM = /^sha256:[0-9a-f]{64}$/ + +export function isAccountKeyDigestTerm(value: string): boolean { + return ACCOUNT_KEY_DIGEST_TERM.test(value) +} + +// Digest term of a search text, surrounding whitespace ignored. Null for an empty text and where crypto.subtle is +// unavailable (insecure context): the text can then only be searched as a name. +export async function accountKeyDigestTerm(text: string): Promise { + const trimmed = text.trim() + const subtle = globalThis.crypto?.subtle + if (!trimmed || !subtle) return null + const digest = await subtle.digest('SHA-256', new TextEncoder().encode(trimmed)) + return ACCOUNT_KEY_DIGEST_PREFIX + Array.from(new Uint8Array(digest), n => n.toString(16).padStart(2, '0')).join('') +} diff --git a/frontend/src/views/admin/AccountsView.vue b/frontend/src/views/admin/AccountsView.vue index 0b8e4ae25daf..232952988a71 100644 --- a/frontend/src/views/admin/AccountsView.vue +++ b/frontend/src/views/admin/AccountsView.vue @@ -767,6 +767,7 @@ import { getAccountPlanType } from '@/utils/accountPresentation' import Icon from '@/components/icons/Icon.vue' import ErrorPassthroughRulesModal from '@/components/admin/ErrorPassthroughRulesModal.vue' import TLSFingerprintProfilesModal from '@/components/admin/TLSFingerprintProfilesModal.vue' +import { accountKeyDigestTerm, isAccountKeyDigestTerm } from '@/utils/accountKeySearch' import { fetchAllAccountIds } from '@/utils/accountSelection' import { buildGrokUsageRefreshKey, buildOpenAIUsageRefreshKey } from '@/utils/accountUsageRefresh' import { formatDateTime, formatRelativeTime } from '@/utils/format' @@ -872,6 +873,14 @@ const consoleFilters = ref({ privacy_mode: queryString('privacy_mode'), account_ids: initialSensitiveConsoleState.account_ids }) +// What requests carry as "search" for the text in the search box: the trimmed text, or its digest term when the text +// is exactly the API key of some account, so that the key itself is never sent. `text` is the text `value` was +// resolved for. +let resolvedSearch = { text: '', value: '' } +type ConsoleSearchResolution = 'resolved' | 'failed' | 'superseded' +type ConsoleSearchLookup = { text: string; result: Promise } +// The lookup in flight, if any. Its result is dropped once another one, or none, stands here. +let pendingSearchLookup: ConsoleSearchLookup | null = null const folders = ref([]) const tags = ref([]) const facetFolders = computed(() => facets.value?.folders || []) @@ -1539,7 +1548,7 @@ const buildConsoleAPIParams = (includeFolder = true) => { account_ids: state.account_ids.length ? state.account_ids.join(',') : undefined, group_id: state.group_id || undefined, privacy_mode: state.privacy_mode || undefined, - search: state.search.trim() || undefined, + search: resolvedSearch.value || undefined, sort_by: sortState.sort_by, sort_order: sortState.sort_order } @@ -1605,15 +1614,93 @@ const loadTaxonomy = async () => { } } -const handleConsoleFiltersChanged = () => { +const lookUpConsoleSearch = async (lookup: ConsoleSearchLookup): Promise => { + const { text } = lookup + let value = text + let failure: unknown = null + try { + const digestTerm = await accountKeyDigestTerm(text) + if (digestTerm && await adminAPI.accounts.hasAPIKeyDigest(digestTerm)) value = digestTerm + } catch (error) { + failure = error ?? new Error('account search lookup failed') + } + if (pendingSearchLookup !== lookup) return 'superseded' + pendingSearchLookup = null + if (!failure) { + resolvedSearch = { text, value } + return 'resolved' + } + console.error('Failed to resolve the account search:', failure) + // A lookup repeated for the applied text fails quietly: requests keep what it resolved to before. + if (text !== resolvedSearch.text) { + appStore.showError(extractApiErrorMessage(failure, t('admin.accounts.failedToLoad'))) + // The text cannot be applied, and sending it as it is could send a key: the box goes back to the search the + // list shows, unless it was edited in the meantime. + if (consoleFilters.value.search.trim() === text) { + consoleFilters.value = { ...consoleFilters.value, search: resolvedSearch.text } + saveSensitiveConsoleState() + } + } + return 'failed' +} + +// Brings resolvedSearch in line with the search box. The answer is immediate when there is nothing to look up (an +// empty text, or a text already resolved unless `force` asks again); otherwise it is the outcome of the lookup. A +// lookup of the same text already in flight answers for this call as well; one of another text is dropped. Until a +// lookup succeeds, requests keep the value resolved before it: the text is never sent in its place. +const resolveConsoleSearch = (force = false): 'resolved' | Promise => { + const text = consoleFilters.value.search.trim() + if (pendingSearchLookup?.text === text) return pendingSearchLookup.result + pendingSearchLookup = null + if (!text) { + resolvedSearch = { text, value: text } + return 'resolved' + } + if (!force && text === resolvedSearch.text) return 'resolved' + const lookup = { text } as ConsoleSearchLookup + pendingSearchLookup = lookup + lookup.result = lookUpConsoleSearch(lookup) + return lookup.result +} + +// Looks an applied name search up again: accounts added or changed since may use that text as their key. A key +// search, or none, stays what it is. False when a newer resolution took over, which then applies the filters itself. +const refreshConsoleSearch = async (): Promise => { + const searchBefore = resolvedSearch.value + const resolution = resolveConsoleSearch(Boolean(searchBefore) && !isAccountKeyDigestTerm(searchBefore)) + if (resolution !== 'resolved' && await resolution === 'superseded') return false + if (resolvedSearch.value !== searchBefore) { + // The search now selects other accounts: start again from their first page, facets included. + pagination.page = 1 + clearSelection() + syncConsoleRoute() + void loadFacets() + } + return true +} + +const applyConsoleFilters = () => { pagination.page = 1 - saveSensitiveConsoleState() syncConsoleRoute() syncConsoleParams() debouncedReload() void loadFacets() } +const handleConsoleFiltersChanged = () => { + saveSensitiveConsoleState() + const resolution = resolveConsoleSearch() + if (resolution === 'resolved') { + applyConsoleFilters() + return + } + // A changed search text is applied once it is looked up. If that fails, the box is back at the applied search and + // the other filters are applied with it. + void resolution.then((outcome) => { + if (outcome !== 'superseded') applyConsoleFilters() + }) +} + const handleFolderSelect = (folder: string) => { if (activeFolder.value === folder) return activeFolder.value = folder @@ -2038,6 +2125,7 @@ const refreshAccountsIncrementally = async () => { } const handleManualRefresh = async () => { + if (!await refreshConsoleSearch()) return await Promise.all([load(), loadUpstreamBillingProbeGlobalState()]) // Force usage cells to refetch /usage on explicit user refresh. usageManualRefreshToken.value += 1 @@ -2571,6 +2659,12 @@ const selectImportedResults = async (jobIDs = completedImportJobIDs.value, expec } } +// Reloads the list, and the facets when asked, after this page added or changed accounts. That can turn the applied +// name search into a key search, so the search is looked up again before anything is requested with it. +const reloadChangedAccounts = async (run: () => Promise = load, withFacets = false) => { + if (await refreshConsoleSearch()) await Promise.all([run(), ...(withFacets ? [loadFacets()] : [])]) +} + const refreshCompletedImports = async () => { if (importCompletionRefreshRunning) { importCompletionRefreshQueued = true @@ -2583,7 +2677,8 @@ const refreshCompletedImports = async () => { const completed = new Map(completedImportQueue) completedImportQueue.clear() completedImportJobIDs.value = [...completed.keys()] - await Promise.all([load(), loadFacets(), loadTaxonomy()]) + const searchCurrent = await refreshConsoleSearch() + await Promise.all([...(searchCurrent ? [load(), loadFacets()] : []), loadTaxonomy()]) const selectable = [...completed].filter(([, revision]) => revision === importSelectionRevision).map(([id]) => id) if (selectable.length) await selectImportedResults(selectable) } while (importCompletionRefreshQueued) @@ -2603,8 +2698,7 @@ watch( if (isTerminalAccountJob(job) && accountJobsStore.completedJobs?.some(done => done.id === job.id) && !refreshedOperations.has(job.id)) { refreshedOperations.add(job.id) if (!pendingDataImportJobIDs.has(job.id) && job.kind !== 'account_duplicate_review') { - void load() - if (['account_bulk_taxonomy', 'account_bulk_update', 'account_batch_delete', 'account_duplicate_merge'].includes(job.kind)) void loadFacets() + void reloadChangedAccounts(load, ['account_bulk_taxonomy', 'account_bulk_update', 'account_batch_delete', 'account_duplicate_merge'].includes(job.kind)) } } const revision = pendingDataImportJobIDs.get(job.id) @@ -2621,7 +2715,7 @@ const handleAccountCreated = (job?: AccountJob) => { accountJobsStore.track(job, { open: false }) return } - void reload() + void reloadChangedAccounts(reload) } const ACCOUNT_UNGROUPED_GROUP_QUERY_VALUE = 'ungrouped' const ACCOUNT_PRIVACY_MODE_UNSET_QUERY_VALUE = '__unset__' @@ -2673,7 +2767,8 @@ const accountMatchesCurrentFilters = (account: Account) => { return false } } - const search = state.search.trim().toLowerCase() + // An account found by its API key is not checked by name: the key cannot be compared here. + const search = isAccountKeyDigestTerm(resolvedSearch.value) ? '' : resolvedSearch.value.toLowerCase() if (search && !account.name.toLowerCase().includes(search)) return false return true } @@ -3077,7 +3172,11 @@ onMounted(async () => { } } - await Promise.all([load(), loadFacets(), loadTaxonomy(), loadAPIKeyVisibility()]) + // A search text restored from the session is resolved before the first requests can carry it. If that fails, the + // box is empty again and the list loads without a search; a newer resolution loads the list itself. + const searchResolution = resolveConsoleSearch() + const searchOutcome = searchResolution === 'resolved' ? searchResolution : await searchResolution + await Promise.all([...(searchOutcome === 'superseded' ? [] : [load(), loadFacets()]), loadTaxonomy(), loadAPIKeyVisibility()]) loadUpstreamBillingProbeGlobalState() const [proxiesResult, groupsResult] = await Promise.allSettled([ adminAPI.proxies.getAll(), @@ -3106,6 +3205,8 @@ onMounted(async () => { }) onUnmounted(() => { + // A search lookup still in flight has nothing left to apply to. + pendingSearchLookup = null upstreamBillingRateAbortController?.abort() if (typeof document !== 'undefined' && document.fonts) { document.fonts.removeEventListener('loadingdone', remeasureAccountNames) diff --git a/frontend/src/views/admin/__tests__/AccountsView.console.spec.ts b/frontend/src/views/admin/__tests__/AccountsView.console.spec.ts index 92f17edef205..f90530122f23 100644 --- a/frontend/src/views/admin/__tests__/AccountsView.console.spec.ts +++ b/frontend/src/views/admin/__tests__/AccountsView.console.spec.ts @@ -1,13 +1,16 @@ vi.mock('@/components/admin/account-jobs/AccountOperationDialog.vue', () => ({ default: { name: 'AccountOperationDialog', props: ['job', 'show'], template: '
' } })) +import { createHash } from 'node:crypto' import { beforeEach, describe, expect, it, vi } from 'vitest' import { flushPromises, mount } from '@vue/test-utils' import { createMemoryHistory, createRouter } from 'vue-router' import AccountsView from '../AccountsView.vue' +import { adminAPI } from '@/api/admin' import { flattenStackedColumns } from '@/components/common/columnStack' import modelDisplayContract from '../../../../../backend/internal/service/testdata/account_available_models_contract.json' const { listAccounts, + hasAPIKeyDigest, getById, getAvailableModels, listWithEtag, @@ -16,6 +19,7 @@ const { listTags, getBatchTodayStats, getUpstreamBillingProbeSettings, + showError, showSuccess, jobTrack, reviewDuplicates, @@ -25,6 +29,7 @@ const { accountJobsState } = vi.hoisted(() => ({ listAccounts: vi.fn(), + hasAPIKeyDigest: vi.fn(), getById: vi.fn(), getAvailableModels: vi.fn(), listWithEtag: vi.fn(), @@ -33,6 +38,7 @@ const { listTags: vi.fn(), getBatchTodayStats: vi.fn(), getUpstreamBillingProbeSettings: vi.fn(), + showError: vi.fn(), showSuccess: vi.fn(), jobTrack: vi.fn(), reviewDuplicates: vi.fn(), @@ -62,6 +68,7 @@ vi.mock('@/api/admin', () => ({ adminAPI: { accounts: { list: listAccounts, + hasAPIKeyDigest, getById, getAPIKeyVisibility: vi.fn().mockResolvedValue({ enabled: false }), getAvailableModels, @@ -84,7 +91,7 @@ vi.mock('@/api/admin', () => ({ vi.mock('@/stores/app', () => ({ useAppStore: () => ({ - showError: vi.fn(), + showError, showSuccess, showInfo: vi.fn(), showWarning: vi.fn() @@ -175,6 +182,12 @@ const ImportDataModalStub = { template: '' } +const ConsoleFiltersStub = { + props: ['modelValue'], + emits: ['update:modelValue', 'change'], + template: '
{{ modelValue.account_ids.join(\',\') }}
' +} + const FolderBarStub = { props: ['folders', 'total'], template: '
{{ folders[0]?.account_count ?? -1 }}{{ total }}
' @@ -205,7 +218,7 @@ const commonStubs = { template: '
{{ accounts.length }}
' }, AccountViewModeSwitcher: ViewModeStub, - AccountConsoleFilters: { props: ['modelValue'], template: '
{{ modelValue.account_ids.join(\',\') }}
' }, + AccountConsoleFilters: ConsoleFiltersStub, AccountFolderBar: FolderBarStub, AccountTableActions: { emits: ['refresh'], @@ -256,6 +269,32 @@ const mountView = ( global: { stubs: commonStubs, plugins } }) +// what the search box does: the text reaches the page at once, the change once typing pauses +const typeSearch = (wrapper: ReturnType, text: string) => { + const filters = wrapper.findComponent(ConsoleFiltersStub) + filters.vm.$emit('update:modelValue', { ...filters.props('modelValue'), search: text }) + filters.vm.$emit('change') +} + +const keyDigestTerm = (text: string) => `sha256:${createHash('sha256').update(text).digest('hex')}` + +// the list reloads a debounce after the search is resolved +const SEARCH_RELOAD_WAIT = { timeout: 3000 } + +// useTableLoader hands every list request the same params object, so the search of a request is copied as it is made +const recordListSearches = () => { + const searches: unknown[] = [] + listAccounts.mockImplementation(async (_page: number, _pageSize: number, filters?: { search?: string }) => { + searches.push(filters?.search) + return { items: [account], total: 1, page: 1, page_size: 20, pages: 1 } + }) + return searches +} + +// every argument of every request the page made through the accounts API +const accountRequestArguments = () => + JSON.stringify(Object.values(adminAPI.accounts).map(request => vi.mocked(request).mock.calls)) + describe('admin AccountsView Cockpit console', () => { it('preserves the submitted bulk-edit selection in the original window after clearing page selection', async () => { const wrapper = mountView() @@ -278,6 +317,8 @@ describe('admin AccountsView Cockpit console', () => { sessionStorage.clear() listAccounts.mockReset().mockResolvedValue({ items: [account], total: 1, page: 1, page_size: 20, pages: 1 }) + // by default a search text is not the API key of any account + hasAPIKeyDigest.mockReset().mockResolvedValue(false) getById.mockReset().mockResolvedValue(account) getAvailableModels.mockReset().mockResolvedValue(structuredClone(modelDisplayContract.expected)) listWithEtag.mockReset().mockResolvedValue({ notModified: true, etag: null, data: null }) @@ -286,6 +327,7 @@ describe('admin AccountsView Cockpit console', () => { listTags.mockReset().mockResolvedValue([]) getBatchTodayStats.mockReset().mockResolvedValue({ stats: {} }) getUpstreamBillingProbeSettings.mockReset().mockResolvedValue({ enabled: true, interval_minutes: 30 }) + showError.mockReset() showSuccess.mockReset() jobTrack.mockReset() reviewDuplicates.mockReset() @@ -530,11 +572,14 @@ describe('admin AccountsView Cockpit console', () => { await router.push('/admin/accounts?folder=7&statuses=active&group_id=ungrouped&sort_by=status&sort_order=desc&page=2&page_size=50') await router.isReady() const wrapper = mountView([router]) - await flushPromises() - expect(listAccounts).toHaveBeenCalledWith(2, 50, expect.objectContaining({ - folder: '7', statuses: 'active', group_id: 'ungrouped', search: 'private search', account_ids: '1', sort_by: 'status', sort_order: 'desc' - }), expect.any(Object)) + // the restored search text is looked up before the first list request + await vi.waitFor(() => { + expect(listAccounts).toHaveBeenCalledWith(2, 50, expect.objectContaining({ + folder: '7', statuses: 'active', group_id: 'ungrouped', search: 'private search', account_ids: '1', sort_by: 'status', sort_order: 'desc' + }), expect.any(Object)) + }) + await flushPromises() expect(router.currentRoute.value.query.search).toBeUndefined() expect(router.currentRoute.value.query.account_ids).toBeUndefined() @@ -549,6 +594,152 @@ describe('admin AccountsView Cockpit console', () => { wrapper.unmount() }) + it('searches by name when the text is not the API key of any account', async () => { + const wrapper = mountView() + await flushPromises() + const searches = recordListSearches() + + typeSearch(wrapper, ' console ') + await vi.waitFor(() => expect(searches).toEqual(['console']), SEARCH_RELOAD_WAIT) + await flushPromises() + expect(hasAPIKeyDigest.mock.calls).toEqual([[keyDigestTerm('console')]]) + expect(getFacets).toHaveBeenLastCalledWith(expect.objectContaining({ search: 'console' })) + wrapper.unmount() + }) + + it('searches by digest term when the text is the API key of an account and never sends the key', async () => { + const key = 'sk-console-upstream-key' + hasAPIKeyDigest.mockResolvedValue(true) + const wrapper = mountView() + await flushPromises() + const searches = recordListSearches() + + typeSearch(wrapper, ` ${key} `) + await vi.waitFor(() => expect(searches).toEqual([keyDigestTerm(key)]), SEARCH_RELOAD_WAIT) + await flushPromises() + expect(getFacets).toHaveBeenLastCalledWith(expect.objectContaining({ search: keyDigestTerm(key) })) + + // found by its key, the account stays listed after an edit although its name does not contain the text + const vm = wrapper.vm as any + vm.handleAccountUpdated({ ...account, priority: 3 }) + expect(vm.accounts).toHaveLength(1) + expect(accountRequestArguments()).not.toContain(key) + wrapper.unmount() + }) + + it('resolves a restored search text before the first list and facets requests', async () => { + const key = 'sk-restored-upstream-key' + sessionStorage.setItem('account-console-sensitive-filters-v1', JSON.stringify({ search: key, account_ids: [] })) + let answerLookup!: (inUse: boolean) => void + hasAPIKeyDigest.mockReturnValue(new Promise((resolve) => { answerLookup = resolve })) + + const wrapper = mountView() + await vi.waitFor(() => expect(hasAPIKeyDigest).toHaveBeenCalledWith(keyDigestTerm(key))) + expect(listAccounts).not.toHaveBeenCalled() + expect(getFacets).not.toHaveBeenCalled() + + answerLookup(true) + await flushPromises() + expect(listAccounts).toHaveBeenCalledWith(1, expect.any(Number), expect.objectContaining({ search: keyDigestTerm(key) }), expect.any(Object)) + expect(getFacets).toHaveBeenCalledWith(expect.objectContaining({ search: keyDigestTerm(key) })) + expect(accountRequestArguments()).not.toContain(key) + wrapper.unmount() + }) + + it('reports a failed key lookup, puts the search box back and never sends the text', async () => { + const text = 'sk-unresolved-upstream-key' + const wrapper = mountView() + await flushPromises() + hasAPIKeyDigest.mockRejectedValue({ status: 503 }) + const searches = recordListSearches() + + typeSearch(wrapper, text) + await vi.waitFor(() => expect(showError).toHaveBeenCalledWith('admin.accounts.failedToLoad')) + // the filters are applied with the search the list had before, which the box shows again + await vi.waitFor(() => expect(searches).toEqual([undefined]), SEARCH_RELOAD_WAIT) + await flushPromises() + expect(wrapper.findComponent(ConsoleFiltersStub).props('modelValue').search).toBe('') + expect(JSON.parse(sessionStorage.getItem('account-console-sensitive-filters-v1') || '{}').search).toBe('') + expect(accountRequestArguments()).not.toContain(text) + wrapper.unmount() + }) + + it('looks a name search up again once the page adds an account, which may use that text as its key', async () => { + const key = 'sk-added-upstream-key' + const wrapper = mountView() + await flushPromises() + const searches = recordListSearches() + typeSearch(wrapper, key) + await vi.waitFor(() => expect(searches).toEqual([key]), SEARCH_RELOAD_WAIT) + await flushPromises() + + hasAPIKeyDigest.mockResolvedValue(true) + ;(wrapper.vm as any).handleAccountCreated() + await vi.waitFor(() => expect(searches).toEqual([key, keyDigestTerm(key)])) + await flushPromises() + expect(getFacets).toHaveBeenLastCalledWith(expect.objectContaining({ search: keyDigestTerm(key) })) + wrapper.unmount() + }) + + it('looks a name search up again before the list and facets reload after an import', async () => { + const key = 'sk-imported-upstream-key' + const wrapper = mountView() + await flushPromises() + const searches = recordListSearches() + typeSearch(wrapper, key) + await vi.waitFor(() => expect(searches).toEqual([key]), SEARCH_RELOAD_WAIT) + await flushPromises() + + // the import adds an account that uses the text as its key; the lookup has no answer yet + let answerLookup!: (inUse: boolean) => void + hasAPIKeyDigest.mockReturnValue(new Promise((resolve) => { answerLookup = resolve })) + searches.length = 0 + getFacets.mockClear() + await wrapper.get('[data-test="emit-import-result"]').trigger('click') + accountJobsState.store.recentJobs.push({ id: 71, status: 'succeeded' }) + await vi.waitFor(() => expect(hasAPIKeyDigest).toHaveBeenLastCalledWith(keyDigestTerm(key))) + await flushPromises() + expect(searches).toEqual([]) + expect(getFacets).not.toHaveBeenCalled() + + answerLookup(true) + await vi.waitFor(() => expect(searches).toEqual([keyDigestTerm(key)])) + await flushPromises() + expect(getFacets).toHaveBeenCalled() + expect(getFacets.mock.calls.map(([filters]) => filters.search)).not.toContain(key) + wrapper.unmount() + }) + + it('lets a filter change wait for a lookup in flight instead of sending the text it is checking', async () => { + const key = 'sk-refreshed-upstream-key' + const wrapper = mountView() + await flushPromises() + const searches = recordListSearches() + typeSearch(wrapper, key) + await vi.waitFor(() => expect(searches).toEqual([key]), SEARCH_RELOAD_WAIT) + await flushPromises() + + // an account now uses the text as its key; the refresh asks again and has no answer yet + let answerLookup!: (inUse: boolean) => void + hasAPIKeyDigest.mockReturnValue(new Promise((resolve) => { answerLookup = resolve })) + searches.length = 0 + getFacets.mockClear() + await wrapper.get('[data-test="page-refresh"]').trigger('click') + const filters = wrapper.findComponent(ConsoleFiltersStub) + filters.vm.$emit('update:modelValue', { ...filters.props('modelValue'), statuses: ['active'] }) + filters.vm.$emit('change') + await flushPromises() + expect(searches).toEqual([]) + expect(getFacets).not.toHaveBeenCalled() + + answerLookup(true) + // the refresh loads at once, the filter change a debounce later + await vi.waitFor(() => expect(searches).toEqual([keyDigestTerm(key), keyDigestTerm(key)]), SEARCH_RELOAD_WAIT) + await flushPromises() + expect(getFacets).toHaveBeenLastCalledWith(expect.objectContaining({ search: keyDigestTerm(key), statuses: 'active' })) + wrapper.unmount() + }) + it('preserves the account list and performs no reads when account testing closes', async () => { const router = createRouter({ history: createMemoryHistory(),