[keycloak-dev] Preferred way to make KeyCloak custom changes - Allow for extra entities in Hibernate besides persistence.xml

[Stian Thorgersen]

On 16 November 2015 at 13:26, Erik Mulder <erik.mulder at docdatapayments.com>
wrote:

> *@Christian:* Actually in Hibernate 5 the Integrator does not offer the
> add entity feature anymore. It seems this 'feature' was actually an
> unintended side effect of the Integrator in Hibernate 4 (
> http://stackoverflow.com/questions/32918808/how-to-use-integrator-service-in-hibernate-5-for-adding-annotated-classes
> - *"but the docs clearly point towards that not being intentional as this
> should be done at the time of initialization"*). This complies with my
> finding that adding the entity classes at this 'late' stage skips some
> validation. So I guess I'll 'revert' my solution to the one I found
> previously, which adds the entity classes to the JPA persistence unit
> configuration. I'll investigate if that is Hibernate 4 and 5 compatible.
>
> *@Stian: *It seems like almost all questions have been answered and we
> can start to build the feature and contribute it to KeyCloak. Maybe you can
> provide some extra information / requirements / guidelines to us. (we did
> read the 'Hacking on Keycloak' page)
> - Is there a KeyCloak checkstyle configuration?
>

No, we have not introduced one yet. However, we'd like code to follow code
style from WildFly. You can get IDE configuration from here
https://github.com/wildfly/wildfly-core/tree/master/ide-configs


> - Do we have to create one entry in the KeyCloak JIRA for this or several
> under one epic?
>

One is sufficient


> - Is there some peer review process? How does it work?
>

Step one is to send an email to Keycloak Dev mailing list about what you
are doing. This is obviously already done.

Second stage is when you've prepared a PR one of the core developers will
review it before merging.



> - What are the requirements for the documentation?
>

No hard requirements, but in general for a new feature to be added it needs
to be mentioned on the documentation. For this particular feature I think
adding a section to
http://keycloak.github.io/docs/userguide/keycloak-server/html/providers.html
would be good.


>
>
>
>
> On 12/11/15 17:20, Christian Beikov wrote:
>
> You need different code for Hibernate 4 and 5 because the integrator
> interfaces are not compatible.
> The integrator is a service that is loaded through a
> java.util.ServiceLoader so I am not sure why you would need the property.
>
> Mit freundlichen Grüßen,
> ------------------------------
> *Christian Beikov*
> Am 12.11.2015 um 17:14 schrieb Erik Mulder:
>
> *@Christian Beikov*: Thanks for the hint on integrators!
>
> I was afraid I could not use integrators when working with the JPA
> 'route', but I found a way. There is a property EntityManagerFactoryBuilderImpl.INTEGRATOR_PROVIDER
> that you can use to supply custom integrators. This even works with the
> original 'pure' JPA call: Persistence.createEntityManagerFactory. In the
> integrator you can add annotated classes to the Hibernate Configuration. So
> the solution is still Hibernate only of course.
>
> The only downside is that there is less validation on the supplied
> classes, they are just added to the config directly. For instance I tested
> with adding Object.class as annotated class. This doesn't raise an
> exception and seems to be silently ignored by Hibernate. Not sure if it
> might result in problems during runtime though. Either way, normally this
> should be fine and with a little extra documentation on how these extra
> classes are handled I think using an integrator is the least intrusive thus
> best way to go. Agreed?
>
> Ok, List as collection type is fine too, indeed easier to use.
> Conceptually I like Set, because List seems to imply there is some kind of
> ordering involved which in this case there isn't. But that's just a minor
> matter of taste.
>
> By exotic ProviderLoader(Factory) I meant the
> org.keycloak.provider.ProviderLoader(Factory) that you can use to load
> Provider(Factory)'s from other locations than a file system based
> classpath. If that were to be a 'read once' kindof location (like some
> stream) there could be a problem. But with the integrator solution that
> shouldn't matter anymore, since it's just the Class object that we need.
>
> As for Hibernate 4/5, I'll try to make a solution that works for both
> Hibernate 4 and 5 in the same way. If that's not possible, I guess we could
> have separate code paths for 4 and 5, depending on the runtime version. I
> hope there is an easy way of figuring this out, maybe some static Hibernate
> class holding a version or so. A quick Google returns some useful results,
> so I'm sure we'll get that sorted if needed.
>
> End of the month is probably too soon indeed, so let's aim for 1.8. Should
> I (or someone else) create one (or several) JIRA tickets for this?
>
>
> On 12/11/15 14:32, Stian Thorgersen wrote:
>
>
>
> On 12 November 2015 at 13:58, Erik Mulder <erik.mulder at docdatapayments.com
> > wrote:
>
>> On 11/11/15 13:54, Stian Thorgersen wrote:
>>
>> Would you be interested in contributing this feature? ATM we don't have
>> anyone available that could work on it. A contribution would also need to
>> include functional tests and documentation.
>>
>>
>> Yes, I'd like to contribute this feature. I'm not sure about the timeline
>> though. I hope to be able to do it as part of our current project, but I
>> might have to use my spare time as well. Is there some kind of deadline to
>> be included in a certain release version?
>>
>
> We do a release every ~6 weeks. It's already a bit late for 1.7 (it's due
> end of the month) so would have to aim for 1.8 in either case (early
> January).
>
>
>>
>>
>> If so I'm happy with going down the route of using the Hibernate specific
>> classes. The remaining issue is figuring out how to deal with classloading.
>>
>> Looks like the following should work:
>>
>> * Add JpaEntitySPI, JpaEntityProviderFactory and JpaEntityProvider
>>
>>
>> I've done this and it works fine, successfully providing the extra
>> classes to the EntityManagerFactory build process in
>> DefaultJpaConnectionProviderFactory.
>>
>> * JpaEntityProvider should have a single method "Class<?> getEntities"
>>
>>
>> Yes, only we need some kind of collection type, so you can provide
>> multiple entity classes per provider. I guess you were intending this,
>> considering the plural name 'getEntities'. I suggest either
>> Collection<Class<?>> or Set<Class<?>> depending on what is most
>> consistent with the rest of the system. Do you have a preference?
>>
>
> Yup - List would be fine, that's what we tend to use as it's nicer to use
> than collection or set.
>
>
>>
>>
>> * Implement org.hibernate.boot.registry.classloading.spi.ClassLoaderService
>> - looks like this can just return null for everything except classForName
>> where it would return the classes returned by the JpaEntityProvider
>> implementations
>>
>>
>> I see no way to interfere in the creation of the ClassLoaderService. The
>> official way is using the BootstrapServiceRegistryBuilder, but with the
>> JPA / EntityManagerFactoryBuilderImpl route this happens 'out of reach'.
>> I did find another way that works just as well: you can provide a 'custom'
>> classloader to the Bootstrap.getEntityManagerFactoryBuilder. We can
>> define a classloader that will return the extra JpaEntityProvider classes
>> if requested. Only tricky part here is that Hibernate not only calls
>> loadClass on a classloader, but before that also getResource to get a URL
>> with an InputStream to the class bytes. It uses that to scan for
>> annotations with Jandex. I fixed this by forwarding that request to the
>> ClassLoader of the JpaEntityProvider provided class (through
>> Class.getClassLoader()). This works fine and shouldn't be problem for
>> any drop in jars. I can imagine though that if you use some exotic
>> ProviderLoader(Factory), you might somehow get in trouble if the class
>> byte[] is not available anymore after class loading. But this is a problem
>> with the way Hibernate works, not with the way we extend Hibernate in this
>> case. So I think it's fine to have a warning about this in the
>> documentation, since it will probably never be a real problem. If you
>> consider this as a no-go, please let me know.
>>
>
> Sounds OK, but not sure what you mean about exotic ProviderLoader(Factory)
> is that a Hibernate thing?
>
>
>>
>>
>> Last question I have considers the Hibernate version of KeyCloak.
>> Currently it's 4.3.10, are there any plans to upgrade to 5? The code
>> related to classloading etc is refactored considerably in Hibernate 5. So
>> it would be a shame to fully get it working for 4.3.10, only to have to
>> upgrade soon after that. I didn't look into the details of Hibernate 5 and
>> I think the solution we came up with should remain more or less intact, but
>> you never know, so that's why I ask.
>>
>
> We are soon moving to WildFly 10 which includes Hibernate 5, but we still
> need to support EAP 6.4 which includes Hibernate 4. At some point next year
> we will drop support for EAP 6.4 and move on to EAP 7.
>
> We either have to support both Hibernate 4 and 5 for a while, or we make
> it use the old approach on Hibernate 4 (so now custom entity class support
> on EAP 6.4) and the new approach on Hibernate 5. That would probably
> require some magic reflection code though.
>
>
>>
>>
>>
>> On 7 November 2015 at 23:39, Erik Mulder <erik.mulder at docdatapayments.com
>> > wrote:
>>
>>> *On 06/11/15 14:46, Stian Thorgersen wrote:*
>>> *> We could use Hibernate directly to boostrap as long as it can return
>>> an EntityManager. Do you know if that's possible?*
>>>
>>> I was a little quick to state that with Hibernate you can add extra
>>> entity class names besides the one in persistence.xml, since I spotted a
>>> few answers on StackOverflow that said it could be done. But they resolve
>>> around classpath scanning or using a Spring managed Hibernate. Then I
>>> thought: 'if Spring can do it, I can do it too' so I investigated the
>>> Hibernate source code 'behind'
>>> Persistence.createEntityManagerFactory(unitName, properties). After
>>> some digging it turns out it's pretty simple to get extra class names in
>>> the configuration. See code sample below.
>>>
>>> The only problem is that Hibernate will only find classes that are part
>>> of the 'main' KeyCloak application, because of the way the Wildfly module
>>> system and ClassLoader strategy work. The debugger showed me Hibernate has
>>> these 3 class loaders available to look for classes:
>>> 1. ModuleClassLoader for Module "deployment.keycloak-server.war:main"
>>> from Service Module Loader
>>> 2. ModuleClassLoader for Module "org.hibernate:main" from local module
>>> loader
>>> 3. sun.misc.Launcher$AppClassLoader
>>>
>>> Number 1 has all other KeyCloak modules in it, so the entity classes
>>> from model-jpa will be found, but the wildfly-extensions module is missing,
>>> so entities in classes in a jar in the providers folder cannot be found.
>>> Now you guys obviously know a lot more about these internals, but as
>>> currently configured, it seems to me there is no way to let Hibernate 'see'
>>> these extra classes, since only the KeyCloak services module has a
>>> dependency on wildfly-extensions.
>>>
>>> So I think it boils down to these decisions:
>>> A. Do you accept a non-pure-JPA way of building the EntityManagerFactory
>>> that has some ties to the Hibernate internals?
>>> B. If A is no, than we're done. If yes, then you must find some way to
>>> get the extra configured classes 'into' Hibernate. You could get the
>>> wildfly-extensions module into scope of the Hibernate classloading. There
>>> are serveral ways to configure Hibernate classloading or you could flip
>>> some switches / dependencies in the module configuration. Another
>>> alternative is to create a separate 'dropfolder' besides themes and
>>> providers for JPA extensions, like 'models' or so and have that one be on
>>> the Hibernate classpath. But I don't know the exact design principles
>>> behind KeyCloak or the Wildfly module system. So maybe you have a better
>>> solution or maybe you conclude that this is 'not done' in terms of the
>>> architecture.
>>>
>>> Either way, I'd really appreciate some feedback on this and some
>>> thoughts on whether this could be a possible addition to KeyCloak in your
>>> eyes.
>>>
>>> Thanks, Erik
>>>
>>>
>>> Current JPA way. No way to 'interfere':
>>> emf = Persistence.createEntityManagerFactory(unitName, properties);
>>>
>>> Alternative Hibernate only way with adding extra entity class names:
>>> // Let Hibernate find and parse all 'persistence.xml' files found on the
>>> classpath.
>>> List<ParsedPersistenceXmlDescriptor> persistenceUnits =
>>> PersistenceXmlParser.locatePersistenceUnits(properties);
>>> // Assume there is only one persistence unit found and that is the one
>>> we need. This can be made more robust by checking on the persistence unit
>>> name.
>>> ParsedPersistenceXmlDescriptor persistenceUnitDescriptor =
>>> persistenceUnits.get(0);
>>> // Add extra class names. These could come from a 'JPA class name
>>> provider' SPI or something alike.
>>>
>>> persistenceUnitDescriptor.addClasses("org.keycloak.models.jpa.entities.UserMerchantEntity",
>>> "org.keycloak.models.jpa.entities.MerchantEntity");
>>> // Let Hibernate create an EntityManagerFactory out of the (enriched)
>>> persistence unit configuration.
>>> emf =
>>> Bootstrap.getEntityManagerFactoryBuilder(persistenceUnitDescriptor,
>>> properties).build();
>>>
>>>
>>>
>>>
>>> On 6 November 2015 at 14:29, Erik Mulder <
>>> <erik.mulder at docdatapayments.com>erik.mulder at docdatapayments.com> wrote:
>>>
>>>> Thanks for pointing me explicitly to the SPI documentation. Of course
>>>> that is exactly what I was looking for in my original question. I don't
>>>> know how I overlooked this earlier! Probably I was not picking it up,
>>>> because of almost a decade of developing on Spring projects, where this
>>>> type of thing works differently. :-)
>>>>
>>>> I tried a quick test with a jar with one extra ProtocolMapper
>>>> configured, put it in the providers folder and it worked like a charm!
>>>>
>>>> As for the JPA: We'll probably go with your suggestion of the separate
>>>> EntityManagerFactory. Indeed there seems to be no way to 'programmatically
>>>> extend' the list of entity classes in JPA besides editing or overwriting
>>>> the persistence.xml. As you probably know it can be done in Hibernate, but
>>>> I guess KeyCloak wants to stick to a generic JPA solution. That said, we
>>>> might consider the Hibernate specific solution for our case, since being
>>>> able to switch the JPA provider is not a requirement for us. And keeping
>>>> the same connection/transaction is a lot easier in reasoning and debugging.
>>>>
>>>
>>> We could use Hibernate directly to boostrap as long as it can return an
>>> EntityManager. Do you know if that's possible?
>>>
>>>
>>>>
>>>>
>>>>
>>>> On 05/11/15 10:52, Stian Thorgersen wrote:
>>>>
>>>> The way to extend Keycloak is by implementing your own custom providers
>>>> of the many SPIs we provide. Some SPIs are more stable (so marked as
>>>> public) and others are not (so marked as private). If there are things that
>>>> you want to customize that can't be done with an existing SPI then let us
>>>> know and we may consider adding additional SPIs.
>>>>
>>>> On 4 November 2015 at 17:16, Erik Mulder <
>>>> <erik.mulder at docdatapayments.com>erik.mulder at docdatapayments.com>
>>>> wrote:
>>>>
>>>>> Thanks for your response!
>>>>>
>>>>> Indeed we already did a proof of concept where we added a custom mapper
>>>>> the way you described (didn't know it was 'protected' territory :). The
>>>>> question is: do we have to override the file
>>>>> 'org.keycloak.protocol.ProtocolMapper' for this and add the new mapper
>>>>> in the original project or is there another way where we don't need to
>>>>> touch the original sources and keep all our changes in a separate
>>>>> project? And how can we do it such that it stays easy to upgrade to
>>>>> newer KeyCloak releases?
>>>>>
>>>>
>>>> Each jar has it's own org.keycloak.protocol.ProtocolMapper. Take a look
>>>> at the docs (
>>>> <http://keycloak.github.io/docs/userguide/keycloak-server/html/providers.html>
>>>> http://keycloak.github.io/docs/userguide/keycloak-server/html/providers.html)
>>>> and examples for other provider (
>>>> <https://github.com/keycloak/keycloak/blob/master/examples/providers/event-listener-sysout/src/main/resources/META-INF/services/org.keycloak.events.EventListenerProviderFactory>
>>>> https://github.com/keycloak/keycloak/blob/master/examples/providers/event-listener-sysout/src/main/resources/META-INF/services/org.keycloak.events.EventListenerProviderFactory
>>>> )
>>>>
>>>>
>>>>>
>>>>> As for JPA: it would be easier to integrate with the existing JPA
>>>>> project. Again we are wondering whether to start modifying original
>>>>> sources (like persistence.xml) or try to 'externalize' our changes
>>>>> somehow and integrate them using existing 'hooks' in the system or
>>>>> maybe
>>>>> merge projects during build.
>>>>>
>>>>> Maybe there is no good answer to this and we'll always be having some
>>>>> manual merge pains when upgrading to new KeyCloak versions. We just
>>>>> wanted to check if there are preferred ways to add functionality with
>>>>> the least amount of impact on the original sources.
>>>>>
>>>>
>>>> I initially wanted the ability to add custom entities to the
>>>> JpaConnectProvider, but couldn't find a way to define entities
>>>> programatically with JPA. To add your own persistence.xml you would have to
>>>> define your own implementation of JpaConnectionProvider and change what is
>>>> loaded by default (connectionsJpa/provider attribute in
>>>> keycloak-server.json).
>>>>
>>>> Alternative, which is cleaner, but you end up with separate
>>>> connection/transaction, is to create your own EntityManagerFactory. If it's
>>>> only used by one provider (for example a custom UserFederationProvider)
>>>> there's no need to add a connect provider (that's just a way to share one
>>>> EntityManagerFactory between multiple providers) and you can just create it
>>>> in the MyUserFederationProviderFactory.
>>>>
>>>>
>>>>>
>>>>>
>>>>> On 04/11/15 15:30, Bill Burke wrote:
>>>>> > Custom mappers should be possible.  I didn't document it as I wasn't
>>>>> > sure if we wanted to make the SPI public.  Custom mappers should just
>>>>> > follow the Provider SPI and they will be picked up.  If you see the
>>>>> > META-INF/services/... file in the resources directory of the
>>>>> "services"
>>>>> > or "broker" modules you'll see how to set this up.
>>>>> >
>>>>> > As for extending the JPA datamodel, what you could do is write a new
>>>>> JPA
>>>>> > Connections Provider and plug that in.  See connections/jpa.  I'm not
>>>>> > sure how you would handle the liquibase db migration.
>>>>> >
>>>>> > On 11/4/2015 6:03 AM, Erik Mulder wrote:
>>>>> >> Hi everybody,
>>>>> >>
>>>>> >> Quick intro: I’m part of a development team in The Netherlands that
>>>>> is
>>>>> >> building a company-wide SSO solution. We’ve chosen KeyCloak to
>>>>> realize
>>>>> >> this and will use OpenID Connect to secure our REST services. It’s a
>>>>> >> great product and seems to be the only one having both support for
>>>>> all
>>>>> >> kinds of security standards and a model and GUI for users and roles.
>>>>> >> Thanks for creating it! J
>>>>> >>
>>>>> >> (if this should be asked instead on the users mailing list, please
>>>>> >> correct me and I’ll post it there)
>>>>> >>
>>>>> >> So far, so good, but we have some extra requirements that do not fit
>>>>> >> into the base KeyCloak data model. See below for details if you’re
>>>>> >> interested. My question is: what is the preferred way / best
>>>>> practice to
>>>>> >> extend the functionality of KeyCloak while keeping the impact on the
>>>>> >> original sources to a minimum? Of course we could just fork the most
>>>>> >> recent version and start hacking away, but we’d like to be able to
>>>>> >> upgrade to newer versions of KeyCloak without too much hassle.
>>>>> >> Possibilities that we’ve come up with so far:
>>>>> >>
>>>>> >> 1.Create completely separate modules that will extend the
>>>>> functionality
>>>>> >> the way we need.
>>>>> >>
>>>>> >> 2.Fork on Github, apply custom changes, and try to merge in updates
>>>>> from
>>>>> >> the master / release branches / tags
>>>>> >>
>>>>> >> 3.Apply custom changes on KeyCloak artifacts using a Maven plugin,
>>>>> such
>>>>> >> as Truezip
>>>>> >> ( <http://www.mojohaus.org/truezip/truezip-maven-plugin/index.html>
>>>>> http://www.mojohaus.org/truezip/truezip-maven-plugin/index.html) -
>>>>> >> manipulate zip files by adding/removing/replacing or Shade
>>>>> >> ( <http://maven.apache.org/plugins/maven-shade-plugin/>
>>>>> http://maven.apache.org/plugins/maven-shade-plugin/) - combine
>>>>> multiple
>>>>> >> jars to 1 'uber-jar' containing the contents of both and when
>>>>> >> overlapping decide on conflicts through configuration.
>>>>> >>
>>>>> >> Of course number 1 is preferred, but I do not see how to add custom
>>>>> >> mappers or JPA entities without making changes in the original
>>>>> module
>>>>> >> files. The other options seem like valid alternatives, but maybe
>>>>> there
>>>>> >> is better / standard way to do this. So any help / insight / shared
>>>>> >> experience on this is much appreciated, thanks!
>>>>> >>
>>>>> >> Kind regards,
>>>>> >>
>>>>> >> Erik Mulder
>>>>> >>
>>>>> >> Senior Software Engineer
>>>>> >>
>>>>> >> Docdata Payments – NL
>>>>> >>
>>>>> >> P.S. Details on why we want to extend the KeyCloak data model: (any
>>>>> >> feedback on the contents of this P.S. is also welcome!)
>>>>> >>
>>>>> >> Our clients are merchants that have several webshops. We manage
>>>>> their
>>>>> >> online payments (shopping cart checkout). We want to be able to let
>>>>> a
>>>>> >> merchant manage their own users and let a user have different roles
>>>>> for
>>>>> >> different webshops within the same merchant. The overall possible
>>>>> roles
>>>>> >> are fixed though, no specific roles per merchant. We could create a
>>>>> >> separate realm for every merchant, but then we need to duplicate all
>>>>> >> roles every time. Furthermore, in KeyCloak there is no concept of a
>>>>> role
>>>>> >> within a certain context. This is very understandable, since every
>>>>> >> situation has it’s own requirements. We did a proof of concept by
>>>>> adding
>>>>> >> tables and entities for Merchant, UserMerchant, UserMerchantRole
>>>>> etc.
>>>>> >> and adding a custom mapper that can put this information on the
>>>>> Access
>>>>> >> token. Worked like a charm! But it does need some changes in the
>>>>> >> KeyCloak modules and sources to work, hence the question above.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> _______________________________________________
>>>>> >> keycloak-dev mailing list
>>>>> >> <keycloak-dev at lists.jboss.org> <keycloak-dev at lists.jboss.org>
>>>>> keycloak-dev at lists.jboss.org
>>>>> >> <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>>>> <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>> >>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> keycloak-dev mailing list
>>>>> <keycloak-dev at lists.jboss.org>keycloak-dev at lists.jboss.org
>>>>> <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
>
> _______________________________________________
> keycloak-dev mailing listkeycloak-dev at lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20151116/953a3091/attachment-0001.html