v1.1.0 panics when Hostname is used rather than Host in the oauth flow.

[williammartin]

Description

#63 introduced a new safe parsing function NewGitHubHost which ensures that the URL parsing is valid. Consumers of this library can use it in order to pass in Host in the OauthFlow configuration struct:

oauth/oauth.go

Line 68 in afffc8e

Host *Host

However, it was possible to provide the deprecated Hostname string and both the device code and oauth flows would do the parsing internally. .

In #63, this parsing was changed to use the new function (which is great) but in doing so, some unintentional variable shadowing happened. Previously, the result of GitHubHost was assigned to the host variable which was used later on. Now the result of NewGitHubHost is assigned to host, err via short variable declaration (:=). This results in a new host variable being declared in the inner scope. Although we set oa.Host = host, the rest of the functions still use host, which results in a nil pointer deference.

Reproduction

Use these tests:

package oauth_test

import (
	"testing"

	"github.com/cli/oauth"
)

func TestDeviceFlowHostname(_ *testing.T) {
	flow := &oauth.Flow{
		Hostname: "github.com",
	}

	_, err := flow.DeviceFlow()
	if err != nil {
		panic(err)
	}
}

func TestWebFlowHostname(_ *testing.T) {
	flow := &oauth.Flow{
		Hostname: "github.com",
	}

	_, err := flow.WebAppFlow()
	if err != nil {
		panic(err)
	}
}

They will panic on:

github.com/cli/oauth.(*Flow).DeviceFlow(0x1400007aeb0)
	/Users/williammartin/workspace/oauth/oauth_device.go:42 +0x188

oauth/oauth_device.go

Line 42 in afffc8e

code, err := device.RequestCode(httpClient, host.DeviceCodeURL, oa.ClientID, oa.Scopes)

github.com/cli/oauth.(*Flow).WebAppFlow(0x140001462c0)
	/Users/williammartin/workspace/oauth/oauth_webapp.go:37 +0x16c

oauth/oauth_webapp.go

Lines 36 to 37 in afffc8e

	}
	browserURL, err := flow.BrowserURL(host.AuthorizeURL, params)

Note that these tests aren't otherwise intended to pass (e.g. Device flow will panic for other reasons on an earlier commit), just to demonstrate the issue.