[JIRA] (JCOMOIDC-41) Move OIDC Signature Validation resolvers and parameter classes to commons

[Brent Putman (Jira)]

Brent Putman ( https://shibboleth.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A97de0981-adc3-4044-95ed-131622fad81e ) *commented* on JCOMOIDC-41 ( https://shibboleth.atlassian.net/browse/JCOMOIDC-41?atlOrigin=eyJpIjoiZWRiMjk1YjJlNTAyNGRiNGJlMDRhMjk0ODlkNzMwYTUiLCJwIjoiaiJ9 )

Re: Move OIDC Signature Validation resolvers and parameter classes to commons ( https://shibboleth.atlassian.net/browse/JCOMOIDC-41?atlOrigin=eyJpIjoiZWRiMjk1YjJlNTAyNGRiNGJlMDRhMjk0ODlkNzMwYTUiLCJwIjoiaiJ9 )

I was (finally) able to finishing reviewing these things. Not too much I see to comment on:

The only thing that I wanted to ask about was: In JWTSecurityParametersContext, for signing and encryption params, you still have properties for the XML-oriented ones in OpenSAML. Those doesn’t seem appropriate for long-term use since they have a lot of KeyInfo stuff on them. I assume those are just placeholders and eventually there would be OIDC-specific ones?

I personally don’t see much that’s worth trying to refactor to common base classes in OpenSAML. Esp for the -Configuration, -Parameters, and -Resolvers, which all go together. The -Configuration and -Parameters are really just beans with properties, and it doesn’t seem worth pulling out a couple of properties to a base class. The -Resolvers are just really specific to each case, not sure there’s much to be done there. If there’s something specific you had in mind for common refactoring, let me know.

On the ‘jku’ resolution, I see that you have the RemoteJwkUtils that already does the basics. For stuff like that, for purely stylistic reasons, I usually inject the HttpClient into the relevant component and then put all the response processing into a ResponseHandler<T> that returns whatever type is to be processed from the client HttpResponse. I just like it stylistically, but also it takes care of closing the response and so forth so you don’t have to worry about.

But then I see you also have the RemoteJwkSetCache, which seems to wrap all that in a caching abstraction. So is the intention then that you inject that into components which need to fetch a ‘jku’, rather than the HttpClient itself?

Otherwise, I don’t see too much else to comment on. If there’s anything specific that I missed, let me know.

( https://shibboleth.atlassian.net/browse/JCOMOIDC-41#add-comment?atlOrigin=eyJpIjoiZWRiMjk1YjJlNTAyNGRiNGJlMDRhMjk0ODlkNzMwYTUiLCJwIjoiaiJ9 ) Add Comment ( https://shibboleth.atlassian.net/browse/JCOMOIDC-41#add-comment?atlOrigin=eyJpIjoiZWRiMjk1YjJlNTAyNGRiNGJlMDRhMjk0ODlkNzMwYTUiLCJwIjoiaiJ9 )

Get Jira notifications on your phone! Download the Jira Cloud app for Android ( https://play.google.com/store/apps/details?id=com.atlassian.android.jira.core&referrer=utm_source%3DNotificationLink%26utm_medium%3DEmail ) or iOS ( https://itunes.apple.com/app/apple-store/id1006972087?pt=696495&ct=EmailNotificationLink&mt=8 ) This message was sent by Atlassian Jira (v1001.0.0-SNAPSHOT#100204- sha1:d476756 )
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/commits/attachments/20220805/4adf5e49/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jira-generated-image-static-comment-icon-393e590e-f546-402e-8edc-bd8fd9b73524
Type: image/png
Size: 1084 bytes
Desc: not available
URL: <http://shibboleth.net/pipermail/commits/attachments/20220805/4adf5e49/attachment-0003.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jira-generated-image-static-footer-desktop-logo-572b3197-a3cc-4e92-a5ff-4e61bb44d21a
Type: image/png
Size: 10805 bytes
Desc: not available
URL: <http://shibboleth.net/pipermail/commits/attachments/20220805/4adf5e49/attachment-0004.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jira-generated-image-avatar-80da75c1-10a2-4e2e-8b0c-5e4e3d983157
Type: image/png
Size: 468 bytes
Desc: not available
URL: <http://shibboleth.net/pipermail/commits/attachments/20220805/4adf5e49/attachment-0005.png>