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

Bugfix/js runtime errors #359

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

leonpahole
Copy link

Hello,

First of all thanks for the great package!

When I ran this project in a production environment, it failed for a few pages, such as registration and update password - white screen was shown and there was a JavaScript error in the console. I located the error to be in the printIfExists function. In my case, the function had if statements with empty bodies, so no matter what was passed in, the error was thrown:

"printIfExists": function (fieldName, x) {
    if(fieldName === "global" ){
    }
    if(fieldName === "userLabel" ){
    }
    if(fieldName === "username" ){
    }

    /* ... and so forth ... */
                 
    throw new Error("There is no " + fieldName + " field");
},

I noticed that inside the ftl file, there was an attempt / recover block, and in my case (I am using Keycloak 11.0.2), the recover block was being triggered, but that recover block did not generate any code, which led to empty if statements.

I fixed this issue by adding a return statement in the recover block. I am not sure if this is the right way to do it, could you advise?

@garronej
Copy link
Collaborator

garronej commented Jun 14, 2023

Hi @leonpahole,

I greatly appreciate your positive feedback and the effort you've put into exploring the situation.

It appears that the issue at hand might be due to the utilization of custom fields, which, by default, aren't listed in the system. To rectify this, you need to declare these fields in your package.json. For further guidance on this, please refer to our documentation: https://docs.keycloakify.dev/build-options#keycloakify.customuserattributes

We're excited to share that in upcoming versions of Keycloakify, we will incorporate automatic code analysis. This feature will eliminate the need for manual specification of such parameters, further enhancing user experience.

Stay tuned for these updates and thank you again for your constructive engagement with our platform.

Best regards,

@leonpahole
Copy link
Author

@garronej Thank you for your response.

I am not using any custom fields in the Keycloak - it is a standard, clean (no configuration changed) Keycloak 11.0.2 instance. I can prepare a repro repository if needed.

@garronej
Copy link
Collaborator

Oh my bad for assuming this was what's happening.

Would you share the exact message of the JavaScript runtime error you're getting?

@leonpahole
Copy link
Author

Here is the error:

image

image

@garronej
Copy link
Collaborator

garronej commented Jun 14, 2023

Oh! I see! My bad, you are right. What you did in the PR seems like the sensible thing to do. But before merging I neet to investigate a little bit. There must be a lot of error in your Keycoak log if the FTL gets rendered this way. Do you occur?

@garronej
Copy link
Collaborator

garronej commented Jun 15, 2023

Ok I understand what's going on. You are using an old version of Keycloak that where the messagesPerField.existsError didn't existed yet.
I'll try to do without it so that the Keycloakify is retrocompatible with your version.

https://github.com/keycloak/keycloak/blob/11.0.2/services/src/main/java/org/keycloak/theme/beans/MessagesPerFieldBean.java

https://github.com/keycloak/keycloak/blob/808988fe20103a7a625f141ec67698345b4b8277/services/src/main/java/org/keycloak/theme/beans/MessagesPerFieldBean.java#L61

return <#if messagesPerField.existsError('username', 'password')>x<#else>undefined</#if>;
<#else>
return <#if messagesPerField.existsError('${fieldName}')>x<#else>undefined</#if>;

garronej added a commit that referenced this pull request Jun 15, 2023
@garronej
Copy link
Collaborator

The issue should be solved in Keycloakify 7.12.3 now being deployed.
Thank you for your contribution!

@garronej garronej closed this Jun 15, 2023
@leonpahole
Copy link
Author

@garronej thanks for the quick feedback and the solution 🙌

@garronej
Copy link
Collaborator

Hold on, after futher examination, I see other use of existsError that I need to remove.

garronej added a commit that referenced this pull request Jun 15, 2023
@garronej
Copy link
Collaborator

@leonpahole I'm sorry, I had to rollback. I'll release a patched version tomorrow. It's much more complicated that I enticipated.
If you can't wait you can still update to Keycloak 12. It will fix it.

garronej added a commit that referenced this pull request Jun 16, 2023
@garronej
Copy link
Collaborator

Probably fixed in 7.12.6-rc.0. I didn't test yet though

garronej added a commit that referenced this pull request Jun 16, 2023
garronej added a commit that referenced this pull request Jun 16, 2023
@garronej
Copy link
Collaborator

Should be working now

@leonpahole
Copy link
Author

leonpahole commented Jun 18, 2023

Thanks! One thing I've noticed though: the username and password fields are marked with an error immediately upon page load.

image

@garronej
Copy link
Collaborator

@leonpahole, ohhhhh 😞
I spend a considerable amount of hours trying to ensure the compatibility with Keycloak 11 but seems like I messed up.
Could you please try with 7.13.2-rc.0 and show me what you're getting HTML wise?

Just like you did when you shared this screenshot:

image

@leonpahole
Copy link
Author

Here is what I am getting:

  "printIfExists": function (fieldName, text) {


                if(fieldName === "totp" ){





                            return undefined;



                }
                if(fieldName === "userLabel" ){





                            return undefined;



                }
                if(fieldName === "password" ){





                                return text;



                }
                if(fieldName === "password-confirm" ){





                            return undefined;



                }
                if(fieldName === "username" ){





                                return text;



                }
                if(fieldName === "email" ){





                            return undefined;



                }
                if(fieldName === "firstName" ){





                            return undefined;



                }
                if(fieldName === "lastName" ){





                            return undefined;



                }
                if(fieldName === "global" ){





                            return undefined;



                }

            throw new Error(fieldName + "is probably runtime generated, see: https://docs.keycloakify.dev/limitations#field-names-cant-be-runtime-generated");
        },

@garronej
Copy link
Collaborator

Thanks for sharing the log.

So you are positive that this is what you get when you are using Keycloakify 7.13.2-rc.0?

Unfortunately, I can't easyly spin up a Keycloak 11 container on my M1 mac.

Could you:

  • Share with me your src/login/pages/Register.tsx page?
  • Try with the default Keycloak theme, see if you have the same issue.

Thanks a lot for you help for getting to the bottom of this.

@leonpahole
Copy link
Author

Hello @garronej , sorry for slow response, It's been a busy week :)

Yes, I am sure I am using Keycloakify 7.13.2-rc.0.

  • I don't have Register.tsx page - it was not ejected and is thus the built-in one
  • I tried default Keycloak theme, and there is no issue there:

image

garronej added a commit that referenced this pull request Apr 13, 2024
garronej added a commit that referenced this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants