1

My project overrides the built-in OidcLogoutActionBuilder with a custom implementation. Specifically, we override the getLogoutAction method for getting a RedirectionAction.

In the base method, the idToken JWT is retrieved in this line. It checks that currentProfile is an instance of OidcProfile before casting it.

val idToken = ((OidcProfile) currentProfile).getIdToken();

In our implementation, currentProfile is a CiviFormProfileData, which does not contain an ID token. We would now like to add the ID token to the LogoutRequest's params.

To do so, I tried using ProfileManager.getProfile(OidcProfile.class), but that turns out to return an empty Optional. I believe this indicates that the user isn't logged in.

Questions:

  1. Is it really possible that the user isn't logged in at the moment that OidcLogoutActionBuilder.getLogoutAction begins executing? If so, how? That method appears to be initiating a logout, not called after a logout, though I might have misunderstood.

  2. Where is the currentProfile parameter coming from in getLogoutAction? I alluded to the fact we use CiviFormProfileData over OidcProfile in our codebase, but I don't know how the framework is deciding what exactly to pass to this method.

  3. Is there a better, more robust way to get the ID token in our case than what I suggested earlier with ProfileManager.getProfile?

oblivion54
  • 11
  • 1

1 Answers1

1

General considerations:

If you use the OIDC protocol, your custom profile should inherit from the OidcProfile. Though, seeing the name for your custom profile: CivFormProfileData, I guess you also use it for form authentication. Maybe two different profiles here: one inheriting from CommonProfile and the other one inheriting from OidcProfile are the solution if one does not work.

And maybe in the future pac4j v6, we should turn the OidcProfile class into an OidcProfile interface to make things easier.

To reply your questions:

  1. In regular use cases (through the DefaultLogoutLogic), the OidcLogoutActionBuilder.getLogoutAction method cannot be called without an ODIC profile. Though, you can manually called this method if you want and this must be handled
  2. In regular use cases, the logout endpoint triggers the DefaultLogoutLogic which loops through the profiles if the centralLogout property is true: https://github.com/pac4j/pac4j/blob/pac4j-parent-5.7.0/pac4j-core/src/main/java/org/pac4j/core/engine/DefaultLogoutLogic.java#L106 These are the current authenticated user profiles
  3. You should not call the manager.getProfile, nor override the OidcLogoutActionBuilder, your custom profile should certainly inherit from OidcProfile: everything would work out-of-the-box this way.
jleleu
  • 2,309
  • 1
  • 13
  • 9
  • This is an extremely helpful answer. Seeking clarification on (3): do you mean that we should not take these steps for this particular use case, or more generally, that overriding the logout action is discouraged? Our [logout action](https://github.com/civiform/civiform/blob/1ec125cb2b39a6964990493aca430506b264ff9f/server/app/auth/oidc/CustomOidcLogoutRequest.java) performs some custom processing on the parameters of the URL. – yotommy Aug 22 '23 at 13:45
  • 1
    Components like `LogoutActionBuilder`, `ProfileCreator` and many others are meant to be be overriden for custom logic. Though, it should be done only in last resort if the fix cannot be contributed to pac4j. – jleleu Aug 24 '23 at 15:28
  • Since the `CiviFormProfileData` object can be associated with profiles that were obtained through other protocol means (e.g., SAML), it's not easy to make it inherit from `OidcProfile`. I see that `pac4j` supports multiple profiles in a web session. Could we store *two* profiles upon authentication, one `CiviFormProfileData` instance and one `OidcProfile` instance? I'm wondering if the loop you point out in answer (2) could then trigger the required logic when it encounters the `OidcProfile`. Though I am concerned that the `break` statement may cause the loop to terminate early. – yotommy Aug 29 '23 at 16:03
  • 1
    The idea is to have one profile per authentication and indeed, the first client associated to a profile which returns a logout action stops the loop of the profiles. If you want to perfom a form login authentication and an OIDC logout, you could maybe use an `AuthorizationGenerator` to rebuild one profile from the other. – jleleu Aug 30 '23 at 06:43