JAMES-4205: Fix OIDC mailbox provisioning

[felixauringer]

Benoit asked in https://issues.apache.org/jira/browse/JAMES-4205 that independent of the authentication mechanism, James should perform provisioning.

To fulfill that requirement, I introduced (or rather repurposed) the method authSuccess to perform all operations that are necessary after a successful operation.
While I was at it, I also introduced authFailure which is called quite often and therefore should reduce the complexity quite a bit.
This also changes the semantics a little in some places, mainly because every failure is now consistently logged with AuditTrail and effects the failure count.

I am still running tests but I would also appreciate if somebody more familiar with the code base could validate that I did not accidentally change the semantics in some place other than log messages.

Additionally, I am not sure where session.stopDetectingCommandInjection(); is supposed to be used but I could not find any logic to how it was used before and changed it so that it is consistently called after every processed request.

[chibenwa]

Good with me.

A 🍏 AI run and it shall be good to go!

[chibenwa]

session.stopDetectingCommandInjection();

We are either authenticated or STARTTLS is active

changed it so that it is consistently called after every processed request

It will do

Rationals: we need to verify STARTTLS command is not altered midayy to batch extra commands in. This costly check is no longer needed as soon as auth or starttls is passed. CF https://nostarttls.secvuln.info/ for rationnals.

[felixauringer]

We are either authenticated or STARTTLS is active

Okay, I removed all calls of stopDetectingCommandInjection except one in authSuccess. Before, it was also called if authentication had failed.

Another question: After a successful authentication, the server tries to provision the mailboxes for the user but ignores potential errors. The authentication is not aborted. Is that the behavior you would like in that case?

Before my changes, the server would send a NO reply but not reset the session, so I guess the session would have been authenticated. That's weird behavior that I would like to get rid off but I am not sure what the correct behavior would be.

[chibenwa]

Another question: After a successful authentication, the server tries to provision the mailboxes for the user but ignores potential errors. The authentication is not aborted. Is that the behavior you would like in that case?

Good question

I think being lenient prevents not accessing his mail...

So current behaviour is likely good.

Before my changes, the server would send a NO reply but not reset the session, so I guess the session would have been authenticated. That's weird behavior that I would like to get rid off but I am not sure what the correct behavior would be.

I think answering yes anyway and doing provisionning in a best effort way is indeed better.

[felixauringer]

The crowdsec tests are now working again. However, all the matchers needed a placeholder like %{DATA:data} somewhere in the string although it should be a perfect match. I haven't understood why, maybe someone can enlighten me.

Additionally, if you merge this, there are also some small changes necessary in tmail. I will open a MR on that repo, too.

[chibenwa]

Additionally, if you merge this, there are also some small changes necessary in tmail. I will open a MR on that repo, too.

As said there that's very nice, thanks a lot! Contribs are welcome there too. That being said while appreciated that is by no way a necessary condition for a merge on James repos, and it is indeed LINAGORA job to maintain Twake mail on sync with latests James (but many thanks again ^^)

[chibenwa]

Looks nice to me!

[chibenwa]

The crowdsec tests are now working again. However, all the matchers needed a placeholder like %{DATA:data} somewhere in the string although it should be a perfect match. I haven't understood why, maybe someone can enlighten me.

@quantranhong1999 maybe ?

[felixauringer]

As said there that's very nice, thanks a lot! Contribs are welcome there too. That being said while appreciated that is by no way a necessary condition for a merge on James repos, and it is indeed LINAGORA job to maintain Twake mail on sync with latests James (but many thanks again ^^)

We are using Tmail for the team mailboxes and are therefore interested in having a working Tmail. I therefore test all of my changes in Tmail anyway, so it's really not a lot of additional effort :)