Embedded CLI returns an error is macaroon is invalid

sdg0919 · May 19, 2021 · c95b4f4 · c95b4f4
1 parent b9abfb7
commit c95b4f4
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 7 deletions.
diff --git a/api/authentication/interactor.go b/api/authentication/interactor.go
@@ -17,7 +17,7 @@ import (
 
 const authMethod = "juju_userpass"
 
-// Visitor is a httpbakery.Visitor that will login directly
+// Interactor is a httpbakery.Interactor that will login directly
 // to the Juju controller using password authentication. This
 // only applies when logging in as a local user.
 type Interactor struct {
@@ -118,3 +118,22 @@ func (i *Interactor) LegacyInteract(ctx context.Context, client *httpbakery.Clie
 	}
 	return &jsonError
 }
+
+// NewNotSupportedInteractor returns an interactor that does
+// not support any discharge workflow.
+func NewNotSupportedInteractor() httpbakery.Interactor {
+	return &notSupportedInteractor{}
+}
+
+type notSupportedInteractor struct {
+}
+
+// Kind implements httpbakery.Interactor for the Interactor.
+func (i notSupportedInteractor) Kind() string {
+	return authMethod
+}
+
+// Interact implements httpbakery.Interactor for the Interactor.
+func (i notSupportedInteractor) Interact(_ context.Context, _ *httpbakery.Client, location string, _ *httpbakery.Error) (*httpbakery.DischargeToken, error) {
+	return nil, errors.NotSupportedf("interaction for %s", location)
+}
diff --git a/api/authentication/interactor_test.go b/api/authentication/interactor_test.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery"
 	"github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery/form"
+	"github.com/juju/errors"
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
@@ -46,6 +47,13 @@ func (s *InteractorSuite) SetUpTest(c *gc.C) {
 	s.AddCleanup(func(c *gc.C) { s.server.Close() })
 }
 
+func (s *InteractorSuite) TestNotSupportedInteract(c *gc.C) {
+	v := authentication.NewNotSupportedInteractor()
+	c.Assert(v.Kind(), gc.Equals, "juju_userpass")
+	_, err := v.Interact(context.TODO(), nil, "", nil)
+	c.Assert(err, jc.Satisfies, errors.IsNotSupported)
+}
+
 func (s *InteractorSuite) TestLegacyInteract(c *gc.C) {
 	v := authentication.NewInteractor("bob", func(username string) (string, error) {
 		c.Assert(username, gc.Equals, "bob")

diff --git a/apiserver/apiserver_test.go b/apiserver/apiserver_test.go
@@ -173,9 +173,14 @@ func (s *apiserverConfigFixture) SetUpTest(c *gc.C) {
 				fmt.Fprintf(ctx.Stderr, err.Error())
 				return 1
 			}
-			cmdStr := fmt.Sprintf("%s@%s:%s -> %s", ad.User, ctrl, model, cmdPlusArgs)
-			fmt.Fprintf(ctx.Stdout, cmdStr)
-			fmt.Fprintf(ctx.Stdout, "\n")
+			if strings.Contains(cmdPlusArgs, "macaroon error") {
+				fmt.Fprintf(ctx.Stderr, "ERROR: cannot get discharge from https://controller")
+				fmt.Fprintf(ctx.Stderr, "\n")
+			} else {
+				cmdStr := fmt.Sprintf("%s@%s:%s -> %s", ad.User, ctrl, model, cmdPlusArgs)
+				fmt.Fprintf(ctx.Stdout, cmdStr)
+				fmt.Fprintf(ctx.Stdout, "\n")
+			}
 			return 0
 		},
 	}
@@ -424,6 +429,16 @@ func (s *apiserverSuite) TestEmbeddedCommandInvalidUser(c *gc.C) {
 	s.assertEmbeddedCommand(c, cmdArgs, "", &params.Error{Message: `user name "123@" not valid`})
 }
 
+func (s *apiserverSuite) TestEmbeddedCommandInvalidMacaroon(c *gc.C) {
+	cmdArgs := params.CLICommands{
+		User:     "fred",
+		Commands: []string{"status macaroon error"},
+	}
+	s.assertEmbeddedCommand(c, cmdArgs, "", &params.Error{
+		Code:    params.CodeDischargeRequired,
+		Message: `macaroon discharge required: cannot get discharge from https://controller`})
+}
+
 func (s *apiserverSuite) assertEmbeddedCommand(c *gc.C, cmdArgs params.CLICommands, expected string, resultErr *params.Error) {
 	address := s.server.Listener.Addr().String()
 	path := fmt.Sprintf("/model/%s/commands", s.State.ModelUUID())

diff --git a/apiserver/embeddedcli.go b/apiserver/embeddedcli.go
@@ -7,8 +7,10 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"strings"
 	"time"
 
+	"github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery"
 	gorillaws "github.com/gorilla/websocket"
 	"github.com/juju/cmd"
 	"github.com/juju/errors"
@@ -174,6 +176,16 @@ done:
 				// Wait for cmd result.
 				continue
 			}
+			// If there's been a macaroon discharge required, we don't yet
+			// process it in embedded mode so just return it so the caller
+			// can deal with it, eg login again to get another macaroon.
+			// This string is hard coded in the bakery library.
+			idx := strings.Index(line, "cannot get discharge from")
+			if idx >= 0 {
+				return apiservererrors.ServerError(&apiservererrors.DischargeRequiredError{
+					Cause: &bakery.DischargeRequiredError{Message: line[idx:]},
+				})
+			}
 
 			if err := ws.WriteJSON(params.CLICommandStatus{
 				Output: []string{line},

diff --git a/cmd/modelcmd/apicontext.go b/cmd/modelcmd/apicontext.go
@@ -15,6 +15,7 @@ import (
 	"github.com/juju/idmclient/v2/ussologin"
 	"gopkg.in/juju/environschema.v1/form"
 
+	"github.com/juju/juju/api/authentication"
 	"github.com/juju/juju/jujuclient"
 )
 
@@ -65,9 +66,12 @@ func newAPIContext(ctxt *cmd.Context, opts *AuthOpts, store jujuclient.CookieSto
 	}
 	var interactor httpbakery.Interactor
 	embedded := ctxt != nil && opts != nil && opts.Embedded
-	noBrowser := ctxt != nil && opts != nil && opts.NoBrowser
-	if !embedded {
+	if embedded {
+		// Embedded commands don't yet support macaroon discharge workflow.
+		interactor = authentication.NewNotSupportedInteractor()
+	} else {
 		// Only support discharge interactions if command is not embedded.
+		noBrowser := ctxt != nil && opts != nil && opts.NoBrowser
 		if noBrowser {
 			filler := &form.IOFiller{
 				In:  ctxt.Stdin,

diff --git a/cmd/modelcmd/apicontext_test.go b/cmd/modelcmd/apicontext_test.go
@@ -4,12 +4,14 @@
 package modelcmd_test
 
 import (
+	"context"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 
 	"github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery"
 	"github.com/juju/cmd"
+	"github.com/juju/errors"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
@@ -101,7 +103,10 @@ func (s *APIContextSuite) TestNewAPIContextEmbedded(c *gc.C) {
 	opts := modelcmd.AuthOpts{Embedded: true}
 	ctx, err := modelcmd.NewAPIContext(cmdCtx, &opts, store, "testcontroller")
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(modelcmd.Interactor(ctx), gc.IsNil)
+	interactor := modelcmd.Interactor(ctx)
+	c.Assert(interactor, gc.Not(gc.IsNil))
+	_, err = interactor.Interact(context.TODO(), nil, "", nil)
+	c.Assert(err, jc.Satisfies, errors.IsNotSupported)
 }
 
 func (s *APIContextSuite) TestNewAPIContextNoBrowser(c *gc.C) {