sst / ion

❍ — a new engine for SST
https://ion.sst.dev
MIT License
1.07k stars 118 forks source link

Auth still broken on OIDC & OAuth Adapters #475

Closed nicholasgubbins closed 1 month ago

nicholasgubbins commented 1 month ago

Looking at the PR that was recently merged in #191 - the function for handling the lifecycle from /:provider/authorize -> /:provider/callback was changed from: const callback = c.req.url.replace(/authorize$/, "callback"); to const callback = c.req.url.replace(/authorize\/.*$/, "callback");

However, given most of the documentation and the standard initial path for the /:provider/authorize will lack a trailing forwardslash, and should be able to forward on query params, this regex is surely incorrect?/authorize\/.*$/ means match the string "authorize/" including trailing backspace + extraneous further pattern. The replace function will also remove any query params provided

I propose we replace this with const callback = c.req.url.replace.replace(/\/authorize\b/, '/callback'); which will simply replace the substring "/authorize" with "/callback" and leave query params in place - keen to hear thoughts from @antonmoller and @TheKnightCoder who have been working on this recently?

nicholasgubbins commented 1 month ago

Also references #276

nicholasgubbins commented 1 month ago

closing for me being stupid and not realising the sst upgrade wasn't actually upgrading my local setup, and instead i needed to run brew upgrade sst

would be nice if the cli could recognise the means of installation, because this is a lil deceiving image