Skip to content

fix: The site layout editor refuses a draft it can't tell apart from another user's, and a deleted user's draft sites are removed EXO-91427 - #604

Merged
boubaker merged 1 commit into
developfrom
backport/EXO-91427-draft-of-another-user
Oct 27, 2026
Merged

boubaker merged 1 commit into
developfrom
backport/EXO-91427-draft-of-another-user

Conversation

@boubaker

Copy link
Copy Markdown
Member

Symptom: some draft site names also read as another user's draft: a site name holding _draft_ or ending with _draft, or a username starting with draft_. On such a name, opening the site layout editor removed and re-created the draft by name, which could drop the other user's draft. Once portal refuses such a name (Meeds-io/portal#1345), the editor stopped on a generic error instead. A deleted user's draft sites stayed, and a user whose name fits a leftover name could open them.

Cause: SiteLayoutService#createDraftSite builds <type>_<name>_draft_<user> and removes any site of that name before creating it, without checking that the user may edit that name. Nothing removed a user's draft sites when the user was deleted.

Fix:

  • createDraftSite checks LayoutAclService#canEditSite(PortalConfig, String), a new overload, on the draft it built in memory, before removing or creating anything. When the check fails it refuses with IllegalArgumentException("layout.siteDraftNameConflict"). SiteLayoutRest answers 400 with that message code, and the editor shows its text. Administrators pass the check.
  • SiteLayoutService#deleteUserDraftSites (@ContainerTransactional) removes every DRAFT site whose name ends with _draft_<username>, read in one unpaged listing. A removal that fails is logged and skipped. Two listeners call it off the deleting thread: DraftSiteOwnerAccountDeletedListener on social's social.account.deleted (@Asynchronous), and DraftSiteOwnerUserDeletedListener on UserEventListener#postDelete (CompletableFuture.runAsync).
  • Tests:
    • SiteLayoutServiceTest: the refusal leaves the existing draft untouched; the removal by suffix; a failed removal; a blank username; the container binding.
    • LayoutAclServiceTest: a site not stored yet.
    • SiteLayoutRestTest: the 400.
    • Both listener tests.

Known limits (accepted):

  • an administrator passes the check;
  • resuming a draft through the user's co-editing revision shows the generic error, not the conflict message;
  • an owner whose draft name collides with another existing user's is refused that draft;
  • page and navigation-node drafts (<name>_draft_<user>) share the name ambiguity and are saved over by name;
  • the draft sites stay for users deleted before this change, and for a user deleted through social's DELETE /v1/social/users/{id}, which fires no event;
  • the removals run in one persistence context;
  • a co-editing revision can still name a removed draft;
  • removing a deleted user's draft sites also removes a draft whose name another user could own: while the deleted user existed, that name was refused to every non-administrator, and an administrator's draft of that name is removed too.

Stacked on #581. Merge after it and after Meeds-io/portal#1345.

Knowledge: Meeds-io/eng-standards#572, #522

This change is classified N1 (a new permission check on the draft site's write path): its approver must be an Archi/Dev who knows it is N1, not an approval on AI review alone, and author ≠ approver.

🤖 Generated with Claude Code

@boubaker

Copy link
Copy Markdown
Member Author

Backport evidence

Source: #590, squash-merged into feature/maintenance. Its change is on origin/feature/maintenance at f5a9180825 (matched against the branch's current head, 2026-10-27). The PO validated it on the maintenance acceptance server on 2026-10-27.

Picks: ea6c223293 = git cherry-pick -x f5a9180825, on the EXO-91183 backport branch (af94c10a25, #603).

Adaptations: none. The pick applied cleanly, and no commit was added.

Verification

Level: N1, restated from the source. A backport from feature/maintenance whose every pick's patch-id equals its source's diff merges on its first approval (ai-review-and-merge.md §3). It lands by squash, with the source's body as its message.

Jihed525
Jihed525 previously approved these changes Oct 27, 2026
@boubaker
boubaker added this pull request to stack #605 October 27, 2026 12:47
@boubaker
boubaker removed the request for review from ahamdi October 27, 2026 14:25
Base automatically changed from backport/EXO-91183-draft-of-an-unresolved-site to develop October 27, 2026 14:26
…another user's, and a deleted user's draft sites are removed EXO-91427 (#590)

**Symptom**: some draft site names also read as another user's draft: a
site name holding `_draft_` or ending with `_draft`, or a username
starting with `draft_`. On such a name, opening the site layout editor
removed and re-created the draft by name, which could drop the other
user's draft. Once portal refuses such a name (Meeds-io/portal#1345),
the editor stopped on a generic error instead. A deleted user's draft
sites stayed, and a user whose name fits a leftover name could open
them.

**Cause**: `SiteLayoutService#createDraftSite` builds
`<type>_<name>_draft_<user>` and removes any site of that name before
creating it, without checking that the user may edit that name. Nothing
removed a user's draft sites when the user was deleted.

**Fix**:
- `createDraftSite` checks `LayoutAclService#canEditSite(PortalConfig,
String)`, a new overload, on the draft it built in memory, before
removing or creating anything. When the check fails it refuses with
`IllegalArgumentException("layout.siteDraftNameConflict")`.
`SiteLayoutRest` answers 400 with that message code, and the editor
shows its text. Administrators pass the check.
- `SiteLayoutService#deleteUserDraftSites` (`@ContainerTransactional`)
removes every `DRAFT` site whose name ends with `_draft_<username>`,
read in one unpaged listing. A removal that fails is logged and skipped.
Two listeners call it off the deleting thread:
`DraftSiteOwnerAccountDeletedListener` on social's
`social.account.deleted` (`@Asynchronous`), and
`DraftSiteOwnerUserDeletedListener` on `UserEventListener#postDelete`
(`CompletableFuture.runAsync`).
- Tests:
- `SiteLayoutServiceTest`: the refusal leaves the existing draft
untouched; the removal by suffix; a failed removal; a blank username;
the container binding.
  - `LayoutAclServiceTest`: a site not stored yet.
  - `SiteLayoutRestTest`: the 400.
  - Both listener tests.

**Known limits** (accepted):
- an administrator passes the check;
- resuming a draft through the user's co-editing revision shows the
generic error, not the conflict message;
- an owner whose draft name collides with another existing user's is
refused that draft;
- page and navigation-node drafts (`<name>_draft_<user>`) share the name
ambiguity and are saved over by name;
- the draft sites stay for users deleted before this change, and for a
user deleted through social's `DELETE /v1/social/users/{id}`, which
fires no event;
- the removals run in one persistence context;
- a co-editing revision can still name a removed draft;
- removing a deleted user's draft sites also removes a draft whose name
another user could own: while the deleted user existed, that name was
refused to every non-administrator, and an administrator's draft of that
name is removed too.

Stacked on #581. Merge after it and after
Meeds-io/portal#1345.

Knowledge: Meeds-io/eng-standards#572, #522

This change is classified N1 (a new permission check on the draft site's
write path): its approver must be an Archi/Dev who knows it is N1, not
an approval on AI review alone, and author ≠ approver.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit f5a9180)
@boubaker
boubaker dismissed Jihed525’s stale review October 27, 2026 14:26

The merge-base changed after approval.

@boubaker
boubaker force-pushed the backport/EXO-91427-draft-of-another-user branch from ea6c223 to e38a075 Compare October 27, 2026 14:26
@boubaker
boubaker merged commit 5f5be6a into develop Oct 27, 2026
2 checks passed
@boubaker
boubaker deleted the backport/EXO-91427-draft-of-another-user branch October 27, 2026 14:27
@sonarqubecloud

Copy link
Copy Markdown

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