Re: ACL Support - cont. from server-user
Hi, sorry for the delay.. I will try to review it this evening... Thanks, Norman 2012/1/23 Jochen Gazda gazdahims...@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 norman.mau...@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 gazdahims...@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. us...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
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 norman.mau...@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 gazdahims...@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. us...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
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 gazdahims...@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. us...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
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 e...@apache.org wrote: Hi Jochen, You will need to replace all references in sever and app pom to apache-james-server-imap-* with groupIdorg.apache.james.protocols/groupId artifactIdprotocols-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 Charlese...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org -- eric | http://about.echarles.net | @echarles - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
It will get added back soon ;) But as Eric already said better use imap/trunk for now. Bye, Norman 2012/1/13 Jochen Gazda gazdahims...@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 e...@apache.org wrote: Hi Jochen, You will need to replace all references in sever and app pom to apache-james-server-imap-* with groupIdorg.apache.james.protocols/groupId artifactIdprotocols-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 Charlese...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org -- eric | http://about.echarles.net | @echarles - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
Hi Jochen, You will need to replace all references in sever and app pom to apache-james-server-imap-* with groupIdorg.apache.james.protocols/groupId artifactIdprotocols-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 Charlese...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org -- eric | http://about.echarles.net | @echarles - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
Eric, On Fri, Jan 6, 2012 at 6:34 PM, Eric Charles e...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
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.MailboxId 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,
Re: ACL Support - cont. from server-user
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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
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 gazdahims...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: ACL Support - cont. from server-user
@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.MailboxId 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:
Re: ACL Support - cont. from server-user
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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
