Skip to content

Commit

Permalink
feat: Disallow space members from editing or removing articles they d…
Browse files Browse the repository at this point in the history
…id not author - EXO-75398 - Meeds-io/meeds#2603
  • Loading branch information
sofyenne committed Nov 20, 2024
1 parent ca4349c commit b68b038
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ public News getNewsByIdAndLang(String newsId,
throw new IllegalAccessException("User " + currentIdentity.getUserId() + " is not authorized to view News");
}
news.setCanEdit(canEditNews(news, currentIdentity.getUserId()));
news.setCanDelete(canDeleteNews(currentIdentity, news.getAuthor(), news.getSpaceId()));
news.setCanDelete(canDeleteNews(currentIdentity, news));
news.setCanPublish(NewsUtils.canPublishNews(news.getSpaceId(), currentIdentity));
Space space = spaceService.getSpaceById(news.getSpaceId());
if (space != null) {
Expand Down Expand Up @@ -578,7 +578,7 @@ public List<News> getNews(NewsFilter filter, Identity currentIdentity) throws Ex
}
newsList.stream().filter(Objects::nonNull).forEach(news -> {
news.setCanEdit(canEditNews(news, currentIdentity.getUserId()));
news.setCanDelete(canDeleteNews(currentIdentity, news.getAuthor(), news.getSpaceId()));
news.setCanDelete(canDeleteNews(currentIdentity, news));
news.setCanPublish(NewsUtils.canPublishNews(news.getSpaceId(), currentIdentity));
Space space = spaceService.getSpaceById(news.getSpaceId());
if (space != null) {
Expand Down Expand Up @@ -817,8 +817,8 @@ public boolean canScheduleNews(Space space, Identity currentIdentity, News artic
boolean isArticleAuthor = article.getAuthor() != null && article.getAuthor().equals(currentIdentity.getUserId());
boolean spaceMemberCanSchedule = isArticleAuthor && spaceService.isMember(space, currentIdentity.getUserId());
return spaceMemberCanSchedule || spaceService.isManager(space, currentIdentity.getUserId())
|| spaceService.isRedactor(space, currentIdentity.getUserId())
|| NewsUtils.canPublishNews(space.getId(), currentIdentity);
|| spaceService.isRedactor(space, currentIdentity.getUserId())
|| NewsUtils.canPublishNews(space.getId(), currentIdentity);
}

/**
Expand Down Expand Up @@ -1580,16 +1580,27 @@ private boolean canEditNews(News news, String authenticatedUser) {
LOG.warn("Can't find user with id {} when checking access on news with id {}", authenticatedUser, news.getId());
return false;
}
return NewsUtils.canPublishNews(news.getSpaceId(), authenticatedUserIdentity)
|| spaceService.canRedactOnSpace(space, authenticatedUserIdentity);
boolean isSpaceMemberCanEdit = isArticleAuthor(news, authenticatedUser) && spaceService.isMember(spaceId, authenticatedUser);
return isSpaceMemberCanEdit || NewsUtils.canPublishNews(news.getSpaceId(), authenticatedUserIdentity)
|| (spaceService.isManager(space, authenticatedUser)
|| spaceService.isSuperManager(space, authenticatedUser)
|| spaceService.isRedactor(space, authenticatedUser));
}

private boolean canDeleteNews(Identity currentIdentity, String posterId, String spaceId) {
Space space = spaceId == null ? null : spaceService.getSpaceById(spaceId);
private boolean isArticleAuthor(News article, String userName) {
return StringUtils.isNotEmpty(article.getAuthor()) && article.getAuthor().equals(userName);
}

private boolean canDeleteNews(Identity currentIdentity, News news) {
Space space = news.getSpaceId() == null ? null : spaceService.getSpaceById(news.getSpaceId());
if (space == null) {
return false;
}
return spaceService.canRedactOnSpace(space, currentIdentity);
boolean isMemberCanDelete = isArticleAuthor(news, currentIdentity.getUserId())
&& spaceService.isMember(space.getId(), currentIdentity.getUserId());
return isMemberCanDelete || (spaceService.isManager(space, currentIdentity.getUserId())
|| spaceService.isSuperManager(space, currentIdentity.getUserId())
|| spaceService.isRedactor(space, currentIdentity.getUserId()));
}

private boolean canViewSharedInSpaces(News news, String username) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ public void testUpdateDraftArticle() throws Exception {
assertThrows(IllegalAccessException.class, () -> newsService.updateNews(news, "john", false, false, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase(), CONTENT_AND_TITLE.name()));

// Given
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(true);
org.exoplatform.social.core.identity.model.Identity identity1 =
mock(org.exoplatform.social.core.identity.model.Identity.class);
when(identityManager.getOrCreateUserIdentity(anyString())).thenReturn(identity1);
Expand Down Expand Up @@ -365,9 +365,13 @@ public void testDeleteDraftArticle() throws Exception {
when(rootPage.getName()).thenReturn(NEWS_ARTICLES_ROOT_NOTE_PAGE_NAME);
when(noteService.getDraftNoteById(anyString(), anyString())).thenReturn(draftPage);
NEWS_UTILS.when(() -> NewsUtils.getUserIdentity(anyString())).thenReturn(identity);
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);

//
assertThrows(IllegalAccessException.class, () -> newsService.deleteNews(draftPage.getId(), identity, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase()));

// When
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(true);

newsService.deleteNews(draftPage.getId(), identity, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase());

// Then
Expand Down Expand Up @@ -616,8 +620,7 @@ public void testCreateDraftArticleForExistingPage() throws Exception {
assertThrows(IllegalAccessException.class, () -> newsService.updateNews(news, "john", false, false, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase(), CONTENT_AND_TITLE.name()));

// Given
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);
when(spaceService.isSuperManager(anyString())).thenReturn(true);
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(true);
org.exoplatform.social.core.identity.model.Identity identity1 =
mock(org.exoplatform.social.core.identity.model.Identity.class);
when(identityManager.getOrCreateUserIdentity(anyString())).thenReturn(identity1);
Expand Down Expand Up @@ -700,8 +703,7 @@ public void testUpdateDraftArticleForExistingPage() throws Exception {
assertThrows(IllegalAccessException.class, () -> newsService.updateNews(news, "john", false, false, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase(), CONTENT_AND_TITLE.name()));

// Given
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);
when(spaceService.isSuperManager(anyString())).thenReturn(true);
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(true);
org.exoplatform.social.core.identity.model.Identity identity1 =
mock(org.exoplatform.social.core.identity.model.Identity.class);
when(identityManager.getOrCreateUserIdentity(anyString())).thenReturn(identity1);
Expand Down Expand Up @@ -776,8 +778,7 @@ public void testUpdateNewsArticle() throws Exception {
assertThrows(IllegalAccessException.class, () -> newsService.updateNews(news, "john", false, false, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase(), CONTENT_AND_TITLE.name()));

// Given
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);
when(spaceService.isSuperManager(anyString())).thenReturn(true);
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(true);
org.exoplatform.social.core.identity.model.Identity identity1 =
mock(org.exoplatform.social.core.identity.model.Identity.class);
when(identityManager.getOrCreateUserIdentity(anyString())).thenReturn(identity1);
Expand Down Expand Up @@ -829,7 +830,11 @@ public void testDeleteNewsArticle() throws Exception {
assertThrows(IllegalAccessException.class, () -> newsService.deleteNews(existingPage.getId(), identity, ARTICLE.name().toLowerCase()));

// when
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(false);
when(spaceService.isRedactor(any(Space.class), anyString())).thenReturn(false);
when(spaceService.isManager(any(Space.class), anyString())).thenReturn(false);
when(spaceService.isMember(anyString(), anyString())).thenReturn(true);

when(noteService.deleteNote(existingPage.getWikiType(), existingPage.getWikiOwner(), existingPage.getName())).thenReturn(true);
DraftPage draftPage = mock(DraftPage.class);
NotePageProperties draftProperties = new NotePageProperties();
Expand Down Expand Up @@ -1046,8 +1051,7 @@ public void testAddNewsArticleTranslation() throws Exception {
assertThrows(IllegalAccessException.class, () -> newsService.updateNews(news, "john", false, false, NewsUtils.NewsObjectType.DRAFT.name().toLowerCase(), CONTENT_AND_TITLE.name()));

// Given
when(spaceService.canRedactOnSpace(space, identity)).thenReturn(true);
when(spaceService.isSuperManager(anyString())).thenReturn(true);
when(spaceService.isSuperManager(any(Space.class), anyString())).thenReturn(true);
org.exoplatform.social.core.identity.model.Identity identity1 =
mock(org.exoplatform.social.core.identity.model.Identity.class);
when(identityManager.getOrCreateUserIdentity(anyString())).thenReturn(identity1);
Expand Down

0 comments on commit b68b038

Please sign in to comment.