fix(admin): keep server traits when an operator saves traits (#2622)

This commit is contained in:
Hampus
2026-09-09 00:13:07 +02:00
committed by GitHub
parent 1b22d14f3d
commit 38e2c8db3e
4 changed files with 94 additions and 1 deletions
@@ -560,6 +560,8 @@ fn traits_form(
}
}
const DERIVED_TRAITS: [&str; 1] = ["premium"];
fn parse_trait_definitions(limit_config: Option<&LimitConfigResponse>) -> Vec<&str> {
limit_config
.map(|response| {
@@ -569,6 +571,7 @@ fn parse_trait_definitions(limit_config: Option<&LimitConfigResponse>) -> Vec<&s
.iter()
.map(|value| value.trim())
.filter(|value| !value.is_empty())
.filter(|value| !DERIVED_TRAITS.contains(value))
.collect()
})
.unwrap_or_default()
@@ -579,5 +582,6 @@ fn custom_traits<'a>(user: &'a AdminUser, trait_definitions: &[&str]) -> Vec<&'a
.iter()
.map(String::as_str)
.filter(|trait_name| !trait_definitions.contains(trait_name))
.filter(|trait_name| !DERIVED_TRAITS.contains(trait_name))
.collect()
}
@@ -45,6 +45,7 @@ import {Logger} from '../../Logger';
import {getInstanceConfigRepository} from '../../middleware/ServiceSingletons';
import type {IRiskHistoryRepository} from '../../risk/HistoricalOutcomeRepository';
import type {HistoricalOutcomeCode} from '../../risk/RiskHistoryTypes';
import {resolveAssignedTraits} from '../../user/UserTraits';
import {getIpAddressReverse, getLocationLabelFromIp} from '../../utils/IpUtils';
import {resolveSessionClientInfo} from '../../utils/SessionClientIdentity';
import {mapUserToAdminResponse} from '../models/UserTypes';
@@ -383,7 +384,8 @@ export class AdminUserSecurityService {
if (!user) {
throw new UnknownUserError();
}
const traitSet = data.traits.length > 0 ? new Set(data.traits) : null;
const assigned = resolveAssignedTraits(user.traits ?? [], data.traits);
const traitSet = assigned.size > 0 ? assigned : null;
const updatedUser = await userRepository.patchUpsert(
userId,
{
@@ -0,0 +1,56 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import {describe, expect, it} from 'vitest';
import {isDerivedTrait, isServerManagedTrait, resolveAssignedTraits} from './UserTraits';
describe('UserTraits', () => {
it('treats premium as derived rather than assignable', () => {
expect(isDerivedTrait('premium')).toBe(true);
expect(isDerivedTrait('beta-tester')).toBe(false);
});
it('recognises every trait shape the server manages', () => {
expect(isServerManagedTrait('sso')).toBe(true);
expect(isServerManagedTrait('sso:acme')).toBe(true);
expect(isServerManagedTrait('sso_provider:0123456789abcdef')).toBe(true);
expect(isServerManagedTrait('sso_identity:0123456789abcdef')).toBe(true);
expect(isServerManagedTrait('registration_pending_approval')).toBe(true);
expect(isServerManagedTrait('registration_rejected')).toBe(true);
expect(isServerManagedTrait('beta-tester')).toBe(false);
});
it('keeps the operator traits it was given', () => {
expect([...resolveAssignedTraits([], ['beta-tester', 'experimental'])]).toEqual(['beta-tester', 'experimental']);
});
it('drops premium because nothing reads the stored value', () => {
expect([...resolveAssignedTraits([], ['premium'])]).toEqual([]);
expect([...resolveAssignedTraits([], ['beta-tester', 'premium'])]).toEqual(['beta-tester']);
});
it('keeps a premium user premium when the operator saves other traits', () => {
expect([...resolveAssignedTraits(['premium'], ['beta-tester'])]).toEqual(['beta-tester']);
});
it('preserves server managed traits the operator did not send', () => {
const resolved = resolveAssignedTraits(
['sso', 'sso:acme', 'sso_identity:0123456789abcdef', 'beta-tester'],
['experimental'],
);
expect([...resolved].sort()).toEqual(['experimental', 'sso', 'sso:acme', 'sso_identity:0123456789abcdef']);
});
it('keeps a pending approval user pending when their traits are cleared', () => {
expect([...resolveAssignedTraits(['registration_pending_approval'], [])]).toEqual([
'registration_pending_approval',
]);
});
it('refuses to let an operator forge a server managed trait', () => {
expect([...resolveAssignedTraits([], ['sso', 'sso_identity:forged'])]).toEqual([]);
});
it('ignores empty trait names', () => {
expect([...resolveAssignedTraits([], ['', 'beta-tester'])]).toEqual(['beta-tester']);
});
});
+31
View File
@@ -0,0 +1,31 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
const DERIVED_TRAITS = new Set(['premium']);
const SERVER_MANAGED_TRAITS = new Set(['sso', 'registration_pending_approval', 'registration_rejected']);
const SERVER_MANAGED_TRAIT_PREFIXES = ['sso:', 'sso_provider:', 'sso_identity:'];
export function isDerivedTrait(trait: string): boolean {
return DERIVED_TRAITS.has(trait);
}
export function isServerManagedTrait(trait: string): boolean {
return SERVER_MANAGED_TRAITS.has(trait) || SERVER_MANAGED_TRAIT_PREFIXES.some((prefix) => trait.startsWith(prefix));
}
export function resolveAssignedTraits(current: Iterable<string>, requested: Iterable<string>): Set<string> {
const next = new Set<string>();
for (const trait of requested) {
if (!trait || isDerivedTrait(trait) || isServerManagedTrait(trait)) {
continue;
}
next.add(trait);
}
for (const trait of current) {
if (isServerManagedTrait(trait)) {
next.add(trait);
}
}
return next;
}