1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-04-10 01:16:39 +02:00

feat: adapt user logic to better adapt to SAs (#2917)

https://linear.app/unleash/issue/2-579/improve-user-like-behaviour-for-service-accounts-accounts-concept

<img width="803" alt="image"
src="https://user-images.githubusercontent.com/14320932/213011584-75870595-988d-49bc-a7bf-cd1ffd146bca.png">

Makes SAs behave more like users. 

Even though they share the same `users` database table, the `is_service`
column distinguishes them. This PR makes the distinction a bit less
obvious by not filtering out SAs for some methods in the user store,
returning both account types and their respective account type
information so we can handle them properly on the UI.

We felt like this was a good enough approach for now, and a decent
compromise to move SAs forward. In the future, we may want to make a
full refactor with the `accounts` concept in mind, which we've
experimented with in the
[accounts-refactoring](https://github.com/Unleash/unleash/tree/accounts-refactoring)
branches (both OSS and Enterprise).
 
https://github.com/Unleash/unleash/pull/2918 - Moves this a bit further,
by introducing the account service and store.
This commit is contained in:
Nuno Góis 2023-01-18 12:12:44 +00:00 committed by GitHub
parent e0d7603eb3
commit d63b3c69fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 176 additions and 44 deletions

View File

@ -7,6 +7,8 @@ import { useUsers } from 'hooks/api/getters/useUsers/useUsers';
import { IGroupUser } from 'interfaces/group';
import { UG_USERS_ID } from 'utils/testIds';
import { caseInsensitiveSearch } from 'utils/search';
import { useServiceAccounts } from 'hooks/api/getters/useServiceAccounts/useServiceAccounts';
import { IServiceAccount } from 'interfaces/service-account';
const StyledOption = styled('div')(({ theme }) => ({
display: 'flex',
@ -44,7 +46,11 @@ const renderOption = (
/>
<StyledOption>
<span>{option.name || option.username}</span>
<span>{option.email}</span>
<span>
{option.name && option.username
? option.username
: option.email}
</span>
</StyledOption>
</li>
);
@ -57,6 +63,10 @@ const renderTags = (value: IGroupUser[]) => (
</StyledTags>
);
type UserOption = IUser & {
type: string;
};
interface IGroupFormUsersSelectProps {
users: IGroupUser[];
setUsers: React.Dispatch<React.SetStateAction<IGroupUser[]>>;
@ -67,6 +77,27 @@ export const GroupFormUsersSelect: VFC<IGroupFormUsersSelectProps> = ({
setUsers,
}) => {
const { users: usersAll } = useUsers();
const { serviceAccounts } = useServiceAccounts();
const options = [
...usersAll
.map((user: IUser) => ({ ...user, type: 'USERS' }))
.sort((a: IUser, b: IUser) => {
const aName = a.name || a.username || '';
const bName = b.name || b.username || '';
return aName.localeCompare(bName);
}),
...serviceAccounts
.map((serviceAccount: IServiceAccount) => ({
...serviceAccount,
type: 'SERVICE ACCOUNTS',
}))
.sort((a, b) => {
const aName = a.name || a.username || '';
const bName = b.name || b.username || '';
return aName.localeCompare(bName);
}),
];
return (
<StyledGroupFormUsersSelect>
@ -77,7 +108,7 @@ export const GroupFormUsersSelect: VFC<IGroupFormUsersSelectProps> = ({
limitTags={1}
openOnFocus
disableCloseOnSelect
value={users}
value={users as UserOption[]}
onChange={(event, newValue, reason) => {
if (
event.type === 'keydown' &&
@ -88,13 +119,10 @@ export const GroupFormUsersSelect: VFC<IGroupFormUsersSelectProps> = ({
}
setUsers(newValue);
}}
options={[...usersAll].sort((a, b) => {
const aName = a.name || a.username || '';
const bName = b.name || b.username || '';
return aName.localeCompare(bName);
})}
groupBy={option => option.type}
options={options}
renderOption={(props, option, { selected }) =>
renderOption(props, option as IUser, selected)
renderOption(props, option as UserOption, selected)
}
filterOptions={(options, { inputValue }) =>
options.filter(
@ -105,7 +133,7 @@ export const GroupFormUsersSelect: VFC<IGroupFormUsersSelectProps> = ({
)
}
isOptionEqualToValue={(option, value) => option.id === value.id}
getOptionLabel={(option: IUser) =>
getOptionLabel={(option: UserOption) =>
option.email || option.name || option.username || ''
}
renderInput={params => (

View File

@ -34,6 +34,7 @@ import {
PA_USERS_GROUPS_TITLE_ID,
} from 'utils/testIds';
import { caseInsensitiveSearch } from 'utils/search';
import { IServiceAccount } from 'interfaces/service-account';
const StyledForm = styled('form')(() => ({
display: 'flex',
@ -99,6 +100,7 @@ interface IProjectAccessAssignProps {
selected?: IProjectAccess;
accesses: IProjectAccess[];
users: IUser[];
serviceAccounts: IServiceAccount[];
groups: IGroup[];
roles: IProjectRole[];
}
@ -107,6 +109,7 @@ export const ProjectAccessAssign = ({
selected,
accesses,
users,
serviceAccounts,
groups,
roles,
}: IProjectAccessAssignProps) => {
@ -152,6 +155,21 @@ export const ProjectAccessAssign = ({
entity: user,
type: ENTITY_TYPE.USER,
})),
...serviceAccounts
.filter(
(serviceAccount: IServiceAccount) =>
edit ||
!accesses.some(
({ entity: { id }, type }) =>
serviceAccount.id === id &&
type === ENTITY_TYPE.SERVICE_ACCOUNT
)
)
.map((serviceAccount: IServiceAccount) => ({
id: serviceAccount.id,
entity: serviceAccount,
type: ENTITY_TYPE.SERVICE_ACCOUNT,
})),
];
const [selectedOptions, setSelectedOptions] = useState<IAccessOption[]>(
@ -167,7 +185,11 @@ export const ProjectAccessAssign = ({
const payload = {
users: selectedOptions
?.filter(({ type }) => type === ENTITY_TYPE.USER)
?.filter(
({ type }) =>
type === ENTITY_TYPE.USER ||
type === ENTITY_TYPE.SERVICE_ACCOUNT
)
.map(({ id }) => ({ id })),
groups: selectedOptions
?.filter(({ type }) => type === ENTITY_TYPE.GROUP)
@ -182,7 +204,10 @@ export const ProjectAccessAssign = ({
try {
if (!edit) {
await addAccessToProject(projectId, role.id, payload);
} else if (selected?.type === ENTITY_TYPE.USER) {
} else if (
selected?.type === ENTITY_TYPE.USER ||
selected?.type === ENTITY_TYPE.SERVICE_ACCOUNT
) {
await changeUserRole(projectId, role.id, selected.entity.id);
} else if (selected?.type === ENTITY_TYPE.GROUP) {
await changeGroupRole(projectId, role.id, selected.entity.id);
@ -205,7 +230,10 @@ export const ProjectAccessAssign = ({
return `curl --location --request ${edit ? 'PUT' : 'POST'} '${
uiConfig.unleashUrl
}/api/admin/projects/${projectId}/${
selected?.type === ENTITY_TYPE.USER ? 'users' : 'groups'
selected?.type === ENTITY_TYPE.USER ||
selected?.type === ENTITY_TYPE.SERVICE_ACCOUNT
? 'users'
: 'groups'
}/${selected?.entity.id}/roles/${role?.id}' \\
--header 'Authorization: INSERT_API_KEY'`;
}
@ -250,7 +278,11 @@ export const ProjectAccessAssign = ({
<span>
{optionUser?.name || optionUser?.username}
</span>
<span>{optionUser?.email}</span>
<span>
{optionUser?.name && optionUser?.username
? optionUser?.username
: optionUser?.email}
</span>
</StyledUserOption>
}
/>
@ -321,7 +353,11 @@ export const ProjectAccessAssign = ({
renderOption(props, option, selected)
}
getOptionLabel={(option: IAccessOption) => {
if (option.type === ENTITY_TYPE.USER) {
if (
option.type === ENTITY_TYPE.USER ||
option.type ===
ENTITY_TYPE.SERVICE_ACCOUNT
) {
const optionUser =
option.entity as IUser;
return (
@ -336,7 +372,11 @@ export const ProjectAccessAssign = ({
}}
filterOptions={(options, { inputValue }) =>
options.filter((option: IAccessOption) => {
if (option.type === ENTITY_TYPE.USER) {
if (
option.type === ENTITY_TYPE.USER ||
option.type ===
ENTITY_TYPE.SERVICE_ACCOUNT
) {
const optionUser =
option.entity as IUser;
return (

View File

@ -7,9 +7,9 @@ export const ProjectAccessCreate = () => {
const projectId = useRequiredPathParam('projectId');
const { access } = useProjectAccess(projectId);
const { users, groups } = useAccess();
const { users, serviceAccounts, groups } = useAccess();
if (!access || !users || !groups) {
if (!access || !users || !serviceAccounts || !groups) {
return null;
}
@ -17,6 +17,7 @@ export const ProjectAccessCreate = () => {
<ProjectAccessAssign
accesses={access.rows}
users={users}
serviceAccounts={serviceAccounts}
groups={groups}
roles={access.roles}
/>

View File

@ -10,9 +10,9 @@ export const ProjectAccessEditGroup = () => {
const groupId = useRequiredPathParam('groupId');
const { access } = useProjectAccess(projectId);
const { users, groups } = useAccess();
const { users, serviceAccounts, groups } = useAccess();
if (!access || !users || !groups) {
if (!access || !users || !serviceAccounts || !groups) {
return null;
}
@ -26,6 +26,7 @@ export const ProjectAccessEditGroup = () => {
accesses={access.rows}
selected={group}
users={users}
serviceAccounts={serviceAccounts}
groups={groups}
roles={access.roles}
/>

View File

@ -10,14 +10,15 @@ export const ProjectAccessEditUser = () => {
const userId = useRequiredPathParam('userId');
const { access } = useProjectAccess(projectId);
const { users, groups } = useAccess();
const { users, serviceAccounts, groups } = useAccess();
if (!access || !users || !groups) {
if (!access || !users || !serviceAccounts || !groups) {
return null;
}
const user = access.rows.find(
row => row.entity.id === Number(userId) && row.type === ENTITY_TYPE.USER
row =>
row.entity.id === Number(userId) && row.type !== ENTITY_TYPE.GROUP
);
return (
@ -25,6 +26,7 @@ export const ProjectAccessEditUser = () => {
accesses={access.rows}
selected={user}
users={users}
serviceAccounts={serviceAccounts}
groups={groups}
roles={access.roles}
/>

View File

@ -151,7 +151,7 @@ export const ProjectAccessTable: VFC = () => {
id: 'username',
Header: 'Username',
accessor: (row: IProjectAccess) => {
if (row.type === ENTITY_TYPE.USER) {
if (row.type !== ENTITY_TYPE.GROUP) {
const userRow = row.entity as IUser;
return userRow.username || userRow.email;
}
@ -194,7 +194,7 @@ export const ProjectAccessTable: VFC = () => {
id: 'lastLogin',
Header: 'Last login',
accessor: (row: IProjectAccess) => {
if (row.type === ENTITY_TYPE.USER) {
if (row.type !== ENTITY_TYPE.GROUP) {
const userRow = row.entity as IUser;
return userRow.seenAt || '';
}
@ -228,7 +228,9 @@ export const ProjectAccessTable: VFC = () => {
permission={UPDATE_PROJECT}
projectId={projectId}
to={`edit/${
row.type === ENTITY_TYPE.USER ? 'user' : 'group'
row.type === ENTITY_TYPE.GROUP
? 'group'
: 'user'
}/${row.entity.id}`}
disabled={access?.rows.length === 1}
tooltipProps={{
@ -344,13 +346,13 @@ export const ProjectAccessTable: VFC = () => {
if (!userOrGroup) return;
const { id, roleId } = userOrGroup.entity;
let name = userOrGroup.entity.name;
if (userOrGroup.type === ENTITY_TYPE.USER) {
if (userOrGroup.type !== ENTITY_TYPE.GROUP) {
const user = userOrGroup.entity as IUser;
name = name || user.email || user.username || '';
}
try {
if (userOrGroup.type === ENTITY_TYPE.USER) {
if (userOrGroup.type !== ENTITY_TYPE.GROUP) {
await removeUserFromRole(projectId, roleId, id);
} else {
await removeGroupFromRole(projectId, roleId, id);

View File

@ -3,9 +3,11 @@ import { formatApiPath } from 'utils/formatPath';
import handleErrorResponses from '../httpErrorResponseHandler';
import { IGroup } from 'interfaces/group';
import { IUser } from 'interfaces/user';
import { IServiceAccount } from 'interfaces/service-account';
export interface IUseAccessOutput {
users?: IUser[];
serviceAccounts?: IServiceAccount[];
groups?: IGroup[];
loading: boolean;
refetch: () => void;
@ -19,7 +21,12 @@ export const useAccess = (): IUseAccessOutput => {
);
return {
users: data?.users,
users: (data?.users as IUser[])?.filter(
({ accountType }) => accountType === 'User'
),
serviceAccounts: (data?.users as IServiceAccount[])?.filter(
({ accountType }) => accountType === 'Service Account'
),
groups: data?.groups,
loading: !error && !data,
refetch: () => mutate(),

View File

@ -6,10 +6,12 @@ import { IProjectRole } from 'interfaces/role';
import { IGroup } from 'interfaces/group';
import { IUser } from 'interfaces/user';
import { mapGroupUsers } from '../useGroup/useGroup';
import { IServiceAccount } from 'interfaces/service-account';
export enum ENTITY_TYPE {
USER = 'USERS',
GROUP = 'GROUPS',
SERVICE_ACCOUNT = 'SERVICE ACCOUNTS',
}
export interface IProjectAccess {
@ -63,7 +65,12 @@ const useProjectAccess = (
if (data) {
return formatAccessData({
roles: data.roles,
users: data.users,
users: (data.users as IUser[]).filter(
({ accountType }) => accountType === 'User'
),
serviceAccounts: (data.users as IUser[]).filter(
({ accountType }) => accountType === 'Service Account'
),
groups:
data?.groups.map((group: any) => ({
...group,
@ -83,15 +90,20 @@ const useProjectAccess = (
const formatAccessData = (access: any): IProjectAccessOutput => {
const users = access.users || [];
const serviceAccounts = access.serviceAccounts || [];
const groups = access.groups || [];
return {
...access,
rows: [
...users.map((user: any) => ({
...users.map((user: IUser) => ({
entity: user,
type: ENTITY_TYPE.USER,
})),
...groups.map((group: any) => ({
...serviceAccounts.map((serviceAccount: IServiceAccount) => ({
entity: serviceAccount,
type: ENTITY_TYPE.SERVICE_ACCOUNT,
})),
...groups.map((group: IGroup) => ({
entity: group,
type: ENTITY_TYPE.GROUP,
})),

View File

@ -1,3 +1,6 @@
export const AccountTypes = ['User', 'Service Account'] as const;
type AccountType = typeof AccountTypes[number];
export interface IUser {
id: number;
email: string;
@ -13,6 +16,7 @@ export interface IUser {
isAPI: boolean;
paid?: boolean;
addedAt?: string;
accountType?: AccountType;
}
export interface IPermission {

View File

@ -14,17 +14,6 @@ import {
const TABLE = 'users';
const USER_COLUMNS = [
'id',
'name',
'username',
'email',
'image_url',
'login_attempts',
'seen_at',
'created_at',
];
const USER_COLUMNS_PUBLIC = [
'id',
'name',
@ -32,8 +21,11 @@ const USER_COLUMNS_PUBLIC = [
'email',
'image_url',
'seen_at',
'is_service',
];
const USER_COLUMNS = [...USER_COLUMNS_PUBLIC, 'login_attempts', 'created_at'];
const emptify = (value) => {
if (!value) {
return undefined;
@ -63,6 +55,7 @@ const rowToUser = (row) => {
loginAttempts: row.login_attempts,
seenAt: row.seen_at,
createdAt: row.created_at,
isService: row.is_service,
});
};
@ -133,6 +126,11 @@ class UserStore implements IUserStore {
}
async getAll(): Promise<User[]> {
const users = await this.activeAll().select(USER_COLUMNS);
return users.map(rowToUser);
}
async getAllUsers(): Promise<User[]> {
const users = await this.activeUsers().select(USER_COLUMNS);
return users.map(rowToUser);
}
@ -147,7 +145,7 @@ class UserStore implements IUserStore {
}
async getAllWithId(userIdList: number[]): Promise<User[]> {
const users = await this.activeUsers()
const users = await this.activeAll()
.select(USER_COLUMNS_PUBLIC)
.whereIn('id', userIdList);
return users.map(rowToUser);

View File

@ -1,4 +1,5 @@
import { FromSchema } from 'json-schema-to-ts';
import { AccountTypes } from '../../types';
export const userSchema = {
$id: '#/components/schemas/userSchema',
@ -45,6 +46,10 @@ export const userSchema = {
type: 'string',
format: 'date-time',
},
accountType: {
type: 'string',
enum: AccountTypes,
},
},
components: {},
} as const;

View File

@ -262,7 +262,7 @@ export default class UserAdminController extends Controller {
}
async getUsers(req: Request, res: Response<UsersSchema>): Promise<void> {
const users = await this.userService.getAll();
const users = await this.userService.getAllUsers();
const rootRoles = await this.accessService.getRootRoles();
const inviteLinks = await this.resetTokenService.getActiveInvitations();
@ -317,6 +317,7 @@ export default class UserAdminController extends Controller {
name: u.name,
username: u.username,
email: u.email,
accountType: u.accountType,
} as IUser;
});

View File

@ -161,6 +161,20 @@ class UserService {
return usersWithRootRole;
}
async getAllUsers(): Promise<IUserWithRole[]> {
const users = await this.store.getAllUsers();
const defaultRole = await this.accessService.getRootRole(
RoleName.VIEWER,
);
const userRoles = await this.accessService.getRootRoleForAllUsers();
const usersWithRootRole = users.map((u) => {
const rootRole = userRoles.find((r) => r.userId === u.id);
const roleId = rootRole ? rootRole.roleId : defaultRole.id;
return { ...u, rootRole: roleId };
});
return usersWithRootRole;
}
async getUser(id: number): Promise<IUserWithRole> {
const roles = await this.accessService.getUserRootRoles(id);
const defaultRole = await this.accessService.getRootRole(

View File

@ -25,6 +25,7 @@ export interface IUserStore extends Store<IUser, number> {
upsert(user: ICreateUser): Promise<IUser>;
hasUser(idQuery: IUserLookup): Promise<number | undefined>;
search(query: string): Promise<IUser[]>;
getAllUsers(): Promise<IUser[]>;
getAllWithId(userIdList: number[]): Promise<IUser[]>;
getByQuery(idQuery: IUserLookup): Promise<IUser>;
getPasswordHash(userId: number): Promise<string>;

View File

@ -1,6 +1,9 @@
import Joi from 'joi';
import { generateImageUrl } from '../util/generateImageUrl';
export const AccountTypes = ['User', 'Service Account'] as const;
type AccountType = typeof AccountTypes[number];
export interface UserData {
id: number;
name?: string;
@ -10,6 +13,7 @@ export interface UserData {
seenAt?: Date;
loginAttempts?: number;
createdAt?: Date;
isService?: boolean;
}
export interface IUser {
@ -24,6 +28,7 @@ export interface IUser {
loginAttempts: number;
isAPI: boolean;
imageUrl: string;
accountType?: AccountType;
}
export interface IProjectUser extends IUser {
@ -51,6 +56,8 @@ export default class User implements IUser {
createdAt: Date;
accountType?: AccountType = 'User';
constructor({
id,
name,
@ -60,6 +67,7 @@ export default class User implements IUser {
seenAt,
loginAttempts,
createdAt,
isService,
}: UserData) {
if (!id) {
throw new TypeError('Id is required');
@ -76,6 +84,7 @@ export default class User implements IUser {
this.seenAt = seenAt;
this.loginAttempts = loginAttempts;
this.createdAt = createdAt;
this.accountType = isService ? 'Service Account' : 'User';
}
generateImageUrl(): string {

View File

@ -3543,6 +3543,9 @@ exports[`should serve the OpenAPI spec 1`] = `
"userSchema": {
"additionalProperties": false,
"properties": {
"accountType": {
"type": "string",
},
"createdAt": {
"format": "date-time",
"type": "string",

View File

@ -112,6 +112,10 @@ class UserStoreMock implements IUserStore {
throw new Error('Not implemented');
}
async getAllUsers(): Promise<IUser[]> {
throw new Error('Not implemented');
}
async getAllWithId(): Promise<IUser[]> {
throw new Error('Not implemented');
}