All work

Select view

Select search mode

 

Relocate and improve lookup functions for request message contents

Completed

Description

The current lookup functions for the authorization (OAuth2) and authentication (OIDC) requests extend the abstract functions (AbstractAuthorizationRequestLookupFunction and AbstractAuthenticationRequestLookupFunction) in a way that they only implement an abstract doLookup(AuthoirizationRequest/AuthenticationRequest) method.

If request object is involved, they fetch it via getter that returns the object via class variable set by abstract parent class for the lookup function. This also means that the functions are not stateless and their bean scope is thus required to be prototype. That’s one thing that should be improved.

The lookup functions could also make use of new additional details about the request, brought by OP 4.2.0. At least the fact if the request object was built by the PAR endpoint would be useful data that may impact what the lookup function should return.

As the lookup functions are very closely related to the SWF actions that deal with the corresponding message parameters, it seems to make sense to move them into the -impl module.

Environment

None

Details

Assignee

Reporter

Components

Fix versions

Created September 13, 2024 at 11:58 AM
Updated October 23, 2024 at 6:53 AM
Resolved October 21, 2024 at 4:30 PM

Activity

Henri MikkonenOctober 1, 2024 at 11:00 AM

Refactored the authorization/authentication request lookup functions' useOnlyRequestObjectPredicate to exploit whether the request object is built by the PAR endpoint. The predicate is now of type BiPredicate<Pair<AuthorizationRequest,Boolean>, String>, where the second part of the Pair is the flag indicating whether the request contains a request object provided by the PAR endpoint.

The predicate may be customized via following bean IDs:

  • PAR endpoint: idp.oauth2.par.useOnlyRequestObject

  • Authorize endpoint: idp.oauth2.authorize.useOnlyRequestObject

Henri MikkonenSeptember 26, 2024 at 3:29 PM

The already existing functions in the API-module could not be refactored in a way that they would satisfy the desired new requirements and also serve the existing implementations without modifications. Thus it felt best to restore them into the previous release and deprecate them. New improved versions of the lookups that directly deal with the request message contents were created to the IMPL-module and its net.shibboleth.idp.plugin.oidc.op.messaging.context.navigate package.

The new versions of the functions that deal with authorization and authentication requests are now inheriting AbstractInitializableComponent and they’re @ThreadSafeAfterInit. They’re always wired to the SWF actions via XML: no default value via Java anymore.

Henri MikkonenSeptember 13, 2024 at 12:19 PM

As the lookup functions are located in OP’s API-module, ideally we should probably deprecate the existing instead of replacing them with new methods. We’ve declared though that OP itself should be treated as implementation.

This is what I drafted:

The new abstract method (implemented by the field-specific lookup functions):

The first two parameters are obtained from the inbound message context and the other two from the OIDCAuthenticationResponseContext. It (at least currently) feels to be the satisfactory set for covering the currently known requirements of the lookup functions. Obviously providing the contexts (even PRC) in addition to those would be the safest bet, but might be overhead.

The current one would be deprecated and switched into using the new one above in the following way:

The getRequestObject() there would also be deprecated, as the new method gets it via method-parameters in a stateless way.

Flag notifications