You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "chibenwa (via GitHub)" <gi...@apache.org> on 2023/03/29 09:59:29 UTC
[GitHub] [james-project] chibenwa commented on a diff in pull request #1505: IMAP - Make reactive SetACLProcessor/DeleteACLProcessor
chibenwa commented on code in PR #1505:
URL: https://github.com/apache/james-project/pull/1505#discussion_r1151688865
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/SetACLProcessor.java:
##########
@@ -62,19 +67,41 @@ public SetACLProcessor(MailboxManager mailboxManager, StatusResponseFactory fact
}
@Override
- protected void processRequest(SetACLRequest request, ImapSession session, Responder responder) {
-
+ protected Mono<Void> processRequestReactive(SetACLRequest request, ImapSession session, Responder responder) {
final MailboxManager mailboxManager = getMailboxManager();
final MailboxSession mailboxSession = session.getMailboxSession();
final String mailboxName = request.getMailboxName();
final String identifier = request.getIdentifier();
- try {
-
- /* parsing the rights is the the cheapest thing to begin with */
- EditMode editMode = EditMode.REPLACE;
- String rights = request.getRights();
- if (rights != null && rights.length() > 0) {
- switch (rights.charAt(0)) {
+ MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
+
+ return checkLookupRight(request, responder, mailboxManager, mailboxSession, mailboxPath)
+ .filter(FunctionalUtils.identityPredicate())
+ .flatMap(hasLookupRight -> checkAdminRight(request, responder, mailboxManager, mailboxSession, mailboxName, mailboxPath))
+ .filter(FunctionalUtils.identityPredicate())
+ .flatMap(hasAdminRight -> applyRight(mailboxManager, mailboxSession, identifier, request, mailboxPath)
+ .then(Mono.fromRunnable(() -> okComplete(request, responder)))
+ .then())
+ .onErrorResume(UnsupportedRightException.class,
+ error -> Mono.fromRunnable(() -> taggedBad(request, responder,
+ new HumanReadableText(
+ HumanReadableText.UNSUPPORTED_RIGHT_KEY,
+ HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE,
+ error.getUnsupportedRight()))))
+ .onErrorResume(MailboxNotFoundException.class,
+ error -> Mono.fromRunnable(() -> no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND)))
+ .onErrorResume(MailboxException.class,
+ error -> {
+ LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, error);
Review Comment:
Idem applymdc context upon logs
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/DeleteACLProcessor.java:
##########
@@ -59,79 +62,64 @@ public DeleteACLProcessor(MailboxManager mailboxManager, StatusResponseFactory f
}
@Override
- protected void processRequest(DeleteACLRequest request, ImapSession session, Responder responder) {
-
+ protected Mono<Void> processRequestReactive(DeleteACLRequest request, ImapSession session, Responder responder) {
final MailboxManager mailboxManager = getMailboxManager();
final MailboxSession mailboxSession = session.getMailboxSession();
final String mailboxName = request.getMailboxName();
final String identifier = request.getIdentifier();
- try {
-
- MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
- // Check that mailbox exists
- mailboxManager.getMailbox(mailboxPath, mailboxSession);
- /*
- * RFC 4314 section 6.
- * An implementation MUST make sure the ACL commands themselves do
- * not give information about mailboxes with appropriately
- * restricted ACLs. For example, when a user agent executes a GETACL
- * command on a mailbox that the user has no permission to LIST, the
- * server would respond to that request with the same error that
- * would be used if the mailbox did not exist, thus revealing no
- * existence information, much less the mailbox’s ACL.
- */
- if (!mailboxManager.hasRight(mailboxPath, MailboxACL.Right.Lookup, mailboxSession)) {
- no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
- } else if (!mailboxManager.hasRight(mailboxPath, MailboxACL.Right.Administer, mailboxSession)) {
- /* RFC 4314 section 4. */
- Object[] params = new Object[] {
- MailboxACL.Right.Administer.toString(),
- request.getCommand().getName(),
- mailboxName
- };
- HumanReadableText text = new HumanReadableText(HumanReadableText.UNSUFFICIENT_RIGHTS_KEY, HumanReadableText.UNSUFFICIENT_RIGHTS_DEFAULT_VALUE, params);
- no(request, responder, text);
- } else {
-
- EntryKey key = EntryKey.deserialize(identifier);
-
- // FIXME check if identifier is a valid user or group
- // FIXME Servers, when processing a command that has an identifier as a
- // parameter (i.e., any of SETACL, DELETEACL, and LISTRIGHTS commands),
- // SHOULD first prepare the received identifier using "SASLprep" profile
- // [SASLprep] of the "stringprep" algorithm [Stringprep]. If the
- // preparation of the identifier fails or results in an empty string,
- // the server MUST refuse to perform the command with a BAD response.
- // Note that Section 6 recommends additional identifier’s verification
- // steps.
-
- mailboxManager.applyRightsCommand(
- mailboxPath,
- MailboxACL.command().key(key).noRights().asReplacement(),
- mailboxSession
- );
-
- okComplete(request, responder);
- // FIXME should we send unsolicited responses here?
- // unsolicitedResponses(session, responder, false);
- }
- } catch (UnsupportedRightException e) {
- /*
- * RFc 4314, section 3.1
- * Note that an unrecognized right MUST cause the command to return the
- * BAD response. In particular, the server MUST NOT silently ignore
- * unrecognized rights.
- * */
- Object[] params = new Object[] {e.getUnsupportedRight()};
- HumanReadableText text = new HumanReadableText(HumanReadableText.UNSUPPORTED_RIGHT_KEY, HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE, params);
- taggedBad(request, responder, text);
- } catch (MailboxNotFoundException e) {
- no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
- } catch (MailboxException e) {
- LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, e);
- no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING);
- }
+ MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
+
+ return checkLookupRight(request, responder, mailboxManager, mailboxSession, mailboxPath)
+ .filter(FunctionalUtils.identityPredicate())
+ .flatMap(hasLookupRight -> checkAdminRight(request, responder, mailboxManager, mailboxSession, mailboxName, mailboxPath))
+ .filter(FunctionalUtils.identityPredicate())
+ .flatMap(hasAdminRight -> applyRight(mailboxManager, mailboxSession, identifier, mailboxPath)
+ .then(Mono.fromRunnable(() -> okComplete(request, responder)))
+ .then())
+ .onErrorResume(UnsupportedRightException.class,
+ error -> Mono.fromRunnable(() -> taggedBad(request, responder,
+ new HumanReadableText(
+ HumanReadableText.UNSUPPORTED_RIGHT_KEY,
+ HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE,
+ error.getUnsupportedRight()))))
+ .onErrorResume(MailboxNotFoundException.class,
+ error -> Mono.fromRunnable(() -> no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND)))
+ .onErrorResume(MailboxException.class,
+ error -> {
+ LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, error);
Review Comment:
Please manage to get contextualized logd
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org