-
Notifications
You must be signed in to change notification settings - Fork 22
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
Callback not firing #1
Comments
Hi @garmeeh, I just read through the issue that you posted in next-auth. I'm not familiar with that project directly, however, I vaguely remember dealing with the We use the signature Beyond that, I'm not exactly certain what the cleanest solution is, given that this is largely handled by the oauth2 boilerplate. |
No worries @svarlamov, thanks for the looking into it for me. The auth server is great for reference 👍 |
Sure, let me know if there's anything that I can do in the mainline keycloak-passport to support next-auth. Would be happy to do that as I'm sure it would benefit other users |
Hi @svarlamov sorry about the delay in replying. I'm working with @woppers to try resolve #2. In regards to my original issue, the only chagne we had to make to this.name = 'Keycloak'; to this.name = 'keycloak'; I'm pretty sure this is something we could open a PR for on |
I see... I'm concerned that mainlining a name change might break other implementations. Is there a particular reason why it needs to be lowercase for next auth? |
I agree, making that change would probably break other implementations. I am not sure if there was a particular reason for the lowercase, but I would hope that I could open a PR for next-auth and add an option to disable the lowercase. |
What is that name actually used for in next-auth? |
@svarlamov seems they pick a strategy by name and they get it from url not sure why they need |
@aol-nnov Thanks for clarifying the use case in next-auth. I'm not sure what specifically I can add here, but if we get clarity on the requirements for compatibility with next-auth I would be happy to mainline the changes so long as they don't break other impls |
Anyway, I've opened a PR on that, because no rules enforcing strategy name to be lower case were found. |
Hi, I am currently trying to use this Strategy with next-auth. I logged an issue on next-auth because the callback didn't seem to be firing for keycloak-passport. Wondering would you maybe have any insight on why it wouldn't be firing?
The text was updated successfully, but these errors were encountered: