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 > > > >>> > > > >>> > > > >> > > > > > > > > >
