Repository: danielgtaylor/restish (go)

Issue:
Is opening OAuth Authorization Url in Browser broken on windows due missing url escaping?
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](https://stackoverflow.com/a/1327476) .

### 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!

Pull request:
Escape '&' for Windows command line oAuth flow
Fixes #248.

PR escapes '&' character to enable oAuth flow for Windows operating systems.
Aside of the test @DominicBortmes as able to verify the fix for his context.

Patch:
diff --git a/oauth/authcode.go b/oauth/authcode.go
index c3c2a6f..4886479 100644
--- a/oauth/authcode.go
+++ b/oauth/authcode.go
@@ -135,6 +135,7 @@ func open(url string) error {
 	case "windows":
 		cmd = "cmd"
 		args = []string{"/c", "start"}
+		url = encodeUrlWindows(url)
 	case "darwin": // mac, ios
 		cmd = "open"
 	default: // "linux", "freebsd", "openbsd", "netbsd"
@@ -144,6 +145,25 @@ func open(url string) error {
 	return exec.Command(cmd, args...).Start()
 }
 
+// Windows OS need strings to be encoded
+// for proper command line interface handling.
+func encodeUrlWindows(url string) string {
+	// escape '&'
+	sp := strings.Split(url, "&")
+	var escaped string
+	for i, p := range sp {
+		// Skip adding escape at the beginning
+		if i == 0 {
+			escaped += p
+			continue
+		}
+		escaped += "^&" + p
+	}
+
+	// keep protocol
+	return escaped
+}
+
 // getInput waits for user input and sends it to the input channel with the
 // trailing newline removed.
 func getInput(input chan string) {


Test patch:
diff --git a/oauth/authcode_test.go b/oauth/authcode_test.go
new file mode 100644
index 0000000..ec74e47
--- /dev/null
+++ b/oauth/authcode_test.go
@@ -0,0 +1,20 @@
+package oauth
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEncodeUrlWindowsSuccess(t *testing.T) {
+	u := "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"
+
+	r := encodeUrlWindows(u)
+	//t.Log(r)
+
+	assert.NotEqual(t, u, r)
+	assert.Contains(t, r, "^&")
+	assert.False(t, strings.HasPrefix(r, "^&"))
+	assert.False(t, strings.HasSuffix(r, "^&"))
+}