Hi Jochen,
comments inside..
Am Freitag, 6. Januar 2012 um 12:25 schrieb Jochen Gazda:
> Gentlemen,
>
> Please read and comment.
>
> I have started to implement the ACL support. In the very first phase I
> would like to
> - add support for IMAP GETACL command and
> - add support for storing ACL at least for one storage backend.
>
> During this first phase I would like to learn how to do things in a
> James-compiliant way so that I will be able to implement other ACL
> commands later.
>
> 1. A new interface org.apache.james.mailbox.MailboxACL and default
> implementation org.apache.james.mailbox.store.SimpleMailboxACL
> Stores an ACL applicable to a mailbox. Inspired by RFC4314 IMAP4
> Access Control List (ACL) Extension.
> Note: An ACL class that could serve our purpose exists in
> com.sun.mail.imap. We will not stick to a proprietary API, will we?
>
>
Nope we should not use it here. I would even be in favor to remove the usage of
javamail at all in mailbox but thats another story..
>
> 2. MailboxACL usage:
> org.apache.james.mailbox.store.mail.model.Mailbox<Id> and
> implementations will be extended to store the related ACL:
> MailboxACL getACL();
> void setACL(MailboxACL acl);
> This is probably OK.
>
>
Yes
>
> 3. MailboxACL usage: org.apache.james.mailbox.MessageManager.MetaData
> will be extended to offer a read access to the ACL of the related
> mailbox. This was approved by Norman Maurer.
> MailboxACL getACL();
>
> 4. MailboxACL usage: Under org.apache.james.imap ACL-related requests,
> decoders, responses, encoders and processors will use MailboxACL.
> It is probably OK as there is a lot of org.apache.james.mailbox.*
> usage under org.apache.james.imap.
>
>
yeah...
>
> 5. MailboxACL instantiation: Clearly, there will be (at least) two
> places where MailboxACL will be instantiated:
> i. In Mailbox implementations when reading ACLs from their backends
> ii. In org.apache.james.imap decoders when parsing the ACL related
> IMAP requests.
>
> At the moment I have hardcoded a constructor of my default
> implementation on both places. I am asking myself if some kind of
> factory pattern should be used instead.
>
>
Just use the constructor for now, we can change later if needed...
>
> 6. How to access an ACL stored in a mailbox backend from a subclass of
> org.apache.james.imap.processor.AbstractMailboxProcessor:
> At the moment I have this code to prepare a response for GETACL IMAP command:
> MessageManager messageManager =
> getMailboxManager().getMailbox(buildFullPath(session, mailboxName),
> mailboxSession);
> MetaData metaData = messageManager.getMetaData(false,
> mailboxSession, FetchGroup.NO_COUNT);
>
> I have not tested it yet, but principally, it should work. I only
> wonder at two things with that code:
>
> i. org.apache.james.mailbox.MailboxManager.getMailbox(MailboxPath,
> MailboxSession)
> It returns a MessageManager rather than a Mailbox. Should it not
> rather be called getMessageManager?
>
>
Yeah it should. Its called getMailbox(..) for historic reasons.. I guess we
should change it like you suggested..
>
> ii. org.apache.james.mailbox.MessageManager.getMetaData(boolean,
> MailboxSession, FetchGroup)
> Javadoc says "Gets current meta data for the mailbox." Why is
> this method in MessageManager and not in MailboxManager?
>
>
As the info you need to retrieve is depend to the MessageManager you got. Like
getting the message count, unseen messages etc..
>
> 7. What and in which order should the GetACLProcessor say after
> sending the ACL response?
> i. Should it send also unsolicitedResponses?
> ii. unsolicitedResponses should be sent before or after okComplete?
> iii. on MailboxException, we send no() and I wonder what should its
> HumanReadableText say?
> iv. is the no() sufficient for all exceptions? What if somebody is
> asking for ACL of a folder which
> (a) does not exist or
> (b) cannot be looked up by the current user?
> Isn't taggedBad more suitable for (a) and/or (b)?
>
I will need to review the RFC for this to answer..
>
> Here is what I have now:
>
> @Override
> protected void doProcess(GetACLRequest message, ImapSession
> session, String tag, ImapCommand command, Responder responder) {
> final MailboxManager mailboxManager = getMailboxManager();
> final MailboxSession mailboxSession =
> ImapSessionUtils.getMailboxSession(session);
> try {
> String mailboxName = message.getMailboxName();
> MessageManager messageManager =
> mailboxManager.getMailbox(buildFullPath(session, mailboxName),
> mailboxSession);
> MetaData metaData = messageManager.getMetaData(false,
> mailboxSession, FetchGroup.NO_COUNT);
> ACLResponse aclResponse = new ACLResponse(mailboxName,
> metaData.getACL());
> responder.respond(aclResponse);
> okComplete(command, tag, responder);
> //FIXME should we send unsolicited responses here?
> //unsolicitedResponses(session, responder, false);
> } catch (MailboxException e) {
> // FIXME: be more specific in the human readable text.
> no(command, tag, responder,
> HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING);
> }
> }
>
>
> 8. Interpretation of ACLs:
> To have ACL stored on every mailbox is far from being able to tell if
> the given user can perform the given operation for the given
> mailbox(es when copying/moving).
> A new service responsible for resolving of ACLs is necessary. I
> propose to call it MailboxACLResolver.
> In which package should it be placed? Also in org.apache.james.mailbox?
> Probably every single operation between IMAP and mailbox stores should
> pass through this service. Where in the code should such a permission
> enforcement be placed?
> How should MailboxACLResolver be instantiated?
>
>
Yeah mailbox api I think… It should be instanced once and get injected in the
constructor.
>
> 9. A new event type:
> org.apache.james.mailbox.MailboxListener.MailboxACLUpdated
> It is quite clear what it should look like. It is also quite clear to
> me where and how it should be fired.
> But I cannot figure out who should listen to it and what the listener should
> do.
>
I need to read the rfc to answer this..
>
>
> Thanks,
>
> Gazda
>
>
--
Norman Maurer
