Merge pull request #4422 from wallyworld/loginv3

Increment login to use version 3 so older logins can be rejected

Increment login to v3  - copy admin_v2 to admin_v3 and increment the version.
Delete support for older logins.
Retain v2 in a test helper so we can test older clients.
When an older client logs in, return an error saying the client is too old. This will stop 1.x clients trying to log into 2.0 Juju and succeeding and then having stuff break.

(Review request: http://reviews.vapour.ws/r/3862/)

api/apiclient.go

@@ -221,12 +221,10 @@ func (t *hostSwitchingTransport) RoundTrip(req *http.Request) (*http.Response, e
 func OpenWithVersion(info *Info, opts DialOpts, loginVersion int) (Connection, error) {
 	var loginFunc func(st *state, tag names.Tag, pwd, nonce string) error
 	switch loginVersion {
-	case 0:
-		loginFunc = (*state).loginV0
-	case 1:
-		loginFunc = (*state).loginV1
 	case 2:
 		loginFunc = (*state).loginV2
+	case 3:
+		loginFunc = (*state).loginV3
 	default:
 		return nil, errors.NotSupportedf("loginVersion %d", loginVersion)
 	}

api/client_test.go

@@ -488,17 +488,12 @@ func (s *clientSuite) TestConnectStreamAtUUIDPath(c *gc.C) {
 
 func (s *clientSuite) TestOpenUsesEnvironUUIDPaths(c *gc.C) {
 	info := s.APIInfo(c)
-	// Backwards compatibility, passing ModelTag = "" should just work
-	info.ModelTag = names.NewModelTag("")
-	apistate, err := api.Open(info, api.DialOpts{})
-	c.Assert(err, jc.ErrorIsNil)
-	apistate.Close()
 
-	// Passing in the correct model UUID should also work
+	// Passing in the correct model UUID should work
 	environ, err := s.State.Model()
 	c.Assert(err, jc.ErrorIsNil)
 	info.ModelTag = environ.ModelTag()
-	apistate, err = api.Open(info, api.DialOpts{})
+	apistate, err := api.Open(info, api.DialOpts{})
 	c.Assert(err, jc.ErrorIsNil)
 	apistate.Close()
 

api/state.go

@@ -40,18 +40,20 @@ import (
 // method is usually called automatically by Open. The machine nonce
 // should be empty unless logging in as a machine agent.
 func (st *state) Login(tag names.Tag, password, nonce string) error {
-	err := st.loginV2(tag, password, nonce)
-	if params.IsCodeNotImplemented(err) {
-		err = st.loginV1(tag, password, nonce)
-		if params.IsCodeNotImplemented(err) {
-			// TODO (cmars): remove fallback once we can drop v0 compatibility
-			return errors.Trace(st.loginV0(tag, password, nonce))
-		}
-	}
+	err := st.loginV3(tag, password, nonce)
 	return errors.Trace(err)
 }
 
+// loginV2 is retained for testing logins from older clients.
 func (st *state) loginV2(tag names.Tag, password, nonce string) error {
+	return st.loginForVersion(tag, password, nonce, 2)
+}
+
+func (st *state) loginV3(tag names.Tag, password, nonce string) error {
+	return st.loginForVersion(tag, password, nonce, 3)
+}
+
+func (st *state) loginForVersion(tag names.Tag, password, nonce string, vers int) error {
 	var result params.LoginResultV1
 	request := &params.LoginRequest{
 		AuthTag:     tagToString(tag),
@@ -62,22 +64,8 @@ func (st *state) loginV2(tag names.Tag, password, nonce string) error {
 		// Add any macaroons that might work for authenticating the login request.
 		request.Macaroons = httpbakery.MacaroonsForURL(st.bakeryClient.Client.Jar, st.cookieURL)
 	}
-	err := st.APICall("Admin", 2, "", "Login", request, &result)
+	err := st.APICall("Admin", vers, "", "Login", request, &result)
 	if err != nil {
-		// If the server complains about an empty tag it may be that we are
-		// talking to an older server version that does not understand facades and
-		// expects a params.Creds request instead of a params.LoginRequest. We
-		// return a CodeNotImplemented error to force login down to V1, which
-		// supports older server logins. This may mask an actual empty tag in
-		// params.LoginRequest, but that would be picked up in loginV1. V1 will
-		// also produce a warning that we are ignoring an invalid API, so we do not
-		// need to add one here.
-		if err.Error() == `"" is not a valid tag` {
-			return &params.Error{
-				Message: err.Error(),
-				Code:    params.CodeNotImplemented,
-			}
-		}
 		return errors.Trace(err)
 	}
 	if result.DischargeRequired != nil {
@@ -101,7 +89,7 @@ func (st *state) loginV2(tag names.Tag, password, nonce string) error {
 		// Add the macaroons that have been saved by HandleError to our login request.
 		request.Macaroons = httpbakery.MacaroonsForURL(st.bakeryClient.Client.Jar, st.cookieURL)
 		result = params.LoginResultV1{} // zero result
-		err = st.APICall("Admin", 2, "", "Login", request, &result)
+		err = st.APICall("Admin", vers, "", "Login", request, &result)
 		if err != nil {
 			return errors.Trace(err)
 		}
@@ -122,60 +110,6 @@ func (st *state) loginV2(tag names.Tag, password, nonce string) error {
 	return nil
 }
 
-func (st *state) loginV1(tag names.Tag, password, nonce string) error {
-	var result struct {
-		// TODO (cmars): remove once we can drop 1.18 login compatibility
-		params.LoginResult
-
-		params.LoginResultV1
-	}
-	err := st.APICall("Admin", 1, "", "Login", &params.LoginRequestCompat{
-		LoginRequest: params.LoginRequest{
-			AuthTag:     tagToString(tag),
-			Credentials: password,
-			Nonce:       nonce,
-		},
-		// TODO (cmars): remove once we can drop 1.18 login compatibility
-		Creds: params.Creds{
-			AuthTag:  tagToString(tag),
-			Password: password,
-			Nonce:    nonce,
-		},
-	}, &result)
-	if err != nil {
-		return err
-	}
-
-	// We've either logged into an Admin v1 facade, or a pre-facade (1.18) API
-	// server.  The JSON field names between the structures are disjoint, so only
-	// one should have an model tag set.
-
-	var modelTag string
-	var controllerTag string
-	var servers [][]network.HostPort
-	var facades []params.FacadeVersions
-	// For quite old servers, it is possible that they don't send down
-	// the modelTag.
-	if result.LoginResult.ModelTag != "" {
-		modelTag = result.LoginResult.ModelTag
-		// If the server doesn't support login v1, it doesn't support
-		// multiple models, so don't store a server tag.
-		servers = params.NetworkHostsPorts(result.LoginResult.Servers)
-		facades = result.LoginResult.Facades
-	} else if result.LoginResultV1.ModelTag != "" {
-		modelTag = result.LoginResultV1.ModelTag
-		controllerTag = result.LoginResultV1.ControllerTag
-		servers = params.NetworkHostsPorts(result.LoginResultV1.Servers)
-		facades = result.LoginResultV1.Facades
-	}
-
-	err = st.setLoginResult(tag, modelTag, controllerTag, servers, facades)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 func (st *state) setLoginResult(tag names.Tag, modelTag, controllerTag string, servers [][]network.HostPort, facades []params.FacadeVersions) error {
 	st.authTag = tag
 	st.modelTag = modelTag
@@ -199,24 +133,6 @@ func (st *state) setLoginResult(tag names.Tag, modelTag, controllerTag string, s
 	return nil
 }
 
-func (st *state) loginV0(tag names.Tag, password, nonce string) error {
-	var result params.LoginResult
-	err := st.APICall("Admin", 0, "", "Login", &params.Creds{
-		AuthTag:  tagToString(tag),
-		Password: password,
-		Nonce:    nonce,
-	}, &result)
-	if err != nil {
-		return err
-	}
-	servers := params.NetworkHostsPorts(result.Servers)
-	// Don't set a server tag.
-	if err = st.setLoginResult(tag, result.ModelTag, "", servers, result.Facades); err != nil {
-		return err
-	}
-	return nil
-}
-
 // slideAddressToFront moves the address at the location (serverIndex, addrIndex) to be
 // the first address of the first server.
 func slideAddressToFront(servers [][]network.HostPort, serverIndex, addrIndex int) {

apiserver/admin.go

@@ -15,6 +15,7 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/rpc"
+	"github.com/juju/juju/rpc/rpcreflect"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/state/presence"
 	"github.com/juju/juju/version"
@@ -83,7 +84,7 @@ func (a *admin) doLogin(req params.LoginRequest, loginVersion int) (params.Login
 		isUser = true
 	}
 
-	serverOnlyLogin := loginVersion > 1 && a.root.modelUUID == ""
+	serverOnlyLogin := a.root.modelUUID == ""
 
 	entity, lastConnection, err := doCheckCreds(a.root.state, req, !serverOnlyLogin, a.srv.authCtxt)
 	if err != nil {
@@ -471,7 +472,7 @@ type errRoot struct {
 	err error
 }
 
-// Admin conforms to the same API as initialRoot, but we'll always return (nil, err)
-func (r *errRoot) Admin(id string) (*adminV0, error) {
+// FindMethod conforms to the same API as initialRoot, but we'll always return (nil, err)
+func (r *errRoot) FindMethod(rootName string, version int, methodName string) (rpcreflect.MethodCaller, error) {
 	return nil, r.err
 }