Hi guys,
I would like to bring on the table some proposition to refactor our
session usage within James.
# Context
The problem starts from
https://issues.apache.org/jira/browse/JAMES-2993, more particularly the
API `POST /jmap/messages/:messageId?action=recomputeJmapPreview` to
recompute a preview of a message with its ID
(https://github.com/linagora/james-project/pull/3035).
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. But it is not possible actually to access
a mailbox without knowing the user it belongs to (in mailbox-api). Thus
started to emerge the idea of giving greater access to a system session
for solving this case.
Discussion:
https://github.com/linagora/james-project/pull/3035#discussion_r363684700
# Current API
In my understanding, regarding the session matter giving the rights to
user to do some operations on mailboxes, it seems working like this to
me at the moment :
* `MailboxSession` is the session object giving the user permissions to
do operations on mailboxes. It contains actually two defined
`SessionType` : `System` and `User`.
* `SessionProvider` is the API that provides the session with different
methods. `SessionProviderImpl` is its implementation.
Actually, the `SessionProvider` allows to provide sessions in those ways:
* `login` => explicitly authenticates and authorizes a user to do
actions on mailboxes he has access to
* `loginAsOtherUser` => authenticates an admin and authorizes him to do
actions on behald of an other user (impersonation)
* `createSystemSession` => used for anything else, intended for internal
use in the system
The `login` method returns a SessionType.User, while `loginAsOtherUser`
and `createSystemSession` return a SessionType.System. However, it looks
like that the session types are not being really used in the code, and
there is no difference in rights whatsoever between the two. Probably
useful for the trace logs, to see what kind of action has been done by
which type of session or account, but even then it's probably insufficient.
# 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, ...?)
* 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)
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...
What do you think of it? Any feedback / discussion is welcomed of course
! Thank you for reading this.
Best regards,
Rene.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
