Skip to content

Add container sharing (collaborators) feature#388

Draft
cmyers-mieweb wants to merge 5 commits into
mainfrom
cmyers_shareables
Draft

Add container sharing (collaborators) feature#388
cmyers-mieweb wants to merge 5 commits into
mainfrom
cmyers_shareables

Conversation

@cmyers-mieweb

@cmyers-mieweb cmyers-mieweb commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Introduce container sharing/collaborators so containers can be shared with additional users.

Details:

  • Backend: add ContainerCollaborator model and migration with unique (containerId, username) constraint and CASCADE deletion.
  • DB/model changes: Container.hasMany(ContainerCollaborator) association and eager-load collaborators in container queries/serializer.
  • Permissions: implement userCanAccess / userCanManage helpers and loadContainerForSession to authorize view vs manage actions.
  • Listing/filtering: applyOwnershipFilter folds shared containers into the "All" view for non-admins and preserves admin behavior for '*'.
  • Share API: GET/POST/DELETE /sites/:siteId/containers/:id/collaborators to list, add, and remove collaborators; share/unshare endpoints validate users and return the updated collaborator list.
  • Creation: Container create accepts additionalOwners array; creator-specified shares validated and stored in the same transaction.
  • Deletion: container deletion removes collaborator grants explicitly and enforces owner/admin-only deletion.
  • Client: add queries.shareContainer/unshareContainer, Container.collaborators type, and React components (CollaboratorChips, AddCollaboratorField, CollaboratorsManager); integrate into ContainerFormPage (create/edit) and ContainersListPage (share modal and row action).

This change enables collaborative workflows while enforcing owner/admin management and consistent UI feedback.

image image image image image image

Introduce container sharing/collaborators so containers can be shared with additional users.

Details:
- Backend: add ContainerCollaborator model and migration with unique (containerId, username) constraint and CASCADE deletion.
- DB/model changes: Container.hasMany(ContainerCollaborator) association and eager-load collaborators in container queries/serializer.
- Permissions: implement userCanAccess / userCanManage helpers and loadContainerForSession to authorize view vs manage actions.
- Listing/filtering: applyOwnershipFilter folds shared containers into the "All" view for non-admins and preserves admin behavior for '*'.
- Share API: GET/POST/DELETE /sites/:siteId/containers/:id/collaborators to list, add, and remove collaborators; share/unshare endpoints validate users and return the updated collaborator list.
- Creation: Container create accepts additionalOwners array; creator-specified shares validated and stored in the same transaction.
- Deletion: container deletion removes collaborator grants explicitly and enforces owner/admin-only deletion.
- Client: add queries.shareContainer/unshareContainer, Container.collaborators type, and React components (CollaboratorChips, AddCollaboratorField, CollaboratorsManager); integrate into ContainerFormPage (create/edit) and ContainersListPage (share modal and row action).

This change enables collaborative workflows while enforcing owner/admin management and consistent UI feedback.
Enhance the Logs, Share, Edit and Delete action buttons in ContainersListPage.tsx with transition and hover color/background classes (including dark mode variants) to improve hover feedback and visual consistency. The changes add Tailwind utility className props to each Button without altering behavior.
@cmyers-mieweb

Copy link
Copy Markdown
Collaborator Author

Modifying "all" container toggle with a more clear and defined filter system to resolve #390

Also additional bug fixes to resolve
#389
#391

image image image

@cmyers-mieweb

Copy link
Copy Markdown
Collaborator Author

Also added color over hovered buttons for better UI
image

Introduce a reusable ContainerFilters component (with a MultiSelect/popover) and wire it into ContainersListPage so filter state lives in the URL (user,status,template,q). ContainersListPage now parses comma-separated user lists (and wildcard '*'), computes user/status/template option lists, performs client-side refinement by status/template/hostname, and updates UI/empty-state logic (removes the old Mine/All toggle). On the API side applyOwnershipFilter was refactored to accept a parsed user list, support wildcard semantics, let admins query arbitrary lists, and restrict non-admins to their own+shared containers. Added helper functions parseUserFilter and ownVisibleClauses to centralize parsing and visible-clause construction.

@runleveldev runleveldev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding my large number of comments in create-a-container/routers/api/v1/containers.js:

Comment thread create-a-container/openapi.v1.yaml Outdated
Comment thread create-a-container/routers/api/v1/containers.js Outdated
Comment on lines +266 to +279
/**
* Sequelize OR clauses matching every container a non-admin may see: their own
* plus any shared with them. An empty shared set collapses to just their own.
* @param {string} username - The requesting user's uid.
* @returns {Promise<object[]>}
*/
async function ownVisibleClauses(username) {
const shares = await ContainerCollaborator.findAll({
where: { username },
attributes: ['containerId'],
});
const sharedIds = shares.map((s) => s.containerId);
return [{ username }, ...(sharedIds.length > 0 ? [{ id: sharedIds }] : [])];
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function runs a query to get it's information before sticking that into another query later on to find what we're actually looking for. If I'm reading it correctly, this should just be a JOIN to the "containers" model which should be more efficient since it's one less round trip to the database.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've attempted to make this function more effecient by having OwnVisibleClauses no longer run its own query and instead the shared container check is now an [id IN (SELECT containerId FROM ContainerCollaborators WHERE username = … so visibility resolves in one trip

function ownVisibleClauses(username) {
  // Built with the dialect's query generator so identifier quoting and value
  // escaping stay correct across sqlite/mysql/postgres. selectQuery emits a
  // trailing ';' which is invalid inside IN (…), hence the slice.
  const shared = sequelize.dialect.queryGenerator
    .selectQuery(
      ContainerCollaborator.getTableName(),
      { attributes: ['containerId'], where: { username } },
      ContainerCollaborator,
    )
    .slice(0, -1);
  return [{ username }, { id: { [Sequelize.Op.in]: Sequelize.literal(`(${shared})`) } }];
}

@runleveldev runleveldev Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not have been clear because I left so many comments. I wanted the logic in ownVisibleClauses and applyOwnershipFilter (which is the sole caller of ownVisibleClauses) to be folded into buildContainerListWhere so that it becomes the sole function arbiting the "where" clause for container queries.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've consolidated the logic behind applyOwnershipFilter and ownVisibleClauses into buildContainerListWhere so it becomes a single function that accepts the session along with the query and Node ID's.

Comment on lines +281 to +304
/**
* Whether a session may view/edit a container: its owner, a collaborator it is
* shared with, or an admin. Requires `collaborators` to be loaded on the record.
* @param {object} container - Container instance with `collaborators` included.
* @param {object} session - req.session ({ user, isAdmin }).
* @returns {boolean}
*/
function userCanAccess(container, session) {
if (session.isAdmin) return true;
if (container.username === session.user) return true;
return (container.collaborators || []).some((c) => c.username === session.user);
}

/**
* Whether a session may manage a container's sharing (add/remove collaborators)
* or delete it: only its primary owner or an admin. Collaborators can use a
* shared container but cannot re-share or delete it.
* @param {object} container - Container instance.
* @param {object} session - req.session ({ user, isAdmin }).
* @returns {boolean}
*/
function userCanManage(container, session) {
return session.isAdmin || container.username === session.user;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These permission checks should be methods on the container model. Container#canView(username) checks that that username is allowed to view the container and Container#canEdit(username) check that that username is allowed to edit it. Those methods can encapsulate more complex logic down the line.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's this instead

  const authorized =
    req.session.isAdmin ||
    (requireManage
      ? container.canEdit(req.session.user)
      : container.canView(req.session.user));
      

Comment thread create-a-container/routers/api/v1/containers.js Outdated
Comment thread create-a-container/client/src/pages/containers/ContainersListPage.tsx Outdated
label: STATUS_LABELS[s],
})),
[],
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably fine, but we should probably derive this from the actual statuses from the API to keep the dropdown clean.

* A compact checkbox-popover multiselect. Selected values are controlled by the
* parent so filter state stays in the URL. Closes on outside click.
*/
function MultiSelect({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a "select all" option which I would prefer to the custom "Everone" option we have.

Comment thread create-a-container/client/src/pages/containers/CollaboratorsManager.tsx Outdated
Comment thread create-a-container/client/src/pages/containers/ContainerFormPage.tsx Outdated

@runleveldev runleveldev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is happening when loading the containers list page. If I get my way I really want the behavior of /api/v1/sites/:id/containers to be equivelent to the ?user=* form (making user=* redundant and obselete and hopefully removing the need for the * special handling in the backend).

Image

Move container UI bits into reusable components and add dedicated components (HttpLinks, Meta, NodeLink, RowActions, SshLinks, StatusBadge, shared helpers). Normalize sharing terminology from "additionalOwners" to "collaborators" across client types, forms and OpenAPI, and wire the UI to the new components (including ServiceBadge and Dropdown multiSelect). Change list filtering to send user filters as an array and update client queries/types accordingly. Update server-side router/model logic: loadSite signature, authorize via Container.canView/canEdit, apply ownership filter using an IN (SELECT …) subquery for shared visibility, and adjust container loading/authorization to use the new semantics. Minor UI/behavior tweaks to filter summaries and owner visibility in the list view.
Comment on lines +25 to +33
<Link to={`/jobs/${c.creationJobId}`}>
<Button
variant="ghost"
size="sm"
className="transition-colors hover:bg-slate-100 hover:text-slate-700 dark:hover:bg-slate-800 dark:hover:text-slate-200"
>
Logs
</Button>
</Link>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the builtin Button fade is being overridden by the Link wrapper. Can we fix that instead of coding our own transition?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed where it was being overridden, however I kept similar logic for the delete button so that it would have a distinct color when hovered over.

Client: Simplify RowActions buttons by removing custom transition CSS from several ghost buttons and slightly adjust the Delete button's hover classes (keep built-in ghost transition and add a comment).

Server: Consolidate and harden container-list visibility logic by folding the previous ownership-filter helpers into `buildContainerListWhere(query, nodeIds, session)`. The function now documents behavior, accepts `session`, understands `query.user` (array) and enforces admin vs non-admin visibility rules (admins may narrow by owner; non-admins are limited to their own and shared containers and cannot widen visibility). The shared set is implemented as an `IN (SELECT ...)` subquery built with the dialect query generator. Updated router to default `req.query.user` and call the new `buildContainerListWhere` with `req.session`.

Overall: makes the list endpoint the single arbiter of what containers a caller may see and prevents non-admins from widening visibility via the `user` filter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants