Hi, I was asked by Stian to post my proposal around https://issues.jboss.org/browse/KEYCLOAK-2671 to be discussed here with wider KC dev team. What we need is to pass some additional attributes into Login and Account freemarker templates as part of our extensions - eg. to configure client side validations for registration form based on actual authentication session. Other use case we need is selection of Theme based on calling client. There are already Login and Account Form providers which may be customized (they are SPI), only problem is that current Freemarker providers use private fields and methods, so it is hard to customize them (I have to copy complete code which is hardly maintainable during keycloak upgrades). I believe we should resolve the problem by small refactoring of existing FreeMarkerLoginProvider and FreeMarkerAccountProvider providers similar you already done in FreeMarkerEmailTemplateProvider. So things like: * change fields and methods from private to protected to allow use/override in subclasses * refactor some features to protected methods (eg. Template loading from template provider - again, you did it in FreeMarkerEmailTemplateProvider provider already) to allow override in subclasses * add one protected callback method called just before template and attributes are passed to the freemarker engine for the processing - this allow subclass simply add additional attributes to be passed into template Only bigger change (and blocker for one of our important features) is passing of current AuthenticationSessionModel to the LoginFormsProvider instance at all places where the form provider is called. This is really missing now to be able customize GUI based on current client and authentication flow needs. I don't think those are big changes, but they will make life of extension developers much easier. I believe I'm able to provide pull request for this change if no better solution will be found there by experienced KC dev team. Thanks a lot in advance for any comments to my proposal. Vlastimil -- Vlastimil Elias Principal Software Engineer, Middleware Engineering Services Red Hat _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

LoginFormsProvider has a setAttribute() method. Isn't that enough?

We have to let authenticator plugins drive the forms as down the road we might be required to build a SaaS which means our themes need to be secure enough to run in a multi-tenant environment. On Fri, Sep 29, 2017 at 7:30 AM, Stian Thorgersen <sthorger(a)redhat.com&gt; wrote: > Making the required methods protected is a simple way to work around the > issue, but bear in mind that the FreeMarker providers are likely to change > in the future. > > Another option could be to introduce an FreeMarkerAttributeProviderSPI, the > current freemarker providers would call all available providers to allow > them to add additional attributes. Something like: > > * registerAccountAttribute(FormContext..) > * registerLoginAttribute(FormContext..) > > Where FormContext would have access to KeycloakSession, > AuthenticationSession, etc.. > > Slightly more work, but would be more future proof IMO. > > > On 27 September 2017 at 15:20, Vlastimil Elias <velias(a)redhat.com&gt; wrote: > >> Hi, >> >> I was asked by Stian to post my proposal around >> https://issues.jboss.org/browse/KEYCLOAK-2671 to be discussed here with >> wider KC dev team. >> >> What we need is to pass some additional attributes into Login and >> Account freemarker templates as part of our extensions - eg. to >> configure client side validations for registration form based on actual >> authentication session. Other use case we need is selection of Theme >> based on calling client. >> >> There are already Login and Account Form providers which may be >> customized (they are SPI), only problem is that current Freemarker >> providers use private fields and methods, so it is hard to customize >> them (I have to copy complete code which is hardly maintainable during >> keycloak upgrades). >> >> I believe we should resolve the problem by small refactoring of existing >> FreeMarkerLoginProvider and FreeMarkerAccountProvider providers similar >> you already done in FreeMarkerEmailTemplateProvider. So things like: >> >> * change fields and methods from private to protected to allow >> use/override in subclasses >> * refactor some features to protected methods (eg. Template loading >> from template provider - again, you did it in >> FreeMarkerEmailTemplateProvider provider already) to allow override >> in subclasses >> * add one protected callback method called just before template and >> attributes are passed to the freemarker engine for the processing - >> this allow subclass simply add additional attributes to be passed >> into template >> >> Only bigger change (and blocker for one of our important features) is >> passing of current AuthenticationSessionModel to the LoginFormsProvider >> instance at all places where the form provider is called. This is really >> missing now to be able customize GUI based on current client and >> authentication flow needs. >> >> I don't think those are big changes, but they will make life of >> extension developers much easier. >> >> I believe I'm able to provide pull request for this change if no better >> solution will be found there by experienced KC dev team. >> >> Thanks a lot in advance for any comments to my proposal. >> >> Vlastimil >> >> -- >> Vlastimil Elias >> Principal Software Engineer, Middleware Engineering Services >> Red Hat >> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev

Making the required methods protected is a simple way to work around the issue, but bear in mind that the FreeMarker providers are likely to change in the future.

Another option could be to introduce an FreeMarkerAttributeProviderSPI, the current freemarker providers would call all available providers to allow them to add additional attributes. Something like: * registerAccountAttribute(FormContext..) * registerLoginAttribute(FormContext..) Where FormContext would have access to KeycloakSession, AuthenticationSession, etc.. Slightly more work, but would be more future proof IMO.

On 27 September 2017 at 15:20, Vlastimil Elias <velias(a)redhat.com <mailto:velias@redhat.com>> wrote: Hi, I was asked by Stian to post my proposal around https://issues.jboss.org/browse/KEYCLOAK-2671 <https://issues.jboss.org/browse/KEYCLOAK-2671> to be discussed here with wider KC dev team. What we need is to pass some additional attributes into Login and Account freemarker templates as part of our extensions  - eg. to configure client side validations for registration form based on actual authentication session. Other use case we need is selection of Theme based on calling client. There are already Login and Account Form providers which may be customized (they are SPI), only problem is that current Freemarker providers use private fields and methods, so it is hard to customize them (I have to copy complete code which is hardly maintainable during keycloak upgrades). I believe we should resolve the problem by small refactoring of existing FreeMarkerLoginProvider and FreeMarkerAccountProvider providers similar you already done in FreeMarkerEmailTemplateProvider. So things like:   * change fields and methods from private to protected to allow     use/override in subclasses   * refactor some features to protected methods (eg. Template loading     from template provider - again, you did it in     FreeMarkerEmailTemplateProvider provider already) to allow override     in subclasses   * add one protected callback method called just before template and     attributes are passed to the freemarker engine for the processing -     this allow subclass simply add additional attributes to be passed     into template Only bigger change (and blocker for one of our important features) is passing of current AuthenticationSessionModel to the LoginFormsProvider instance at all places where the form provider is called. This is really missing now to be able customize GUI based on current client and authentication flow needs. I don't think those are big changes, but they will make life of extension developers much easier. I believe I'm able to provide pull request for this change if no better solution will be found there by experienced KC dev team. Thanks a lot in advance for any comments to my proposal. Vlastimil -- Vlastimil Elias Principal Software Engineer, Middleware Engineering Services Red Hat _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org> https://lists.jboss.org/mailman/listinfo/keycloak-dev <https://lists.jboss.org/mailman/listinfo/keycloak-dev>

I'm happy with a simple PR then to just change the methods you need to protected.

We won't be able to look at any SPIs for 7.2 anyways unless you decided to contribute it ;)

For the template selections. Are you picking different templates per-client or different themes?

On 2 October 2017 at 14:12, Vlastimil Elias <velias(a)redhat.com <mailto:velias@redhat.com>> wrote: Hi, On 29.9.2017 13:30, Stian Thorgersen wrote: > Making the required methods protected is a simple way to work > around the issue, but bear in mind that the FreeMarker providers > are likely to change in the future. I'm aware of a potential change of the FreeMarker providers in the future. For now this is preferred way for me as we also need to change mechanism of Template selection (based on client), so we have to extend these providers anyway. > > Another option could be to introduce an > FreeMarkerAttributeProviderSPI, the current freemarker providers > would call all available providers to allow them to add > additional attributes. Something like: > > * registerAccountAttribute(FormContext..) > * registerLoginAttribute(FormContext..) > > Where FormContext would have access to KeycloakSession, > AuthenticationSession, etc.. > > Slightly more work, but would be more future proof IMO. If you have a room to implement this SPI and make it stable (and supported in RH-SSO ;-), then I'm not against it, it may be very useful for most of custom extensions. It is likely that we will get rid of template selection mechanism in the future, so we should switch to this SPI if it will be available. Still some refactoring of FreeMarker providers may be performed as part of this change to make them easily usable for more complicated custom extensions ;-) Thanks a lot Vlastimil > > On 27 September 2017 at 15:20, Vlastimil Elias <velias(a)redhat.com > <mailto:velias@redhat.com>> wrote: > > Hi, > > I was asked by Stian to post my proposal around > https://issues.jboss.org/browse/KEYCLOAK-2671 > <https://issues.jboss.org/browse/KEYCLOAK-2671> to be > discussed here with > wider KC dev team. > > What we need is to pass some additional attributes into Login and > Account freemarker templates as part of our extensions  - eg. to > configure client side validations for registration form based > on actual > authentication session. Other use case we need is selection > of Theme > based on calling client. > > There are already Login and Account Form providers which may be > customized (they are SPI), only problem is that current > Freemarker > providers use private fields and methods, so it is hard to > customize > them (I have to copy complete code which is hardly > maintainable during > keycloak upgrades). > > I believe we should resolve the problem by small refactoring > of existing > FreeMarkerLoginProvider and FreeMarkerAccountProvider > providers similar > you already done in FreeMarkerEmailTemplateProvider. So > things like: > >   * change fields and methods from private to protected to allow >     use/override in subclasses >   * refactor some features to protected methods (eg. Template > loading >     from template provider - again, you did it in >     FreeMarkerEmailTemplateProvider provider already) to > allow override >     in subclasses >   * add one protected callback method called just before > template and >     attributes are passed to the freemarker engine for the > processing - >     this allow subclass simply add additional attributes to > be passed >     into template > > Only bigger change (and blocker for one of our important > features) is > passing of current AuthenticationSessionModel to the > LoginFormsProvider > instance at all places where the form provider is called. > This is really > missing now to be able customize GUI based on current client and > authentication flow needs. > > I don't think those are big changes, but they will make life of > extension developers much easier. > > I believe I'm able to provide pull request for this change if > no better > solution will be found there by experienced KC dev team. > > Thanks a lot in advance for any comments to my proposal. > > Vlastimil > > -- > Vlastimil Elias > Principal Software Engineer, Middleware Engineering Services > Red Hat > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > <mailto:keycloak-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/keycloak-dev > <https://lists.jboss.org/mailman/listinfo/keycloak-dev> > > -- Vlastimil Elias Principal Software Engineer, Middleware Engineering Services Red Hat

On 2 October 2017 at 14:41, Vlastimil Elias <velias(a)redhat.com <mailto:velias@redhat.com>> wrote: Hi, On 2.10.2017 14:32, Stian Thorgersen wrote: > I'm happy with a simple PR then to just change the methods you > need to protected. great, I'll try to prepare PR against master branch and send it  (hopefully this week). > > We won't be able to look at any SPIs for 7.2 anyways unless you > decided to contribute it ;) :-D > For the template selections. Are you picking different templates > per-client or different themes? We select complete Theme per client in the code, using realm's default if nothing special is defined for the client. I reckon we can fairly easily add a ThemeSelectorSPI for that purpose. Vl. > > On 2 October 2017 at 14:12, Vlastimil Elias <velias(a)redhat.com > <mailto:velias@redhat.com>> wrote: > > Hi, > > > On 29.9.2017 13:30, Stian Thorgersen wrote: >> Making the required methods protected is a simple way to >> work around the issue, but bear in mind that the FreeMarker >> providers are likely to change in the future. > > I'm aware of a potential change of the FreeMarker providers > in the future. For now this is preferred way for me as we > also need to change mechanism of Template selection (based on > client), so we have to extend these providers anyway. > >> >> Another option could be to introduce an >> FreeMarkerAttributeProviderSPI, the current freemarker >> providers would call all available providers to allow them >> to add additional attributes. Something like: >> >> * registerAccountAttribute(FormContext..) >> * registerLoginAttribute(FormContext..) >> >> Where FormContext would have access to KeycloakSession, >> AuthenticationSession, etc.. >> >> Slightly more work, but would be more future proof IMO. > > If you have a room to implement this SPI and make it stable > (and supported in RH-SSO ;-), then I'm not against it, it may > be very useful for most of custom extensions. > It is likely that we will get rid of template selection > mechanism in the future, so we should switch to this SPI if > it will be available. > Still some refactoring of FreeMarker providers may be > performed as part of this change to make them easily usable > for more complicated custom extensions ;-) > > > Thanks a lot > Vlastimil > > >> >> On 27 September 2017 at 15:20, Vlastimil Elias >> <velias(a)redhat.com <mailto:velias@redhat.com>> wrote: >> >> Hi, >> >> I was asked by Stian to post my proposal around >> https://issues.jboss.org/browse/KEYCLOAK-2671 >> <https://issues.jboss.org/browse/KEYCLOAK-2671> to be >> discussed here with >> wider KC dev team. >> >> What we need is to pass some additional attributes into >> Login and >> Account freemarker templates as part of our extensions  >> - eg. to >> configure client side validations for registration form >> based on actual >> authentication session. Other use case we need is >> selection of Theme >> based on calling client. >> >> There are already Login and Account Form providers which >> may be >> customized (they are SPI), only problem is that current >> Freemarker >> providers use private fields and methods, so it is hard >> to customize >> them (I have to copy complete code which is hardly >> maintainable during >> keycloak upgrades). >> >> I believe we should resolve the problem by small >> refactoring of existing >> FreeMarkerLoginProvider and FreeMarkerAccountProvider >> providers similar >> you already done in FreeMarkerEmailTemplateProvider. So >> things like: >> >>   * change fields and methods from private to protected >> to allow >>     use/override in subclasses >>   * refactor some features to protected methods (eg. >> Template loading >>     from template provider - again, you did it in >> FreeMarkerEmailTemplateProvider provider already) to >> allow override >>     in subclasses >>   * add one protected callback method called just before >> template and >>     attributes are passed to the freemarker engine for >> the processing - >>     this allow subclass simply add additional attributes >> to be passed >>     into template >> >> Only bigger change (and blocker for one of our important >> features) is >> passing of current AuthenticationSessionModel to the >> LoginFormsProvider >> instance at all places where the form provider is >> called. This is really >> missing now to be able customize GUI based on current >> client and >> authentication flow needs. >> >> I don't think those are big changes, but they will make >> life of >> extension developers much easier. >> >> I believe I'm able to provide pull request for this >> change if no better >> solution will be found there by experienced KC dev team. >> >> Thanks a lot in advance for any comments to my proposal. >> >> Vlastimil >> >> -- >> Vlastimil Elias >> Principal Software Engineer, Middleware Engineering Services >> Red Hat >> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> <mailto:keycloak-dev@lists.jboss.org> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> <https://lists.jboss.org/mailman/listinfo/keycloak-dev> >> >> > > -- > Vlastimil Elias > Principal Software Engineer, Middleware Engineering Services > Red Hat > > -- Vlastimil Elias Principal Software Engineer, Middleware Engineering Services Red Hat