lukin / keywind

Keywind is a component-based Keycloak Login Theme built with Tailwind CSS
Apache License 2.0
737 stars 261 forks source link

WebAuthn registration failure #57

Open rhawdor opened 9 months ago

rhawdor commented 9 months ago

Hello !

I recently updated to Keycloak 22 (more precisely 22.0.1, was using version 21 before), and I think I spotted a bug on WebAuthn security key registration.

Caused by: freemarker.core.NonStringOrTemplateOutputException: For "${...}" content: Expected a string or something automatically convertible to string (number, date or boolean), or "template output" , but this has evaluated to a sequence (wrapper: f.t.SimpleSequence):
==> signatureAlgorithms  [in template "webauthn-register.ftl" at line 47, column 31]
----
FTL stack trace ("~" means nesting-related):
 - Failed at: ${signatureAlgorithms}  [in template "webauthn-register.ftl" at line 47, column 29]
----

Could this be related to #20831 or am I missing something ?

paulwer commented 9 months ago

Hey @rhawdor, does this error was also present on kc 21.0.1?

rhawdor commented 9 months ago

Hey @paulwer ! To be extremely sure I reproduced it in a clean environment with fresh install from KC and keywind.

So, to answer, no the error was not present on KC 21.0.1.

paulwer commented 9 months ago

i guess this is due to the following change in keycloak: https://github.com/keycloak/keycloak/commit/1af4a7a5325c248da2275759379ec00e495045df

@lukin should we support the old and the new way for backwards compatibility?

Clemv95 commented 9 months ago

I have this error too and this is annoying :/

paulwer commented 9 months ago

a workaround for now would be to stay on 21 from my pov, if this is an option😕

Clemv95 commented 9 months ago

it's not really an option :/ How much time do you think it's gonna take ?

paulwer commented 9 months ago

i dont know. are you able to debug yourself and PR a fix? maybe we can start from there or move support to only the actual version?

tipp: look at the official integration mentioned beforw, what they changed...maybe this helps

Clemv95 commented 9 months ago

I can try myself, but i dont think it's gonna be easy and fast, i'm not really familiar with typescript and tailwind

paulwer commented 9 months ago

from my pov its an issue from mapping internal data by ftl and output it to the script. ytring changed to array. so the mapping behavior should be edited and not even necessary the typescript script

Clemv95 commented 9 months ago

Where is the mapping behavior ? Maybe i can try

paulwer commented 9 months ago

the line, which is failing according to the error message is: https://github.com/lukin/keywind/blob/master/theme/keywind/login/webauthn-register.ftl#L47C31-L47C50

the variable within keycloak before was: a string separeted by "," and afterwards its a List in java.

Maybe try something like this:

<#if signatureAlgorithms?is_enumerable>${signatureAlgorithms?join(",")}<#else>${signatureAlgorithms}</#if>

(means: if its an array => join it to string, seperated with "," OR if its anything else (probably a string, from versions before) => just display it as string in the html)

and here is the line of official keycloak: https://github.com/keycloak/keycloak/blob/main/themes/src/main/resources/theme/base/login/webauthn-register.ftl#L42

scheibling commented 4 months ago

the line, which is failing according to the error message is: https://github.com/lukin/keywind/blob/master/theme/keywind/login/webauthn-register.ftl#L47C31-L47C50

the variable within keycloak before was: a string separeted by "," and afterwards its a List in java.

Maybe try something like this:

<#if signatureAlgorithms?is_enumerable>${signatureAlgorithms?join(",")}<#else>${signatureAlgorithms}</#if>

(means: if its an array => join it to string, seperated with "," OR if its anything else (probably a string, from versions before) => just display it as string in the html)

and here is the line of official keycloak: https://github.com/keycloak/keycloak/blob/main/themes/src/main/resources/theme/base/login/webauthn-register.ftl#L42

@paulwer Cheers man, this solved it for me. I've opened up a pull request with that line replaced