Skip to content

Commit

Permalink
feat: adapt user logic to better adapt to SAs (#2917)
Browse files Browse the repository at this point in the history
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).
 
#2918 - Moves this a bit further,
by introducing the account service and store.
  • Loading branch information
nunogois committed Jan 18, 2023
1 parent e0d7603 commit d63b3c6
Show file tree
Hide file tree
Showing 17 changed files with 176 additions and 44 deletions.
Expand Up @@ -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',
Expand Down Expand Up @@ -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>
);
Expand All @@ -57,6 +63,10 @@ const renderTags = (value: IGroupUser[]) => (
</StyledTags>
);

type UserOption = IUser & {
type: string;
};

interface IGroupFormUsersSelectProps {
users: IGroupUser[];
setUsers: React.Dispatch<React.SetStateAction<IGroupUser[]>>;
Expand All @@ -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>
Expand All @@ -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' &&
Expand All @@ -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(
Expand All @@ -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 => (
Expand Down
Expand Up @@ -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',
Expand Down Expand Up @@ -99,6 +100,7 @@ interface IProjectAccessAssignProps {
selected?: IProjectAccess;
accesses: IProjectAccess[];
users: IUser[];
serviceAccounts: IServiceAccount[];
groups: IGroup[];
roles: IProjectRole[];
}
Expand All @@ -107,6 +109,7 @@ export const ProjectAccessAssign = ({
selected,
accesses,
users,
serviceAccounts,
groups,
roles,
}: IProjectAccessAssignProps) => {
Expand Down Expand Up @@ -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[]>(
Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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'`;
}
Expand Down Expand Up @@ -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>
}
/>
Expand Down Expand Up @@ -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 (
Expand All @@ -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 (
Expand Down
Expand Up @@ -7,16 +7,17 @@ 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;
}

return (
<ProjectAccessAssign
accesses={access.rows}
users={users}
serviceAccounts={serviceAccounts}
groups={groups}
roles={access.roles}
/>
Expand Down
Expand Up @@ -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;
}

Expand All @@ -26,6 +26,7 @@ export const ProjectAccessEditGroup = () => {
accesses={access.rows}
selected={group}
users={users}
serviceAccounts={serviceAccounts}
groups={groups}
roles={access.roles}
/>
Expand Down
Expand Up @@ -10,21 +10,23 @@ 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 (
<ProjectAccessAssign
accesses={access.rows}
selected={user}
users={users}
serviceAccounts={serviceAccounts}
groups={groups}
roles={access.roles}
/>
Expand Down
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 || '';
}
Expand Down Expand Up @@ -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={{
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/hooks/api/getters/useAccess/useAccess.ts
Expand Up @@ -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;
Expand All @@ -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(),
Expand Down

0 comments on commit d63b3c6

Please sign in to comment.