fix: uppercase letters in username won't allow login (until restart) https://github.com/rejetto/hfs/issues/85

Massimo Melina committed Jan 6, 2023 at 10:48 UTC fbc92ea0631ffb6b799382ac91ee1ad6807dbb86
5 files changed +30 -28
admin/src/AccountForm.ts
+6 -5
@@ -32,7 +32,8 @@ export default function AccountForm({ account, done, groups, close }: FormProps)
32 ],
33 fields: [
34 { k: 'username', label: group ? 'Group name' : undefined, autoComplete: 'off', required: true, xl: group ? 12 : 4,
35 - getError: v => v !== account.username && apiCall('get_account', { username: v }).then(() => "already used", () => false),
35 + getError: v => v !== account.username && apiCall('get_account', { username: v })
36 + .then(got => got.username === account.username ? "usernames are case-insensitive" : "already used", () => false),
37 },
38 !group && { k: 'password', md: 6, xl: 4, type: 'password', autoComplete: 'new-password', required: add,
39 label: add ? "Password" : "Change password"
@@ -58,23 +59,23 @@ export default function AccountForm({ account, done, groups, close }: FormProps)
59 const { password='', password2, adminActualAccess, ...withoutPassword } = values
60 const { username } = values
61 if (add) {
61 - await apiCall('add_account', withoutPassword)
62 + const got = await apiCall('add_account', withoutPassword)
63 if (password)
64 try { await apiNewPassword(username, password) }
65 catch(e) {
66 apiCall('del_account', { username }).then() // best effort, don't wait
67 throw e
68 }
68 - done(username)
69 + done(got.username)
70 return alertDialog("Account created", 'success')
71 }
71 - await apiCall('set_account', {
72 + const got = await apiCall('set_account', {
73 username: account.username,
74 changes: withoutPassword,
75 })
76 if (password)
77 await apiNewPassword(username, password)
77 - done(username)
78 + done(got.username)
79 return alertDialog("Account modified", 'success')
80 }
81 }
src/api.accounts.ts
+4 -4
@@ -50,15 +50,15 @@ const apis: ApiHandlers = {
50 changes.admin = undefined
51 else if (admin !== undefined && typeof admin !== 'boolean')
52 return new ApiError(400, "invalid admin")
53 - return setAccount(username, changes) ? {} : new ApiError(400)
53 + const acc = setAccount(username, changes)
54 + return acc ? _.pick(acc, 'username') : new ApiError(400)
55 },
56
57 add_account({ username, ...rest }) {
58 if (getAccount(username))
59 return new ApiError(FORBIDDEN)
59 - if (!addAccount(username, rest))
60 - return new ApiError(400)
61 - return {}
60 + const acc = addAccount(username, rest)
61 + return acc ? _.pick(acc, 'username') : new ApiError(400)
62 },
63
64 del_account({ username }) {
src/api.auth.ts
+3 -5
@@ -1,6 +1,6 @@
1 // This file is part of HFS - Copyright 2021-2022, Massimo Melina <a@rejetto.com> - License https://www.gnu.org/licenses/gpl-3.0.txt
2
3 -import { Account, getAccount, getCurrentUsername } from './perm'
3 +import { Account, getAccount, getCurrentUsername, normalizeUsername } from './perm'
4 import { verifyPassword } from './crypt'
5 import { ApiError, ApiHandler } from './apiMiddleware'
6 import { SRPParameters, SRPRoutines, SRPServerSession, SRPServerSessionStep1 } from 'tssrp6a'
@@ -24,7 +24,7 @@ async function loggedIn(ctx:Koa.Context, username: string | false) {
24 ctx.cookies.set('csrf', '')
25 return
26 }
27 - s.username = username
27 + s.username = normalizeUsername(username)
28 await prepareState(ctx, async ()=>{}) // updating the state is necessary to send complete session data so that frontend shows admin button
29 delete s.login
30 ctx.cookies.set('csrf', randomId(), { signed:false, httpOnly: false })
@@ -37,7 +37,6 @@ function makeExp() {
37 export const login: ApiHandler = async ({ username, password }, ctx) => {
38 if (!username || !password) // some validation
39 return new ApiError(400)
40 - username = username.toLocaleLowerCase() // normalize username, to be case-insensitive
40 const acc = getAccount(username)
41 if (!acc)
42 return new ApiError(UNAUTHORIZED)
@@ -54,7 +53,6 @@ export const login: ApiHandler = async ({ username, password }, ctx) => {
53 export const loginSrp1: ApiHandler = async ({ username }, ctx) => {
54 if (!username)
55 return new ApiError(400)
57 - username = username.toLocaleLowerCase()
56 const account = getAccount(username)
57 if (!ctx.session)
58 return new ApiError(500)
@@ -109,7 +107,7 @@ export const loginSrp2: ApiHandler = async ({ pubKey, proof }, ctx) => {
107 export const logout: ApiHandler = async ({}, ctx) => {
108 if (!ctx.session)
109 return new ApiError(500)
112 - loggedIn(ctx, false)
110 + await loggedIn(ctx, false)
111 // 401 is a convenient code for OK: the browser clears a possible http authentication (hopefully), and Admin automatically triggers login dialog
112 return new ApiError(401)
113 }
src/middlewares.ts
+2 -3
@@ -14,7 +14,7 @@ import { serveGuiFiles } from './serveGuiFiles'
14 import mount from 'koa-mount'
15 import { Readable } from 'stream'
16 import { applyBlock } from './block'
17 -import { getAccount, getCurrentUsername } from './perm'
17 +import { getAccount } from './perm'
18 import { socket2connection, updateConnection, normalizeIp } from './connections'
19 import basicAuth from 'basic-auth'
20 import { SRPClientSession, SRPParameters, SRPRoutines } from 'tssrp6a'
@@ -126,7 +126,7 @@ export function getProxyDetected() {
126 }
127 export const prepareState: Koa.Middleware = async (ctx, next) => {
128 // calculate these once and for all
129 - ctx.state.account = await getHttpAccount(ctx) ?? getAccount(getCurrentUsername(ctx))
129 + ctx.state.account = await getHttpAccount(ctx) ?? getAccount(ctx.session?.username, false)
130 const conn = ctx.state.connection = socket2connection(ctx.socket)
131 await next()
132 if (conn)
@@ -141,7 +141,6 @@ async function getHttpAccount(ctx: Koa.Context) {
141 }
142
143 async function srpCheck(username: string, password: string) {
144 - username = username.toLocaleLowerCase()
144 const account = getAccount(username)
145 if (!account?.srp || !password) return false
146 const { step1, salt, pubKey } = await srpStep1(account)
src/perm.ts
+15 -11
@@ -9,7 +9,7 @@ import { createVerifierAndSalt, SRPParameters, SRPRoutines } from 'tssrp6a'
9 import events from './events'
10
11 export interface Account {
12 - username: string, // we'll have username in it, so we don't need to pass it separately
12 + username: string, // we keep username property (hidden) so we don't need to pass it separately
13 password?: string
14 hashed_password?: string
15 srp?: string
@@ -27,7 +27,7 @@ export function getAccounts() {
27 }
28
29 export function getCurrentUsername(ctx: Koa.Context): string {
30 - return ctx.state.account?.username || ctx.session?.username || ''
30 + return ctx.state.account?.username || ''
31 }
32
33 // provides the username and all other usernames it inherits based on the 'belongs' attribute. Useful to check permissions
@@ -44,7 +44,9 @@ export function getCurrentUsernameExpanded(ctx: Koa.Context) {
44 return ret
45 }
46
47 -export function getAccount(username:string) : Account | undefined {
47 +export function getAccount(username:string, normalize=true) : Account | undefined {
48 + if (normalize)
49 + username = normalizeUsername(username)
50 return username ? accounts[username] : undefined
51 }
52
@@ -92,13 +94,14 @@ accountsConfig.sub(async v => {
94 if (!rec) // an empty object in yaml is stored as null
95 rec = accounts[norm] = { username: norm }
96 else
95 - objRenameKey(accounts, k, norm)
97 + if (objRenameKey(accounts, k, norm))
98 + saveAccountsAsap()
99 setHidden(rec, { username: norm })
97 - await updateAccount(rec)
100 + await updateAccount(rec) // work password fields
101 }))
102 })
103
101 -function normalizeUsername(username: string) {
104 +export function normalizeUsername(username: string) {
105 return username.toLocaleLowerCase()
106 }
107
@@ -129,10 +132,11 @@ export function renameAccount(from: string, to: string) {
132 const assignableProps: (keyof Account)[] = ['redirect','ignore_limits','belongs','admin']
133
134 export function addAccount(username: string, props: Partial<Account>) {
132 - if (!username || accounts[username])
135 + username = normalizeUsername(username)
136 + if (!username || getAccount(username, false))
137 return
134 - const copy: Account = setHidden(_.pickBy(_.pick(props, assignableProps), Boolean),
135 - { username }) // have the field in the object but hidden so that stringification won't include it
138 + const filteredProps = _.pickBy(_.pick(props, assignableProps), Boolean)
139 + const copy: Account = setHidden(filteredProps, { username }) // have the field in the object but hidden so that stringification won't include it
140 accountsConfig.set(accounts =>
141 Object.assign(accounts, { [username]: copy }))
142 saveAccountsAsap().then()
@@ -151,14 +155,14 @@ export function setAccount(username: string, changes: Partial<Account>) {
155 if (changes.username)
156 renameAccount(username, changes.username)
157 saveAccountsAsap().then()
154 - return true
158 + return acc
159 }
160
161 export function delAccount(username: string) {
162 if (!getAccount(username))
163 return false
164 accountsConfig.set(accounts =>
161 - Object.assign(accounts, { [username]: undefined }))
165 + Object.assign(accounts, { [normalizeUsername(username)]: undefined }))
166 saveAccountsAsap().then()
167 return true
168 }