[prev in list] [next in list] [prev in thread] [next in thread]
List: geronimo-dev
Subject: Re: Principal Injection in Geronimo-JWT-Auth
From: Romain Manni-Bucau <rmannibucau () gmail ! com>
Date: 2018-11-05 8:23:52
Message-ID: CACLE=7OXxbAoST6LtO_TKivcMhzUypHpdnjgn2fDDOmZiKQkLw () mail ! gmail ! com
[Download RAW message or body]
@Mark: well, this is not a workaround, all these built-in beans in OWB are
assumed being contextual by their instance, not by their lookup, the
principal is not an exception so we must ensure our implementations are, it
has been fixed in meecrowave, tomee can easily fix it with an
implementation which is close (it has the hook for that already) so we just
have our tomcat integration in OWB to enhance AFAIK
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-performanc=
e>
Le lun. 5 nov. 2018 =C3=A0 09:19, Mark Struberg <struberg@yahoo.de> a =C3=
=A9crit :
> Nonetheless, if the PrincipalBean in OWB really caches the instance then
> this is just wrong.
> We need to fix it there as well - regardless whether we found a valid
> workaround for it or not.
>
> LieGrue,
> strub
>
>
> > Am 02.11.2018 um 17:43 schrieb Romain Manni-Bucau <rmannibucau@gmail.co=
m
> >:
> >
> > Yes
> >
> > I pushed the fix in g-jwt-auth to make it compliant with JWT-AUTH, the
> side note being that injecting a JsonWebToken as principal is not CDI
> compliant (you get a Principal proxy which is useless and not castable to=
a
> JsonWebToken unless you veto principal instances to replace them with the
> jsonwebtoken one which breaks apps in other ways - likely worse?). This i=
s
> also why the TCK don't abuse of that injection since it cant really use i=
t
> and it works with the bad TckSecurityService impl.
> >
> > So long story short the issue is that OWB enforces the built-in beans t=
o
> be "application scoped" (ie the provider returns a proxy otherwise the
> injections will not be contextual as expected) and default impl is not. O=
n
> that aspect tomee ManagedSecurityService#getCurrentPrincipal impl is
> clearly wrong.
> >
> > side note: in meecrowave we have a config to know what we proxy and
> return back always the same proxy which solves that. We can likely push i=
t
> back in openwebbeans but only in tomcat integration which is the only OW=
B
> impl with a principal management (which has the same bug btw - likely cau=
se
> "current" is about the "current app" and not the "current context" in
> "getCurrentPrincipal" which is very misleading and not doc-ed).
> >
> > Hope it is clearer now and helps
> >
> > Romain Manni-Bucau
> > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
> >
> >
> > Le ven. 2 nov. 2018 =C3=A0 17:10, jgallimore <jonathan.gallimore@gmail.=
com>
> a =C3=A9crit :
> > Is this what you're referring to:
> >
> https://github.com/apache/meecrowave/blob/trunk/meecrowave-core/src/main/=
java/org/apache/meecrowave/openwebbeans/MeecrowaveSecurityService.java
> > - with the unwrap?
> >
> > That would make some sense, but is not catered for in
> >
> https://github.com/apache/geronimo-jwt-auth/blob/master/src/test/java/org=
/apache/geronimo/microprofile/impl/jwtauth/tck/TckSecurityService.java
> .
> >
> > I can update my PR to include something similar, but it seems like a ha=
rd
> > workaround for something that ought not to be a problem in the first
> place.
> >
> > Jon
> >
> >
> >
> > --
> > Sent from:
> http://apache-geronimo.328035.n3.nabble.com/Development-f342155.html
>
>
[Attachment #3 (text/html)]
<div dir="ltr">@Mark: well, this is not a workaround, all these built-in beans in OWB \
are assumed being contextual by their instance, not by their lookup, the principal is \
not an exception so we must ensure our implementations are, it has been fixed in \
meecrowave, tomee can easily fix it with an implementation which is close (it has the \
hook for that already) so we just have our tomcat integration in OWB to enhance \
AFAIK<br clear="all"><div><div dir="ltr" class="gmail_signature" \
data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div \
dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div \
dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><br><span \
style="font-size:small">Romain Manni-Bucau</span><br><a \
href="https://twitter.com/rmannibucau" target="_blank">@rmannibucau</a> | <a \
href="https://rmannibucau.metawerx.net/" target="_blank">Blog</a> | <a \
href="http://rmannibucau.wordpress.com" target="_blank">Old Blog</a> | <a \
href="https://github.com/rmannibucau" target="_blank">Github</a> | <a \
href="https://www.linkedin.com/in/rmannibucau" target="_blank">LinkedIn</a> | <a \
href="https://www.packtpub.com/application-development/java-ee-8-high-performance" \
target="_blank">Book</a></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div><br></div><br><div \
class="gmail_quote"><div dir="ltr">Le lun. 5 nov. 2018 Ã 09:19, Mark Struberg \
<<a href="mailto:struberg@yahoo.de">struberg@yahoo.de</a>> a écrit \
:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">Nonetheless, if the PrincipalBean in OWB really caches \
the instance then this is just wrong.<br> We need to fix it there as well - \
regardless whether we found a valid workaround for it or not.<br> <br>
LieGrue,<br>
strub<br>
<br>
<br>
> Am 02.11.2018 um 17:43 schrieb Romain Manni-Bucau <<a \
href="mailto:rmannibucau@gmail.com" \
target="_blank">rmannibucau@gmail.com</a>>:<br> > <br>
> Yes<br>
> <br>
> I pushed the fix in g-jwt-auth to make it compliant with JWT-AUTH, the side note \
being that injecting a JsonWebToken as principal is not CDI compliant (you get a \
Principal proxy which is useless and not castable to a JsonWebToken unless you veto \
principal instances to replace them with the jsonwebtoken one which breaks apps in \
other ways - likely worse?). This is also why the TCK don't abuse of that \
injection since it cant really use it and it works with the bad TckSecurityService \
impl.<br> > <br>
> So long story short the issue is that OWB enforces the built-in beans to be \
"application scoped" (ie the provider returns a proxy otherwise the \
injections will not be contextual as expected) and default impl is not. On that \
aspect tomee ManagedSecurityService#getCurrentPrincipal impl is clearly wrong.<br> \
> <br> > side note: in meecrowave we have a config to know what we proxy and \
return back always the same proxy which solves that. We can likely push it back in \
openwebbeans but only in tomcat integration which is the only OWB impl with a \
principal management (which has the same bug btw - likely cause "current" \
is about the "current app" and not the "current context" in \
"getCurrentPrincipal" which is very misleading and not doc-ed).<br> > \
<br> > Hope it is clearer now and helps<br>
> <br>
> Romain Manni-Bucau<br>
> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book<br>
> <br>
> <br>
> Le ven. 2 nov. 2018 Ã 17:10, jgallimore <<a \
href="mailto:jonathan.gallimore@gmail.com" \
target="_blank">jonathan.gallimore@gmail.com</a>> a écrit :<br> > Is this what \
you're referring to:<br> > <a \
href="https://github.com/apache/meecrowave/blob/trunk/meecrowave-core/src/main/java/org/apache/meecrowave/openwebbeans/MeecrowaveSecurityService.java" \
rel="noreferrer" target="_blank">https://github.com/apache/meecrowave/blob/trunk/meecr \
owave-core/src/main/java/org/apache/meecrowave/openwebbeans/MeecrowaveSecurityService.java</a><br>
> - with the unwrap?<br>
> <br>
> That would make some sense, but is not catered for in<br>
> <a href="https://github.com/apache/geronimo-jwt-auth/blob/master/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/tck/TckSecurityService.java" \
rel="noreferrer" target="_blank">https://github.com/apache/geronimo-jwt-auth/blob/mast \
er/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/tck/TckSecurityService.java</a>.<br>
> <br>
> I can update my PR to include something similar, but it seems like a hard<br>
> workaround for something that ought not to be a problem in the first place.<br>
> <br>
> Jon<br>
> <br>
> <br>
> <br>
> --<br>
> Sent from: <a href="http://apache-geronimo.328035.n3.nabble.com/Development-f342155.html" \
rel="noreferrer" target="_blank">http://apache-geronimo.328035.n3.nabble.com/Development-f342155.html</a><br>
<br>
</blockquote></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic