Pass macaroons from cookiejar for HTTPS requests

The following passes the macaroons for all https requests if you
logout and then login. As we rightly do not store the new password once
you've changed it, we then expect to use the macaroons from the cookie
jar. We do that correctly when passing to login, but we fail to do it
when calling the HTTPS requests.

The following fix just creates a new openFunc middleware that just adds
the macaroons if the password and the length of macaroons is empty. For
most if not all cases this should not be called, yet for uploading
charms in production environments where the password for the user has
been changed it clearly a benefit.

api/apiclient.go

@@ -245,25 +245,19 @@ func Open(info *Info, opts DialOpts) (Connection, error) {
 
 	// Prefer the SNI hostname or controller name for the cookie URL
 	// so that it is stable when used with a HA controller cluster.
-	host := info.SNIHostName
-	if host == "" && info.ControllerUUID != "" {
-		host = info.ControllerUUID
-	}
+	host := PerferredHost(info)
 	if host == "" {
 		host = dialResult.addr
 	}
+
 	st := &state{
-		ctx:    context.Background(),
-		client: client,
-		conn:   dialResult.conn,
-		clock:  opts.Clock,
-		addr:   dialResult.addr,
-		ipAddr: dialResult.ipAddr,
-		cookieURL: &url.URL{
-			Scheme: "https",
-			Host:   host,
-			Path:   "/",
-		},
+		ctx:                 context.Background(),
+		client:              client,
+		conn:                dialResult.conn,
+		clock:               opts.Clock,
+		addr:                dialResult.addr,
+		ipAddr:              dialResult.ipAddr,
+		cookieURL:           CookieURLFromHost(host),
 		pingerFacadeVersion: facadeVersions["Pinger"],
 		serverScheme:        "https",
 		serverRootAddress:   dialResult.addr,
@@ -302,6 +296,24 @@ func Open(info *Info, opts DialOpts) (Connection, error) {
 	return st, nil
 }
 
+// CookieURLFromHost creates a url.URL from a given host.
+func CookieURLFromHost(host string) *url.URL {
+	return &url.URL{
+		Scheme: "https",
+		Host:   host,
+		Path:   "/",
+	}
+}
+
+// PerferredHost returns the perferred host from a Info.
+func PerferredHost(info *Info) string {
+	host := info.SNIHostName
+	if host == "" && info.ControllerUUID != "" {
+		host = info.ControllerUUID
+	}
+	return host
+}
+
 // loginWithContext wraps st.Login with code that terminates
 // if the context is cancelled.
 // TODO(rogpeppe) pass Context into Login (and all API calls) so

cmd/modelcmd/base.go

@@ -589,10 +589,31 @@ func newAPIConnectionParams(
 		AccountDetails: accountDetails,
 		ModelUUID:      modelUUID,
 		DialOpts:       dialOpts,
-		OpenAPI:        apiOpen,
+		OpenAPI:        OpenAPIFuncWithMacaroons(apiOpen, store, controllerName),
 	}, nil
 }
 
+// OpenAPIFuncWithMacaroons is a middleware to ensure that we have a set of
+// macaroons for a given open request.
+func OpenAPIFuncWithMacaroons(apiOpen api.OpenFunc, store jujuclient.ClientStore, controllerName string) api.OpenFunc {
+	return func(info *api.Info, dialOpts api.DialOpts) (api.Connection, error) {
+		// When attempting to connect to the non websocket fronted HTTPS
+		// endpoints, we need to ensure that we have a series of macaroons
+		// correctly set if there isn't a password.
+		if info.Password == "" && len(info.Macaroons) == 0 {
+			cookieJar, err := store.CookieJar(controllerName)
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+
+			cookieURL := api.CookieURLFromHost(api.PerferredHost(info))
+			info.Macaroons = httpbakery.MacaroonsForURL(cookieJar, cookieURL)
+		}
+
+		return apiOpen(info, dialOpts)
+	}
+}
+
 // NewGetBootstrapConfigParamsFunc returns a function that, given a controller name,
 // returns the params needed to bootstrap a fresh copy of that controller in the given client store.
 func NewGetBootstrapConfigParamsFunc(

cmd/modelcmd/base_test.go

@@ -5,16 +5,23 @@ package modelcmd_test
 
 import (
 	"io/ioutil"
+	"net/http"
+	"net/url"
 	"strings"
+	"time"
 
 	"github.com/juju/cmd/v3"
 	"github.com/juju/cmd/v3/cmdtesting"
 	"github.com/juju/errors"
+	cookiejar "github.com/juju/persistent-cookiejar"
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
+	"gopkg.in/macaroon-bakery.v2/httpbakery"
+	"gopkg.in/macaroon.v2"
 
 	"github.com/juju/juju/api"
+	apitesting "github.com/juju/juju/api/testing"
 	"github.com/juju/juju/cloud"
 	"github.com/juju/juju/cmd/modelcmd"
 	"github.com/juju/juju/core/model"
@@ -268,3 +275,133 @@ func (p *mockEnvironProvider) FinalizeCredential(
 func (p *mockEnvironProvider) CredentialSchemas() map[cloud.AuthType]cloud.CredentialSchema {
 	return map[cloud.AuthType]cloud.CredentialSchema{cloud.EmptyAuthType: {}}
 }
+
+type OpenAPIFuncSuite struct {
+	testing.IsolationSuite
+	store *jujuclient.MemStore
+}
+
+var _ = gc.Suite(&OpenAPIFuncSuite{})
+
+func (s *OpenAPIFuncSuite) SetUpTest(c *gc.C) {
+	s.IsolationSuite.SetUpTest(c)
+
+	s.store = jujuclient.NewMemStore()
+}
+
+func (s *OpenAPIFuncSuite) TestOpenAPIFunc(c *gc.C) {
+	var (
+		expected = &api.Info{
+			Password:  "meshuggah",
+			Macaroons: []macaroon.Slice{{}},
+		}
+		received *api.Info
+	)
+	origin := func(info *api.Info, dialOpts api.DialOpts) (api.Connection, error) {
+		received = info
+		return nil, nil
+	}
+	openFunc := modelcmd.OpenAPIFuncWithMacaroons(origin, s.store, "foo")
+	_, err := openFunc(expected, api.DialOpts{})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(received, jc.DeepEquals, expected)
+}
+
+func (s *OpenAPIFuncSuite) TestOpenAPIFuncWithNoPassword(c *gc.C) {
+	var (
+		expected = &api.Info{
+			Macaroons: []macaroon.Slice{{}},
+		}
+		received *api.Info
+	)
+	origin := func(info *api.Info, dialOpts api.DialOpts) (api.Connection, error) {
+		received = info
+		return nil, nil
+	}
+	openFunc := modelcmd.OpenAPIFuncWithMacaroons(origin, s.store, "foo")
+	_, err := openFunc(expected, api.DialOpts{})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(received, jc.DeepEquals, expected)
+}
+
+func (s *OpenAPIFuncSuite) TestOpenAPIFuncWithNoMacaroons(c *gc.C) {
+	var (
+		expected = &api.Info{
+			Password: "meshuggah",
+		}
+		received *api.Info
+	)
+	origin := func(info *api.Info, dialOpts api.DialOpts) (api.Connection, error) {
+		received = info
+		return nil, nil
+	}
+	openFunc := modelcmd.OpenAPIFuncWithMacaroons(origin, s.store, "foo")
+	_, err := openFunc(expected, api.DialOpts{})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(received, jc.DeepEquals, expected)
+}
+
+func (s *OpenAPIFuncSuite) TestOpenAPIFuncUsesStore(c *gc.C) {
+	mac, err := apitesting.NewMacaroon("id")
+	c.Assert(err, jc.ErrorIsNil)
+	jar, err := cookiejar.New(nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	addCookie(c, jar, mac, api.CookieURLFromHost("foo"))
+	s.store.CookieJars["foo"] = jar
+
+	var (
+		expected = &api.Info{
+			ControllerUUID: "foo",
+			Macaroons:      []macaroon.Slice{{mac}},
+		}
+		received *api.Info
+	)
+	origin := func(info *api.Info, dialOpts api.DialOpts) (api.Connection, error) {
+		received = info
+		return nil, nil
+	}
+	openFunc := modelcmd.OpenAPIFuncWithMacaroons(origin, s.store, "foo")
+	_, err = openFunc(&api.Info{
+		ControllerUUID: "foo",
+	}, api.DialOpts{})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(received, jc.DeepEquals, expected)
+}
+
+func (s *OpenAPIFuncSuite) TestOpenAPIFuncUsesStoreWithSNIHost(c *gc.C) {
+	mac, err := apitesting.NewMacaroon("id")
+	c.Assert(err, jc.ErrorIsNil)
+	jar, err := cookiejar.New(nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	addCookie(c, jar, mac, api.CookieURLFromHost("foo"))
+	s.store.CookieJars["foo"] = jar
+
+	var (
+		expected = &api.Info{
+			SNIHostName:    "foo",
+			ControllerUUID: "bar",
+			Macaroons:      []macaroon.Slice{{mac}},
+		}
+		received *api.Info
+	)
+	origin := func(info *api.Info, dialOpts api.DialOpts) (api.Connection, error) {
+		received = info
+		return nil, nil
+	}
+	openFunc := modelcmd.OpenAPIFuncWithMacaroons(origin, s.store, "foo")
+	_, err = openFunc(&api.Info{
+		SNIHostName:    "foo",
+		ControllerUUID: "bar",
+	}, api.DialOpts{})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(received, jc.DeepEquals, expected)
+}
+
+func addCookie(c *gc.C, jar http.CookieJar, mac *macaroon.Macaroon, url *url.URL) {
+	cookie, err := httpbakery.NewCookie(nil, macaroon.Slice{mac})
+	c.Assert(err, jc.ErrorIsNil)
+	cookie.Expires = time.Now().Add(time.Hour) // only persistent cookies are stored
+	jar.SetCookies(url, []*http.Cookie{cookie})
+}