From 54a256010d73c60d7332229419ae92face6d7872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Kr=C3=BCmpelmann?= Date: Thu, 22 Oct 2026 06:24:15 +0200 Subject: [PATCH 1/2] feat(user): manage groups over HTTP and in the CLI GroupRepository could create and delete groups and maintain sub-group relations, but nothing exposed it: UserController only had GET routes, which is why the AddGroup/RemoveGroup/UpdateGroup enum slots in the CLI were dead code and were removed. Four routes now cover it (Admin-User): POST/DELETE /user/groups and POST/DELETE /user/groups/{name}/subgroups/{sub}, with the matching UserApi methods and the CLI commands group_add, group_remove, group_add_subgroup and group_remove_subgroup. Deleting a group that some other group contains was broken: deleteOf removed the groups row but left the group_groups_relation rows behind, so the next initGroups() dereferenced a missing group (m_groupMap.at) and the process could not start. It now removes the relations in both directions, scrubs the in-memory member lists and has a test that restarts the repository on the same database. --- CHANGELOG.md | 9 + .../base/repositories/GroupRepository.cpp | 26 ++ .../components/UserManagementCliComponent.cpp | 71 +++- src/features/http/controllers/UserApi.cpp | 73 ++++ .../http/controllers/UserController.cpp | 248 +++++++++++++ .../components/UserManagementCliComponent.h | 9 + .../features/http/controllers/UserApi.h | 31 ++ .../http/controllers/UserController.h | 8 + swagger.md | 139 +++++++ swagger.yaml | 140 +++++++ .../UserManagementCliComponentMockTest.cpp | 144 ++++++++ tests/http/UserApiTest.cpp | 70 ++++ tests/http/UserControllerTest.cpp | 348 +++++++++++++++++- tests/mocks/MockUserApi.h | 4 + .../repositories/UserGroupRepositoryTest.cpp | 28 ++ 15 files changed, 1342 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72284afb..69fb516c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Group management over HTTP and in the CLI.** `POST /user/groups`, `DELETE /user/groups/{name}` + and `POST`/`DELETE` `/user/groups/{name}/subgroups/{sub}` (all `Admin-User`) expose the group + operations `GroupRepository` already had; the CLI gains `group_add`, `group_remove`, + `group_add_subgroup` and `group_remove_subgroup`. Virtual groups stay code-defined and are + rejected with 400, and a group that a user still belongs to cannot be deleted (409). - **Migration hook for shared memory segments.** `lintel::shm::SegmentLayoutPolicy` lets the application decide what happens when a segment was written with another object layout: refuse (default), recreate, or run a registered migrator that sees the untouched segment file and can @@ -297,6 +302,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Hypodermic was an unmaintained dependency; `lintel::di` replaces it. ### Fixed +- **Deleting a group left dangling sub-group relations.** `GroupRepository::deleteOf()` removed the + `groups` row but not the `group_groups_relation` rows pointing at it, so the next `initGroups()` + dereferenced the gone group (`m_groupMap.at`) and aborted the start. It now removes the relations + in both directions and drops the group from the member lists of its parents. - **The SHM message queue was not an IPC mechanism.** Every process built its own ring in the segment, so `MessageQueueShmCore` only worked inside one process. A fork-based test now proves that a child process finds the queue by name and that a message crosses the boundary. diff --git a/src/features/base/repositories/GroupRepository.cpp b/src/features/base/repositories/GroupRepository.cpp index 62a26230..350cc942 100644 --- a/src/features/base/repositories/GroupRepository.cpp +++ b/src/features/base/repositories/GroupRepository.cpp @@ -175,12 +175,38 @@ void GroupRepository::deleteOf(const Group &group) "group_name = ?", builder); if (result.getSize() > 0) { throw std::runtime_error("cannot remove group which is in usage of an user"); } + // A group can be the sub-group of another one. Those rows point at it from + // both directions, and leaving them behind makes the next initGroups() + // dereference a group that no longer exists (m_groupMap.at). + db::ParameterBuilder relationBuilder(m_connectionEntry); + relationBuilder.add(group.getGroupName()).add(group.getGroupName()); + statement.execute("delete from group_groups_relation where group_name = ? or base_group_name = ?", relationBuilder); statement.execute("delete from groups where name = ?", builder); m_groupMap.erase(group.getGroupName()); m_groups.erase( std::remove_if( m_groups.begin(), m_groups.end(), [&](const Group &g) { return g.getGroupName() == group.getGroupName(); }), m_groups.end()); + // drop it from the member lists of the groups that contained it, so that + // allOf() does not keep reporting a group that is gone + const auto withoutDeleted = [&group](const std::vector &members) { + std::vector kept; + kept.reserve(members.size()); + for (const auto &member : members) { + if (member.getGroupName() != group.getGroupName()) { kept.push_back(member); } + } + return kept; + }; + for (auto &[name, candidate] : m_groupMap) { + const std::vector members = withoutDeleted(candidate.getGroups()); + if (members.size() == candidate.getGroups().size()) { continue; } + candidate = Group(candidate.getGroupName(), members, candidate.isVirtual()); + } + for (auto &candidate : m_groups) { + const std::vector members = withoutDeleted(candidate.getGroups()); + if (members.size() == candidate.getGroups().size()) { continue; } + candidate = Group(candidate.getGroupName(), members, candidate.isVirtual()); + } } void GroupRepository::addGroupOf(const Group &group, const Group &add) diff --git a/src/features/cli/components/UserManagementCliComponent.cpp b/src/features/cli/components/UserManagementCliComponent.cpp index c97d85fd..b5efef8e 100644 --- a/src/features/cli/components/UserManagementCliComponent.cpp +++ b/src/features/cli/components/UserManagementCliComponent.cpp @@ -17,6 +17,25 @@ UserManagementCliComponent::UserManagementCliComponent(std::shared_ptr .addArgument( { "--is-virtual", "-v" }, &m_isVirtualGroup, "Limits search result to virtual or non virtual groups"), AllGroups); + m_commandParser.addCommand( + Command("group_add", "Creates a group!") + .addArgument({ "--group", "-g" }, &m_groupName, "Name of the group which should be created", false) + .addArgument({ "--virtual", "-v" }, &m_isVirtualGroup, "Creates a virtual group", true), + AddGroup); + m_commandParser.addCommand( + Command("group_remove", "Deletes a group!") + .addArgument({ "--group", "-g" }, &m_groupName, "Name of the group which should be deleted", false), + RemoveGroup); + m_commandParser.addCommand( + Command("group_add_subgroup", "Adds a virtual group as member of a collecting group!") + .addArgument({ "--group", "-g" }, &m_groupName, "Name of the collecting group", false) + .addArgument({ "--subgroup", "-s" }, &m_subGroupName, "Name of the virtual group to add", false), + AddSubGroup); + m_commandParser.addCommand( + Command("group_remove_subgroup", "Removes a virtual group from a collecting group!") + .addArgument({ "--group", "-g" }, &m_groupName, "Name of the collecting group", false) + .addArgument({ "--subgroup", "-s" }, &m_subGroupName, "Name of the virtual group to remove", false), + RemoveSubGroup); m_commandParser.addCommand( Command("show_users", "Show all available users!") .addArgument( @@ -60,10 +79,12 @@ UserManagementCliComponent::UserManagementCliComponent(std::shared_ptr * Dispatches an entered CLI command to the UserApi. * * Flow: the parser splits the input into a command and its parameters, then a - * switch over the eight commands executes the matching UserApi operation and - * prints the result. All filters (group name, virtual group) are taken from the - * previously set session context, not from the parameters - the CLI session - * carries them. + * switch over the registered commands executes the matching UserApi operation + * and prints the result. Filter flags (group name, virtual group) of the + * listing commands are taken from the parsed parameters of that command, not + * from previously executed ones - the parser resets them for every command. + * The group management commands take the target names from the parameters as + * well. * * Every call is wrapped in a try/catch: a failed UserApi operation should not * end the menu loop but show up as an error message. The handler therefore @@ -80,7 +101,7 @@ void UserManagementCliComponent::onCommand(const UserDto &userDto, try { Commands command = m_commandParser.parse(input, parameters); switch (command) { - // groups: filters come from the session context, not from the parameters. + // groups: the four listing filter combinations of name and virtual flag. case AllGroups: if (!m_groupName.has_value() && !m_isVirtualGroup.has_value()) { printGroups(m_userApi->allOf()); @@ -92,6 +113,46 @@ void UserManagementCliComponent::onCommand(const UserDto &userDto, printGroups(m_userApi->allOf(m_groupName.value(), m_isVirtualGroup.value())); } break; + // groups: create and delete take the name from the parameters; the + // virtual flag is only meaningful for the creation. + case AddGroup: { + if (!m_groupName.has_value()) { + std::cerr << "ERROR: please set the group name\n"; + break; + } + GroupDto groupDto(Group(m_groupName.value(), {}, m_isVirtualGroup.value_or(false))); + if (!m_userApi->createGroupOf(groupDto)) { std::cerr << "ERROR: failed to create group\n"; } + break; + } + case RemoveGroup: { + if (!m_groupName.has_value()) { + std::cerr << "ERROR: please set the group name\n"; + break; + } + if (!m_userApi->deleteGroupOf(m_groupName.value())) { std::cerr << "ERROR: failed to delete group\n"; } + break; + } + // subgroups: a collecting group plus the virtual group to add or remove. + case AddSubGroup: { + if (!m_groupName.has_value() || !m_subGroupName.has_value()) { + std::cerr << "ERROR: please set the group and subgroup name\n"; + break; + } + if (!m_userApi->addSubGroupOf(m_groupName.value(), m_subGroupName.value())) { + std::cerr << "ERROR: failed to add subgroup\n"; + } + break; + } + case RemoveSubGroup: { + if (!m_groupName.has_value() || !m_subGroupName.has_value()) { + std::cerr << "ERROR: please set the group and subgroup name\n"; + break; + } + if (!m_userApi->removeSubGroupOf(m_groupName.value(), m_subGroupName.value())) { + std::cerr << "ERROR: failed to remove subgroup\n"; + } + break; + } // users: same, the same four filter combinations. case AllUsers: { if (m_userName.has_value()) { diff --git a/src/features/http/controllers/UserApi.cpp b/src/features/http/controllers/UserApi.cpp index d50f46a9..7e631379 100644 --- a/src/features/http/controllers/UserApi.cpp +++ b/src/features/http/controllers/UserApi.cpp @@ -248,6 +248,79 @@ std::vector UserApi::allOf(bool isVirtualGroup) return groupsDto.getGroups(); } +bool UserApi::createGroupOf(const GroupDto &groupDto) +{ + std::string body; + try { + body = groupDto.JsonSerializable::serialize(); + } catch (const std::exception &exception) { + LOG_ERROR("failed to create group {}", groupDto.getGroupName()); + return false; + } + httplib::Result result = m_client->post("/user/groups", body, "application/json"); + if (!HttpClientHelper::hasResponse(result)) { return false; } + HttpStatusCodes status(result->status); + switch (status) { + case HttpStatusCodes::Unauthorized: + throw HttpUnauthorizedException(); + case HttpStatusCodes::OK: + return true; + default: + LOG_ERROR("failed to create group {} ({})", groupDto.getGroupName(), status.getCode()); + return false; + } +} + +bool UserApi::deleteGroupOf(const std::string &groupName) +{ + httplib::Result result = m_client->deletes("/user/groups/" + groupName, "", "application/json"); + if (!HttpClientHelper::hasResponse(result)) { return false; } + HttpStatusCodes status(result->status); + switch (status) { + case HttpStatusCodes::Unauthorized: + throw HttpUnauthorizedException(); + case HttpStatusCodes::OK: + return true; + default: + LOG_ERROR("failed to delete group {} ({})", groupName, status.getCode()); + return false; + } +} + +bool UserApi::addSubGroupOf(const std::string &groupName, const std::string &subGroupName) +{ + httplib::Result result = + m_client->post("/user/groups/" + groupName + "/subgroups/" + subGroupName, "", "application/json"); + if (!HttpClientHelper::hasResponse(result)) { return false; } + HttpStatusCodes status(result->status); + switch (status) { + case HttpStatusCodes::Unauthorized: + throw HttpUnauthorizedException(); + case HttpStatusCodes::OK: + return true; + default: + LOG_ERROR("failed to add group {} to {} ({})", subGroupName, groupName, status.getCode()); + return false; + } +} + +bool UserApi::removeSubGroupOf(const std::string &groupName, const std::string &subGroupName) +{ + httplib::Result result = + m_client->deletes("/user/groups/" + groupName + "/subgroups/" + subGroupName, "", "application/json"); + if (!HttpClientHelper::hasResponse(result)) { return false; } + HttpStatusCodes status(result->status); + switch (status) { + case HttpStatusCodes::Unauthorized: + throw HttpUnauthorizedException(); + case HttpStatusCodes::OK: + return true; + default: + LOG_ERROR("failed to remove group {} from {} ({})", subGroupName, groupName, status.getCode()); + return false; + } +} + void UserApi::createOf(const UserDto &userDto) { std::string body; diff --git a/src/features/http/controllers/UserController.cpp b/src/features/http/controllers/UserController.cpp index 0a84e59b..3835670e 100644 --- a/src/features/http/controllers/UserController.cpp +++ b/src/features/http/controllers/UserController.cpp @@ -1,9 +1,14 @@ #include "lintel/features/http/controllers/UserController.h" +#include +#include +#include + #include #include #include "lintel/core/utils/Cryption.h" +#include "lintel/core/utils/JsonUtils.h" #include "lintel/core/utils/StringUtils.h" #include "lintel/features/base/controller/UserDto.h" #include "lintel/features/base/controller/UserGroupDto.h" @@ -16,6 +21,53 @@ #include "lintel/features/http/service/HttpStatusCodes.h" namespace lintel { +namespace { + /** + * @return {"group_name":"","removed":} + * + * rapidjson writes the body so that a group name coming from the request + * (quotes, backslashes, control characters) cannot break the JSON. + */ + std::string userGroupRemovedBody(const std::string &groupName, bool removed) + { + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + writer.StartObject(); + writer.Key("group_name"); + writer.String(groupName.c_str(), static_cast(groupName.size())); + writer.Key("removed"); + writer.Bool(removed); + writer.EndObject(); + return std::string(buffer.GetString(), buffer.GetSize()); + } + + /** @return {"group_name":"","sub_group_name":"","":} */ + std::string + userSubGroupBody(const std::string &groupName, const std::string &subGroupName, const char *key, bool value) + { + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + writer.StartObject(); + writer.Key("group_name"); + writer.String(groupName.c_str(), static_cast(groupName.size())); + writer.Key("sub_group_name"); + writer.String(subGroupName.c_str(), static_cast(subGroupName.size())); + writer.Key(key); + writer.Bool(value); + writer.EndObject(); + return std::string(buffer.GetString(), buffer.GetSize()); + } + + /** @return true if the group already lists @p subGroupName as a member */ + bool hasSubGroupOf(const Group &group, const std::string &subGroupName) + { + const std::vector &members = group.getGroups(); + return std::any_of(members.begin(), members.end(), [&subGroupName](const Group &member) -> bool { + return member.getGroupName() == subGroupName; + }); + } +}// namespace + void UserController::loginOfPost(const httplib::Request &request, httplib::Response &response, const ContentType &contentType, @@ -163,6 +215,202 @@ void UserController::allGroupsOfGroupNameAndIsVirtualGroupGet(const httplib::Req } } +void UserController::createGroupOfPost(const httplib::Request &request, + httplib::Response &response, + const ContentType &contentType, + const std::optional &user) +{ + if (!user.has_value()) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (!user->m_user.has(m_adminUser)) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (contentType != ContentType::ApplicationJson) { + response.status = HttpStatusCodes::Forbidden; + response.set_content("", contentType.getName()); + return; + } + rapidjson::Document document; + document.Parse(request.body.c_str()); + if (document.HasParseError() || !document.IsObject()) { + LOG_WARN("{} sent an unparsable group body", user->m_user.getUserName()); + response.status = HttpStatusCodes::BadRequest; + response.set_content("", contentType.getName()); + return; + } + std::string groupName; + if (!json::getString(document, "group_name", groupName) || groupName.empty()) { + LOG_WARN("{} sent a group without a name", user->m_user.getUserName()); + response.status = HttpStatusCodes::BadRequest; + response.set_content("", contentType.getName()); + return; + } + // The virtual flag is optional and defaults to a regular (persisted) group. + bool isVirtual = false; + json::getBool(document, "virtual", isVirtual); + if (isVirtual) { + // Virtual groups carry access rights and are defined in the code, so the + // repository rejects creating them over the API. + LOG_WARN("{} tried to create the virtual group {}", user->m_user.getUserName(), groupName); + response.status = HttpStatusCodes::BadRequest; + response.set_content("", contentType.getName()); + return; + } + if (m_groupRepository->of(groupName).has_value()) { + LOG_WARN("{} tried to create the existing group {}", user->m_user.getUserName(), groupName); + response.status = HttpStatusCodes::Conflict; + response.set_content("", contentType.getName()); + return; + } + const Group group(groupName, {}, false); + m_groupRepository->createOf(group); + GroupDto groupDto(group); + response.status = HttpStatusCodes::OK; + response.set_content(groupDto.JsonSerializable::serialize(), contentType.getName()); +} + +void UserController::deleteGroupOfDelete(const httplib::Request &request, + httplib::Response &response, + const ContentType &contentType, + const std::optional &user) +{ + if (!user.has_value()) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (!user->m_user.has(m_adminUser)) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (contentType != ContentType::ApplicationJson) { + response.status = HttpStatusCodes::Forbidden; + response.set_content("", contentType.getName()); + return; + } + const std::string groupName = request.matches[1]; + const std::optional group = m_groupRepository->of(groupName); + if (!group.has_value()) { + response.status = HttpStatusCodes::NotFound; + response.set_content("", contentType.getName()); + return; + } + try { + m_groupRepository->deleteOf(group.value()); + } catch (const std::exception &exception) { + // deleteOf() refuses groups that are still assigned to a user + LOG_WARN("{} failed to delete group {}: {}", user->m_user.getUserName(), groupName, exception.what()); + response.status = HttpStatusCodes::Conflict; + response.set_content("", contentType.getName()); + return; + } + LOG_INFO("{} deleted group {}", user->m_user.getUserName(), groupName); + response.status = HttpStatusCodes::OK; + response.set_content(userGroupRemovedBody(groupName, true), contentType.getName()); +} + +void UserController::addGroupMemberOfPost(const httplib::Request &request, + httplib::Response &response, + const ContentType &contentType, + const std::optional &user) +{ + if (!user.has_value()) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (!user->m_user.has(m_adminUser)) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (contentType != ContentType::ApplicationJson) { + response.status = HttpStatusCodes::Forbidden; + response.set_content("", contentType.getName()); + return; + } + const std::string groupName = request.matches[1]; + const std::string subGroupName = request.matches[2]; + const std::optional group = m_groupRepository->of(groupName); + if (!group.has_value()) { + response.status = HttpStatusCodes::NotFound; + response.set_content("", contentType.getName()); + return; + } + const std::optional subGroup = m_groupRepository->of(subGroupName); + if (!subGroup.has_value()) { + response.status = HttpStatusCodes::NotFound; + response.set_content("", contentType.getName()); + return; + } + if (group->isVirtual()) { + // Virtual groups are the leaves of the hierarchy, they cannot collect members + LOG_WARN("{} tried to add a member to the virtual group {}", user->m_user.getUserName(), groupName); + response.status = HttpStatusCodes::BadRequest; + response.set_content("", contentType.getName()); + return; + } + if (!subGroup->isVirtual()) { + // Only virtual groups can be a member: they carry the access rights + LOG_WARN("{} tried to add the non virtual group {} to {}", user->m_user.getUserName(), subGroupName, groupName); + response.status = HttpStatusCodes::BadRequest; + response.set_content("", contentType.getName()); + return; + } + const bool alreadyMember = hasSubGroupOf(group.value(), subGroupName); + if (!alreadyMember) { m_groupRepository->addGroupOf(group.value(), subGroup.value()); } + response.status = HttpStatusCodes::OK; + response.set_content(userSubGroupBody(groupName, subGroupName, "added", !alreadyMember), contentType.getName()); +} + +void UserController::removeGroupMemberOfDelete(const httplib::Request &request, + httplib::Response &response, + const ContentType &contentType, + const std::optional &user) +{ + if (!user.has_value()) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (!user->m_user.has(m_adminUser)) { + response.status = HttpStatusCodes::Unauthorized; + response.set_content("", contentType.getName()); + return; + } + if (contentType != ContentType::ApplicationJson) { + response.status = HttpStatusCodes::Forbidden; + response.set_content("", contentType.getName()); + return; + } + const std::string groupName = request.matches[1]; + const std::string subGroupName = request.matches[2]; + const std::optional group = m_groupRepository->of(groupName); + if (!group.has_value()) { + response.status = HttpStatusCodes::NotFound; + response.set_content("", contentType.getName()); + return; + } + const bool wasMember = hasSubGroupOf(group.value(), subGroupName); + if (wasMember) { + const std::optional subGroup = m_groupRepository->of(subGroupName); + if (!subGroup.has_value()) { + response.status = HttpStatusCodes::NotFound; + response.set_content("", contentType.getName()); + return; + } + m_groupRepository->removeGroupOf(group.value(), subGroup.value()); + } + response.status = HttpStatusCodes::OK; + response.set_content(userSubGroupBody(groupName, subGroupName, "removed", wasMember), contentType.getName()); +} + void UserController::allUsersOfGet(const httplib::Request &request, httplib::Response &response, const ContentType &contentType, diff --git a/src/include/lintel/features/cli/components/UserManagementCliComponent.h b/src/include/lintel/features/cli/components/UserManagementCliComponent.h index b5feb162..40943c25 100644 --- a/src/include/lintel/features/cli/components/UserManagementCliComponent.h +++ b/src/include/lintel/features/cli/components/UserManagementCliComponent.h @@ -24,6 +24,10 @@ class UserManagementCliComponent : public CommandLineComponent Undefined, // group operations AllGroups, + AddGroup, + RemoveGroup, + AddSubGroup, + RemoveSubGroup, // user operations AllUsers, AddUser, @@ -45,6 +49,11 @@ class UserManagementCliComponent : public CommandLineComponent */ std::optional m_groupName; std::optional m_isVirtualGroup; + /* + * Command: AddSubGroup + * Command: RemoveSubGroup + */ + std::optional m_subGroupName; /* * Command: AllUsers * Command: AddUser diff --git a/src/include/lintel/features/http/controllers/UserApi.h b/src/include/lintel/features/http/controllers/UserApi.h index 5247eee6..a7dcab76 100644 --- a/src/include/lintel/features/http/controllers/UserApi.h +++ b/src/include/lintel/features/http/controllers/UserApi.h @@ -6,6 +6,7 @@ #include #include +#include "lintel/features/base/controller/GroupDto.h" #include "lintel/features/base/controller/UserDto.h" #include "lintel/features/base/controller/UserLoginDto.h" #include "lintel/features/base/controller/UserPasswordChangeDto.h" @@ -66,6 +67,36 @@ class UserApi /** @param isVirtualGroup filter by virtual flag; return matching groups */ virtual std::vector allOf(bool isVirtualGroup); + /** + * Create a group. + * @param groupDto name and virtual flag of the new group + * @return true on success + */ + virtual bool createGroupOf(const GroupDto &groupDto); + + /** + * Delete a group. + * @param groupName name of the group to delete + * @return true on success + */ + virtual bool deleteGroupOf(const std::string &groupName); + + /** + * Add a virtual group as member of a collecting group. + * @param groupName name of the collecting group + * @param subGroupName name of the virtual group to add + * @return true on success + */ + virtual bool addSubGroupOf(const std::string &groupName, const std::string &subGroupName); + + /** + * Remove a virtual group from a collecting group. + * @param groupName name of the collecting group + * @param subGroupName name of the virtual group to remove + * @return true on success + */ + virtual bool removeSubGroupOf(const std::string &groupName, const std::string &subGroupName); + /** @return all users */ virtual std::vector allUsersOf(); diff --git a/src/include/lintel/features/http/controllers/UserController.h b/src/include/lintel/features/http/controllers/UserController.h index 71b28984..e58f422a 100644 --- a/src/include/lintel/features/http/controllers/UserController.h +++ b/src/include/lintel/features/http/controllers/UserController.h @@ -29,6 +29,14 @@ class UserController : public Controller ADD_HANDLER_METHOD(R"(/user/groups/([^\/]+)/([^\/]+))", Get, allGroupsOfGroupNameAndIsVirtualGroup); + ADD_HANDLER_METHOD(R"(/user/groups)", Post, createGroupOf); + + ADD_HANDLER_METHOD(R"(/user/groups/([^\/]+))", Delete, deleteGroupOf); + + ADD_HANDLER_METHOD(R"(/user/groups/([^\/]+)/subgroups/([^\/]+))", Post, addGroupMemberOf); + + ADD_HANDLER_METHOD(R"(/user/groups/([^\/]+)/subgroups/([^\/]+))", Delete, removeGroupMemberOf); + ADD_HANDLER_METHOD(R"(/user/users)", Get, allUsersOf); ADD_HANDLER_METHOD(R"(/user/users/([^\/]+))", Get, allUsersOfUserName); diff --git a/swagger.md b/swagger.md index b39a06d6..aad773d0 100644 --- a/swagger.md +++ b/swagger.md @@ -1093,6 +1093,38 @@ Get all groups | --------------- | ------ | | bearerAuth | | +#### POST +##### Summary + +Create a group + +##### Description + +Creates a non-virtual (collecting) group. Virtual groups carry the access rights and are defined in the code itself, so "virtual": true is rejected with 400. The response is the created GroupDto. + +##### Parameters + +| Name | Located in | Description | Required | Schema | +| ---- | ---------- | ----------- | -------- | ------ | +| body | body | | Yes | GroupDto | + +##### Responses + +| Code | Description | +| ---- | ----------- | +| 200 | A successful response (the created GroupDto) | +| 400 | Bad request (malformed body, empty group_name, or virtual=true) | +| 401 | Unauthorized | +| 403 | Forbidden (wrong content type) | +| 409 | Conflict (a group with this name already exists) | +| 500 | Internal server error | + +##### Security + +| Security Schema | Scopes | +| --------------- | ------ | +| bearerAuth | | + ### /user/groups/{groupNameOrIsVirtual} #### GET @@ -1121,6 +1153,38 @@ Get all groups by name or virtual flag | --------------- | ------ | | bearerAuth | | +#### DELETE +##### Summary + +Delete a group + +##### Description + +Deletes the group named by the path segment (the parameter name is shared with the GET operation of this path). The response reports the outcome as {"group_name":"","removed":true}. + +##### Parameters + +| Name | Located in | Description | Required | Schema | +| ---- | ---------- | ----------- | -------- | ------ | +| groupNameOrIsVirtual | path | Name of the group to delete | Yes | string | + +##### Responses + +| Code | Description | +| ---- | ----------- | +| 200 | A successful response ({"group_name":...,"removed":true}) | +| 401 | Unauthorized | +| 403 | Forbidden (wrong content type) | +| 404 | Unknown group | +| 409 | Conflict (the group is still assigned to a user) | +| 500 | Internal server error | + +##### Security + +| Security Schema | Scopes | +| --------------- | ------ | +| bearerAuth | | + ### /user/groups/{groupName}/{isVirtual} #### GET @@ -1150,6 +1214,73 @@ Get all groups by name and virtual flag | --------------- | ------ | | bearerAuth | | +### /user/groups/{groupName}/subgroups/{subGroupName} + +#### POST +##### Summary + +Add a subgroup to a group + +##### Description + +Adds a virtual group as member of a non-virtual (collecting) group. Both groups must exist; the parent group must not be virtual and the added group must be virtual, otherwise the request is rejected with 400. Adding an existing member is idempotent: the response then reports "added": false. + +##### Parameters + +| Name | Located in | Description | Required | Schema | +| ---- | ---------- | ----------- | -------- | ------ | +| groupName | path | Collecting group | Yes | string | +| subGroupName | path | Virtual group to add | Yes | string | + +##### Responses + +| Code | Description | +| ---- | ----------- | +| 200 | A successful response ({"group_name":...,"sub_group_name":...,"added":true\|false}) | +| 400 | Bad request (virtual parent group or non-virtual subgroup) | +| 401 | Unauthorized | +| 403 | Forbidden (wrong content type) | +| 404 | Unknown group or subgroup | +| 500 | Internal server error | + +##### Security + +| Security Schema | Scopes | +| --------------- | ------ | +| bearerAuth | | + +#### DELETE +##### Summary + +Remove a subgroup from a group + +##### Description + +Removes a virtual group from a non-virtual (collecting) group. Removing a member that is not listed is idempotent: the response then reports "removed": false. + +##### Parameters + +| Name | Located in | Description | Required | Schema | +| ---- | ---------- | ----------- | -------- | ------ | +| groupName | path | Collecting group | Yes | string | +| subGroupName | path | Virtual group to remove | Yes | string | + +##### Responses + +| Code | Description | +| ---- | ----------- | +| 200 | A successful response ({"group_name":...,"sub_group_name":...,"removed":true\|false}) | +| 401 | Unauthorized | +| 403 | Forbidden (wrong content type) | +| 404 | Unknown group or subgroup | +| 500 | Internal server error | + +##### Security + +| Security Schema | Scopes | +| --------------- | ------ | +| bearerAuth | | + ### /user/users #### GET @@ -1491,6 +1622,14 @@ Update a property ## Models +### GroupDto + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| group_name | string | Unique group name | Yes | +| groups | array[GroupDto] | Members of a collecting group (omitted while empty) | No | +| virtual | boolean | True for virtual groups, which are defined in the code and not persisted | No | + ### EventBusDto | Name | Type | Description | Required | diff --git a/swagger.yaml b/swagger.yaml index 0b5cc439..a10625a5 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -1250,6 +1250,33 @@ paths: description: Forbidden (wrong content type) "500": description: Internal server error + post: + summary: Create a group + description: | + Creates a non-virtual (collecting) group. Virtual groups carry the access + rights and are defined in the code itself, so "virtual": true is rejected + with 400. The response is the created GroupDto. + security: + - bearerAuth: [] + parameters: + - name: body + in: body + required: true + schema: + $ref: "#/definitions/GroupDto" + responses: + "200": + description: A successful response (the created GroupDto) + "400": + description: Bad request (malformed body, empty group_name, or virtual=true) + "401": + description: Unauthorized + "403": + description: Forbidden (wrong content type) + "409": + description: Conflict (a group with this name already exists) + "500": + description: Internal server error /user/groups/{groupNameOrIsVirtual}: get: summary: Get all groups by name or virtual flag @@ -1270,6 +1297,33 @@ paths: description: Forbidden (wrong content type) "500": description: Internal server error + delete: + summary: Delete a group + description: | + Deletes the group named by the path segment (the parameter name is shared + with the GET operation of this path). The response reports the outcome + as {"group_name":"","removed":true}. + security: + - bearerAuth: [] + parameters: + - name: groupNameOrIsVirtual + in: path + required: true + type: string + description: Name of the group to delete + responses: + "200": + description: A successful response ({"group_name":...,"removed":true}) + "401": + description: Unauthorized + "403": + description: Forbidden (wrong content type) + "404": + description: Unknown group + "409": + description: Conflict (the group is still assigned to a user) + "500": + description: Internal server error /user/groups/{groupName}/{isVirtual}: get: summary: Get all groups by name and virtual flag @@ -1293,6 +1347,71 @@ paths: description: Forbidden (wrong content type) "500": description: Internal server error + /user/groups/{groupName}/subgroups/{subGroupName}: + post: + summary: Add a subgroup to a group + description: | + Adds a virtual group as member of a non-virtual (collecting) group. Both + groups must exist; the parent group must not be virtual and the added + group must be virtual, otherwise the request is rejected with 400. + Adding an existing member is idempotent: the response then reports + "added": false. + security: + - bearerAuth: [] + parameters: + - name: groupName + in: path + required: true + type: string + description: Collecting group + - name: subGroupName + in: path + required: true + type: string + description: Virtual group to add + responses: + "200": + description: A successful response ({"group_name":...,"sub_group_name":...,"added":true|false}) + "400": + description: Bad request (virtual parent group or non-virtual subgroup) + "401": + description: Unauthorized + "403": + description: Forbidden (wrong content type) + "404": + description: Unknown group or subgroup + "500": + description: Internal server error + delete: + summary: Remove a subgroup from a group + description: | + Removes a virtual group from a non-virtual (collecting) group. Removing a + member that is not listed is idempotent: the response then reports + "removed": false. + security: + - bearerAuth: [] + parameters: + - name: groupName + in: path + required: true + type: string + description: Collecting group + - name: subGroupName + in: path + required: true + type: string + description: Virtual group to remove + responses: + "200": + description: A successful response ({"group_name":...,"sub_group_name":...,"removed":true|false}) + "401": + description: Unauthorized + "403": + description: Forbidden (wrong content type) + "404": + description: Unknown group or subgroup + "500": + description: Internal server error /user/users: get: summary: Get all users @@ -1588,6 +1707,27 @@ paths: "500": description: Internal server error definitions: + GroupDto: + type: object + description: > + A user group. Non-virtual groups collect virtual groups, which carry the + access rights checked by the controllers. + properties: + group_name: + type: string + description: Unique group name + example: Administrators + groups: + type: array + description: Members of a collecting group (omitted while empty) + items: + $ref: "#/definitions/GroupDto" + virtual: + type: boolean + description: True for virtual groups, which are defined in the code and not persisted + example: false + required: + - group_name EventBusDto: type: object description: > diff --git a/tests/cli/UserManagementCliComponentMockTest.cpp b/tests/cli/UserManagementCliComponentMockTest.cpp index 2acf3ee5..c3335a4d 100644 --- a/tests/cli/UserManagementCliComponentMockTest.cpp +++ b/tests/cli/UserManagementCliComponentMockTest.cpp @@ -387,4 +387,148 @@ TEST_CASE("UserManagementCliComponent: an exception from the api is caught and p std::cerr.rdbuf(oldBuf); REQUIRE(oss.str().find("boom") != std::string::npos); } +TEST_CASE("UserManagementCliComponent: group_add with -g calls createGroupOf") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + REQUIRE_CALL(*mockUserApi, createGroupOf(ANY(const GroupDto &))) + .WITH(_1.getGroupName() == "ops" && !_1.isVirtual()) + .TIMES(1) + .LR_RETURN(true); + + UserDto userDto; + component.onCommand(userDto, "group_add", { "-g", "ops" }); +} + +TEST_CASE("UserManagementCliComponent: group_add with -g -v calls createGroupOf for a virtual group") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + REQUIRE_CALL(*mockUserApi, createGroupOf(ANY(const GroupDto &))) + .WITH(_1.getGroupName() == "ops" && _1.isVirtual()) + .TIMES(1) + .LR_RETURN(true); + + UserDto userDto; + component.onCommand(userDto, "group_add", { "-g", "ops", "-v" }); +} + +TEST_CASE("UserManagementCliComponent: group_add without -g prints error") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + FORBID_CALL(*mockUserApi, createGroupOf(ANY(const GroupDto &))); + + UserDto userDto; + std::ostringstream oss; + auto oldBuf = std::cerr.rdbuf(oss.rdbuf()); + + component.onCommand(userDto, "group_add", {}); + + std::cerr.rdbuf(oldBuf); + REQUIRE(oss.str().find("ERROR") != std::string::npos); +} + +TEST_CASE("UserManagementCliComponent: a failed group_add prints an error") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + REQUIRE_CALL(*mockUserApi, createGroupOf(ANY(const GroupDto &))).TIMES(1).LR_RETURN(false); + + std::ostringstream oss; + auto oldBuf = std::cerr.rdbuf(oss.rdbuf()); + + component.onCommand(UserDto{}, "group_add", { "-g", "ops" }); + + std::cerr.rdbuf(oldBuf); + REQUIRE(oss.str().find("ERROR") != std::string::npos); +} + +TEST_CASE("UserManagementCliComponent: group_remove calls deleteGroupOf") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + REQUIRE_CALL(*mockUserApi, deleteGroupOf(std::string{ "ops" })).TIMES(1).LR_RETURN(true); + + UserDto userDto; + component.onCommand(userDto, "group_remove", { "-g", "ops" }); +} + +TEST_CASE("UserManagementCliComponent: group_remove without -g prints error") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + FORBID_CALL(*mockUserApi, deleteGroupOf(ANY(std::string))); + + UserDto userDto; + std::ostringstream oss; + auto oldBuf = std::cerr.rdbuf(oss.rdbuf()); + + component.onCommand(userDto, "group_remove", {}); + + std::cerr.rdbuf(oldBuf); + REQUIRE(oss.str().find("ERROR") != std::string::npos); +} + +TEST_CASE("UserManagementCliComponent: group_add_subgroup calls addSubGroupOf") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + REQUIRE_CALL(*mockUserApi, addSubGroupOf(std::string{ "ops" }, std::string{ "admin" })).TIMES(1).LR_RETURN(true); + + UserDto userDto; + component.onCommand(userDto, "group_add_subgroup", { "-g", "ops", "-s", "admin" }); +} + +TEST_CASE("UserManagementCliComponent: group_add_subgroup without -s prints error") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + FORBID_CALL(*mockUserApi, addSubGroupOf(ANY(std::string), ANY(std::string))); + + UserDto userDto; + std::ostringstream oss; + auto oldBuf = std::cerr.rdbuf(oss.rdbuf()); + + component.onCommand(userDto, "group_add_subgroup", { "-g", "ops" }); + + std::cerr.rdbuf(oldBuf); + REQUIRE(oss.str().find("ERROR") != std::string::npos); +} + +TEST_CASE("UserManagementCliComponent: group_remove_subgroup calls removeSubGroupOf") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + REQUIRE_CALL(*mockUserApi, removeSubGroupOf(std::string{ "ops" }, std::string{ "admin" })).TIMES(1).LR_RETURN(true); + + UserDto userDto; + component.onCommand(userDto, "group_remove_subgroup", { "-g", "ops", "-s", "admin" }); +} + +TEST_CASE("UserManagementCliComponent: group_remove_subgroup without -g prints error") +{ + auto mockUserApi = std::make_shared(); + UserManagementCliComponent component(mockUserApi); + + FORBID_CALL(*mockUserApi, removeSubGroupOf(ANY(std::string), ANY(std::string))); + + UserDto userDto; + std::ostringstream oss; + auto oldBuf = std::cerr.rdbuf(oss.rdbuf()); + + component.onCommand(userDto, "group_remove_subgroup", { "-s", "admin" }); + + std::cerr.rdbuf(oldBuf); + REQUIRE(oss.str().find("ERROR") != std::string::npos); +} }// namespace lintel diff --git a/tests/http/UserApiTest.cpp b/tests/http/UserApiTest.cpp index 04805805..57fd302d 100644 --- a/tests/http/UserApiTest.cpp +++ b/tests/http/UserApiTest.cpp @@ -467,4 +467,74 @@ TEST_CASE("UserApi: deleteOf covers OK, unauthorized and error") api.deleteOf({ "ada" }); } +TEST_CASE("UserApi: createGroupOf covers OK, unauthorized and error") +{ + userApiBackend backend; + UserApi api(userApiProviderOf(backend.port())); + GroupDto groupDto(Group("ops", std::vector{}, false)); + + backend.answerWithStatus(200, "{}"); + REQUIRE(api.createGroupOf(groupDto)); + REQUIRE(backend.paths.back() == "/user/groups"); + REQUIRE(backend.methods.back() == "POST"); + REQUIRE(backend.bodies.back().find("\"group_name\":\"ops\"") != std::string::npos); + + backend.answerWithStatus(401, "{}"); + REQUIRE_THROWS_AS(api.createGroupOf(groupDto), HttpUnauthorizedException); + + backend.answerWithStatus(409, "{}"); + REQUIRE_FALSE(api.createGroupOf(groupDto)); +} + +TEST_CASE("UserApi: deleteGroupOf covers OK, unauthorized and error") +{ + userApiBackend backend; + UserApi api(userApiProviderOf(backend.port())); + + backend.answerWithStatus(200, "{}"); + REQUIRE(api.deleteGroupOf("ops")); + REQUIRE(backend.paths.back() == "/user/groups/ops"); + REQUIRE(backend.methods.back() == "DELETE"); + + backend.answerWithStatus(401, "{}"); + REQUIRE_THROWS_AS(api.deleteGroupOf("ops"), HttpUnauthorizedException); + + backend.answerWithStatus(404, "{}"); + REQUIRE_FALSE(api.deleteGroupOf("ops")); +} + +TEST_CASE("UserApi: addSubGroupOf covers OK, unauthorized and error") +{ + userApiBackend backend; + UserApi api(userApiProviderOf(backend.port())); + + backend.answerWithStatus(200, "{}"); + REQUIRE(api.addSubGroupOf("ops", "admin")); + REQUIRE(backend.paths.back() == "/user/groups/ops/subgroups/admin"); + REQUIRE(backend.methods.back() == "POST"); + + backend.answerWithStatus(401, "{}"); + REQUIRE_THROWS_AS(api.addSubGroupOf("ops", "admin"), HttpUnauthorizedException); + + backend.answerWithStatus(400, "{}"); + REQUIRE_FALSE(api.addSubGroupOf("ops", "admin")); +} + +TEST_CASE("UserApi: removeSubGroupOf covers OK, unauthorized and error") +{ + userApiBackend backend; + UserApi api(userApiProviderOf(backend.port())); + + backend.answerWithStatus(200, "{}"); + REQUIRE(api.removeSubGroupOf("ops", "admin")); + REQUIRE(backend.paths.back() == "/user/groups/ops/subgroups/admin"); + REQUIRE(backend.methods.back() == "DELETE"); + + backend.answerWithStatus(401, "{}"); + REQUIRE_THROWS_AS(api.removeSubGroupOf("ops", "admin"), HttpUnauthorizedException); + + backend.answerWithStatus(404, "{}"); + REQUIRE_FALSE(api.removeSubGroupOf("ops", "admin")); +} + }// namespace lintel diff --git a/tests/http/UserControllerTest.cpp b/tests/http/UserControllerTest.cpp index 2a24a77e..2c59d68e 100644 --- a/tests/http/UserControllerTest.cpp +++ b/tests/http/UserControllerTest.cpp @@ -116,7 +116,12 @@ namespace { int m_port = -1; std::unique_ptr m_client; - UserControllerFixture() + /** + * @param withVirtualGroup seed an additional virtual group "ucVirtual" for + * the subgroup tests; it stays absent for the listing tests, which + * assert the seeded set of non-virtual groups + */ + explicit UserControllerFixture(bool withVirtualGroup = false) { static int counter = 0; m_dbPath = (std::filesystem::temp_directory_path() @@ -179,6 +184,10 @@ namespace { db::ParameterBuilder(connectionEntry).add(std::string("ucAdmins")).add(false)); seedStatement.execute("insert into groups(name, virtual) values(?, ?)", db::ParameterBuilder(connectionEntry).add(std::string("ucUsers")).add(false)); + if (withVirtualGroup) { + seedStatement.execute("insert into groups(name, virtual) values(?, ?)", + db::ParameterBuilder(connectionEntry).add(std::string("ucVirtual")).add(true)); + } } m_groupRepository->onAwake(); } @@ -575,6 +584,332 @@ TEST_CASE("UserController::allGroupsOfGroupNameAndIsVirtualGroupGet combines nam REQUIRE(notMatching->body == "[]"); } +// -------------------------------------------------------------------------------------- +// /user/groups (write operations) +// -------------------------------------------------------------------------------------- + +TEST_CASE("UserController::createGroupOfPost stores a new group", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Post( + "/user/groups", ucAuthHeaders(ucTokenAdmin), R"({"group_name":"ucCreated","virtual":false})", "application/json"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 200); + REQUIRE(response->body.find("ucCreated") != std::string::npos); + + const auto created = fixture.m_groupRepository->of("ucCreated"); + REQUIRE(created.has_value()); + REQUIRE_FALSE(created->isVirtual()); +} + +TEST_CASE("UserController::createGroupOfPost returns 409 for an existing group", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Post( + "/user/groups", ucAuthHeaders(ucTokenAdmin), R"({"group_name":"ucAdmins","virtual":false})", "application/json"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 409); + REQUIRE(response->body.empty()); +} + +TEST_CASE("UserController::createGroupOfPost returns 400 for an empty or missing group name", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + ALLOW_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto emptyName = fixture.m_client->Post( + "/user/groups", ucAuthHeaders(ucTokenAdmin), R"({"group_name":"","virtual":false})", "application/json"); + REQUIRE(emptyName != nullptr); + REQUIRE(emptyName->status == 400); + + auto missingName = + fixture.m_client->Post("/user/groups", ucAuthHeaders(ucTokenAdmin), R"({"virtual":false})", "application/json"); + REQUIRE(missingName != nullptr); + REQUIRE(missingName->status == 400); + + auto malformed = fixture.m_client->Post("/user/groups", ucAuthHeaders(ucTokenAdmin), "not json", "application/json"); + REQUIRE(malformed != nullptr); + REQUIRE(malformed->status == 400); +} + +TEST_CASE("UserController::createGroupOfPost returns 400 for a virtual group", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Post( + "/user/groups", ucAuthHeaders(ucTokenAdmin), R"({"group_name":"ucVirtualNew","virtual":true})", "application/json"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 400); + REQUIRE_FALSE(fixture.m_groupRepository->of("ucVirtualNew").has_value()); +} + +TEST_CASE("UserController::createGroupOfPost returns 401 for a caller without Admin-User", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucPlainUser, ucTokenPlain)); + + auto response = fixture.m_client->Post( + "/user/groups", ucAuthHeaders(ucTokenPlain), R"({"group_name":"ucDenied","virtual":false})", "application/json"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 401); + REQUIRE_FALSE(fixture.m_groupRepository->of("ucDenied").has_value()); +} + +TEST_CASE("UserController::createGroupOfPost returns 403 for a non json content type", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Post( + "/user/groups", ucAuthHeaders(ucTokenAdmin), R"({"group_name":"ucPlainBody","virtual":false})", "text/plain"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 403); + REQUIRE_FALSE(fixture.m_groupRepository->of("ucPlainBody").has_value()); +} + +TEST_CASE("UserController::deleteGroupOfDelete removes a group", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Delete("/user/groups/ucUsers", ucAuthHeaders(ucTokenAdmin)); + REQUIRE(response != nullptr); + REQUIRE(response->status == 200); + REQUIRE(response->body.find("\"removed\":true") != std::string::npos); + REQUIRE_FALSE(fixture.m_groupRepository->of("ucUsers").has_value()); +} + +TEST_CASE("UserController::deleteGroupOfDelete returns 404 for an unknown group", "[user_controller]") +{ + UserControllerFixture fixture; + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Delete("/user/groups/ucGhost", ucAuthHeaders(ucTokenAdmin)); + REQUIRE(response != nullptr); + REQUIRE(response->status == 404); +} + +TEST_CASE("UserController::deleteGroupOfDelete returns 409 for a group still in use", "[user_controller]") +{ + UserControllerFixture fixture; + const User member("Mem", "Ber", User::Male, "member@uc.test", "ucMember", "hash", {}); + fixture.m_userRepository->createOf(member); + fixture.m_userRepository->addGroupsOf(member, { Group("ucAdmins", {}, false) }); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Delete("/user/groups/ucAdmins", ucAuthHeaders(ucTokenAdmin)); + REQUIRE(response != nullptr); + REQUIRE(response->status == 409); + REQUIRE(fixture.m_groupRepository->of("ucAdmins").has_value()); +} + +TEST_CASE("UserController::addGroupMemberOfPost adds a virtual group to a collecting group", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Post( + "/user/groups/ucAdmins/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 200); + REQUIRE(response->body.find("\"added\":true") != std::string::npos); + + const auto group = fixture.m_groupRepository->of("ucAdmins"); + REQUIRE(group.has_value()); + REQUIRE(group->getGroups().size() == 1); + REQUIRE(group->getGroups()[0].getGroupName() == "ucVirtual"); +} + +TEST_CASE("UserController::addGroupMemberOfPost is idempotent for an existing member", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + ALLOW_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto first = fixture.m_client->Post( + "/user/groups/ucAdmins/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(first != nullptr); + REQUIRE(first->status == 200); + + auto second = fixture.m_client->Post( + "/user/groups/ucAdmins/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(second != nullptr); + REQUIRE(second->status == 200); + REQUIRE(second->body.find("\"added\":false") != std::string::npos); + + const auto group = fixture.m_groupRepository->of("ucAdmins"); + REQUIRE(group.has_value()); + REQUIRE(group->getGroups().size() == 1); +} + +TEST_CASE("UserController::addGroupMemberOfPost returns 404 for an unknown group or subgroup", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + ALLOW_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto unknownGroup = fixture.m_client->Post( + "/user/groups/ucGhost/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(unknownGroup != nullptr); + REQUIRE(unknownGroup->status == 404); + + auto unknownSubGroup = fixture.m_client->Post( + "/user/groups/ucAdmins/subgroups/ucGhost", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(unknownSubGroup != nullptr); + REQUIRE(unknownSubGroup->status == 404); +} + +TEST_CASE("UserController::addGroupMemberOfPost rejects a non virtual subgroup with 400", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Post( + "/user/groups/ucAdmins/subgroups/ucUsers", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(response != nullptr); + REQUIRE(response->status == 400); + + const auto group = fixture.m_groupRepository->of("ucAdmins"); + REQUIRE(group.has_value()); + REQUIRE(group->getGroups().empty()); +} + +TEST_CASE("UserController::removeGroupMemberOfDelete removes a member", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + ALLOW_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto added = fixture.m_client->Post( + "/user/groups/ucAdmins/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin), "", "application/json"); + REQUIRE(added != nullptr); + REQUIRE(added->status == 200); + + auto removed = fixture.m_client->Delete("/user/groups/ucAdmins/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin)); + REQUIRE(removed != nullptr); + REQUIRE(removed->status == 200); + REQUIRE(removed->body.find("\"removed\":true") != std::string::npos); + + const auto group = fixture.m_groupRepository->of("ucAdmins"); + REQUIRE(group.has_value()); + REQUIRE(group->getGroups().empty()); +} + +TEST_CASE("UserController::removeGroupMemberOfDelete reports an absent member as not removed", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Delete("/user/groups/ucAdmins/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin)); + REQUIRE(response != nullptr); + REQUIRE(response->status == 200); + REQUIRE(response->body.find("\"removed\":false") != std::string::npos); +} + +TEST_CASE("UserController::removeGroupMemberOfDelete returns 404 for an unknown group", "[user_controller]") +{ + UserControllerFixture fixture(true); + auto controller = fixture.makeController(); + fixture.startServing(controller); + REQUIRE(fixture.isReady()); + + REQUIRE_CALL(*fixture.m_authService, onAccessOf(ANY(const UserTokenLogin &))) + .TIMES(1) + .LR_RETURN(ucTokenOf(ucAdminUser, ucTokenAdmin)); + + auto response = fixture.m_client->Delete("/user/groups/ucGhost/subgroups/ucVirtual", ucAuthHeaders(ucTokenAdmin)); + REQUIRE(response != nullptr); + REQUIRE(response->status == 404); +} + // -------------------------------------------------------------------------------------- // /user/users // -------------------------------------------------------------------------------------- @@ -1174,6 +1509,10 @@ TEST_CASE("UserController: every guarded handler rejects a missing bearer token REQUIRE(fixture.m_client->Post("/user/add", "{}", "application/json")->status == 401); REQUIRE(fixture.m_client->Put("/user/update", "{}", "application/json")->status == 401); REQUIRE(fixture.m_client->Put("/user/password", "{}", "application/json")->status == 401); + REQUIRE(fixture.m_client->Post("/user/groups", "{}", "application/json")->status == 401); + REQUIRE(fixture.m_client->Delete("/user/groups/ucAdmins")->status == 401); + REQUIRE(fixture.m_client->Post("/user/groups/ucAdmins/subgroups/ucUsers", "", "application/json")->status == 401); + REQUIRE(fixture.m_client->Delete("/user/groups/ucAdmins/subgroups/ucUsers")->status == 401); for (const auto &[path, status] : plainDeletes) { REQUIRE(fixture.m_client->Delete(path)->status == status); } } @@ -1240,6 +1579,13 @@ TEST_CASE("UserController: every json-only handler rejects a non json content ty REQUIRE(fixture.m_client->Put("/user/update", ucAuthHeaders(ucTokenAdmin), "{}", "text/plain")->status == 403); REQUIRE(fixture.m_client->Put("/user/password", ucAuthHeaders(ucTokenAdmin), "{}", "text/plain")->status == 403); REQUIRE(fixture.m_client->Delete("/user/delete", getHeaders)->status == 403); + REQUIRE(fixture.m_client->Post("/user/groups", ucAuthHeaders(ucTokenAdmin), "{}", "text/plain")->status == 403); + REQUIRE(fixture.m_client->Delete("/user/groups/ucAdmins", getHeaders)->status == 403); + REQUIRE( + fixture.m_client->Post("/user/groups/ucAdmins/subgroups/ucUsers", ucAuthHeaders(ucTokenAdmin), "", "text/plain") + ->status + == 403); + REQUIRE(fixture.m_client->Delete("/user/groups/ucAdmins/subgroups/ucUsers", getHeaders)->status == 403); } TEST_CASE("UserController::allUsersOfUserNameGet returns the paging envelope when a limit is given", diff --git a/tests/mocks/MockUserApi.h b/tests/mocks/MockUserApi.h index 89f2c133..cb04b04e 100644 --- a/tests/mocks/MockUserApi.h +++ b/tests/mocks/MockUserApi.h @@ -19,6 +19,10 @@ class MockUserApi : public UserApi MAKE_MOCK1(allOf, std::vector(const std::string &), override); MAKE_MOCK2(allOf, std::vector(const std::string &, bool), override); MAKE_MOCK1(allOf, std::vector(bool), override); + MAKE_MOCK1(createGroupOf, bool(const GroupDto &), override); + MAKE_MOCK1(deleteGroupOf, bool(const std::string &), override); + MAKE_MOCK2(addSubGroupOf, bool(const std::string &, const std::string &), override); + MAKE_MOCK2(removeSubGroupOf, bool(const std::string &, const std::string &), override); MAKE_MOCK0(allUsersOf, std::vector(), override); MAKE_MOCK1(allUsersOf, std::vector(const std::string &), override); MAKE_MOCK1(createOf, void(const UserDto &), override); diff --git a/tests/repositories/UserGroupRepositoryTest.cpp b/tests/repositories/UserGroupRepositoryTest.cpp index 8fe665a5..e5380fe0 100644 --- a/tests/repositories/UserGroupRepositoryTest.cpp +++ b/tests/repositories/UserGroupRepositoryTest.cpp @@ -217,6 +217,34 @@ TEST_CASE("GroupRepository::deleteOf removes group", "[group_repository]") REQUIRE_FALSE(found.has_value()); } +TEST_CASE("GroupRepository::deleteOf removes the sub-group relations", "[group_repository]") +{ + // A deleted group may be the sub-group of another one. Leaving those rows in + // group_groups_relation made the next initGroups() dereference the gone group + // (m_groupMap.at) and abort the start. + RepoFixture fixture; + fixture.insertVirtualGroup("Child"); + fixture.groupRepository->onAwake(); + fixture.groupRepository->createOf(fixture.makeGroup("Parent")); + + auto parent = fixture.groupRepository->of("Parent"); + auto child = fixture.groupRepository->of("Child"); + REQUIRE(parent.has_value()); + REQUIRE(child.has_value()); + fixture.groupRepository->addGroupOf(*parent, *child); + REQUIRE(fixture.groupRepository->of("Parent")->getGroups().size() == 1); + + fixture.groupRepository->deleteOf(*child); + + // a fresh repository on the same database is what a restart does + auto restarted = std::make_shared(fixture.connectionConfigurations); + REQUIRE_NOTHROW(restarted->onAwake()); + REQUIRE_FALSE(restarted->of("Child").has_value()); + auto parentAfterRestart = restarted->of("Parent"); + REQUIRE(parentAfterRestart.has_value()); + REQUIRE(parentAfterRestart->getGroups().empty()); +} + TEST_CASE("GroupRepository::deleteOf fails when group is in use by user", "[group_repository]") { RepoFixture fixture; From f8c3233bd5e05768c0929ca5263102d960572dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Kr=C3=BCmpelmann?= Date: Thu, 22 Oct 2026 06:46:35 +0200 Subject: [PATCH 2/2] fix(cli): give the Commands enum of the user component a byte base type performance-enum-size: the values fit into one byte, and the neighbouring components (CommandLineService, EventBusCliComponent) already use std::uint8_t. --- .../features/cli/components/UserManagementCliComponent.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/include/lintel/features/cli/components/UserManagementCliComponent.h b/src/include/lintel/features/cli/components/UserManagementCliComponent.h index 40943c25..fca1db26 100644 --- a/src/include/lintel/features/cli/components/UserManagementCliComponent.h +++ b/src/include/lintel/features/cli/components/UserManagementCliComponent.h @@ -1,6 +1,7 @@ #ifndef LINTEL_USERMANAGEMENTCLICOMPONENT_H #define LINTEL_USERMANAGEMENTCLICOMPONENT_H +#include #include #include #include @@ -20,7 +21,7 @@ class UserManagementCliComponent : public CommandLineComponent "This component is responsible for the total user and group management"; std::shared_ptr m_userApi; - enum Commands { + enum Commands : std::uint8_t { Undefined, // group operations AllGroups,