You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Jochen Gazda <ga...@gmail.com> on 2012/01/06 12:25:21 UTC

ACL Support - cont. from server-user

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?

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.

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.

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.

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?

  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?

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)?

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?

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.


Thanks,

Gazda

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Norman Maurer <no...@googlemail.com>.
Hi,

sorry for the delay.. I will try to review it this evening...


Thanks,
Norman


2012/1/23 Jochen Gazda <ga...@gmail.com>:
> Norman,
>
> I have created https://issues.apache.org/jira/browse/IMAP-351 on 2012-01-18.
> When may I expect that it will be reviewed?
>
> Best,
>
> Gazda
>
> On Wed, Jan 18, 2012 at 2:29 PM, Norman Maurer
> <no...@googlemail.com> wrote:
>> Wow....
>>
>> very nice work. I would be more then happy to include it into our
>> code-base. Please open a jira and attach the patch there (don't forget
>> to "check" the ASL2 box). Once its there I will pull the changes in.
>>
>> Thanks,
>> Norman
>>
>>
>> 2012/1/17 Jochen Gazda <ga...@gmail.com>:
>>> Gentlemen,
>>>
>>> could please somebody have a look at the attached patch?
>>> It should be applied against "current" directory. I hope have created
>>> it properly. I have never done it before.
>>>
>>> The patch adds very basic support for GETACL command. A small javamail
>>> based demo program is also attached. "user1@dom1.com" with password
>>> "1234" needs to be created before ImapGetACLTest is run.
>>>
>>> Within my workspace the following tests failed or were skipped for
>>> taking too long to finish, whereby I cannot say if it was due to my
>>> changes:
>>> james-server-smtpserver
>>> apache-james-mailbox-hbase
>>> james-server-queue-activemq
>>>
>>> What needs to be done:
>>>
>>> (1) I have put some FIXMEs and TODOs in the patch where somebody
>>> experienced should please have a look.
>>>
>>> (2) ACL getter and setter stubs should be filled in Mailbox
>>> implementations. New tests should be added for those getters and
>>> setters.
>>> Due to the fact that there is no ACL persistence support in the
>>> mailbox implementations, the only ACLs that are effective at the
>>> moment are the global ACLs in
>>> org.apache.james.mailbox.UnionMailboxACLResolver. The default values
>>> for these global ACLs are: (a) full rights for mailbox owner if the
>>> mailbox owner is a user and (b) full except for a (administration)
>>> rights if the mailbox is a group. I guess this is roughly how it
>>> worked before my patch.
>>>
>>> (3) Mapping of ACL Rights to READ-WRITE and READ-ONLY Response Codes,
>>> see RFC 4314 section 5.2.
>>>
>>> (4) Enforce rights required to perform different IMAP4rev1 Commands in
>>> IMAP processors, see RFC 4314 section 4. Write tests.
>>>
>>> (5) MailboxACLUpdated event should be fired and litstened to.
>>>
>>> Best,
>>>
>>> Gazda
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Jochen Gazda <ga...@gmail.com>.
Norman,

I have created https://issues.apache.org/jira/browse/IMAP-351 on 2012-01-18.
When may I expect that it will be reviewed?

Best,

Gazda

On Wed, Jan 18, 2012 at 2:29 PM, Norman Maurer
<no...@googlemail.com> wrote:
> Wow....
>
> very nice work. I would be more then happy to include it into our
> code-base. Please open a jira and attach the patch there (don't forget
> to "check" the ASL2 box). Once its there I will pull the changes in.
>
> Thanks,
> Norman
>
>
> 2012/1/17 Jochen Gazda <ga...@gmail.com>:
>> Gentlemen,
>>
>> could please somebody have a look at the attached patch?
>> It should be applied against "current" directory. I hope have created
>> it properly. I have never done it before.
>>
>> The patch adds very basic support for GETACL command. A small javamail
>> based demo program is also attached. "user1@dom1.com" with password
>> "1234" needs to be created before ImapGetACLTest is run.
>>
>> Within my workspace the following tests failed or were skipped for
>> taking too long to finish, whereby I cannot say if it was due to my
>> changes:
>> james-server-smtpserver
>> apache-james-mailbox-hbase
>> james-server-queue-activemq
>>
>> What needs to be done:
>>
>> (1) I have put some FIXMEs and TODOs in the patch where somebody
>> experienced should please have a look.
>>
>> (2) ACL getter and setter stubs should be filled in Mailbox
>> implementations. New tests should be added for those getters and
>> setters.
>> Due to the fact that there is no ACL persistence support in the
>> mailbox implementations, the only ACLs that are effective at the
>> moment are the global ACLs in
>> org.apache.james.mailbox.UnionMailboxACLResolver. The default values
>> for these global ACLs are: (a) full rights for mailbox owner if the
>> mailbox owner is a user and (b) full except for a (administration)
>> rights if the mailbox is a group. I guess this is roughly how it
>> worked before my patch.
>>
>> (3) Mapping of ACL Rights to READ-WRITE and READ-ONLY Response Codes,
>> see RFC 4314 section 5.2.
>>
>> (4) Enforce rights required to perform different IMAP4rev1 Commands in
>> IMAP processors, see RFC 4314 section 4. Write tests.
>>
>> (5) MailboxACLUpdated event should be fired and litstened to.
>>
>> Best,
>>
>> Gazda
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Norman Maurer <no...@googlemail.com>.
Wow....

very nice work. I would be more then happy to include it into our
code-base. Please open a jira and attach the patch there (don't forget
to "check" the ASL2 box). Once its there I will pull the changes in.

Thanks,
Norman


2012/1/17 Jochen Gazda <ga...@gmail.com>:
> Gentlemen,
>
> could please somebody have a look at the attached patch?
> It should be applied against "current" directory. I hope have created
> it properly. I have never done it before.
>
> The patch adds very basic support for GETACL command. A small javamail
> based demo program is also attached. "user1@dom1.com" with password
> "1234" needs to be created before ImapGetACLTest is run.
>
> Within my workspace the following tests failed or were skipped for
> taking too long to finish, whereby I cannot say if it was due to my
> changes:
> james-server-smtpserver
> apache-james-mailbox-hbase
> james-server-queue-activemq
>
> What needs to be done:
>
> (1) I have put some FIXMEs and TODOs in the patch where somebody
> experienced should please have a look.
>
> (2) ACL getter and setter stubs should be filled in Mailbox
> implementations. New tests should be added for those getters and
> setters.
> Due to the fact that there is no ACL persistence support in the
> mailbox implementations, the only ACLs that are effective at the
> moment are the global ACLs in
> org.apache.james.mailbox.UnionMailboxACLResolver. The default values
> for these global ACLs are: (a) full rights for mailbox owner if the
> mailbox owner is a user and (b) full except for a (administration)
> rights if the mailbox is a group. I guess this is roughly how it
> worked before my patch.
>
> (3) Mapping of ACL Rights to READ-WRITE and READ-ONLY Response Codes,
> see RFC 4314 section 5.2.
>
> (4) Enforce rights required to perform different IMAP4rev1 Commands in
> IMAP processors, see RFC 4314 section 4. Write tests.
>
> (5) MailboxACLUpdated event should be fired and litstened to.
>
> Best,
>
> Gazda
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Jochen Gazda <ga...@gmail.com>.
Gentlemen,

could please somebody have a look at the attached patch?
It should be applied against "current" directory. I hope have created
it properly. I have never done it before.

The patch adds very basic support for GETACL command. A small javamail
based demo program is also attached. "user1@dom1.com" with password
"1234" needs to be created before ImapGetACLTest is run.

Within my workspace the following tests failed or were skipped for
taking too long to finish, whereby I cannot say if it was due to my
changes:
james-server-smtpserver
apache-james-mailbox-hbase
james-server-queue-activemq

What needs to be done:

(1) I have put some FIXMEs and TODOs in the patch where somebody
experienced should please have a look.

(2) ACL getter and setter stubs should be filled in Mailbox
implementations. New tests should be added for those getters and
setters.
Due to the fact that there is no ACL persistence support in the
mailbox implementations, the only ACLs that are effective at the
moment are the global ACLs in
org.apache.james.mailbox.UnionMailboxACLResolver. The default values
for these global ACLs are: (a) full rights for mailbox owner if the
mailbox owner is a user and (b) full except for a (administration)
rights if the mailbox is a group. I guess this is roughly how it
worked before my patch.

(3) Mapping of ACL Rights to READ-WRITE and READ-ONLY Response Codes,
see RFC 4314 section 5.2.

(4) Enforce rights required to perform different IMAP4rev1 Commands in
IMAP processors, see RFC 4314 section 4. Write tests.

(5) MailboxACLUpdated event should be fired and litstened to.

Best,

Gazda

Re: ACL Support - cont. from server-user

Posted by Norman Maurer <no...@googlemail.com>.
It will get added back soon ;) But as Eric already said better use
imap/trunk for now.

Bye,
Norman

2012/1/13 Jochen Gazda <ga...@gmail.com>

> Thank you for your advice, Eric, but my efforts with protocols-imap
> are pointless now, as protocols-imap was removed from the svn repo
> yesterday.
>
> Gazda
>
> On Wed, Jan 11, 2012 at 8:38 PM, Eric Charles <er...@apache.org> wrote:
> > Hi Jochen,
> >
> > You will need to replace all references in sever and app pom to
> > apache-james-server-imap-* with
> >
> >    <groupId>org.apache.james.protocols</groupId>
> >    <artifactId>protocols-imap</artifactId>
> >
> >
> > As said earlier, this copy of imap is less stable.
> >
> > Thx,
> >
> > Eric
> >
> >
> >
> > On 10/01/12 19:52, Jochen Gazda wrote:
> >>
> >> Eric,
> >>
> >> On Fri, Jan 6, 2012 at 6:34 PM, Eric Charles<er...@apache.org>  wrote:
> >>>
> >>> As a side note, we are moving IMAP code from
> >>>
> >>> https://svn.apache.org/repos/asf/james/imap/trunk/
> >>> to
> >>> https://svn.apache.org/repos/asf/james/protocols/trunk/imap/
> >>>
> >>> I would be better that you code in the latter.
> >>
> >>
> >> I have coded with /james/protocols/trunk/imap/ and I would like run my
> >> code with James. How can I do it?
> >> The default build results in James with /james/imap/trunk/. How can I
> >> replace it?
> >>
> >> Thanks,
> >>
> >> Gazda
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> >> For additional commands, e-mail: server-dev-help@james.apache.org
> >>
> >
> > --
> > eric | http://about.echarles.net | @echarles
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> > For additional commands, e-mail: server-dev-help@james.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>

Re: ACL Support - cont. from server-user

Posted by Jochen Gazda <ga...@gmail.com>.
Thank you for your advice, Eric, but my efforts with protocols-imap
are pointless now, as protocols-imap was removed from the svn repo
yesterday.

Gazda

On Wed, Jan 11, 2012 at 8:38 PM, Eric Charles <er...@apache.org> wrote:
> Hi Jochen,
>
> You will need to replace all references in sever and app pom to
> apache-james-server-imap-* with
>
>    <groupId>org.apache.james.protocols</groupId>
>    <artifactId>protocols-imap</artifactId>
>
>
> As said earlier, this copy of imap is less stable.
>
> Thx,
>
> Eric
>
>
>
> On 10/01/12 19:52, Jochen Gazda wrote:
>>
>> Eric,
>>
>> On Fri, Jan 6, 2012 at 6:34 PM, Eric Charles<er...@apache.org>  wrote:
>>>
>>> As a side note, we are moving IMAP code from
>>>
>>> https://svn.apache.org/repos/asf/james/imap/trunk/
>>> to
>>> https://svn.apache.org/repos/asf/james/protocols/trunk/imap/
>>>
>>> I would be better that you code in the latter.
>>
>>
>> I have coded with /james/protocols/trunk/imap/ and I would like run my
>> code with James. How can I do it?
>> The default build results in James with /james/imap/trunk/. How can I
>> replace it?
>>
>> Thanks,
>>
>> Gazda
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>
> --
> eric | http://about.echarles.net | @echarles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Eric Charles <er...@apache.org>.
Hi Jochen,

You will need to replace all references in sever and app pom to 
apache-james-server-imap-* with

     <groupId>org.apache.james.protocols</groupId>
     <artifactId>protocols-imap</artifactId>


As said earlier, this copy of imap is less stable.

Thx,

Eric


On 10/01/12 19:52, Jochen Gazda wrote:
> Eric,
>
> On Fri, Jan 6, 2012 at 6:34 PM, Eric Charles<er...@apache.org>  wrote:
>> As a side note, we are moving IMAP code from
>>
>> https://svn.apache.org/repos/asf/james/imap/trunk/
>> to
>> https://svn.apache.org/repos/asf/james/protocols/trunk/imap/
>>
>> I would be better that you code in the latter.
>
> I have coded with /james/protocols/trunk/imap/ and I would like run my
> code with James. How can I do it?
> The default build results in James with /james/imap/trunk/. How can I
> replace it?
>
> Thanks,
>
> Gazda
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

-- 
eric | http://about.echarles.net | @echarles

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Jochen Gazda <ga...@gmail.com>.
Eric,

On Fri, Jan 6, 2012 at 6:34 PM, Eric Charles <er...@apache.org> wrote:
> As a side note, we are moving IMAP code from
>
> https://svn.apache.org/repos/asf/james/imap/trunk/
> to
> https://svn.apache.org/repos/asf/james/protocols/trunk/imap/
>
> I would be better that you code in the latter.

I have coded with /james/protocols/trunk/imap/ and I would like run my
code with James. How can I do it?
The default build results in James with /james/imap/trunk/. How can I
replace it?

Thanks,

Gazda

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Eric Charles <er...@apache.org>.
If protocols-imap is gonna move soon, then 
https://svn.apache.org/repos/asf/james/imap/trunk/ will be better yes.

Eric


On 06/01/12 18:36, Norman Maurer wrote:
> @Eric:
>
> I think Jochen should just code in the imap trunk for now.. We will merge it later then. As the code in protocols need many more work :/
>
> Bye,
> Norman
>
>

-- 
eric | http://about.echarles.net | @echarles

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Norman Maurer <no...@apache.org>.
@Eric:

I think Jochen should just code in the imap trunk for now.. We will merge it later then. As the code in protocols need many more work :/

Bye,
Norman


-- 
Norman Maurer


Am Freitag, 6. Januar 2012 um 18:34 schrieb Eric Charles:

> As a side note, we are moving IMAP code from
> 
> https://svn.apache.org/repos/asf/james/imap/trunk/
> to
> https://svn.apache.org/repos/asf/james/protocols/trunk/imap/
> 
> I would be better that you code in the latter.
> 
> Thx,
> Eric
> 
> 
> On 06/01/12 12:25, Jochen Gazda wrote:
> > 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?
> > 
> > 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.
> > 
> > 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.
> > 
> > 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.
> > 
> > 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?
> > 
> > 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?
> > 
> > 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)?
> > 
> > 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?
> > 
> > 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.
> > 
> > 
> > Thanks,
> > 
> > Gazda
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org (mailto:server-dev-unsubscribe@james.apache.org)
> > For additional commands, e-mail: server-dev-help@james.apache.org (mailto:server-dev-help@james.apache.org)
> > 
> 
> 
> -- 
> eric | http://about.echarles.net | @echarles
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org (mailto:server-dev-unsubscribe@james.apache.org)
> For additional commands, e-mail: server-dev-help@james.apache.org (mailto:server-dev-help@james.apache.org)
> 
>

Re: ACL Support - cont. from server-user

Posted by Eric Charles <er...@apache.org>.
As a side note, we are moving IMAP code from

https://svn.apache.org/repos/asf/james/imap/trunk/
to
https://svn.apache.org/repos/asf/james/protocols/trunk/imap/

I would be better that you code in the latter.

Thx,
Eric


On 06/01/12 12:25, Jochen Gazda wrote:
> 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?
>
> 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.
>
> 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.
>
> 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.
>
> 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?
>
>    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?
>
> 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)?
>
> 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?
>
> 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.
>
>
> Thanks,
>
> Gazda
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

-- 
eric | http://about.echarles.net | @echarles

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Norman Maurer <no...@googlemail.com>.
You should use the MailboxPath for resolve the ACLs.

Where you need to inject the resolver depends on the needs. But I guess you need it in both..

Bye
Norman

Sent from my iPhone. Excuse any typos....

Am 06.01.2012 um 14:18 schrieb Jochen Gazda <ga...@gmail.com>:

> Norman,
> 
>>> 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.
> 
> Hmmm... I thought that MailboxACLResolver would offer a method like
> the following:
> 
>  boolean hasRight(User user, Mailbox mailbox, Right right);
> 
> But org.apache.james.mailbox.store.mail.model.Mailbox is not visible
> in in mailbox-store. Should I place MailboxACLResolver to
> mailbox-store? Or I should rely on
> org.apache.james.mailbox.MailboxPath rather than Mailbox itself?
> 
>> [MailboxACLResolver] should be instanced once and get injected in the constructor.
> 
> Which constructor? MessageManager or MailBoxManager or both?
> 
> Best,
> 
> Gazda
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Jochen Gazda <ga...@gmail.com>.
Norman,

>> 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.

Hmmm... I thought that MailboxACLResolver would offer a method like
the following:

  boolean hasRight(User user, Mailbox mailbox, Right right);

But org.apache.james.mailbox.store.mail.model.Mailbox is not visible
in in mailbox-store. Should I place MailboxACLResolver to
mailbox-store? Or I should rely on
org.apache.james.mailbox.MailboxPath rather than Mailbox itself?

> [MailboxACLResolver] should be instanced once and get injected in the constructor.

Which constructor? MessageManager or MailBoxManager or both?

Best,

Gazda

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: ACL Support - cont. from server-user

Posted by Norman Maurer <no...@apache.org>.
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