better code: had 2 functions for 1 job

Massimo Melina committed Jan 27, 2024 at 18:19 UTC ccfe944dce0be7c05d3fb19815d1d32c092007b9
4 files changed +29 -40
src/api.accounts.ts
+9 -6
@@ -2,7 +2,7 @@
2
3 import { ApiError, ApiHandlers } from './apiMiddleware'
4 import { Account, accountCanLoginAdmin, accountHasPassword, accountsConfig, addAccount, delAccount, getAccount,
5 - setAccount, changeSrpHelper } from './perm'
5 + changeSrpHelper, updateAccount } from './perm'
6 import _ from 'lodash'
7 import { HTTP_BAD_REQUEST, HTTP_CONFLICT, HTTP_NOT_FOUND } from './const'
8 import { getCurrentUsername, invalidSessions } from './auth'
@@ -37,20 +37,23 @@ export default {
37 return { list: _.filter(accountsConfig.get(), accountCanLoginAdmin).map(ac => ac.username) }
38 },
39
40 - set_account({ username, changes }, ctx) {
40 + async set_account({ username, changes }, ctx) {
41 const acc = getAccount(username)
42 if (!acc)
43 return new ApiError(HTTP_BAD_REQUEST)
44 - setAccount(acc, changes)
44 + await updateAccount(acc, changes)
45 if (changes.username && ctx.session?.username === username)
46 ctx.session!.username = changes.username
47 return _.pick(acc, 'username')
48 },
49
50 - add_account({ overwrite, username, ...rest }) {
50 + async add_account({ overwrite, username, ...rest }) {
51 const existing = getAccount(username)
52 - if (existing)
53 - return overwrite ? setAccount(existing, rest) : new ApiError(HTTP_CONFLICT)
52 + if (existing) {
53 + if (overwrite) return new ApiError(HTTP_CONFLICT)
54 + await updateAccount(existing, rest)
55 + return _.pick(existing, 'username')
56 + }
57 const acc = addAccount(username, rest)
58 return acc ? _.pick(acc, 'username') : new ApiError(HTTP_BAD_REQUEST)
59 },
src/auth.ts
+1 -3
@@ -50,9 +50,7 @@ export async function setLoggedIn(ctx: Context, username: string | false) {
50 s.username = normalizeUsername(username)
51 const a = ctx.state.account = getAccount(username)
52 if (a && !a.expire && a.days_to_live)
53 - updateAccount(a, x => {
54 - x.expire = new Date(Date.now() + a.days_to_live! * DAY)
55 - })
53 + updateAccount(a, { expire: new Date(Date.now() + a.days_to_live! * DAY) })
54 }
55
56 export const invalidSessions = new Set<string>() // since session are currently stored in cookies, we need to memorize this until we meet again
src/commands.ts
+1 -3
@@ -66,9 +66,7 @@ const commands = {
66 const acc = getAccount(user)
67 if (!acc)
68 throw "user doesn't exist"
69 - await updateAccount(acc!, acc => {
70 - acc.password = password
71 - })
69 + await updateAccount(acc!, { password })
70 }
71 },
72 config: {
src/perm.ts
+18 -28
@@ -1,13 +1,14 @@
1 // This file is part of HFS - Copyright 2021-2023, Massimo Melina <a@rejetto.com> - License https://www.gnu.org/licenses/gpl-3.0.txt
2
3 import _ from 'lodash'
4 -import { HTTP_BAD_REQUEST, objRenameKey, setHidden, wantArray } from './misc'
4 +import { HTTP_BAD_REQUEST, objRenameKey, objSameKeys, setHidden, wantArray } from './misc'
5 import { defineConfig, saveConfigAsap } from './config'
6 import { createVerifierAndSalt, SRPParameters, SRPRoutines } from 'tssrp6a'
7 import events from './events'
8 import { ApiError } from './apiMiddleware'
9
10 export interface Account {
11 + // we consider all the following fields, when falsy, as equivalent to be missing. If this changes in the future, please adjust addAccount and setAccount
12 username: string, // we keep username property (hidden) so we don't need to pass it separately
13 password?: string
14 srp?: string
@@ -55,19 +56,24 @@ createAdminConfig.sub(v => {
56 createAdminConfig.set('')
57 })
58
58 -export function createAdmin(pass: string, username='admin') {
59 +export function createAdmin(password: string, username='admin') {
60 const acc = addAccount(username, { admin: true })
61 if (!acc) return console.log("cannot create, already exists")
61 - updateAccount(acc!, acc => { acc.password = pass })
62 + updateAccount(acc!, { password })
63 console.log("account admin created")
64 }
65
66 const srp6aNimbusRoutines = new SRPRoutines(new SRPParameters())
67
68 type Changer = (account:Account)=> void | Promise<void>
68 -export async function updateAccount(account: Account, changer?:Changer) {
69 - const was = JSON.stringify(account)
70 - await changer?.(account)
69 +export async function updateAccount(account: Account, change: Partial<Account> | Changer) {
70 + const jsonWas = JSON.stringify(account)
71 + const { username: usernameWas } = account
72 + if (typeof change === 'function')
73 + await change?.(account)
74 + else
75 + Object.assign(account, objSameKeys(change, x => x || undefined))
76 +
77 const { username } = account
78 if (account.password) {
79 console.debug('hashing password for', username)
@@ -83,7 +89,10 @@ export async function updateAccount(account: Account, changer?:Changer) {
89 return true
90 })
91 }
86 - if (was !== JSON.stringify(account))
92 + account.expire &&= new Date(account.expire)
93 + if (username !== usernameWas)
94 + renameAccount(usernameWas, username)
95 + if (jsonWas !== JSON.stringify(account))
96 saveAccountsAsap()
97 }
98
@@ -101,8 +110,7 @@ accountsConfig.sub(obj => {
110 saveAccountsAsap()
111 setHidden(rec, { username: norm })
112 }
104 - updateAccount(rec).then() // work password fields
105 - rec.expire &&= new Date(rec.expire) // parse
113 + updateAccount(rec, {}).then() // work fields
114 })
115 })
116
@@ -133,34 +141,16 @@ export function renameAccount(from: string, to: string) {
141 }
142 }
143
136 -// we consider all the following fields, when falsy, as equivalent to be missing. If this changes in the future, please adjust addAccount and setAccount
137 -const assignableProps: (keyof Account)[] = ['redirect', 'ignore_limits', 'belongs', 'admin', 'disabled',
138 - 'disable_password_change', 'expire', 'days_to_live']
139 -
144 export function addAccount(username: string, props: Partial<Account>) {
145 username = normalizeUsername(username)
146 if (!username || getAccount(username, false))
147 return
144 - const filteredProps = _.pickBy(_.pick(props, assignableProps), Boolean)
145 - const copy: Account = setHidden(filteredProps, { username }) // have the field in the object but hidden so that stringification won't include it
148 + const copy: Account = setHidden(_.pickBy(props, Boolean), { username }) // have the field in the object but hidden so that stringification won't include it
149 accountsConfig.set(accounts =>
150 Object.assign(accounts, { [username]: copy }))
151 return copy
152 }
153
151 -export function setAccount(acc: Account, changes: Partial<Account>) {
152 - const rest = _.pick(changes, assignableProps)
153 - for (const [k,v] of Object.entries(rest))
154 - if (!v)
155 - rest[k as keyof Account] = undefined
156 - Object.assign(acc, rest)
157 - acc.expire &&= new Date(acc.expire)
158 - if (changes.username)
159 - renameAccount(acc.username, changes.username)
160 - saveAccountsAsap()
161 - return acc
162 -}
163 -
154 export function delAccount(username: string) {
155 if (!getAccount(username))
156 return false