fix: error "belongs" in case of a series of accounts deletion
Massimo Melina committed
Mar 12, 2025 at 11:28 UTC
7be159a7994b7596a3c41d915941edbca58574b6
6 files changed
+41
-40
admin/src/AccountForm.ts
+2
-1
@@ -89,7 +89,8 @@ export default function AccountForm({ account, done, groups, addToBar, reload }:
89
h(Btn, {
90
icon: Delete,
91
confirm: `Delete ${account.members.length} accounts?`,
92
- onClick: () => Promise.all(account.members.map(u => apiCall('del_account', { username: u }))).finally(reload)
92
+ onClick: () => apiCall('del_account', { username: account.members }).then(reload),
93
+ sx: { verticalAlign: 'text-top' }
94
}),
95
),
96
isGroup && h(Alert, { severity: 'info' }, `To add users to this group, select the user and then click "Inherit"`),
src/adminApis.ts
+2
-2
@@ -17,7 +17,7 @@ import logApis from './api.log'
17
import certApis from './api.cert'
18
import { getConnections } from './connections'
19
import { apiAssertTypes, debounceAsync, isLocalHost, makeNetMatcher, typedEntries, waitFor } from './misc'
20
-import { accountCanLoginAdmin, accountsConfig } from './perm'
20
+import { accountCanLoginAdmin, accounts } from './perm'
21
import Koa from 'koa'
22
import { cloudflareDetected, getProxyDetected } from './middlewares'
23
import { execFile } from 'child_process'
@@ -205,7 +205,7 @@ const frpDebounced = debounceAsync(async () => {
205
}, { retain: 10_000 })
206
207
export function anyAccountCanLoginAdmin() {
208
- return Boolean(_.find(accountsConfig.get(), accountCanLoginAdmin))
208
+ return Boolean(_.find(accounts.get(), accountCanLoginAdmin))
209
}
210
211
export function allowAdmin(ctx: Koa.Context) {
src/api.accounts.ts
+13
-9
@@ -2,13 +2,13 @@
2
3
import { ApiError, ApiHandlers } from './apiMiddleware'
4
import {
5
- Account, accountCanLoginAdmin, accountHasPassword, accountsConfig, addAccount, delAccount, getAccount,
5
+ Account, accountCanLoginAdmin, accountHasPassword, accounts, addAccount, delAccount, getAccount,
6
changeSrpHelper, updateAccount, accountCanLogin
7
} from './perm'
8
import _ from 'lodash'
9
import { HTTP_BAD_REQUEST, HTTP_CONFLICT, HTTP_NOT_FOUND } from './const'
10
import { getCurrentUsername, invalidateSessionBefore } from './auth'
11
-import { apiAssertTypes, onlyTruthy, with_ } from './misc'
11
+import { apiAssertTypes, objFromKeys, onlyTruthy, with_ } from './misc'
12
13
function prepareAccount(ac: Account | undefined) {
14
return ac && {
@@ -19,8 +19,8 @@ function prepareAccount(ac: Account | undefined) {
19
adminActualAccess: accountCanLoginAdmin(ac),
20
canLogin: accountHasPassword(ac) ? accountCanLogin(ac) : undefined,
21
invalidated: invalidateSessionBefore.get(ac.username),
22
- directMembers: Object.values(accountsConfig.get()).filter(a => a.belongs?.includes(ac.username)).map(x => x.username),
23
- members: with_(Object.values(accountsConfig.get()), accounts => {
22
+ directMembers: Object.values(accounts.get()).filter(a => a.belongs?.includes(ac.username)).map(x => x.username),
23
+ members: with_(Object.values(accounts.get()), accounts => {
24
const ret = []
25
let news = [ac.username]
26
while (news.length) {
@@ -35,7 +35,7 @@ function prepareAccount(ac: Account | undefined) {
35
export default {
36
37
get_usernames() {
38
- return { list: Object.keys(accountsConfig.get()) }
38
+ return { list: Object.keys(accounts.get()) }
39
},
40
41
get_account({ username }, ctx) {
@@ -44,11 +44,11 @@ export default {
44
},
45
46
get_accounts() {
47
- return { list: onlyTruthy(Object.values(accountsConfig.get()).map(prepareAccount)) }
47
+ return { list: onlyTruthy(Object.values(accounts.get()).map(prepareAccount)) }
48
},
49
50
get_admins() {
51
- return { list: _.filter(accountsConfig.get(), accountCanLoginAdmin).map(ac => ac.username) }
51
+ return { list: _.filter(accounts.get(), accountCanLoginAdmin).map(ac => ac.username) }
52
},
53
54
async set_account({ username, changes }, ctx) {
@@ -75,8 +75,12 @@ export default {
75
},
76
77
del_account({ username }) {
78
- apiAssertTypes({ string: { username } })
79
- return delAccount(username) ? {} : new ApiError(HTTP_BAD_REQUEST)
78
+ apiAssertTypes({ string_array: { username } })
79
+ if (Array.isArray(username)) {
80
+ const errors = objFromKeys(username, u => delAccount(u) ? undefined : HTTP_NOT_FOUND)
81
+ return _.isEmpty(errors) ? {} : { errors }
82
+ }
83
+ return delAccount(username) ? {} : new ApiError(HTTP_NOT_FOUND)
84
},
85
86
invalidate_sessions({ username }) {
src/github.ts
+1
-1
@@ -232,7 +232,7 @@ export async function searchPlugins(text='', { skipRepos=[''] }={}) {
232
}
233
234
export const alerts = storedMap.singleSync<string[]>('alerts', [])
235
-const cachedCentralInfo = storedMap.singleSync('cachedCentralInfo', '')
235
+const cachedCentralInfo = storedMap.singleSync('cachedCentralInfo', '') // persisting it could also be useful for no-internet instances, so that you can provide a fresher copy
236
export let blacklistedInstalledPlugins: string[] = []
237
// centralized hosted information, to be used as little as possible
238
const FN = 'central.json'
src/perm.ts
+22
-26
@@ -29,8 +29,6 @@ export interface Account {
29
}
30
interface Accounts { [username:string]: Account }
31
32
-let accounts: Accounts = {}
33
-
32
// provides the username and all other usernames it inherits based on the 'belongs' attribute. Useful to check permissions
33
export function expandUsername(who: string): string[] {
34
const ret = []
@@ -52,13 +50,13 @@ export function ctxBelongsTo(ctx: Koa.Context, usernames: string[]) {
50
}
51
52
export function getUsernames() {
55
- return Object.keys(accounts)
53
+ return Object.keys(accounts.get())
54
}
55
56
export function getAccount(username:string, normalize=true) : Account | undefined {
57
if (normalize)
58
username = normalizeUsername(username)
61
- return username ? accounts[username] : undefined
59
+ return username ? accounts.get()[username] : undefined
60
}
61
62
export function saveSrpInfo(account:Account, salt:string | bigint, verifier: string | bigint) {
@@ -100,7 +98,7 @@ export async function updateAccount(account: Account, change: Partial<Account> |
98
if (account.belongs) {
99
account.belongs = wantArray(account.belongs)
100
_.remove(account.belongs, b => {
103
- if (accounts.hasOwnProperty(b)) return
101
+ if (accounts.get().hasOwnProperty(b)) return
102
console.error(`account ${username} belongs to non-existing ${b}`)
103
return true
104
})
@@ -116,10 +114,10 @@ export async function updateAccount(account: Account, change: Partial<Account> |
114
115
const saveAccountsAsap = saveConfigAsap
116
119
-export const accountsConfig = defineConfig('accounts', {} as Accounts)
120
-accountsConfig.sub(obj => {
121
- // consider some validation here
122
- _.each(accounts = obj, (rec,k) => {
117
+export const accounts = defineConfig('accounts', {} as Accounts)
118
+accounts.sub(_.debounce(obj => {
119
+ // consider some validation here, in case of manual edit of the config
120
+ _.each(obj, (rec,k) => {
121
const norm = normalizeUsername(k)
122
if (rec?.username !== norm) {
123
if (!rec) // an empty object in yaml is parsed as null
@@ -130,7 +128,7 @@ accountsConfig.sub(obj => {
128
}
129
void updateAccount(rec, {}) // work fields
130
})
133
-})
131
+})) // don't trigger in the middle of a series of deletion, as we may have an inconsistent state
132
133
export function normalizeUsername(username: string) {
134
return username.toLocaleLowerCase()
@@ -138,25 +136,24 @@ export function normalizeUsername(username: string) {
136
137
export function renameAccount(from: string, to: string) {
138
from = normalizeUsername(from)
139
+ const as = accounts.get()
140
to = normalizeUsername(to)
142
- if (!to || !accounts[from] || accounts[to])
141
+ if (!to || !as[from] || as[to])
142
return false
143
if (to === from)
144
return true
146
- objRenameKey(accounts, from, to)
147
- updateReferences()
145
+ objRenameKey(as, from, to)
146
+ setHidden(as[to], { username: to })
147
+ // update references
148
+ for (const a of Object.values(as)) {
149
+ const idx = a.belongs?.indexOf(from)
150
+ if (idx !== undefined && idx >= 0)
151
+ a.belongs![idx] = to
152
+ }
153
+ accounts.set(as)
154
+ events.emit('accountRenamed', from, to) // everybody, take care of your stuff
155
saveAccountsAsap()
156
return true
150
-
151
- function updateReferences() {
152
- setHidden(accounts[to], { username: to })
153
- for (const a of Object.values(accounts)) {
154
- const idx = a.belongs?.indexOf(from)
155
- if (idx !== undefined && idx >= 0)
156
- a.belongs![idx] = to
157
- }
158
- events.emit('accountRenamed', from, to) // everybody, take care of your stuff
159
- }
157
}
158
159
export function addAccount(username: string, props: Partial<Account>, updateExisting=false) {
@@ -166,7 +163,7 @@ export function addAccount(username: string, props: Partial<Account>, updateExis
163
if (account && !updateExisting) return
164
account = setHidden(account || {}, { username }) // hidden so that stringification won't include it
165
Object.assign(account, _.pickBy(props, Boolean))
169
- accountsConfig.set(accounts =>
166
+ accounts.set(accounts =>
167
Object.assign(accounts, { [username]: account }))
168
return updateAccount(account, account).then(() => account!)
169
}
@@ -174,8 +171,7 @@ export function addAccount(username: string, props: Partial<Account>, updateExis
171
export function delAccount(username: string) {
172
if (!getAccount(username))
173
return false
177
- accountsConfig.set(accounts =>
178
- _.omit(accounts, normalizeUsername(username)) )
174
+ accounts.set(x => _.omit(x, normalizeUsername(username)) )
175
saveAccountsAsap()
176
return true
177
}
tests/test.ts
+1
-1
@@ -173,7 +173,7 @@ describe('accounts', () => {
173
before(() => login(username))
174
it('get_accounts', reqApi('get_accounts', {}, ({ list }) => _.find(list, { username }) && _.find(list, { username: 'admins' })))
175
const add = 'test-Add'
176
- it('accounts.add', reqApi('add_account', { username: add }, res => res?.username === add.toLowerCase()))
176
+ it('accounts.add', reqApi('add_account', { username: add, overwrite: true }, res => res?.username === add.toLowerCase()))
177
it('accounts.remove', reqApi('del_account', { username: add }, 200))
178
})
179