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

setSingleAttribute doesn't add the attribute inside an EventListenerProvider #14942

Open
DanielScholte opened this issue Oct 17, 2022 · 13 comments
Labels
area/core area/storage Indicates an issue that touches storage (change in data layout or data manipulation) help wanted impact/low kind/bug Categorizes a PR related to a bug priority/normal status/auto-bump status/auto-expire status/bumped-by-bot team/core-iam

Comments

@DanielScholte
Copy link

DanielScholte commented Oct 17, 2022

Describe the bug

I recently updated our Keycloak version from 10.0.2 to 19.0.2. With this, I also updated our plugins.
One of our plugins is a simple EventListener that listens for an EventType.REGISTER event. When the event is triggered for a new account, it adds an attribute to the new account containing the ClientId which the user used to open the register page.
The EventListener is of type EventListenerProvider and uses the setSingleAttribute function on a UserModel.

The issue at hand is that the setSingleAttribute function doesn't actually add the attribute to the user. The code runs fine and without errors, but the attribute is never added.

Version

19.0.2

Expected behavior

For the setSingleAttribute to add and save an attribute to the user when being run inside an EventListener.
This was how it worked in Keycloak version 10.0.2.

Actual behavior

The setSingleAttribute function runs without throwing any errors, but doesn't add the attribute.
The setAttribute function suffers from the same issue.
In Keycloak version 10.0.2 the function works as expected, and adds the attribute to the user.

How to Reproduce?

Here's the source code for the EventListener, which is quite simple:

public class RegistrationClientIdEventListenerProvider implements EventListenerProvider {
    private static final Logger LOG = Logger.getLogger(RegistrationClientIdEventListenerProvider.class);
    private final KeycloakSession session;

    public RegistrationClientIdEventListenerProvider(KeycloakSession session) {
        this.session = session;
    }

    @Override
    public void onEvent(Event event) {
        if (event.getType() != EventType.REGISTER) {
            return;
        }

        UserModel user = this.session.users().getUserById(
                this.session.realms().getRealm(event.getRealmId()),
                event.getUserId()
        );

        if (user == null) {
            return;
        }

        user.setSingleAttribute("registrationClientId", event.getClientId());

        LOG.info("Added registrationClientId for user " + event.getUserId());
    }

    @Override
    public void onEvent(AdminEvent adminEvent, boolean b) {}

    @Override
    public void close() {}
}

The code runs without errors and the LOG.info logs as expected, indicating that it should have added the attribute. But the attribute is never added to the user.

Anything else?

No response

@DanielScholte DanielScholte added kind/bug Categorizes a PR related to a bug status/triage labels Oct 17, 2022
@stianst stianst added the area/storage Indicates an issue that touches storage (change in data layout or data manipulation) label Oct 17, 2022
@Vividious
Copy link
Contributor

Do you have a corresponding RegistrationClientIdEventListenerProviderFactory class as well?

@vramik
Copy link
Contributor

vramik commented Oct 24, 2022

Thank you @DanielScholte for the report. Could you please add another log message to the code (see the line just above setting the attribute) and provide output from it? That would help a lot to triage this issue. Thank you.

public class RegistrationClientIdEventListenerProvider implements EventListenerProvider {
    private static final Logger LOG = Logger.getLogger(RegistrationClientIdEventListenerProvider.class);
    private final KeycloakSession session;

    public RegistrationClientIdEventListenerProvider(KeycloakSession session) {
        this.session = session;
    }

    @Override
    public void onEvent(Event event) {
        if (event.getType() != EventType.REGISTER) {
            return;
        }

        UserModel user = this.session.users().getUserById(
                this.session.realms().getRealm(event.getRealmId()),
                event.getUserId()
        );

        if (user == null) {
            return;
        }

        LOG.debugf("UserModel: %s; clientId: %s ", user.getClass().getSimpleName(), event.getClientId());

        user.setSingleAttribute("registrationClientId", event.getClientId());

        LOG.info("Added registrationClientId for user " + event.getUserId());
    }

    @Override
    public void onEvent(AdminEvent adminEvent, boolean b) {}

    @Override
    public void close() {}
}

@vramik
Copy link
Contributor

vramik commented Nov 6, 2022

Hello @DanielScholte, do you have any update on this? Thanks

@DanielScholte
Copy link
Author

It returns UserModel: UserAdapter; clientId: account-console.
I would like to add that the setSingleAttribute works correctly when using it in the VERIFY_EMAIL event, which I'm doing now as a workaround.

@vramik vramik removed their assignment Nov 9, 2022
@hmlnarik hmlnarik added this to the 22.0.0 milestone Mar 1, 2023
@hmlnarik hmlnarik modified the milestones: 22.0.0, Backlog Mar 1, 2023
@danielpall
Copy link

I can confirm that this is still happening, and it works for more events, e.g. EventType.LOGIN.

So if you have the exact same code but use EventType.LOGIN instead of EventType.REGISTER then setting an attribute works on the UserModel. If it's set to EventType.REGISTER it does not work.

@supersnager
Copy link

Hey guys, I can confirm that this bug also occurs in version 22.0.3.
Both methods setSingleAttribute and setAttribute don't work for EventType.REGISTER but they do work for EventType.LOGIN
If there is any information I can provide to help with diagnosis, please let me know.

@danielpall
Copy link

I was able to get this working by using the same pattern as suggested here: https://github.com/p2-inc/keycloak-events/#user-change-listener.

It creates a KeycloakTransaction that is added to the current KeycloakSession by using the .enlistAfterCompletion method on the TransactionManager class.

  session
    .getTransactionManager()
    .enlistAfterCompletion(<the Keycloak Transaction here>)

@supersnager
Copy link

@danielpall Thank You for the tip. It works like a charm!

@derlin
Copy link
Contributor

derlin commented Nov 6, 2023

What's the status of this issue? @danielpall can you provide a full working example of the workaround?

@danielpall
Copy link

This is still an issue.

This is even worse if the Declarative User Profile is enabled because then the UserModel in the KeycloakSession isn't populated until after the transaction. So I had to move more logic into the transaction that is set in the enlistAfterCompletion.

I'll see if I have time to write up an example of the fix. If I do then I'll ping you @derlin.

But the link I posted in the comment above has a pretty good way to fix this and I recommend following that pattern.

@ghenadiibatalski
Copy link

Hi @danielpall, i'm facing a some kind differnt problem. I've elisted the action and the database is already updated (checked via psql), but if i read the user from the listener session.users().getUserById() the updated state is not loaded from db. Does anybody have a solution? See picture: terms_and_conditions is missing on select via keycloak:
image

image

@pedroigor
Copy link
Contributor

pedroigor commented Mar 6, 2024

AFAIK, the behavior is expected and the solution suggested by @danielpall should work. There were changes to how event listeners are executed where certain operations are only possible within a transaction when at the afterCompletion phase.

@vramik Assigning to you as you have started this. If not an issue, please feel free to close it.

@keycloak-github-bot
Copy link

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a 👍 to the description. We would also welcome a contribution to fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/storage Indicates an issue that touches storage (change in data layout or data manipulation) help wanted impact/low kind/bug Categorizes a PR related to a bug priority/normal status/auto-bump status/auto-expire status/bumped-by-bot team/core-iam
Projects
None yet
Development

No branches or pull requests

10 participants