Repository navigation
Malformed callback redirect when it already contains query string #317
Description
First of all, thanks for the library.
When I have a Provider callback that contains already a query string, the final redirect location seems to be malformed
Callback
http://localhost:3000/connect/keycloak/redirect?referrer=%2Fen%2Fsearch
Redirect Location
http://localhost:3000/connect/keycloak/redirect?referrer=%2Fen%2Fsearch?id_token=abcdefg
As you can see the query symbol (?) is already included in the Callback.
When the Redirect Location is created, the query symbol gets added again (see before id_token).
This issue creates problems when you try to parse the final URL
let url = new URL(`http://localhost:3000/connect/keycloak/redirect?referrer=%2Fen%2Fsearch?id_token=abcdefg`)
console.log(url.searchParams.get('id_token')) // null
console.log(url.searchParams.get('referrer')) // "/en/search?id_token=abcdefg"This comes from this line
Line 108 in 500ab20
Meanwhile, this gets fixed, I'm using this patch workaround
patches/grant+5.4.24.patch
diff --git a/node_modules/grant/lib/response.js b/node_modules/grant/lib/response.js
index e67a013..430f746 100644
--- a/node_modules/grant/lib/response.js
+++ b/node_modules/grant/lib/response.js
@@ -105,7 +105,8 @@ var transport = ({provider, input, input:{params, state, session}, output}) => (
? output
: (!provider.transport || provider.transport === 'querystring')
- ? `${provider.callback || '/'}?${qs.stringify(output)}`
+ /* https://github.com/simov/grant/pull/329 */
+ ? `${provider.callback || '/'}${(provider.callback || '').includes('?') ? '&' : '?'}${qs.stringify(output)}`
: provider.transport === 'session'
? provider.callbackRedirect Location with patch
http://localhost:3000/connect/keycloak/redirect?referrer=%2Fen%2Fsearch&id_token=abcdef
Activity
simov commentedon Feb 19, 2025
I see, thanks for sharing it. Not sure at what point precisely you add that additional querystring parameter to the callback URL (assuming that it is dynamic), but alternatively you can have a handler before the Grant middleware where you read that value and put it in session instead, you can even put it in the Grant session itself https://github.com/simov/grant/blob/master/examples/dynamic-state/express.js and then inside the final
callbackroute you can read it back from the session.unrevised6419 commentedon Feb 19, 2025
I'm using Strapi which uses
grant/koa, and I don't have full control over it.When the user clicks the login URL on
webthese redirects happenweb: the front face of the websitecms: Strapi instancesso: Keycloack instanceWe have these endpoints
sso/login:ssologin endpointcms/keycloack:cmsconnect keycloack endpoint (does not create any user)cms/keycloack-callback:cmsconnect keycloack endpoint callback (does not create any user)web/callback:webendpoint that is called after login to callcmsback to create the usercms/create:cmsendpoint receives provider access token and creates the userCallback
webredirects tocms/keycloackcms/keycloackgoes tosso/loginsso/loginredirects tocms/keycloack-callbackcms/keycloack-callbackredirects toweb/callbackweb/callbackcallscms/createweb/callbackstores the session, and redirects toreferrerfrom URLI cannot change
web/callbackto anything arbitrary, that needs to be an endpoint that callscms/create. That's why I put the real callback where user needs to land in the end in the query string.I hope this was clear 😂
simov commentedon Feb 23, 2025
Thanks for the thorough explanation, I might reconsider this feature 👍
fix(response): allow callback with query string