felixauringer commented on PR #2773: URL: https://github.com/apache/james-project/pull/2773#issuecomment-3710320761
I have rebased on the newest commit on main. If the tests are green, I still have to adapt the documentation (but as far as I can see, this has to be done for everything using the new audience handling). Before the new `validateToken` method was introduced, I had already written tests in the Managesieve implementation for all different cases regarding introspection, local validation, userinfo. Those tests would be better located in `OidcJwtTokenVerifierTest` now in my opinion. Do you agree? If so, I could move them there (currently there are no tests for the different cases in the config as far as I can tell) and focus more on the Managesieve related functionality in the Managesieve tests. To keep my old tests working, I also had to adapt your new code a little bit. In `OidcSASLConfiguration`, there were two static variables relying on System Properties. As they are set once in the JVM, I did not find a good way to run tests with different values for them. They are therefore now read when the configuration is parsed. This should not happen often and therefore these additional calls of `System.getProperty` should be fine in my opinion but please correct me if you know better. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
