The commits are showing for me (at the bottom). Here's the latest one: https://github.com/apache/tomee/commit/7ce1f8033e239331cfa7843e4e5565ed0aa83345
On Tue, Nov 20, 2018 at 2:44 PM Jean-Louis Monteiro < jlmonte...@tomitribe.com> wrote: > Hey Jon, > > I clicked on the link and the diff tab does not show any difference. > Did you push? > -- > Jean-Louis Monteiro > http://twitter.com/jlouismonteiro > http://www.tomitribe.com > > > On Mon, Nov 19, 2018 at 12:36 PM Jonathan Gallimore < > jonathan.gallim...@gmail.com> wrote: > > > I now have the principal injection part of this working - thanks Romain > for > > your help and explanations. Progress is in my fork here: > > https://github.com/jgallimore/tomee/tree/jwt-1.1 (changes here: > > > > > https://github.com/apache/tomee/compare/master...jgallimore:jwt-1.1?expand=1 > > ). > > There are still a couple of TODOs to clean up, and 3 tests to get > passing. > > Any feedback is appreciated. > > > > Jon > > > > On Sat, Nov 3, 2018 at 9:10 AM Jonathan Gallimore < > > jonathan.gallim...@gmail.com> wrote: > > > > > Yep, got it. Thanks for the feedback - makes sense now. > > > > > > Cheers > > > > > > Jon > > > > > > On Fri, 2 Nov 2018, 16:46 Romain Manni-Bucau <rmannibu...@gmail.com > > wrote: > > > > > >> Answered hopefully "long enough" on dev@geronimo so will just do a > > short > > >> one here and shout if not enough: ManagedSecurityService in cdi > package > > of > > >> openejb-core must make the getCurrentPrincipal contextual so hidden > > behind > > >> a proxy. The proxied API must be Principal and JsonWebToken when > > available > > >> (try { add if can load } catch { ignore } works as pattern). The proxy > > >> instance can be created once for all app using the container loader or > > per > > >> app using the app loader and avoiding to leak between apps since the > API > > >> can use different loaders. > > >> > > >> Le ven. 2 nov. 2018 14:44, Jonathan Gallimore < > > >> jonathan.gallim...@gmail.com> > > >> a écrit : > > >> > > >> > Thanks for the reply, but I am confused by your response. The PR I > > >> > referenced adds a single test to the geronimo-jwt-auth project ( > > >> > https://github.com/apache/geronimo-jwt-auth/pull/3), based on > > >> > > > org.eclipse.microprofile.jwt.tck.container.jaxrs.PrincipalInjectionTest > > >> > from the TCK. It fails at present (hopefully we agree on that - my > > >> results > > >> > attached). The geronimo-jwt-auth project doesn't touch TomEE at all > - > > it > > >> > uses OWB/Meecrowave to run the MicroProfile JWT TCK. I have not > > modified > > >> > the project config at all, so it is using the SecurityService code > you > > >> > previously posted. If this additional test were part of the > > MicroProfile > > >> > JWT TCK (and I'm going to propose it), the Geronimo JWT Auth > > >> implementation > > >> > would *not* pass the TCK. > > >> > > > >> > I posted this here as I originally found the issue when continuing > > >> > Roberto's efforts, but this has probably contributed to some > > confusion. > > >> I > > >> > would suggest we continue this over on the Geronimo and OWB lists to > > >> avoid > > >> > further confusion. > > >> > > > >> > Jon > > >> > > > >> > On Fri, Nov 2, 2018 at 12:46 PM Romain Manni-Bucau < > > >> rmannibu...@gmail.com> > > >> > wrote: > > >> > > > >> >> Hi > > >> >> > > >> >> Yes this is an owb misconfiguration/integration > > >> >> > > >> >> Geronimo is fine here so likely tomee owb spi to update as in > > geronimo > > >> tck > > >> >> > > >> >> Le ven. 2 nov. 2018 10:42, Jonathan Gallimore < > > >> >> jonathan.gallim...@gmail.com> > > >> >> a écrit : > > >> >> > > >> >> > Thanks for the reply. I am still sure there is some sort of > issue. > > >> >> Putting > > >> >> > TomEE to one side for the moment, I am able to reproduce this in > > the > > >> >> > Geronimo JWT auth library as well. This PR includes a test to > show > > >> what > > >> >> I > > >> >> > mean: https://github.com/apache/geronimo-jwt-auth/pull/3. > > >> >> > > > >> >> > I can confirm that this change: > > >> >> > https://github.com/apache/openwebbeans/pull/12 enables that new > > >> test to > > >> >> > pass. > > >> >> > > > >> >> > In short, if you @Inject JsonWebToken, or individual claims, or > > >> >> > use @RolesAllowed, I think you're ok, but if you @Inject > Principal, > > >> you > > >> >> > will most likely get the wrong principal because the instance is > > >> cache > > >> >> in a > > >> >> > field in the org.apache.webbeans.portable.ProviderBasedProducer > > >> class, > > >> >> and > > >> >> > that looks like a security issue. > > >> >> > > > >> >> > Jon > > >> >> > > > >> >> > On Tue, Oct 30, 2018 at 5:56 AM Romain Manni-Bucau < > > >> >> rmannibu...@gmail.com> > > >> >> > wrote: > > >> >> > > > >> >> > > Hi Jon, > > >> >> > > > > >> >> > > yes and no, idea is to be fast and for all producers it works > > >> except > > >> >> the > > >> >> > > principal which is broken anyway in CDI 1.x so guess this was > not > > >> >> fixed > > >> >> > > > > >> >> > > in CDI 2 (tomee 8) we can impl it this way: > > >> >> > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > https://github.com/apache/geronimo-jwt-auth/blob/master/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/tck/TckSecurityService.java > > >> >> > > > > >> >> > > Romain Manni-Bucau > > >> >> > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > >> >> > > <https://rmannibucau.metawerx.net/> | Old Blog > > >> >> > > <http://rmannibucau.wordpress.com> | Github < > > >> >> > > https://github.com/rmannibucau> | > > >> >> > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > > >> >> > > < > > >> >> > > > > >> >> > > > >> >> > > >> > > > https://www.packtpub.com/application-development/java-ee-8-high-performance > > >> >> > > > > > >> >> > > > > >> >> > > > > >> >> > > Le mar. 30 oct. 2018 à 00:58, Jonathan Gallimore < > > >> >> > > jonathan.gallim...@gmail.com> a écrit : > > >> >> > > > > >> >> > > > Here's a question, probably for Mark or Romain. If I turn the > > >> proxy > > >> >> > *off* > > >> >> > > > in org.apache.webbeans.component.PrincipalBean, I'm finding > > that > > >> I > > >> >> get > > >> >> > > the > > >> >> > > > wrong principal injected sometimes. Specifically, I get the > > >> >> whatever is > > >> >> > > on > > >> >> > > > the proxyInstance field here: > > >> >> > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/ProviderBasedProducer.java#L51 > > >> >> > > > > > >> >> > > > Should this line (line 66) > > >> >> > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/ProviderBasedProducer.java#L66 > > >> >> > > > , > > >> >> > > > not simply be: > > >> >> > > > > > >> >> > > > return provider.get(); > > >> >> > > > > > >> >> > > > as opposed to > > >> >> > > > > > >> >> > > > proxyInstance = provider.get(); ? > > >> >> > > > > > >> >> > > > That way, the proxyInstance field would never get set if > proxy > > >> mode > > >> >> is > > >> >> > > set > > >> >> > > > to false. When proxy is true, this seems to work correctly > > >> >> (although I > > >> >> > > have > > >> >> > > > other unrelated issues in TomEE). > > >> >> > > > > > >> >> > > > I can probably work around this some other way, but it seems > to > > >> me > > >> >> like > > >> >> > > > that behaviour isn't quite right. > > >> >> > > > > > >> >> > > > Trying to think of a way to test it - I can probably come up > > with > > >> >> > > > something, but I'd appreciate some pointers. Happy to shift > > this > > >> to > > >> >> > > > openwebbeans-dev, and submit a PR. Replying here initially > as I > > >> ran > > >> >> > into > > >> >> > > > this while hacking on the JWT code. > > >> >> > > > > > >> >> > > > Jon > > >> >> > > > > > >> >> > > > On Wed, Oct 17, 2018 at 12:41 AM Roberto Cortez > > >> >> > > > <radcor...@yahoo.com.invalid> > > >> >> > > > wrote: > > >> >> > > > > > >> >> > > > > Please, go ahead. Let me know if need anything. Thanks! > > >> >> > > > > > > >> >> > > > > > On 16 Oct 2018, at 21:53, Jonathan Gallimore < > > >> >> > > > > jonathan.gallim...@gmail.com> wrote: > > >> >> > > > > > > > >> >> > > > > > Any objection if I pick this up and have a go at the last > > >> >> tests, or > > >> >> > > is > > >> >> > > > > > someone already working on this? > > >> >> > > > > > > > >> >> > > > > > On Thu, Sep 27, 2018 at 5:44 PM Romain Manni-Bucau < > > >> >> > > > > rmannibu...@gmail.com> > > >> >> > > > > > wrote: > > >> >> > > > > > > > >> >> > > > > >> Yep this feature. Then it must works since we support > user > > >> >> > principal > > >> >> > > > if > > >> >> > > > > the > > >> >> > > > > >> jwt filter is corretly placed in the filter chain and we > > >> must > > >> >> > > inherit > > >> >> > > > > from > > >> >> > > > > >> the request principal. > > >> >> > > > > >> > > >> >> > > > > >> Le jeu. 27 sept. 2018 18:37, Roberto Cortez > > >> >> > > > <radcor...@yahoo.com.invalid > > >> >> > > > > > > > >> >> > > > > >> a > > >> >> > > > > >> écrit : > > >> >> > > > > >> > > >> >> > > > > >>> I guess you are referring to this, to remove the proxy? > > >> >> > > > > >>> > > >> >> > > > > >>> > > >> >> > > > > >> > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > https://github.com/apache/openwebbeans/commit/a21a949fb19247dcc39ee89292a1554b2cf1388e > > >> >> > > > > >>> < > > >> >> > > > > >>> > > >> >> > > > > >> > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > https://github.com/apache/openwebbeans/commit/a21a949fb19247dcc39ee89292a1554b2cf1388e > > >> >> > > > > >>>> > > >> >> > > > > >>> > > >> >> > > > > >>> Yes, this one step. > > >> >> > > > > >>> > > >> >> > > > > >>> By default, we do inject the generic Principal of > Tomcat. > > >> We > > >> >> > > probably > > >> >> > > > > >> need > > >> >> > > > > >>> to check first about the existence of a JWT Principal > and > > >> then > > >> >> > > > fallback > > >> >> > > > > >> to > > >> >> > > > > >>> the Tomcat one. I think I know how to do it, I was just > > >> >> trying to > > >> >> > > > > broaden > > >> >> > > > > >>> up the conversation about general integration with EE > > >> >> security. > > >> >> > > > > >>> > > >> >> > > > > >>> Cheers, > > >> >> > > > > >>> Roberto > > >> >> > > > > >>> > > >> >> > > > > >>>> On 26 Sep 2018, at 07:21, Romain Manni-Bucau < > > >> >> > > rmannibu...@gmail.com > > >> >> > > > > > > >> >> > > > > >>> wrote: > > >> >> > > > > >>>> > > >> >> > > > > >>>> OWB enable to do it - we did it in geronimo impl to > pass > > >> tck > > >> >> of > > >> >> > > jwt > > >> >> > > > > >> auth > > >> >> > > > > >>>> spec. > > >> >> > > > > >>>> > > >> >> > > > > >>>> Le mer. 26 sept. 2018 03:28, Roberto Cortez > > >> >> > > > > >> <radcor...@yahoo.com.invalid> > > >> >> > > > > >>> a > > >> >> > > > > >>>> écrit : > > >> >> > > > > >>>> > > >> >> > > > > >>>>> Hi, > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> I’ve done some work to push our MP JWT implementation > > >> from > > >> >> 1.0 > > >> >> > to > > >> >> > > > > 1.1. > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> You can check it here: > > >> >> > > > > >>>>> https://github.com/apache/tomee/pull/173 < > > >> >> > > > > >>>>> https://github.com/apache/tomee/pull/173> > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> There are still a couple of tests in the TCK that I > > have > > >> to > > >> >> fix > > >> >> > > > and a > > >> >> > > > > >>> few > > >> >> > > > > >>>>> things that I would like to improve, but I think the > > >> >> majority > > >> >> > of > > >> >> > > > the > > >> >> > > > > >>> work > > >> >> > > > > >>>>> is done. > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> Some time ago, there was a discussion in the list > about > > >> how > > >> >> to > > >> >> > > > > >> integrate > > >> >> > > > > >>>>> MP JWT with EE security: > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> > > >> >> > > > > >>> > > >> >> > > > > >> > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > http://tomee-openejb.979440.n4.nabble.com/Implementing-Microprofile-JWT-td4683212i40.html > > >> >> > > > > >>>>> < > > >> >> > > > > >>>>> > > >> >> > > > > >>> > > >> >> > > > > >> > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > http://tomee-openejb.979440.n4.nabble.com/Implementing-Microprofile-JWT-td4683212i40.html > > >> >> > > > > >>>>>> > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> I believe we need to revisit that conversation and > > figure > > >> >> out > > >> >> > how > > >> >> > > > to > > >> >> > > > > >>> move > > >> >> > > > > >>>>> forward. > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> Right now for instance, we don’t support injecting a > > JWT > > >> >> > > Principal > > >> >> > > > > >> since > > >> >> > > > > >>>>> it clashes with the predefined by CDI. Most likely, > we > > >> would > > >> >> > need > > >> >> > > > to > > >> >> > > > > >>> plugin > > >> >> > > > > >>>>> the JWT Principal lookup in TomcatSecurityService. > I’m > > >> not > > >> >> sure > > >> >> > > if > > >> >> > > > we > > >> >> > > > > >>> want > > >> >> > > > > >>>>> to do it in that way, or if we want to think in > > something > > >> >> else. > > >> >> > > > > >>>>> > > >> >> > > > > >>>>> Cheers, > > >> >> > > > > >>>>> Roberto > > >> >> > > > > >>> > > >> >> > > > > >>> > > >> >> > > > > >> > > >> >> > > > > > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > >> > > > > > >
