fix: auth for external users

Auth for external user was broken because if the user is not local, we
check their username against the logged in user name from state, and the
local username we were checking it against was empty. This was because
of a check against local users that would set their account detials to
empty if they had no macaroons. The macaroons are not stored in the
AccountDetails of the external user in this case, and so the username
it was compared against was empty.

This fix sets the account details to the username and macaroons
(which may be empty).

cmd/modelcmd/base.go

@@ -226,16 +226,12 @@ func processAccountDetails(accountDetails *jujuclient.AccountDetails) *jujuclien
 	} else {
 		u := names.NewUserTag(accountDetails.User)
 		if !u.IsLocal() {
-			if len(accountDetails.Macaroons) == 0 {
-				accountDetails = &jujuclient.AccountDetails{}
-			} else {
-				// If the account has macaroon set, use those to login
-				// to avoid an unnecessary auth round trip.
-				// Used for embedded commands.
-				accountDetails = &jujuclient.AccountDetails{
-					User:      u.Id(),
-					Macaroons: accountDetails.Macaroons,
-				}
+			// If the account has macaroon set, use those to login
+			// to avoid an unnecessary auth round trip.
+			// Used for embedded commands.
+			accountDetails = &jujuclient.AccountDetails{
+				User:      u.Id(),
+				Macaroons: accountDetails.Macaroons,
 			}
 		}
 	}

cmd/modelcmd/base_test.go

@@ -17,6 +17,7 @@ import (
 	cookiejar "github.com/juju/persistent-cookiejar"
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
+	"go.uber.org/mock/gomock"
 	gc "gopkg.in/check.v1"
 	"gopkg.in/macaroon-bakery.v2/httpbakery"
 	"gopkg.in/macaroon.v2"
@@ -25,6 +26,7 @@ import (
 	apitesting "github.com/juju/juju/api/testing"
 	"github.com/juju/juju/cloud"
 	"github.com/juju/juju/cmd/modelcmd"
+	"github.com/juju/juju/cmd/modelcmd/mocks"
 	"github.com/juju/juju/core/model"
 	"github.com/juju/juju/core/network"
 	"github.com/juju/juju/environs"
@@ -190,6 +192,42 @@ To access it run 'juju switch bar:admin/badmodel'.`,
 	}
 }
 
+func (s *BaseCommandSuite) setupMocks(c *gc.C) *gomock.Controller {
+	ctrl := gomock.NewController(c)
+	return ctrl
+}
+
+func (s *BaseCommandSuite) TestNewAPIRootExternalUser(c *gc.C) {
+	ctrl := s.setupMocks(c)
+	conn := mocks.NewMockConnection(ctrl)
+	apiOpen := func(info *api.Info, opts api.DialOpts) (api.Connection, error) {
+		return conn, nil
+	}
+	externalName := "alastair@external"
+	conn.EXPECT().AuthTag().Return(names.NewUserTag(externalName)).MinTimes(1)
+	conn.EXPECT().APIHostPorts()
+	conn.EXPECT().ServerVersion()
+	conn.EXPECT().Addr()
+	conn.EXPECT().IPAddr()
+	conn.EXPECT().PublicDNSName()
+	conn.EXPECT().ControllerAccess().MinTimes(1)
+
+	s.store.Accounts["foo"] = jujuclient.AccountDetails{
+		User: externalName,
+	}
+
+	baseCmd := new(modelcmd.ModelCommandBase)
+	baseCmd.SetClientStore(s.store)
+	baseCmd.SetAPIOpen(apiOpen)
+	modelcmd.InitContexts(&cmd.Context{Stderr: io.Discard}, baseCmd)
+	modelcmd.SetRunStarted(baseCmd)
+
+	c.Assert(baseCmd.SetModelIdentifier("foo:admin/badmodel", false), jc.ErrorIsNil)
+
+	_, err := baseCmd.NewAPIRoot()
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 type NewGetBootstrapConfigParamsFuncSuite struct {
 	testing.IsolationSuite
 }
@@ -462,7 +500,9 @@ func (s *BaseCommandSuite) TestProcessAccountDetails(c *gc.C) {
 		input: jujuclient.AccountDetails{
 			User: names.NewUserTag("[email protected]").String(),
 		},
-		expectedOutput: jujuclient.AccountDetails{},
+		expectedOutput: jujuclient.AccountDetails{
+			User: names.NewUserTag("[email protected]").String(),
+		},
 	}}
 	for i, test := range tests {
 		c.Logf("running test case %d", i)