Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jooby-pac4j oidc: never renew profile? #3425

Closed
tkarlinski opened this issue May 17, 2024 · 3 comments
Closed

jooby-pac4j oidc: never renew profile? #3425

tkarlinski opened this issue May 17, 2024 · 3 comments
Milestone

Comments

@tkarlinski
Copy link
Contributor

tkarlinski commented May 17, 2024

Hello,

I'm having an issue with renewing the profile after has expired (for OIDC flow).

The problem arises because the profile is cleared every time in the ProfileManager, as the class has not set the config. Consequently, the profile is never renewed.

Take a look at the code:
In Pac4JModule the current user is set as follows:

application.setCurrentUser(new Pac4jCurrentUser());

The current user class has the following method:

  public Object apply(Context ctx) {
    Pac4jContext pac4jContext = Pac4jContext.create(ctx);
    ProfileManager pm = new ProfileManager(pac4jContext, pac4jContext.getSessionStore());
    return pm.getProfile().orElse(null);
  }

Here, we have an initialized ProfileManager without a config. The config should probably be set using the public method of ProfileManager setConfig(Config config).

The getProfile method calls the removeOrRenewExpiredProfiles method in ProfileManager, which includes the following condition:

    protected void removeOrRenewExpiredProfiles(final LinkedHashMap<String, UserProfile> profiles, final boolean readFromSession) {
        var profilesUpdated = false;
        for (final var entry : profiles.entrySet()) {
            final var key = entry.getKey();
            final var profile = entry.getValue();
            if (profile.isExpired()) {
                LOGGER.debug("Expired profile: {}", profile);
                profilesUpdated = true;
                profiles.remove(key);
                if (config != null && profile.getClientName() != null) {
                    final var client = config.getClients().findClient(profile.getClientName());
                    if (client.isPresent()) {
                        try {
                            final var newProfile = client.get().renewUserProfile(profile, context, sessionStore);
                            if (newProfile.isPresent()) {
                                LOGGER.debug("Renewed by profile: {}", newProfile);
                                profiles.put(key, newProfile.get());
                            }
                        } catch (final RuntimeException e) {
                            logger.error("Unable to renew the user profile for key: {}", key, e);
                        }
                    }
                }
            }
        }
        if (profilesUpdated) {
            saveAll(profiles, readFromSession);
        }
    }

In the method you see the condition

if (config != null && profile.getClientName() != null) {

I suspect that this condition will always be null when ProfileManager is called by Pac4jCurrentUser, resulting in the profile always being cleared.

Could you please advise me on how to bypass this problem? Or perhaps it should be fixed?

Thank you.

@tkarlinski
Copy link
Contributor Author

ps. The setCurrentUser we can't override because the method add new "current user" instead of set

application.setCurrentUser(new Pac4jCurrentUser());

@edgarespinawt
Copy link
Contributor

Would you like to send a PR?

Thanks

tkarlinski pushed a commit to tkarlinski/jooby that referenced this issue May 18, 2024
@tkarlinski
Copy link
Contributor Author

tkarlinski commented May 18, 2024

Ok, I created PR. That works for my problem.

tkarlinski pushed a commit to tkarlinski/jooby that referenced this issue May 19, 2024
@jknack jknack added this to the 3.1.1 milestone May 19, 2024
@jknack jknack closed this as completed in 40876aa May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants