Callback not firing

[garmeeh]

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?

[svarlamov]

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 req argument issue when I was building keycloak-passport... You can get a sample of how we implement the strategy over here

We use the signature req, accessToken, refreshToken, profile, done which requires this line in the config. Although as discussed in the next-auth issue, it seems as though you're also setting that...

Beyond that, I'm not exactly certain what the cleanest solution is, given that this is largely handled by the oauth2 boilerplate.

[garmeeh]

No worries @svarlamov, thanks for the looking into it for me. The auth server is great for reference 👍

[svarlamov]

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

[svarlamov]

@garmeeh Were you able to find the solution to this issue? Also, any luck re: #2? Thanks!

[garmeeh]

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 keycloak-passport was to change this line

this.name = 'Keycloak';

to

this.name = 'keycloak';

I'm pretty sure this is something we could open a PR for on next-auth to add an option to not .toLowerCase() the provider name option. If we get the flow working I will add comment back here for anyone finding the issue later. I would be happy to consider this issue closed if that suits you.

[svarlamov]

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?

[garmeeh]

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.

[svarlamov]

What is that name actually used for in next-auth?

[aol-nnov]

@svarlamov seems they pick a strategy by name and they get it from url /auth/oauth/<strategyName>

not sure why they need .toLowerCase() though...

[svarlamov]

@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

[aol-nnov]

So, I browsed a handful of strategies, all of them have their name in lower case. Seems like a policy to me..

https://github.com/jaredhanson/passport-local/blob/master/lib/strategy.js#L53
https://github.com/jaredhanson/passport-openid/blob/master/lib/passport-openid/strategy.js#L78
https://github.com/jaredhanson/passport-browserid/blob/master/lib/strategy.js#L52
https://github.com/jaredhanson/passport-facebook/blob/master/lib/strategy.js#L55
https://github.com/jaredhanson/passport-google/blob/master/lib/passport-google/strategy.js#L47
https://github.com/jaredhanson/passport-twitter/blob/master/lib/strategy.js#L54
https://github.com/AzureAD/passport-azure-ad/blob/dev/lib/oidcstrategy.js#L334
https://github.com/jaredhanson/passport-http-bearer/blob/master/lib/strategy.js#L65
https://github.com/auth0/passport-auth0/blob/master/lib/index.js#L55
https://github.com/yanatan16/passport-youtube-v3/blob/master/lib/passport-youtube-v3/strategy.js#L25
https://github.com/piggyman007/passport-line-v2/blob/master/index.js#L7
https://github.com/iszak/passport-voice-it/blob/master/lib/strategy.js#L72

For the sake of brevity I won't paste all 480 policies here, lol!

[aol-nnov]

Anyway, I've opened a PR on that, because no rules enforcing strategy name to be lower case were found.

nextauthjs/next-auth#51