Hi Benoit,

Thank you for your feedback.

Answers are inline as well but overall it looks good feedback to me.

Best regards,
Rene.

On 22/01/2020 12:10, Tellier Benoit wrote:
Hi René,

My answers are inlined...

Best regard,

Benoit

On 22/01/2020 10:21, Rene Cordier wrote:
Hi guys,

I would like to bring on the table some proposition to refactor our
session usage within James.

# Context
[...]
Then I tried to use a `MailboxMapper` defined in the mailbox-store
module to be able to access to the mailbox containing that message
without having the mailbox id. However, we should not rely on this
outside of the mailbox modules, and rely more on the abstract API
defined in mailbox-api module.

In my opinion we would need another architecture decision record on this
consensual topic as this is the heart of our core APIs.

Could you take care about it?

Two ADRs in total then. Ok it makes sense to me, I will try to check all of that.


I would add in the "context" that as part of an upcoming effort to port
our current JMAP implementation to the lastest JMAP specification, we
will have to support JMAP account delegation. Thus being able to
audit/represent these concept efficiently when interacting with the
mailbox store is also related to this work

# Current API
[...]

# What we need

Well we can agree that the actual session is not enough to cover all the
cases that we have at the moment. I think we can have probably 4 types
of accesses within James to do operations on mailboxes :

* User session : user is logged and authenticated explicitly, does
operations on mailboxes he has rights on.
* User delegation session : James is doing user-related operations
internally, it's more of an implicit user session (mailets, listeners,
...?)

I think you are mixing two concepts here...

We have 'implicit' user actions, taken by the system on the behalf of
the user (eg: listener, mailets). These system action originated from a
user action.

Which is different to 'delegation': Bob gives Alice the right to
interact with Bob's mailbox as if she was Bob.

  - This can be revoked
  - We need to track that an action originated from Alice, and not from
Bob (audit)

Thanks for the clarification, that was not clear on my side indeed. Noted !


* Impersonation session : Admin is logged and authenticated, but has the
authorization to act on behalf of an other user
* System session : internal operations not directly related to a user (a
good example would be the recomputation of a jmap preview of a single
message)

We would need then to refactor the code to ensure we have different
types of sessions regarding those cases and review the session usage to
not introduce security vulnerabilities.


# Proposed solution

We could start by having a different session type for each of those
cases above, by also modifying the API in `SessionProvider` to have the
required methods achieved by this :

* `login` => user session : TypeSession.User
* `loginAsOtherUser` => impersonation session :
TypeSession.ImpersonatedUser
* `createDelegatedUserSession` => user delegation session :
TypeSession.DelegatedUser
* `createSystemSession` => system session : TypeSession.System

Those grained differentiations would also help better for auditing and
logging purposes.

We need as well to be careful and refactor the code regarding those new
types of sessions and sets of rights :

* `TypeSession.User` and `TypeSession.ImpersonatedUser` should have the
rights defined for the user
* `TypeSession.ImpersonatedUser` should have the rights of the user that
is being impersonated, not the logged in admin
* `TypeSession.System` should have bigger rights, to allow more global
operations on mailboxes in necessary cases (need to use carefully though)

Define "(need to use carefully though)"... How would you achieve
carefulness here?

Could be adding some more related tests, or having more granularity in the rights for system session, depending what we need at that place, instead of giving full rights. I'm not exactly sure yet though.


We need to define what kind of session is suitable for which parts of
the code and also add/update tests suites accordingly.

We probably should propose an ADR for this too...

That, is necessary, yes.


What do you think of it? Any feedback / discussion is welcomed of course
! Thank you for reading this.

I believe MailboxSession need to cary along different information
depending of the 'SessionType'.

User type just need to cary along the username
In case of delegation we need to have both users
In case of a system session (as defined above) have the name of the
service taking an action is more relevant than a username

Maybe we could have an object representing the Entity using this session
to interact with the mailbox store?

Like an Actor perhaps? UserActor, SystemActor, DelegatedActor, ImpersonatedActor



Best regards,
Rene.

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

Reply via email to