JAMES-2182 Fix rights for STORE

apache · Oct 20, 2024 · ba95f71 · ba95f71
1 parent df6eb01
commit ba95f71
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 59 deletions.
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java
@@ -679,12 +679,43 @@ private void trimFlags(Flags flags, MailboxSession session) {
 
     }
 
-    @Override
-    public Map<MessageUid, Flags> setFlags(final Flags flags, final FlagsUpdateMode flagsUpdateMode, final MessageRange set, MailboxSession mailboxSession) throws MailboxException {
+    public Optional<MailboxException> ensureFlagsWrite(Flags flags, FlagsUpdateMode flagsUpdateMode, MailboxSession mailboxSession) {
+        MailboxACL.Rfc4314Rights myRights = storeRightManager.myRights(mailbox, mailboxSession);
 
-        if (!isWriteable(mailboxSession)) {
-            throw new ReadOnlyException(getMailboxPath());
+        if (flagsUpdateMode.equals(FlagsUpdateMode.REPLACE)) {
+            if (!myRights.contains(MailboxACL.Right.Write, MailboxACL.Right.WriteSeenFlag, MailboxACL.Right.DeleteMessages)) {
+                return Optional.of(new InsufficientRightsException("'stw' rights are needed to reset flags"));
+            }
+            return Optional.empty();
+        }
+
+        if (flags.contains(Flag.SEEN) && !myRights.contains(MailboxACL.Right.WriteSeenFlag)) {
+            return Optional.of(new InsufficientRightsException("'s' right is needed to modify seen flag"));
+        }
+
+        if (flags.contains(Flag.DELETED) && !myRights.contains(MailboxACL.Right.DeleteMessages)) {
+            return Optional.of(new InsufficientRightsException("'t' right is needed to modify deleted flag"));
+        }
+
+        boolean hasOtherFlagChanges = flags.getUserFlags().length > 0
+            || flags.contains(Flag.FLAGGED)
+            || flags.contains(Flag.DRAFT)
+            || flags.contains(Flag.ANSWERED)
+            || flags.contains(Flag.RECENT);
+
+        if (hasOtherFlagChanges && !myRights.contains(MailboxACL.Right.Write)) {
+            return Optional.of(new InsufficientRightsException("'w' right is needed to modify arbitrary flags"));
         }
+        return Optional.empty();
+    }
+
+    @Override
+    public Map<MessageUid, Flags> setFlags(Flags flags, FlagsUpdateMode flagsUpdateMode, MessageRange set, MailboxSession mailboxSession) throws MailboxException {
+
+        ensureFlagsWrite(flags, flagsUpdateMode, mailboxSession)
+            .ifPresent(Throwing.<MailboxException>consumer(e -> {
+                throw e;
+            }).sneakyThrow());
 
         trimFlags(flags, mailboxSession);
 
@@ -709,25 +740,25 @@ public Map<MessageUid, Flags> setFlags(final Flags flags, final FlagsUpdateMode
 
     @Override
     public Publisher<Map<MessageUid, Flags>> setFlagsReactive(Flags flags, FlagsUpdateMode flagsUpdateMode, MessageRange set, MailboxSession mailboxSession) {
-        if (!isWriteable(mailboxSession)) {
-            return Mono.error(new ReadOnlyException(getMailboxPath()));
-        }
-
-        trimFlags(flags, mailboxSession);
-
-        MessageMapper messageMapper = mapperFactory.getMessageMapper(mailboxSession);
-
-        return messageMapper.executeReactive(messageMapper.updateFlagsReactive(getMailboxEntity(), new FlagsUpdateCalculator(flags, flagsUpdateMode), set))
-            .flatMap(updatedFlags -> eventBus.dispatch(EventFactory.flagsUpdated()
-                    .randomEventId()
-                    .mailboxSession(mailboxSession)
-                    .mailbox(getMailboxEntity())
-                    .updatedFlags(updatedFlags)
-                    .build(),
-                new MailboxIdRegistrationKey(mailbox.getMailboxId()))
-                .thenReturn(updatedFlags.stream().collect(ImmutableMap.toImmutableMap(
-                    UpdatedFlags::getUid,
-                    UpdatedFlags::getNewFlags))));
+        return ensureFlagsWrite(flags, flagsUpdateMode, mailboxSession)
+            .map(Mono::<Map<MessageUid, Flags>>error)
+            .orElseGet(() -> {
+                trimFlags(flags, mailboxSession);
+
+                MessageMapper messageMapper = mapperFactory.getMessageMapper(mailboxSession);
+
+                return messageMapper.executeReactive(messageMapper.updateFlagsReactive(getMailboxEntity(), new FlagsUpdateCalculator(flags, flagsUpdateMode), set))
+                    .flatMap(updatedFlags -> eventBus.dispatch(EventFactory.flagsUpdated()
+                                .randomEventId()
+                                .mailboxSession(mailboxSession)
+                                .mailbox(getMailboxEntity())
+                                .updatedFlags(updatedFlags)
+                                .build(),
+                            new MailboxIdRegistrationKey(mailbox.getMailboxId()))
+                        .thenReturn(updatedFlags.stream().collect(ImmutableMap.toImmutableMap(
+                            UpdatedFlags::getUid,
+                            UpdatedFlags::getNewFlags))));
+            });
     }
 
     /**

diff --git a/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRI.test b/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRI.test
@@ -84,21 +84,17 @@ C: F11 FETCH 1 FLAGS
 S: \* 1 FETCH \(FLAGS \(\\Recent\)\)
 S: F11 OK FETCH completed.
 
-# TODO Insert is not flags
 C: F12 STORE 1 +FLAGS (\Deleted)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
-S: F12 OK STORE completed.
+S: F12 NO STORE failed. Save failed.
 
 C: F13 STORE 1 +FLAGS (\Seen)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent \\Seen\)\)
-S: F13 OK STORE completed.
+S: F13 NO STORE failed. Save failed.
 
 C: F14 STORE 1 +FLAGS (\Flagged)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
-S: F14 OK STORE completed.
+S: F14 NO STORE failed. Save failed.
 
 C: F11 FETCH 1 FLAGS
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Recent\)\)
 S: F11 OK FETCH completed.
 
 C: F15 EXPUNGE

diff --git a/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRS.test b/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRS.test
@@ -86,21 +86,18 @@ C: F11 FETCH 1 FLAGS
 S: \* 1 FETCH \(FLAGS \(\\Recent\)\)
 S: F11 OK FETCH completed.
 
-# TODO I shall only be able to manipulate \\Seen...
 C: F12 STORE 1 +FLAGS (\Deleted)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
-S: F12 OK STORE completed.
+S: F12 NO STORE failed. Save failed.
 
 C: F13 STORE 1 +FLAGS (\Seen)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Recent \\Seen\)\)
 S: F13 OK STORE completed.
 
 C: F14 STORE 1 +FLAGS (\Flagged)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
-S: F14 OK STORE completed.
+S: F14 NO STORE failed. Save failed.
 
 C: F11 FETCH 1 FLAGS
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Recent \\Seen\)\)
 S: F11 OK FETCH completed.
 
 C: F15 EXPUNGE

diff --git a/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRT.test b/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRT.test
@@ -90,17 +90,14 @@ C: F12 STORE 1 +FLAGS (\Deleted)
 S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
 S: F12 OK STORE completed.
 
-# TODO I shall only be able to manipulate \\Deleted...
 C: F13 STORE 1 +FLAGS (\Seen)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent \\Seen\)\)
-S: F13 OK STORE completed.
+S: F13 NO STORE failed. Save failed.
 
 C: F14 STORE 1 +FLAGS (\Flagged)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
-S: F14 OK STORE completed.
+S: F14 NO STORE failed. Save failed.
 
 C: F11 FETCH 1 FLAGS
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
 S: F11 OK FETCH completed.
 
 C: F15 EXPUNGE

diff --git a/...imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRTE.test b/...imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRTE.test
@@ -90,17 +90,14 @@ C: F12 STORE 1 +FLAGS (\Deleted)
 S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
 S: F12 OK STORE completed.
 
-# TODO I shall only be able to manipulate \\Deleted...
 C: F13 STORE 1 +FLAGS (\Seen)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent \\Seen\)\)
-S: F13 OK STORE completed.
+S: F13 NO STORE failed. Save failed.
 
 C: F14 STORE 1 +FLAGS (\Flagged)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
-S: F14 OK STORE completed.
+S: F14 NO STORE failed. Save failed.
 
 C: F11 FETCH 1 FLAGS
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
 S: F11 OK FETCH completed.
 
 C: F15 EXPUNGE

diff --git a/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRW.test b/.../imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRW.test
@@ -86,21 +86,18 @@ C: F11 FETCH 1 FLAGS
 S: \* 1 FETCH \(FLAGS \(\\Recent\)\)
 S: F11 OK FETCH completed.
 
-# TODO I shall only be able to manipulate \\Seen...
 C: F12 STORE 1 +FLAGS (\Deleted)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent\)\)
-S: F12 OK STORE completed.
+S: F12 NO STORE failed. Save failed.
 
 C: F13 STORE 1 +FLAGS (\Seen)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Recent \\Seen\)\)
-S: F13 OK STORE completed.
+S: F13 NO STORE failed. Save failed.
 
 C: F14 STORE 1 +FLAGS (\Flagged)
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Flagged \\Recent\)\)
 S: F14 OK STORE completed.
 
 C: F11 FETCH 1 FLAGS
-S: \* 1 FETCH \(FLAGS \(\\Deleted \\Flagged \\Recent \\Seen\)\)
+S: \* 1 FETCH \(FLAGS \(\\Flagged \\Recent\)\)
 S: F11 OK FETCH completed.
 
 C: F15 EXPUNGE

diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java
@@ -404,8 +404,9 @@ private Mono<MailboxMetaData> selectMailbox(MailboxPath mailboxPath, ImapSession
             .<MessageManager>handle(Throwing.biConsumer((mailbox, sink) -> {
                 if (mailboxManager.hasRight(mailbox.getMailboxEntity(), MailboxACL.Right.Read, mailboxSession)) {
                     sink.next(mailbox);
+                } else {
+                    sink.error(new InsufficientRightsException("'r' right is needed to select a mailbox"));
                 }
-                sink.error(new InsufficientRightsException("'r' right is needed to select a mailbox"));
             }))
             .flatMap(Throwing.function(mailbox -> selectMailbox(session, responder, mailbox, currentMailbox)
                 .flatMap(Throwing.function(sessionMailbox ->

diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/StatusProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/StatusProcessor.java
@@ -127,8 +127,9 @@ private Mono<MailboxStatusResponse> sendStatus(MailboxPath mailboxPath, StatusDa
             .<MessageManager>handle(Throwing.biConsumer((mailbox, sink) -> {
                 if (getMailboxManager().hasRight(mailbox.getMailboxEntity(), MailboxACL.Right.Read, mailboxSession)) {
                     sink.next(mailbox);
+                } else {
+                    sink.error(new InsufficientRightsException("'r' right is needed to status a mailbox"));
                 }
-                sink.error(new InsufficientRightsException("'r' right is needed to status a mailbox"));
             }))
             .flatMap(mailbox -> sendStatus(mailbox, statusDataItems, responder, session, mailboxSession));
     }