geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: jwt-auth CDI base version
Date Wed, 25 Apr 2018 19:04:55 GMT
outch, very good one, do you want PR? :angel:


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>

2018-04-25 20:30 GMT+02:00 Rudy De Busscher <rdebusscher@gmail.com>:

> Issue seems indeed to support all types (seems practically impossible
> since wildcards are not supported for return methods of producers (like
> Provider<?>))
>
> Since there is not a lot of objection to the CDI 2.0 based version (here
> and also microprofile mailing list has little response to my question) lets
> drop the CDI 1.2 question (although I don't like the fact that we need to
> remove the NonBinding as it was an explicit design choice by the spec
> people)
>
>  I'll keep my current solution for my customer (not compliant but can use
> token and use it for authorization on WildFly 10.x)
>
> The only real issue I found (for the moment) is a NullPointer when you
> forget to config property geronimo.jwt-auth.issuer.default
>
> at JwtParser#parse line 67
>
> if (!this.kidMapper.loadIssuer(kid).equals(payload.getString(
> Claims.iss.name()))) {
>
> this.kidMapper.loadIssuer(kid) returns null when no default and no issuer
> mapped for kid. :)
>
>
> Rudy
>
>
>
> On 23 April 2018 at 23:55, Romain Manni-Bucau <rmannibucau@gmail.com>
> wrote:
>
>>
>>
>> Le 23 avr. 2018 21:50, "Rudy De Busscher" <rdebusscher@gmail.com> a
>> écrit :
>>
>> Made progress in porting it to CDI 1.2
>>
>> - Also used JSONP 1.0 (instead of 1.1)
>>
>>
>> Wasnt it already the case? Good anyway
>>
>> - Used Producers instead of beans for each @Claim @Inject (as described
>> here https://github.com/eclipse/microprofile-jwt-auth/issues/32)
>>
>>
>> I didnt pick that for two reasons
>>
>> 1. Easier to integrate in tomee and most containers which can bypass
>> scanning with an extension driven impl
>> 2. Easier to configure the scope when needed (spec defaults are crazy and
>> almost not usable :()
>>
>> But if you kept the existing scopes and optimized (by type/claim) impl ut
>> sounds good enough
>>
>> Issue will likely be to support all types
>>
>>
>>
>> Still a bit hacky and not all producers created, but my demo app with
>> injection of JsonWebToken, JsonString Claim and String Claim are already
>> working.
>>
>> Rudy
>>
>>
>>
>> On 23 April 2018 at 16:41, Romain Manni-Bucau <rmannibucau@gmail.com>
>> wrote:
>>
>>> This is what does the extension, the not CDI 1 features used are only
>>> configurators to override the @Claim model (this one is only supported for
>>> OWB >= 2.0.5 even in the spec since more - at least 2.0.0) + to create
>>> beans (this one is easy to solve adding a custom Bean+PassivationCapable
>>> impl)
>>>
>>>
>>> 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>
>>>
>>> 2018-04-23 16:38 GMT+02:00 John D. Ament <johndament@apache.org>:
>>>
>>>> I can look at the code later but what I had to do before is capture all
>>>> of the claim injection points and provide specific producers for each.
>>>>
>>>> On Mon, Apr 23, 2018, 10:35 AM Mark Struberg <struberg@yahoo.de> wrote:
>>>>
>>>>> Qualifiers are per CDI spec not AnnotatedTypes.
>>>>> So if we rely on this then it's not spec compliant anyway.
>>>>>
>>>>> LieGrue,
>>>>> strub
>>>>>
>>>>> > Am 23.04.2018 um 14:30 schrieb Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com>:
>>>>> >
>>>>> > the extension modifies @Claim to remove @NonBinding. This requires
>>>>> the impl to support to read qualifiers as AnnotatedType and only OWB
2.0.5
>>>>> supports it in OWB series ATM
>>>>> >
>>>>> >
>>>>> > Romain Manni-Bucau
>>>>> > @rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book
>>>>> >
>>>>> > 2018-04-23 14:18 GMT+02:00 John D. Ament <johndament@apache.org>:
>>>>> > Whats the qualifier issue you're referring to?
>>>>> >
>>>>> > On Mon, Apr 23, 2018, 8:05 AM Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com> wrote:
>>>>> > Same here, I just doubt we have an owb impl supporting the qualifier
>>>>> model change today so we can stay on OWB 2.0.5 or need to backport it
to
>>>>> 1.x as well (which can likely be the case as well but can need to be
done
>>>>> in parallel).
>>>>> >
>>>>> >
>>>>> > Romain Manni-Bucau
>>>>> > @rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book
>>>>> >
>>>>> > 2018-04-23 13:17 GMT+02:00 John D. Ament <johndament@apache.org>:
>>>>> > If you already have a PR submitted even better.  We should accept
it.
>>>>> >
>>>>> > On Mon, Apr 23, 2018, 7:07 AM Rudy De Busscher <
>>>>> rdebusscher@gmail.com> wrote:
>>>>> > Not that hard, except maybe for the NonBinding thing which is
>>>>> removed from @Claim.
>>>>> >
>>>>> > All the rest was done in 20 minutes or so.
>>>>> >
>>>>> > On 23 April 2018 at 13:03, Jean-Louis MONTEIRO <jeanouii@gmail.com>
>>>>> wrote:
>>>>> > Overall same view here.
>>>>> > How hard is it to make it 1.2 compliant?
>>>>> >
>>>>> >
>>>>> > Le lun. 23 avr. 2018 à 12:25, John D. Ament <johndament@apache.org>
>>>>> a écrit :
>>>>> > MP has made it very clear they don't care about portable libraries,
>>>>> and only care about the vendor provided solutions.  The requirement is
that
>>>>> vendors provide a CDI 1.2 runtime to use.  Liberty provides a way to
switch
>>>>> between them (1.2, 2.0).  I think Swarm may have moved to 2.0; not sure.
>>>>> >
>>>>> > I think Safeguard also compiles against CDI 2.0, but I don't think
>>>>> I'm using any 2.0 features in it so it may run properly against 1.2.
>>>>> >
>>>>> > Personally, if we have a user who wants it for 1.2, and the effort
>>>>> is minimal we should appease that user to help build out the community.
>>>>> >
>>>>> > John
>>>>> >
>>>>> >
>>>>> > On Mon, Apr 23, 2018 at 2:17 AM Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com> wrote:
>>>>> > Hi guys,
>>>>> >
>>>>> > current codebase uses cdi 2.0 which means it can be used on tomee,
>>>>> meecrowave,  openwebbeans etc...
>>>>> >
>>>>> > Rudy opened https://issues.apache.org/jira/browse/GERONIMO-6604
to
>>>>> move it to cdi 1.2 - BTW "Microprofile depends on CDI 1.2, so using 2.0
is
>>>>> wrong." is wrong since some years you can always use a version *>=*
of the
>>>>> minimum requirement for spec impls.
>>>>> > Technically I don't see a strong need to do it but I'd like to get
>>>>> your feeling about it to know what we do of the issue.
>>>>> >
>>>>> >
>>>>> > Romain Manni-Bucau
>>>>> > @rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book
>>>>> >
>>>>> >
>>>>> >
>>>>>
>>>>>
>>>
>>
>>
>

Mime
View raw message