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

Cannot find requested client with clientId #30837

Closed
1 of 2 tasks
MrZajic opened this issue Jun 26, 2024 · 8 comments
Closed
1 of 2 tasks

Cannot find requested client with clientId #30837

MrZajic opened this issue Jun 26, 2024 · 8 comments
Assignees
Labels
area/ldap kind/bug Categorizes a PR related to a bug kind/regression priority/blocker Highest Priority. Has a deadline and it blocks other tasks release/25.0.2 release/26.0.0 team/ui
Milestone

Comments

@MrZajic
Copy link

MrZajic commented Jun 26, 2024

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

ldap

Describe the bug

If I add new mapper role-ldap-mapper for LDAP federation and unchecked Use Realm Roles Mapping with filled Client ID, then it will do nothing after sync and nothing is synced with roles for filled client id.

Version

25.0.1

Regression

  • The issue is a regression

Expected behavior

It will sync roles to specific Client ID.

Actual behavior

User federation -> yourFederation (LDAP: other) -> Mappers -> Add mapper -> role-ldap-mapper

Problematic is unchecked Use Realm Roles Mapping + filled Client ID. This cause warning in keycloak log:

2024-06-26 20:45:06,040 WARN  [org.keycloak.storage.ldap.mappers.membership.role.RoleLDAPStorageMapper] (executor-thread-11) Cannot find requested client with clientId 'ddde338f-d031-45ce-bae4-4d8f8aeb5d5e' in federation mapper 'roles'
2024-06-26 20:45:06,040 WARN  [org.keycloak.storage.ldap.mappers.membership.role.RoleLDAPStorageMapper] (executor-thread-11) Ignored sync for federation mapper 'roles' as client not found: 'ddde338f-d031-45ce-bae4-4d8f8aeb5d5e'

But if I go to Clients - selected my client -> then in URL i can see:
https://{keycloak.ip}/admin/master/console/#/{myRealm}/clients/ddde338f-d031-45ce-bae4-4d8f8aeb5d5e/settings

It seems UUID are fine. Database default H2 without changes. All newly created manually without imports config. With only checked Use Realm Roles Mapping without Client ID works fine.

I have very similar problem when I tried mapping client scope roles with very similar Client ID and too without success. But it is another problem.

How to Reproduce?

Please see actual behaviour.

Anything else?

No response

@MrZajic
Copy link
Author

MrZajic commented Jun 26, 2024

I did small investigating and maybe I see problem.

JpaReaelmProvider.java

    @Override
    public ClientModel getClientByClientId(RealmModel realm, String clientId) {
        logger.tracef("getClientByClientId(%s, %s)%s", realm, clientId, getShortStackTrace());

        TypedQuery<String> query = em.createNamedQuery("findClientIdByClientId", String.class);
        query.setParameter("clientId", clientId);
        query.setParameter("realm", realm.getId());
        List<String> results = query.getResultList();
        if (results.isEmpty()) return null;
        String id = results.get(0);
        return session.clients().getClientById(realm, id);
    }

Input parameter String clientId is UUID of client and It is not intended Client ID.

named query for findClientIdByClientId:

select client.id from ClientEntity client where client.clientId = :clientId and client.realmId = :realm

It failed on client.clientId = :clientId.

There could be used already existed getClientById SQL:

select client from ClientEntity client where client.id = :id and client.realmId = :realm
@Override
    public ClientModel getClientByClientId(RealmModel realm, String clientId) {
        logger.tracef("getClientByClientId(%s, %s)%s", realm, clientId, getShortStackTrace());

        TypedQuery<String> query = em.createNamedQuery("getClientById", String.class);
        query.setParameter("id", clientId);
        query.setParameter("realm", realm.getId());
        List<String> results = query.getResultList();
        if (results.isEmpty()) return null;
        String id = results.get(0);
        return session.clients().getClientById(realm, id);
    }

But question is why String clientId is UUID. Maybe bug is somewhere before and clientId should be name and not UUID.

@MrZajic
Copy link
Author

MrZajic commented Jun 26, 2024

And tests are written for cliendId (name) not UUID.

ClientModelTest.java

final ClientModel client1 = session.clients().getClientByClientId(realm, "client1");

@pedroigor
Copy link
Contributor

@MrZajic Thanks for the investigation.

Yeah, I just checked here and for me I'm seeing the clientId when syncing the roles. Can you check how the mapper config looks like when the Admin UI is fetching the config from /{realm}/components/{componentid}?

Perhaps try to re-create the mapper to see if it helps. If so, not sure why you ended up with the UUID set in the mapper config.

@pedroigor pedroigor self-assigned this Jun 26, 2024
@pedroigor
Copy link
Contributor

I think I know why. See #30523. Looks like re-creating the mapper should help.

@MrZajic
Copy link
Author

MrZajic commented Jun 26, 2024

@pedroigor Thank you. You're right. Unfortunately, fix is not in 25.0.1 tag. Fastly I tested NIGHTLY build and after re-creating mapper it works. And this solving more problems because similar problem was with Clients scope and mapping for example roles to specific Client ID. Now it's work too.

@pedroigor
Copy link
Contributor

Nice. @MrZajic Just to confirm, were you able to solve the problem in 25.0.1? Wondering if we need to backport that change if not there already.

@MrZajic
Copy link
Author

MrZajic commented Jun 27, 2024

@pedroigor Unfortunately, version 25.0.1 not solve problem. I see still old code without change (#30523) for 25.0.1 see:

options={clients.map(({ id, clientId }) => ({
key: id!,
value: clientId!,
}))}

@pedroigor
Copy link
Contributor

Should be fixed by #30865 in the next 25 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ldap kind/bug Categorizes a PR related to a bug kind/regression priority/blocker Highest Priority. Has a deadline and it blocks other tasks release/25.0.2 release/26.0.0 team/ui
Projects
None yet
Development

No branches or pull requests

3 participants