[keycloak-dev] Improving support for LDAP backed Keycloak

Mon Mar 4 14:53:23 EST 2019

Hi All,

Continuing my journey, I found that I was unable to create users through Keycloak, as they had to have the objectClass: posixAccount, which has uidNumber, gidNumber and homeDirectory all as mandatory fields. During creation, first the UsersResource.createUser method would add an empty user to LDAP and then update it with the information passed in. The UserAttributeLDAPStorageMapper.onRegisterUserToLDAP method would use “ “ as the default for mappers marked as mandatory in LDAP, however, “ “ doesn’t work for numeric fields.

The simplest thing would be for Keycloak to store “0" instead of “ “ for mandatory attributes with a numeric syntax, but Keycloak doesn’t have knowledge of the attribute syntax (at least, not from within this method as far as I can tell).

The next minimal change I could determine was to add a “LDAP Default Value” configuration option to the UserAttributeLDAPStorageMapperFactory.getConfigProps method, and use that instead of LDAPConstants.EMPTY_ATTRIBUTE_VALUE (defaulting to this if the config property is missing, for backward compatibility).

I’ve added this to my current work on KEYCLOAK-5571 as it’s intrinsically linked to my work on improving support for LDAP backed Keycloak. Please let me know if this is inappropriate and I’ll raise a separate JIRA (which might already exist) and split the patch out. You can find the commit with this change here: https://github.com/dhardiker/keycloak/commit/67b9b029784074b6ec580c786898ae4ef9e23497

With this, I think this completes our immediate show stopping issues with using Keycloak in front of our OpenLDAP installation. As ever, I welcome code review before I find the time to expand the test coverage and comply with the other PR requirements!

Thanks,

—
Dan Hardiker | Adaptavist
dhardiker at adaptavist.com

Winners of the Atlassian President's Award for Technical Excellence - http://bit.ly/techexc

Adaptavist, Waterside, Unit 2, 44-48 Wharf Road, London, N1 7UX, United Kingdom.
Registered in England and Wales #5456785.

> On 1 Mar 2019, at 23:12, Dan Hardiker <dhardiker at adaptavist.com> wrote:
> 
> Hi All,
> 
> So I’ve continued and have Keycloak extended to support KEYCLOAK-5571. Notably, I can setup LDAP attribute mappers for the enabled (boolean), emailVerified (boolean), and requiredActions (Set<String>) model fields and they work as expected. I did not need to store TOTP, it seems to just work but if I’m missing a scenario where it doesn’t, the same approach should work.
> 
> Required Actions caused me a bit of effort. It does not have a setter on the proxy like the others, instead is has addRequiredAction/removeRequiredAction mutator methods. This means I’ve had to detect the requiredAction model field in a couple of areas to short-circuit so that errors don’t crop up in the logs. It’s still functional without those short-circuits (as Required Actions are handled later in the processes) but the logs can get noisy and confusingly point to a problem that’s not there. Personally I think it’s cleaner to short-circuit. [If I’m not being clear, I can try to explain again differently.]
> 
> Because of my unfamiliarity with Github, I’ve ended up opening a Draft Pull Request. I wanted to do it within my own fork, but ended up doing it live. I’ve left it so you can see where I’m up to. A quick review of my approach would be very welcome as I’ve never committed to Keycloak before and would appreciate a nod that I’m going in the right direction with all this.
> 
> https://github.com/keycloak/keycloak/pull/5908
> 
> I have no automated tests yet, it’s just been a manual development cycle as it was slow enough getting up to speed with the code base, let alone the LDAP side of the test harness! I’ll move onto that next and hopefully with that and completing the rest of the committer guidance, it’ll be soon accepted!
> 
> 
> —
> Dan Hardiker | Adaptavist
> dhardiker at adaptavist.com
> 
> Winners of the Atlassian President's Award for Technical Excellence - http://bit.ly/techexc
> 
> Adaptavist, Waterside, Unit 2, 44-48 Wharf Road, London, N1 7UX, United Kingdom.
> Registered in England and Wales #5456785.
> 
>> On 22 Feb 2019, at 12:54, Dan Hardiker <dhardiker at adaptavist.com> wrote:
>> 
>> Hi All,
>> 
>> Thanks for the pointers. I was able to get Keycloak imported properly into IDEA (a bit of user error, needed a lot more patience). I’ve worked out how to get Keycloak running with a remote debugger, which has been a great help, but I’ve not managed to get Keycloak running from within IDEA itself. Is it standard to run it through mvn during development?
>> 
>> I’ve got a basic implementation working against a custom attribute I’ve set. It’s not tested, I’m first looking for validation on the approach taken. Can someone please have a look at https://github.com/dhardiker/keycloak/commit/e26ff98c2d4c55b98b743522d195226b94b4c8aa - all comments welcome, so I can make sure I’m on the right path. Tests will come before a PR is submitted.
>> 
>> Other questions:
>> 
>> 1. Should I address the other KEYCLOAK-5571 issues too in the same branch of work?
>> 2. I’ve found that boolean fields sometimes map inversely to common LDAP attributes one might choose to back Keycloak with (e.g. UserModel.enabled <> LDAPUser.loginDisabled). Would it be ok to add another setting to the UserAttributeLDAPStorageMapper which would be “Invert boolean values” - so an UserModel.enabled == true maps to setting LDAPUser.loginDisabled = false? If so, any tips on where the areas to extend that in, beyond the Mapper itself (I’m thinking at least a theme or two?).
>> 
>> Thanks for the continued support, I’m starting to feel a little more comfortable with the Keycloak codebase.
>> 
>> 
>> —
>> Dan Hardiker | Adaptavist
>> dhardiker at adaptavist.com
>> 
>> Winners of the Atlassian President's Award for Technical Excellence - http://bit.ly/techexc
>> 
>> Adaptavist, Waterside, Unit 2, 44-48 Wharf Road, London, N1 7UX, United Kingdom.
>> Registered in England and Wales #5456785.
>> 
>>> On 20 Feb 2019, at 13:03, Marek Posolda <mposolda at redhat.com> wrote:
>>> 
>>> On 18/02/2019 18:39, Dan Hardiker wrote:
>>>> Hi All,
>>>> 
>>>> Sorry for such a long first post. Here we go!
>>>> 
>>>> 
>>>> TL;DR:
>>>> I want to look at 
>>>> https://issues.jboss.org/browse/KEYCLOAK-5571 as it is impacting us. I’m happy to contribute code or write a blog on what configuration settings are needed to achieve this. While the SAGA has more context, here’s a few of my currently burning questions:
>>> Cool!
>>>> 1. What implements the org.keycloak.admin.client.resource.UserResource.update(UserRepresentation) and UserRepresentation ...toRepresentation() interface method? (from the integration/admin-client directory - I can’t find the business logic)
>>> The implementation is "auto-generated" proxy class, which just invokes the particular admin REST endpoint - for example org.keycloak.admin.client.resource.UserResource.update is invoked the server-side REST endpoint org.keycloak.services.resources.admin.UserResource.updateUser
>>>> 2. What would be the right approach to wire up the admin ui User Enabled toggle to a LDAP boolean field, and where in the codebase would that go? (if you can cite examples of similar that would be great)
>>> 
>>> UserAttributeLDAPStorageMapper is the class where you can look at. As you can see in the "proxy" method, the proxied model implements setFirstName, setLastNAme, setEmail .
>>> It seems that few more things need to be added (EG. setEnabled, setEmailVerified)
>>> 
>>> The automated test may need to be added somewhere in LDAPProvidersIntegrationTest IMO.
>>> 
>>>> 3. What is the best way to go about setting up an IDE for development? Just importing the root POM into IDEA doesn’t seem to cut it.
>>> Importing the root POM into IDEA works fine for me.
>>>> 4. If I provide a patch for this, is this something that might be considered for pulling into master?
>>> Yes. There needs to be few simple rules met when you want something to be considered into master. We're working on improving guidelines for contributors. See details in this PR: https://github.com/keycloak/keycloak/pull/5886/files . You just already started the discussion on keycloak-dev mailing list, which is nice 1st step. Good luck with moving forward on your contribution!
>>> 
>>> Marek
>>> 
>>>> I am interested in all of the features within KEYCLOAK-5571, as a few other requirements, but I’m happy to start here and treat the others as atomic suggested changes. They may include:
>>>> 
>>>> * Supporting incremented default values for new users (the uidNumber must be unique and it should be 1 greater than the highest uidNumber that the system can see … i.e. the next available UID).
>>>> * Supporting out-of-band password recovery (where by a code is sent via a trusted path [text message, telephone call, in person conversation with the user] which can be used to reset their password - ideally in combination with another stored value, such as their employee id / tax id / post code / something else which is relatively static but relatively unknown) - this could be developed outside of Keycloak of course, but would ideally be within the same system.
>>>> 
>>>> If addressing KEYCLOAK-5571 goes well, I would be interested in continuing to contribute down these paths.
>>>> 
>>>> Thanks for your time, I would love to get involved … I just need a bit of help.
>>>> 
>>>> 
>>>> THE SAGA:
>>>> Apologies if this message should be in keycloak-users, and if any of it seems incoherent. I’ve been fighting in circles all weekend and I have to admit that I’m not entirely sure I’m approaching the problem correctly. Please bear with me as I’m not entirely sure how to articulate things at this point, but I know I need help!
>>>> 
>>>> Problem statement: we are currently using OpenLDAP to manage access to our systems. However, the administration interface is crude and it lacks SAML/OIDC support for integrating systems like Google Suite, AWS Console, Office 365 and others. It also lacks a self service console where users can mange their own accounts. Keycloak at first glance looks ideal - especially as it allows us to continue using OpenLDAP as the primary source of truth, with Keycloak used to enhance the user experience giving self service and integration with SAML/OIDC clients.
>>>> 
>>>> As per the docs, some mapping is required to have OpenLDAP support the storage of Keycloak data within the OpenLDAP schemas. Unfortunately, I’ve not bee able to find documentation for what those fields names in Keycloak can be and how I should alter my OpenLDAP schema to support them. I found KEYCLOAK-5571 which appears to cover at least some of the issues I’m having. Amongst other things, I’m a Java developer, so I’m comfortable with working in code and submitting patches. Assuming that the answer isn’t configuration, is this something that would be valuable to contribute? If so, is there any advice that this list can offer on where to start?
>>>> 
>>>> What follows is my journey as an outsider into trying to figure out things myself. This may or may not be of interest - but given this list gets indexed by Google, it might help someone in future. Seeing that issue (KEYCLOAK-5571) I figured the best place to start would be the admin ui where you enable/disable users. I thought that I would start at the browser and try to figure out what the enabled/disable user toggle did when saved, trace that into the server side endpoint that picked up that representation and hopefully find out where & why it didn’t make it through to LDAP.
>>>> 
>>>> I noticed that there was a PUT to 
>>>> http://localhost/auth/admin/realms/master/users/f:$UUID:$USERNAME
>>>> and as part of the JSON payload was “enabled: false”. At this point I started grepping around in the Keycloak code. I figured that org.keycloak.admin.client.resource.UserResource.update(UserRepresentation) Interface was what was being called, but unfortunately when I opened up the root POM then IDEA only saw the files as plain text and none of the Intelisense worked, I could grep around the code though. When I opened up the integration/admin-client/pom.xml it recognise the Java files, however I wasn’t able to find what was implementing this. If found "public static UserRepresentation toRepresentation(KeycloakSession session, RealmModel realm, UserModel user)” in server-spi-private org.keycloak.models.util.ModelToRepresentation, but couldn’t find the glue which connects them together. I’m guessing there might be some WildFly or other magic going on which I’m not aware of?
>>>> 
>>>> Seeing the “enabled: false” lead me to think that I might be able to create a user-attribute-ldap-mapper from the user model attribute “enabled” to an “enabled” LDAP attribute I added to our schema to test. The LDAP attribute has SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 (a binary attribute) and I’ve checked I can set that to TRUE / FALSE appropriately. I set it to be mandatory in LDAP and set it to be a Binary Attribute - however when I save it says "Error! With Binary attribute enabled, the 'Always read value from LDAP' must be enabled too” - however there is no “Always read value from LDAP” option! However, after enabling Import Users in the LDAP user federation settings, “Always read value from LDAP” becomes available. It’s not clear if Binary Attributes are supported only in this configuration, but ideally I would like to not Import Users as I prefer LDAP to be the authoritative source. After this I can disabled Import Users and the configuration still seemingly remains valid without any errors in the logs. That said, it’s not erroring about not properly persisting the enabled state to LDAP...
>>>> 
>>>> If I go across to the user in the admin ui, even though enabled is set to FALSE in LDAP, the toggle is showing as enabled in the UI. The JSON it gets for the UserRepresentation on the client side is “enabled”:true which explains the state of the toggle. If I stick Wireshark locally, setup a Docker with OpenLDAP and configure it appropriately, sniffing traffic I can see that the enabled attribute for my user comes back as FALSE. So there is something going wrong when trying to build that UserRepresentation. I suspect at the root of the KEYCLOAK-5571 issue. If I change the toggle to false in the UI and save, then reload the page, the toggle is back to true - when it persists to the LDAP server, it’s sending enabled: FALSE - this doesn’t make sense, but it might be just repeating back to LDAP what it read in without changing that field. If I change the name as well, it does send those fields updated, but enabled remains FALSE in the LDAP server.
>>>> 
>>>> Given that I didnt get very far with the UserRepresentation angle, I thought about going down the FederatedStorage - something must map the model into LDAP, as changes to the first name / last name, and the other attributes seem to be persisted and loaded in LDAP just fine. In my grepping around in server-spi I found a org.keycloak.models.UserModel Interface, which had a org.keycloak.storage.adapter.AbstractUserAdapterFederatedStorage implementation with a ENABLED_ATTRIBUTE = “ENABLED” field and isEnabled / setEnabled methods which getFirstAttribute(ENABLED_ATTRIBUTE) / setSingleAttribute(ENABLED_ATTRIBUTE, Boolean.toString(enabled). The class comment has:
>>>> 
>>>> * Assumes everything is managed by federated storage except for username.  getId() returns a default value
>>>> * of "f:" + providerId + ":" + getUsername().  UserModel properties like enabled, firstName, lastName, email, etc. are all
>>>> * stored as attributes in federated storage.
>>>> 
>>>> I’m not sure how the case difference between “enabled” in the UserModel properties and “ENABLED” as listed in the class field is connected - but there must be a mapping somewhere, as “firstName” is similarly “FIRST_NAME” and that maps just fine. I found model/jpa contained org.keycloak.models.jpa.entities/UserEntity which had @Column(name = "ENABLED”) protected boolean enabled, perhaps this is the link and even with Import Users disabled it always goes through the database?
>>>> 
>>>> I’ve yet to find the trigger which calls the mapper to run which persists into the database. Part of the problem is that I’m acutely aware my IDE is not setup to effectively jump around the code base, or to effectively attach my IDE as a debugger so I can add breakpoints and step through the code to figure out what happens where. I’ve just turned on trace logging - but this is giving me a wall of text which may take sometime to process. I’ve also yet to comb through the H2 DB to see if there’s cause there.
>>>> 
>>>> Any assistance on this would be most welcome.
>>>> 
>>>> 
>>>> ON ANOTHER NOTE:
>>>> I checked out the code and ran the build as documented against Java 8 on my mac, but unfortunately it failed. I ignored it and progressed, but here’s some excerpts from the output:
>>>> [INFO] Keycloak Integration TestSuite - deprecated ........ FAILURE [07:04 min]
>>>> 
>>>> [ERROR] Errors:
>>>> [ERROR]   OIDCKeyCloakServerBrokerBasicTest.testLogoutWorksWithTokenTimeout:131 » Processing
>>>> [ERROR]   OIDCKeycloakServerBrokerWithConsentTest.before:84 » Processing java.lang.NoSuc...
>>>> [ERROR]   BrokenUserStorageTest.testBootWithBadProviderId:118 » Processing java.lang.NoS...
>>>> [ERROR]   JaxrsBasicAuthTest.testBasic:120 » NoSuchMethod org.apache.commons.io.output.D...
>>>> [ERROR]   JaxrsFilterTest.testBasic:129 » NoSuchMethod org.apache.commons.io.output.Defe...
>>>> [ERROR] Tests run: 238, Failures: 0, Errors: 5, Skipped: 32
>>>> 
>>>> I guess this shouldn’t happen on a fresh check out & a following of the instruction.
>>>> 
>>>> 
>>>> If you made it this far - bravo!
>>>> 
>>>> Thanks again,
>>>> 
>>>> 
>>>> —
>>>> Dan Hardiker | Adaptavist
>>>> 
>>>> _______________________________________________
>>>> keycloak-dev mailing list
>>>> 
>>>> keycloak-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>> 
>>> 
>> 
> 