MicroProfile JWT 1.1

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

MicroProfile JWT 1.1

Roberto Cortez
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
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Romain Manni-Bucau
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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Roberto Cortez
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 <[hidden email]> 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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Romain Manni-Bucau
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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
> 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
>
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

jgallimore
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 <[hidden email]>
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 <[hidden email]>
> 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 <[hidden email]>
> > 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
> <[hidden email]>
> > 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
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Roberto Cortez
Please, go ahead. Let me know if need anything. Thanks!

> On 16 Oct 2018, at 21:53, Jonathan Gallimore <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
>> 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 <[hidden email]>
>>> 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
>> <[hidden email]>
>>> 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
>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

jgallimore
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 <[hidden email]>
wrote:

> Please, go ahead. Let me know if need anything. Thanks!
>
> > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> [hidden email]> 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 <
> [hidden email]>
> > 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 <[hidden email]
> >
> >> 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 <[hidden email]>
> >>> 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
> >> <[hidden email]>
> >>> 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
> >>>
> >>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Romain Manni-Bucau
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 <
[hidden email]> 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
> <[hidden email]>
> wrote:
>
> > Please, go ahead. Let me know if need anything. Thanks!
> >
> > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> > [hidden email]> 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 <
> > [hidden email]>
> > > 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
> <[hidden email]
> > >
> > >> 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 <[hidden email]
> >
> > >>> 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
> > >> <[hidden email]>
> > >>> 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
> > >>>
> > >>>
> > >>
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

jgallimore
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 <[hidden email]>
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 <
> [hidden email]> 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
> > <[hidden email]>
> > wrote:
> >
> > > Please, go ahead. Let me know if need anything. Thanks!
> > >
> > > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> > > [hidden email]> 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 <
> > > [hidden email]>
> > > > 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
> > <[hidden email]
> > > >
> > > >> 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 <
> [hidden email]
> > >
> > > >>> 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
> > > >> <[hidden email]>
> > > >>> 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
> > > >>>
> > > >>>
> > > >>
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Romain Manni-Bucau
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 <[hidden email]>
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 <[hidden email]>
> 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 <
> > [hidden email]> 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
> > > <[hidden email]>
> > > wrote:
> > >
> > > > Please, go ahead. Let me know if need anything. Thanks!
> > > >
> > > > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> > > > [hidden email]> 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 <
> > > > [hidden email]>
> > > > > 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
> > > <[hidden email]
> > > > >
> > > > >> 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 <
> > [hidden email]
> > > >
> > > > >>> 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
> > > > >> <[hidden email]>
> > > > >>> 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
> > > > >>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

jgallimore
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 <[hidden email]> 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 <[hidden email]>
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 <[hidden email]>
> 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 <
> > [hidden email]> 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
> > > <[hidden email]>
> > > wrote:
> > >
> > > > Please, go ahead. Let me know if need anything. Thanks!
> > > >
> > > > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> > > > [hidden email]> 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 <
> > > > [hidden email]>
> > > > > 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
> > > <[hidden email]
> > > > >
> > > > >> 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 <
> > [hidden email]
> > > >
> > > > >>> 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
> > > > >> <[hidden email]>
> > > > >>> 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
> > > > >>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

Romain Manni-Bucau
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 <[hidden email]>
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 <[hidden email]>
> 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 <
>> [hidden email]>
>> 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 <
>> [hidden email]>
>> > 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 <
>> > > [hidden email]> 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
>> > > > <[hidden email]>
>> > > > wrote:
>> > > >
>> > > > > Please, go ahead. Let me know if need anything. Thanks!
>> > > > >
>> > > > > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
>> > > > > [hidden email]> 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 <
>> > > > > [hidden email]>
>> > > > > > 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
>> > > > <[hidden email]
>> > > > > >
>> > > > > >> 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 <
>> > > [hidden email]
>> > > > >
>> > > > > >>> 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
>> > > > > >> <[hidden email]>
>> > > > > >>> 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
>> > > > > >>>
>> > > > > >>>
>> > > > > >>
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: MicroProfile JWT 1.1

jgallimore
Yep, got it. Thanks for the feedback - makes sense now.

Cheers

Jon

On Fri, 2 Nov 2018, 16:46 Romain Manni-Bucau <[hidden email] 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 <
> [hidden email]>
> 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 <
> [hidden email]>
> > 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 <
> >> [hidden email]>
> >> 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 <
> >> [hidden email]>
> >> > 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 <
> >> > > [hidden email]> 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
> >> > > > <[hidden email]>
> >> > > > wrote:
> >> > > >
> >> > > > > Please, go ahead. Let me know if need anything. Thanks!
> >> > > > >
> >> > > > > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> >> > > > > [hidden email]> 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 <
> >> > > > > [hidden email]>
> >> > > > > > 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
> >> > > > <[hidden email]
> >> > > > > >
> >> > > > > >> 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 <
> >> > > [hidden email]
> >> > > > >
> >> > > > > >>> 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
> >> > > > > >> <[hidden email]>
> >> > > > > >>> 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
> >> > > > > >>>
> >> > > > > >>>
> >> > > > > >>
> >> > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>