danielgtaylor / restish

Restish is a CLI for interacting with REST-ish HTTP APIs with some nice features built-in
https://rest.sh/
MIT License
717 stars 69 forks source link

Is opening OAuth Authorization Url in Browser broken on windows due missing url escaping? #248

Closed DominicBortmes closed 1 month ago

DominicBortmes commented 3 months ago

Hey @danielgtaylor and other maintainers,

Description

I noticed that the oauth code flow seems to break when opening the authorization URL with the default browser on windows (10). I observe that the browser opens a URL with wrong http query params since my IDP returns such an error. We do use the exact same config restish config (target api+oidc settings) for mac and linux which works flawlessly.

Reproduction scenario

Prerequisite

1) You need access to a functional setup of an oauth secured (code flow) API. 2) You need a windows machine (I used windows 10) 3) Use resting v0.20 (latest as of issue date)

Steps

  1. You add the given api to restish config per tui wizard or add it to the restish api json - and call the api "petstore".
  2. execute restish petstore --help to start the authentication before restish would then opt to download the openapi spec.

Observed behavior

You should see an error from your OAuth IDP now, since the URL query params (client_id, and other mandatory defaults of oidc) necessary for the authorization request were omitted. (Remark: Manually copy/pasting the authorize URL printed on the stdout by restish does work)

Hypothesis

While the authorizeURL seems to be a properly URL encoded string, it seems windows' cmd open https://mydomain.auth.us-east-1.amazoncognito.com/oauth2/authorize?response_type=code&client_id=1example23456789&redirect_uri=https://www.example.com&state=abcdefg&scope=openid+profile needs extra string escaping to not drop http query parameters when opening a URL with the default browser .

Expected

Any authentication flow (initial or after token expiry) on Windows should open the browser without dropping http url query params. Once the URL is passed to the open function, as shown below, the open function handler should escape the url as expected by each platform (specifically windows seems to need special care) so nothing of the stringified URL gets modified or cut off mistakenly.

https://github.com/danielgtaylor/restish/blob/d16bdd717c3a73efbda8ba48a3af6bf8f5995b67/oauth/authcode.go#L271

https://github.com/danielgtaylor/restish/blob/d16bdd717c3a73efbda8ba48a3af6bf8f5995b67/oauth/authcode.go#L143-L144

Solution

I'd be great if the community could focus on confirming the hypothesis / bug first. Subsequently, both my Windows and Golang skills are virtually in-existent, but I might be able to contribute even to I think for someone experience it'd be a fairly quick and easy thing to fix. I'd be happy to test a experimental build of the community if who-ever may provide a fix had no access to a windows box.

Generally I appreciate restish a lot. Thanks for this great tool!

DominicBortmes commented 1 month ago

@danielgtaylor : thanks for your review and merge into main. Just wondering if there's a rough target date after which this fix is shipped as part of a downloadable release of compiled binaries? BR, Dominic