Skip to content

Commit

Permalink
Remove accounts
Browse files Browse the repository at this point in the history
  • Loading branch information
barreiro authored and johnaohara committed Jun 14, 2024
1 parent efc5526 commit cf4fece
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ public interface UserService {
@Blocking
void createUser(@RequestBody(required = true) NewUser user);

@DELETE
@Path("{username}")
@Blocking
void removeUser(@PathParam("username") String username);

@GET
@Path("teams")
@Blocking
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
import io.hyperfoil.tools.horreum.entity.user.UserInfo;
import io.hyperfoil.tools.horreum.server.WithRoles;
import io.hyperfoil.tools.horreum.svc.user.UserBackEnd;
import io.quarkus.logging.Log;
import io.quarkus.security.Authenticated;
import io.quarkus.security.identity.SecurityIdentity;
import jakarta.annotation.security.RolesAllowed;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
import jakarta.transaction.Transactional;
import org.jboss.logging.Logger;

import java.security.SecureRandom;
import java.util.HashMap;
Expand All @@ -25,7 +25,7 @@
@Authenticated
@ApplicationScoped
public class UserServiceImpl implements UserService {
private static final Logger LOG = Logger.getLogger(UserServiceImpl.class);

private static final int RANDOM_PASSWORD_LENGTH = 15;

@Inject SecurityIdentity identity;
Expand Down Expand Up @@ -53,7 +53,17 @@ public class UserServiceImpl implements UserService {
userIsManagerForTeam(user.team);
backend.get().createUser(user);
createLocalUser(user.user.username, user.team, user.roles != null && user.roles.contains(Roles.MACHINE) ? user.password : null);
LOG.infov("{0} created user {1} {2} with username {3} on team {4}", identity.getPrincipal().getName(), user.user.firstName, user.user.lastName, user.user.username, user.team);
Log.infov("{0} created user {1} {2} with username {3} on team {4}", identity.getPrincipal().getName(), user.user.firstName, user.user.lastName, user.user.username, user.team);
}

@RolesAllowed({ Roles.ADMIN, Roles.MANAGER })
@Override public void removeUser(String username) {
if (identity.getPrincipal().getName().equals(username)) {
throw ServiceException.badRequest("Cannot remove yourself");
}
backend.get().removeUser(username);
removeLocalUser(username);
Log.infov("{0} removed user {1}", identity.getPrincipal().getName(), username);
}

@Override public List<String> getTeams() {
Expand Down Expand Up @@ -113,14 +123,14 @@ public class UserServiceImpl implements UserService {
@Override public void addTeam(String unsafeTeam) {
String team = validateTeamName(unsafeTeam);
backend.get().addTeam(team);
LOG.infov("{0} created team {1}", identity.getPrincipal().getName(), team);
Log.infov("{0} created team {1}", identity.getPrincipal().getName(), team);
}

@RolesAllowed(Roles.ADMIN)
@Override public void deleteTeam(String unsafeTeam) {
String team = validateTeamName(unsafeTeam);
backend.get().deleteTeam(team);
LOG.infov("{0} deleted team {1}", identity.getPrincipal().getName(), team);
Log.infov("{0} deleted team {1}", identity.getPrincipal().getName(), team);
}

@RolesAllowed(Roles.ADMIN)
Expand Down Expand Up @@ -161,7 +171,7 @@ public class UserServiceImpl implements UserService {
}
String newPassword = new SecureRandom().ints(RANDOM_PASSWORD_LENGTH, '0', 'z' + 1).collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append).toString();
userInfo.setPassword(newPassword);
LOG.infov("{0} reset password of user {1}", identity.getPrincipal().getName(), username);
Log.infov("{0} reset password of user {1}", identity.getPrincipal().getName(), username);
return newPassword;
}

Expand Down Expand Up @@ -225,6 +235,16 @@ void createLocalUser(String username, String defaultTeam, String password) {
}
}

@Transactional
@WithRoles(fromParams = FirstParameter.class)
void removeLocalUser(String username) {
try {
UserInfo.deleteById(username);
} catch (Exception e) {
// ignore
}
}

// --- //

public static final class FirstParameter implements Function<Object[], String[]> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ private void addTeamMembership(UserInfo userInfo, String teamName, TeamRole role
userInfo.teams.add(new TeamMembership(userInfo, storedTeam.orElseGet(() -> Team.getEntityManager().merge(new Team(teamName))), role));
}

@Transactional
@WithRoles(fromParams = RemoveUserParameterConverter.class)
@Override public void removeUser(String username) {
if (!UserInfo.deleteById(username)) {
throw ServiceException.notFound("User does not exist");
}
}

@Transactional
@Override public List<String> getTeams() {
List<Team> teams = Team.listAll();
Expand Down Expand Up @@ -245,6 +253,15 @@ public static final class NewUserParameterConverter implements Function<Object[]
}
}

/**
* Extract usernames from parameters of `removeUser()`
*/
public static final class RemoveUserParameterConverter implements Function<Object[], String[]> {
@Override public String[] apply(Object[] objects) {
return new String[] {(String) objects[0]};
}
}

/**
* Extract usernames from parameters of `updateTeamMembers()`
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,20 @@ private static boolean isTeam(String role) {
}
}

@Override public void removeUser(String username) {
try (Response response = keycloak.realm(realm).users().delete(findMatchingUserId(username))) {
if (response.getStatusInfo().getFamily() != Response.Status.Family.SUCCESSFUL) {
LOG.warnv("Got {0} response for removing user {0}", response.getStatusInfo(), username);
throw ServiceException.serverError(format("Unable to remove user {0}", username));
}
} catch (ServiceException se) {
throw se; // thrown above, re-throw
} catch (Throwable t) {
LOG.warnv(t, "Unable to remove user {0}", username);
throw ServiceException.serverError(format("Unable to remove user {0}", username));
}
}

private static UserRepresentation convertUserRepresentation(UserService.NewUser user) {
UserRepresentation rep = new UserRepresentation();
rep.setUsername(user.user.username);
Expand Down Expand Up @@ -167,7 +181,7 @@ private String findMatchingUserId(String username) { // find the clientID of a s
List<UserRepresentation> matchingUsers = keycloak.realm(realm).users().search(username, true);
if (matchingUsers == null || matchingUsers.isEmpty()) {
LOG.warnv("Cannot find user with username {0}", username);
throw ServiceException.badRequest(format("User {0} does not exist", username));
throw ServiceException.notFound(format("User {0} does not exist", username));
} else if (matchingUsers.size() > 1) {
LOG.warnv("Multiple matches for exact search for username {0}: {1}", username, matchingUsers.stream().map(UserRepresentation::getId).collect(joining(" ")));
throw ServiceException.serverError(format("More than one user with username {0}", username));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public interface UserBackEnd {

void createUser(UserService.NewUser user);

void removeUser(String username);

List<String> getTeams();

Map<String, List<String>> teamMembers(String team);
Expand All @@ -35,5 +37,4 @@ public interface UserBackEnd {
List<UserService.UserData> machineAccounts(String team);

void setPassword(String username, String password);

}
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ private void overrideTestSecurity(String name, Set<String> roles, Runnable runna
// user should be able to authenticate now
given().auth().preemptive().basic(machineUser, "whatever").get("api/user/roles").then().statusCode(SC_UNAUTHORIZED);
given().auth().preemptive().basic(machineUser, newPassword).get("api/user/roles").then().statusCode(SC_OK);

// manager remove account
userService.removeUser(machineUser);
assertThrows(ServiceException.class, () -> userService.removeUser(machineUser));
});

userService.deleteTeam(testTeam);
Expand Down Expand Up @@ -429,6 +433,13 @@ private void assertUserSearch(String search, String... expected) {
// test attempt to set admin role
assertFalse(userService.administrators().stream().anyMatch(data -> thirdUser.equals(data.username)));

// test remove user
userService.removeUser(secondUser);
assertThrows(ServiceException.class, () -> userService.removeUser("some-non-existent-user"));

// test recreate
userService.createUser(anotherUser);

// delete team
userService.deleteTeam(testTeam);
}
Expand All @@ -439,6 +450,7 @@ private void assertUserSearch(String search, String... expected) {
assertThrows(UnauthorizedException.class, () -> userService.searchUsers(null));
assertThrows(UnauthorizedException.class, () -> userService.info(null));
assertThrows(UnauthorizedException.class, () -> userService.createUser(null));
assertThrows(UnauthorizedException.class, () -> userService.removeUser(null));
assertThrows(UnauthorizedException.class, userService::getTeams);
assertThrows(UnauthorizedException.class, userService::defaultTeam);
assertThrows(UnauthorizedException.class, () -> userService.setDefaultTeam(null));
Expand All @@ -448,15 +460,16 @@ private void assertUserSearch(String search, String... expected) {
assertThrows(UnauthorizedException.class, () -> userService.addTeam(null));
assertThrows(UnauthorizedException.class, () -> userService.deleteTeam(null));
assertThrows(UnauthorizedException.class, userService::administrators);
assertThrows(UnauthorizedException.class, () -> userService.updateAdministrators(null));
assertThrows(UnauthorizedException.class, () -> userService.updateAdministrators(List.of()));
assertThrows(UnauthorizedException.class, () -> userService.resetPassword(null, null));

// user authenticated but without the necessary privileges
overrideTestSecurity("unprivileged-user", Set.of(), () -> {
assertThrows(ForbiddenException.class, userService::getAllTeams);
assertThrows(ForbiddenException.class, () -> userService.addTeam(null));
assertThrows(ForbiddenException.class, () -> userService.deleteTeam(null));
assertThrows(ForbiddenException.class, userService::administrators);
assertThrows(ForbiddenException.class, () -> userService.updateAdministrators(null));
assertThrows(ForbiddenException.class, () -> userService.updateAdministrators(List.of()));
assertThrows(ForbiddenException.class, () -> userService.searchUsers(null));
assertThrows(ForbiddenException.class, () -> userService.info(new ArrayList<>()));
});
Expand Down
6 changes: 5 additions & 1 deletion horreum-web/src/domain/admin/Admin.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {useRef} from "react"
import {Card, CardBody, NavItem, PageSection} from "@patternfly/react-core"
import {Card, CardBody, PageSection} from "@patternfly/react-core"

import SavedTabs, {SavedTab, TabFunctions, saveFunc, resetFunc, modifiedFunc} from "../../components/SavedTabs"
import FragmentTabs, {FragmentTab} from "../../components/FragmentTabs"
Expand All @@ -12,6 +12,7 @@ import Administrators from "./Administrators"
import {useSelector} from "react-redux";
import {isAdminSelector, isManagerSelector} from "../../auth";
import Datastores from "./Datastores";
import RemoveUsers from "./RemoveUsers";

export default function Admin() {
const adminFuncsRef = useRef<TabFunctions>()
Expand Down Expand Up @@ -45,6 +46,9 @@ export default function Admin() {
>
<Teams funcs={teamsFuncsRef}/>
</SavedTab>
<FragmentTab title="Remove Users" fragment="remove-users">
<RemoveUsers/>
</FragmentTab>
<FragmentTab title="Global Actions" fragment="actions">
<AllowedSiteList/>
<br/>
Expand Down
71 changes: 71 additions & 0 deletions horreum-web/src/domain/admin/RemoveUsers.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import UserSearch from "../../components/UserSearch";
import {useContext, useState} from "react";
import {UserData} from "../../generated";
import {
Button,
DataList,
DataListAction,
DataListCell,
DataListItem,
DataListItemCells,
DataListItemRow,
Form,
FormGroup
} from "@patternfly/react-core";
import {userName} from "../../auth";
import {userApi} from "../../api";
import {AppContext} from "../../context/appContext";
import {AppContextType} from "../../context/@types/appContextTypes";

export default function RemoveUsers() {
const [users, setUsers] = useState<UserData[]>([])
const {alerting} = useContext(AppContext) as AppContextType;

return (
<>
<Form isHorizontal>
<FormGroup label="Available users" fieldId="users">
<UserSearch
key={0}
onUsers={users => {
setUsers(users)
}}
/>
</FormGroup>
<DataList aria-label="Users" isCompact={true}>
{users.map((u, i) =>
<DataListItem key={"user-" + i} aria-labelledby={"user-" + i}>
<DataListItemRow>
<DataListItemCells
dataListCells={[
<DataListCell key={"content" + i}>
<span id={"user-" + i}>{userName(u)}</span>
</DataListCell>,
]}
/>
<DataListAction key={"action" + i} aria-labelledby={"remove-user-" + i}
aria-label={"remove-user-" + i} id={"remove-user-" + i}>
<Button
variant="danger" size="sm" key={"remove-user" + i}
onClick={() => {
if (confirm("Are you sure you want to remove user " + userName(u) + "?")) {
userApi.removeUser(u.username).then(
_ => {
setUsers(users.filter(user => user != u));
},
error => alerting.dispatchError(error, "REMOVE_ACCOUNT", "Failed to remove account")
)
}
}}
>
Remove
</Button>
</DataListAction>
</DataListItemRow>
</DataListItem>
)}
</DataList>
</Form>
</>
)
}
16 changes: 16 additions & 0 deletions horreum-web/src/domain/user/MachineAccounts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,22 @@ export default function ManageMachineAccounts(props: ManageMachineAccountsProps)
Reset Password
</Button>
</DataListAction>
<DataListAction key={"action" + i} aria-labelledby={"remove-account-" + i}
aria-label={"remove-account-" + i} id={"remove-account-" + i}>
<Button
variant="danger" size="sm" key={"remove-account" + i}
onClick={() => {
if (confirm("Are you sure you want to remove account " + userName(u) + "?")) {
userApi.removeUser(u.username).then(
_ => setMachineUsers(machineUsers.filter(user => user != u)),
error => alerting.dispatchError(error, "REMOVE_ACCOUNT", "Failed to remove account")
)
}
}}
>
Remove Account
</Button>
</DataListAction>
</DataListItemRow>
</DataListItem>
)}
Expand Down

0 comments on commit cf4fece

Please sign in to comment.