Hi David,
I can absolutely understand that this is a bit much to ask for to
review. I think this is what really needs a closer look and maybe needs
to be discussed:
- JWT Validation is using jose.4.j, this introduces a new dependency in
all tomee flavours (wasn't in webprofile before). Maybe it needs to be
added in some notice file?
- Spec mentions a special variable that can be used in the annotation:
${baseURL}, I implemented this with producing an @Named String
- I built a basic delegate in
OpenIdAuthenticationMechanismDefinitionDelegate that automatically
resolves the configuration from the openid provider
- SavedRequest (originally from @LoginToContinue) has been rewritten so
I can serialize it for use in cookies
- Spec is ambiguous on how to handle subjectTypeSupported,
idTokenSigningAlgorithmsSupported and responseTypeSupported (See
CompositeOpenIdProviderMetadata). A user can override these, but it's
not obvious if that has been done or not. I handled these the same way
soteria does, but it's probably worth a spec issue in the future?
- Requests to openid provider are done using JAX-RS Client, maybe we
want to use something else in TomEE? Really the only reason I chose this
was because it's convenient
I appreciate any feedback, will also put this bullet point list in the
PR so it gets some visibility there. The PR is already pretty big so I'm
on your side to merge if it doesn't break anything, reviewing this just
gets harder and harder with every commit I add.
Thanks
Markus
On 12.06.24 00:36, David Blevins wrote:
Are there specific parts you'd like eyes on? Maybe some things you imaged
could be done a few different ways and chose one hoping for feedback if it
wasn't right? Perhaps any new deployment or runtime concepts that were added?
-David
On Jun 11, 2024, at 3:33 PM, David Blevins <david.blev...@gmail.com> wrote:
I've only had a change to take a cursory look at the PR, but I was immediately
impressed.
I can't thank you enough -- what an amazing undertaking!
It might be crickets getting a review for a PR that big especially given our
starved state, so as long as you're a good sport about changes to the code
after it's merged I suspect we should just merge it and see what breaks.
Totally ok to leave it open for a bit as well.
What an amazing contribution, Markus. Seriously. Huge applause.
-David
On Jun 2, 2024, at 8:34 AM, Markus Jung <ju...@apache.org> wrote:
Hi all,
small update after a long time of silence, my implementation now passes the
(admittedly very few) openid tests in the TCK:
[INFO] Reactor Summary for Jakarta Security TCK - main 3.0.1:
[INFO]
[INFO] Jakarta Security TCK - main ........................ SUCCESS [ 2.725 s]
[INFO] common ............................................. SUCCESS [ 0.748 s]
[INFO] app-openid ......................................... SUCCESS [ 6.556 s]
[INFO] app-openid2 ........................................ SUCCESS [ 15.742 s]
[INFO] app-openid3 ........................................ SUCCESS [ 14.100 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 39.986 s
[INFO] Finished at: 2024-06-02T17:24:15+02:00
[INFO] ------------------------------------------------------------------------
I've opened a pull request in the meantime here
https://github.com/apache/tomee/pull/1178 to gather some feedback, the
implementation is pretty much done IMO.
However I'd like to add more unit/integration tests and clean up the code a bit
more, that's why I'm keeping the PR in draft state for now.
Thanks
Markus
On 20. Apr 2024, at 16:13, Richard Zowalla <r...@apache.org> wrote:
Cool! Thanks, Markus!
Feel free to ask any question you might come across while implementing!
Am Freitag, dem 19.04.2024 um 17:14 +0200 schrieb Markus Jung:
Hey all,
just a quick heads up, i've began trying to work on this a couple of
days ago here: https://github.com/jungm/tomee/tree/openid
It's still a huge work in progress right now though
On 13. Apr 2024, at 20:29, Richard Zowalla <r...@apache.org> wrote:
Hi,
looks like we need to implement OpenID connect for Jakarta Security
3.0
- https://jakarta.ee/specifications/security/3.0/
If anyone wants to jump in and dive into the spec -> feel free :)
Gruß
Richard
