chore: harden reliability checks #2
@ -165,8 +165,8 @@ exports.updateMemberRole = async (req, res) => {
|
||||
const { userId } = req.params;
|
||||
const { role } = req.body;
|
||||
|
||||
if (!role || !['admin', 'member'].includes(role)) {
|
||||
return sendError(res, 400, "Invalid role. Must be 'admin' or 'member'");
|
||||
if (!role || !["owner", "admin", "member"].includes(role)) {
|
||||
return sendError(res, 400, "Invalid role. Must be 'owner', 'admin', or 'member'");
|
||||
}
|
||||
|
||||
// Can't change own role
|
||||
@ -182,14 +182,29 @@ exports.updateMemberRole = async (req, res) => {
|
||||
return sendError(res, 403, "Owner role cannot be changed");
|
||||
}
|
||||
|
||||
const updated = await householdModel.updateMemberRole(
|
||||
let updated;
|
||||
if (role === "owner") {
|
||||
if (req.household.role !== "owner") {
|
||||
return sendError(res, 403, "Only the household owner can transfer ownership");
|
||||
}
|
||||
|
||||
updated = await householdModel.transferOwnership(
|
||||
req.params.householdId,
|
||||
req.user.id,
|
||||
parseInt(userId, 10)
|
||||
);
|
||||
} else {
|
||||
updated = await householdModel.updateMemberRole(
|
||||
req.params.householdId,
|
||||
userId,
|
||||
role
|
||||
);
|
||||
}
|
||||
|
||||
res.json({
|
||||
message: "Member role updated successfully",
|
||||
message: role === "owner"
|
||||
? "Household ownership transferred successfully"
|
||||
: "Member role updated successfully",
|
||||
member: updated
|
||||
});
|
||||
} catch (error) {
|
||||
|
||||
@ -162,6 +162,47 @@ exports.updateMemberRole = async (householdId, userId, newRole) => {
|
||||
return result.rows[0];
|
||||
};
|
||||
|
||||
// Transfer household ownership from one member to another atomically.
|
||||
exports.transferOwnership = async (householdId, currentOwnerUserId, nextOwnerUserId) => {
|
||||
const client = await pool.connect();
|
||||
|
||||
try {
|
||||
await client.query("BEGIN");
|
||||
|
||||
const promoteResult = await client.query(
|
||||
`UPDATE household_members
|
||||
SET role = 'owner'
|
||||
WHERE household_id = $1 AND user_id = $2
|
||||
RETURNING user_id, role`,
|
||||
[householdId, nextOwnerUserId]
|
||||
);
|
||||
|
||||
if (promoteResult.rows.length === 0) {
|
||||
throw new Error("TARGET_MEMBER_NOT_FOUND");
|
||||
}
|
||||
|
||||
const demoteResult = await client.query(
|
||||
`UPDATE household_members
|
||||
SET role = 'admin'
|
||||
WHERE household_id = $1 AND user_id = $2
|
||||
RETURNING user_id, role`,
|
||||
[householdId, currentOwnerUserId]
|
||||
);
|
||||
|
||||
if (demoteResult.rows.length === 0) {
|
||||
throw new Error("CURRENT_OWNER_NOT_FOUND");
|
||||
}
|
||||
|
||||
await client.query("COMMIT");
|
||||
return promoteResult.rows[0];
|
||||
} catch (error) {
|
||||
await client.query("ROLLBACK");
|
||||
throw error;
|
||||
} finally {
|
||||
client.release();
|
||||
}
|
||||
};
|
||||
|
||||
// Remove member from household
|
||||
exports.removeMember = async (householdId, userId) => {
|
||||
await pool.query(
|
||||
|
||||
88
backend/tests/households.controller.test.js
Normal file
88
backend/tests/households.controller.test.js
Normal file
@ -0,0 +1,88 @@
|
||||
jest.mock("../models/household.model", () => ({
|
||||
getUserRole: jest.fn(),
|
||||
transferOwnership: jest.fn(),
|
||||
updateMemberRole: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock("../utils/logger", () => ({
|
||||
logError: jest.fn(),
|
||||
}));
|
||||
|
||||
const householdModel = require("../models/household.model");
|
||||
const controller = require("../controllers/households.controller");
|
||||
|
||||
function createResponse() {
|
||||
const res = {};
|
||||
res.status = jest.fn().mockReturnValue(res);
|
||||
res.json = jest.fn().mockReturnValue(res);
|
||||
return res;
|
||||
}
|
||||
|
||||
describe("households.controller updateMemberRole", () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
householdModel.getUserRole.mockResolvedValue("member");
|
||||
householdModel.transferOwnership.mockResolvedValue({ user_id: 7, role: "owner" });
|
||||
householdModel.updateMemberRole.mockResolvedValue({ user_id: 7, role: "admin" });
|
||||
});
|
||||
|
||||
test("owner can transfer household ownership", async () => {
|
||||
const req = {
|
||||
params: { householdId: "3", userId: "7" },
|
||||
body: { role: "owner" },
|
||||
user: { id: 1 },
|
||||
household: { id: 3, role: "owner" },
|
||||
};
|
||||
const res = createResponse();
|
||||
|
||||
await controller.updateMemberRole(req, res);
|
||||
|
||||
expect(householdModel.transferOwnership).toHaveBeenCalledWith("3", 1, 7);
|
||||
expect(householdModel.updateMemberRole).not.toHaveBeenCalled();
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
message: "Household ownership transferred successfully",
|
||||
member: { user_id: 7, role: "owner" },
|
||||
});
|
||||
});
|
||||
|
||||
test("admin cannot transfer household ownership", async () => {
|
||||
const req = {
|
||||
params: { householdId: "3", userId: "7" },
|
||||
body: { role: "owner" },
|
||||
user: { id: 1 },
|
||||
household: { id: 3, role: "admin" },
|
||||
};
|
||||
const res = createResponse();
|
||||
|
||||
await controller.updateMemberRole(req, res);
|
||||
|
||||
expect(householdModel.transferOwnership).not.toHaveBeenCalled();
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(res.json).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
error: expect.objectContaining({
|
||||
message: "Only the household owner can transfer ownership",
|
||||
}),
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
test("owner can still update a member to admin without transfer flow", async () => {
|
||||
const req = {
|
||||
params: { householdId: "3", userId: "7" },
|
||||
body: { role: "admin" },
|
||||
user: { id: 1 },
|
||||
household: { id: 3, role: "owner" },
|
||||
};
|
||||
const res = createResponse();
|
||||
|
||||
await controller.updateMemberRole(req, res);
|
||||
|
||||
expect(householdModel.updateMemberRole).toHaveBeenCalledWith("3", "7", "admin");
|
||||
expect(householdModel.transferOwnership).not.toHaveBeenCalled();
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
message: "Member role updated successfully",
|
||||
member: { user_id: 7, role: "admin" },
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -71,6 +71,7 @@ export default function ManageHousehold() {
|
||||
const [isLeaveModalOpen, setIsLeaveModalOpen] = useState(false);
|
||||
|
||||
const isManager = ["owner", "admin"].includes(activeHousehold?.role);
|
||||
const isOwner = activeHousehold?.role === "owner";
|
||||
const isMemberOnly = activeHousehold?.role === "member";
|
||||
|
||||
useEffect(() => {
|
||||
@ -276,14 +277,27 @@ export default function ManageHousehold() {
|
||||
}
|
||||
};
|
||||
|
||||
const handleUpdateRole = async (memberId, currentRole, memberName) => {
|
||||
if (currentRole === "owner") return;
|
||||
const newRole = currentRole === "admin" ? "member" : "admin";
|
||||
const handleUpdateRole = async (memberId, nextRole, memberName) => {
|
||||
if (!nextRole) return;
|
||||
|
||||
if (
|
||||
nextRole === "owner" &&
|
||||
!window.confirm(`Make ${memberName} the household owner? You will become an admin.`)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
await updateMemberRole(activeHousehold.id, memberId, newRole);
|
||||
await loadMembers();
|
||||
toast.success("Updated member role", `Updated role for ${memberName} to ${newRole}`);
|
||||
await updateMemberRole(activeHousehold.id, memberId, nextRole);
|
||||
await Promise.all([
|
||||
loadMembers(),
|
||||
nextRole === "owner" ? refreshHouseholds() : Promise.resolve(),
|
||||
]);
|
||||
if (nextRole === "owner") {
|
||||
toast.success("Transferred household ownership", `Transferred ownership to ${memberName}`);
|
||||
} else {
|
||||
toast.success("Updated member role", `Updated role for ${memberName} to ${nextRole}`);
|
||||
}
|
||||
} catch (error) {
|
||||
const message = getApiErrorMessage(error, "Failed to update member role");
|
||||
toast.error("Update member role failed", `Update member role failed: ${message}`);
|
||||
@ -560,8 +574,20 @@ export default function ManageHousehold() {
|
||||
</div>
|
||||
{isManager && !isSelf && member.role !== "owner" && (
|
||||
<div className="member-actions">
|
||||
{isOwner && (
|
||||
<button
|
||||
onClick={() => handleUpdateRole(member.id, member.role, member.username)}
|
||||
onClick={() => handleUpdateRole(member.id, "owner", member.username)}
|
||||
className="btn-primary btn-small member-owner-action"
|
||||
>
|
||||
Make Owner
|
||||
</button>
|
||||
)}
|
||||
<button
|
||||
onClick={() => handleUpdateRole(
|
||||
member.id,
|
||||
member.role === "admin" ? "member" : "admin",
|
||||
member.username
|
||||
)}
|
||||
className="btn-secondary btn-small member-role-action"
|
||||
>
|
||||
{member.role === "admin" ? "Make Member" : "Make Admin"}
|
||||
|
||||
@ -495,12 +495,16 @@ body.dark-mode .member-card:hover {
|
||||
display: flex;
|
||||
gap: 0.55rem;
|
||||
flex-wrap: wrap;
|
||||
justify-content: flex-end;
|
||||
justify-content: flex-start;
|
||||
align-items: center;
|
||||
padding-top: 0.75rem;
|
||||
border-top: 1px solid color-mix(in srgb, var(--color-border-light) 82%, transparent);
|
||||
}
|
||||
|
||||
.member-owner-action {
|
||||
box-shadow: inset 0 0 0 1px rgba(255, 255, 255, 0.12);
|
||||
}
|
||||
|
||||
.member-role-action {
|
||||
background: rgba(30, 144, 255, 0.14);
|
||||
color: var(--primary-dark);
|
||||
|
||||
@ -192,3 +192,112 @@ test("household management shows pending invite approvals and can approve them",
|
||||
await expect(page.getByText("No pending join requests right now.")).toBeVisible();
|
||||
await expect(page.getByText("Members (2)")).toBeVisible();
|
||||
});
|
||||
|
||||
test("household owner can transfer ownership from household settings", async ({ page }) => {
|
||||
await seedAuthStorage(page, "owner");
|
||||
await mockConfig(page);
|
||||
|
||||
let households = [{ id: 1, name: "Approval Home", role: "owner", invite_code: "ABCD1234" }];
|
||||
let members = [
|
||||
{ id: 1, username: "manager-user", role: "owner" },
|
||||
{ id: 2, username: "nico-admin", role: "admin" },
|
||||
];
|
||||
|
||||
await page.route("**/households", async (route) => {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify(households),
|
||||
});
|
||||
});
|
||||
|
||||
await page.route("**/households/1/members", async (route) => {
|
||||
const request = route.request();
|
||||
if (request.method() === "PATCH") {
|
||||
const body = request.postDataJSON() as { role?: string };
|
||||
if (body.role === "owner") {
|
||||
households = [{ id: 1, name: "Approval Home", role: "admin", invite_code: "ABCD1234" }];
|
||||
members = [
|
||||
{ id: 1, username: "manager-user", role: "admin" },
|
||||
{ id: 2, username: "nico-admin", role: "owner" },
|
||||
];
|
||||
}
|
||||
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({
|
||||
message: "Household ownership transferred successfully",
|
||||
member: { user_id: 2, role: body.role || "member" },
|
||||
}),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify(members),
|
||||
});
|
||||
});
|
||||
|
||||
await page.route("**/api/groups/join-policy", async (route) => {
|
||||
const request = route.request();
|
||||
if (request.method() === "GET") {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ joinPolicy: "APPROVAL_REQUIRED" }),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ ok: true }),
|
||||
});
|
||||
});
|
||||
|
||||
await page.route("**/api/groups/invites", async (route) => {
|
||||
const request = route.request();
|
||||
if (request.method() === "GET") {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ links: [] }),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
await route.fulfill({
|
||||
status: 201,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ link: { id: 10, token: "new-token" } }),
|
||||
});
|
||||
});
|
||||
|
||||
await page.route("**/api/groups/join-requests", async (route) => {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ requests: [] }),
|
||||
});
|
||||
});
|
||||
|
||||
page.on("dialog", async (dialog) => {
|
||||
await dialog.accept();
|
||||
});
|
||||
|
||||
await page.goto("/manage?tab=household");
|
||||
|
||||
await expect(page.getByRole("button", { name: "Make Owner" })).toBeVisible();
|
||||
|
||||
await page.getByRole("button", { name: "Make Owner" }).click();
|
||||
|
||||
await expect(page.locator(".action-toast.action-toast-success")).toContainText("Transferred household ownership");
|
||||
await expect(page.locator(".action-toast.action-toast-success")).toContainText("nico-admin");
|
||||
await expect(page.getByText("👑 Owner")).toContainText("Owner");
|
||||
await expect(page.getByText("🛠️ Admin")).toContainText("Admin");
|
||||
await expect(page.getByRole("button", { name: "Make Owner" })).toHaveCount(0);
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user