Made RemoveUsers also revoke permissions.

When a users is "deleted" the permissions that user has
need to be removed too.

apiserver/modelmanager/modelmanager.go

@@ -485,7 +485,7 @@ func (m *ModelManagerAPI) ModelInfo(args params.Entities) (params.ModelInfoResul
 	getModelInfo := func(arg params.Entity) (params.ModelInfo, error) {
 		tag, err := names.ParseModelTag(arg.Tag)
 		if err != nil {
-			return params.ModelInfo{}, err
+			return params.ModelInfo{}, errors.Trace(err)
 		}
 		return m.getModelInfo(tag)
 	}
@@ -506,32 +506,32 @@ func (m *ModelManagerAPI) getModelInfo(tag names.ModelTag) (params.ModelInfo, er
 	if errors.IsNotFound(err) {
 		return params.ModelInfo{}, common.ErrPerm
 	} else if err != nil {
-		return params.ModelInfo{}, err
+		return params.ModelInfo{}, errors.Trace(err)
 	}
 	defer st.Close()
 
 	model, err := st.Model()
 	if errors.IsNotFound(err) {
 		return params.ModelInfo{}, common.ErrPerm
 	} else if err != nil {
-		return params.ModelInfo{}, err
+		return params.ModelInfo{}, errors.Trace(err)
 	}
 
 	cfg, err := model.Config()
 	if err != nil {
-		return params.ModelInfo{}, err
+		return params.ModelInfo{}, errors.Trace(err)
 	}
 	controllerCfg, err := st.ControllerConfig()
 	if err != nil {
-		return params.ModelInfo{}, err
+		return params.ModelInfo{}, errors.Trace(err)
 	}
 	users, err := model.Users()
 	if err != nil {
-		return params.ModelInfo{}, err
+		return params.ModelInfo{}, errors.Trace(err)
 	}
 	status, err := model.Status()
 	if err != nil {
-		return params.ModelInfo{}, err
+		return params.ModelInfo{}, errors.Trace(err)
 	}
 
 	owner := model.Owner()

state/controlleruser.go

@@ -70,17 +70,21 @@ func createControllerUserOps(controllerUUID string, user, createdBy names.UserTa
 	return ops
 }
 
-// RemoveControllerUser removes a user from the database.
-func (st *State) removeControllerUser(user names.UserTag) error {
-	ops := []txn.Op{
-		removePermissionOp(controllerKey(st.ControllerUUID()), userGlobalKey(userAccessID(user))),
+func removeControllerUserOps(controllerUUID string, user names.UserTag) []txn.Op {
+	return []txn.Op{
+		removePermissionOp(controllerKey(controllerUUID), userGlobalKey(userAccessID(user))),
 		{
 			C:      controllerUsersC,
 			Id:     userAccessID(user),
 			Assert: txn.DocExists,
 			Remove: true,
 		}}
 
+}
+
+// RemoveControllerUser removes a user from the database.
+func (st *State) removeControllerUser(user names.UserTag) error {
+	ops := removeControllerUserOps(st.ControllerUUID(), user)
 	err := st.runTransaction(ops)
 	if err == txn.ErrAborted {
 		err = errors.NewNotFound(nil, fmt.Sprintf("controller user %q does not exist", user.Canonical()))

state/modeluser.go

@@ -143,17 +143,31 @@ func createModelUserOps(modelUUID string, user, createdBy names.UserTag, display
 	return ops
 }
 
-// removeModelUser removes a user from the database.
-func (st *State) removeModelUser(user names.UserTag) error {
-	ops := []txn.Op{
-		removePermissionOp(modelKey(st.ModelUUID()), userGlobalKey(userAccessID(user))),
+func removeRawModelUserOps(modelUUID, userID string, user names.UserTag) []txn.Op {
+	return []txn.Op{
+		removePermissionOp(modelKey(modelUUID), userGlobalKey(userAccessID(user))),
+		{
+			C:      modelUsersC,
+			Id:     userID,
+			Assert: txn.DocExists,
+			Remove: true,
+		}}
+
+}
+func removeModelUserOps(modelUUID string, user names.UserTag) []txn.Op {
+	return []txn.Op{
+		removePermissionOp(modelKey(modelUUID), userGlobalKey(userAccessID(user))),
 		{
 			C:      modelUsersC,
 			Id:     userAccessID(user),
 			Assert: txn.DocExists,
 			Remove: true,
 		}}
+}
 
+// removeModelUser removes a user from the database.
+func (st *State) removeModelUser(user names.UserTag) error {
+	ops := removeModelUserOps(st.ModelUUID(), user)
 	err := st.runTransaction(ops)
 	if err == txn.ErrAborted {
 		err = errors.NewNotFound(nil, fmt.Sprintf("model user %q does not exist", user.Canonical()))

state/user.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/juju/errors"
 	"github.com/juju/juju/core/description"
+	jujutxn "github.com/juju/txn"
 	"github.com/juju/utils"
 	"gopkg.in/juju/names.v2"
 	"gopkg.in/mgo.v2"
@@ -143,10 +144,46 @@ func (st *State) RemoveUser(tag names.UserTag) error {
 			Assert: txn.DocExists,
 			Update: bson.M{"$set": bson.M{"deleted": true}},
 		}}
+		userModels, err := st.ModelsForUser(tag)
+		if err != nil && !errors.IsNotFound(err) {
+			return nil, errors.Trace(err)
+		}
+		for _, userModel := range userModels {
+			if userModel.Owner() == tag {
+				continue
+			}
+			mt := userModel.ModelTag()
+			uaID := ensureModelUUID(mt.Id(), userAccessID(tag))
+			ops = append(ops, removeRawModelUserOps(mt.Id(), uaID, tag)...)
+		}
+		controllerTag := st.ControllerTag()
+		controllerUser, err := st.UserAccess(tag, controllerTag)
+		if err != nil && !errors.IsNotFound(err) {
+			return nil, errors.Trace(err)
+		}
+		if !description.IsEmptyUserAccess(controllerUser) {
+			ops = append(ops, removeControllerUserOps(controllerTag.Id(), tag)...)
+		}
 		return ops, nil
 	}
-
-	return st.run(buildTxn)
+	for i := 0; i < 3; i++ {
+		ops, err := buildTxn(i)
+		if err == jujutxn.ErrTransientFailure {
+			continue
+		}
+		if err == jujutxn.ErrNoOperations {
+			return nil
+		}
+		if err != nil {
+			return err
+		}
+		if err := st.runRawTransaction(ops); err == nil {
+			return nil
+		} else if err != txn.ErrAborted {
+			return err
+		}
+	}
+	return jujutxn.ErrExcessiveContention
 }
 
 func createInitialUserOps(controllerUUID string, user names.UserTag, password, salt string) []txn.Op {

state/user_test.go

@@ -14,6 +14,7 @@ import (
 	gc "gopkg.in/check.v1"
 	"gopkg.in/juju/names.v2"
 
+	"github.com/juju/juju/core/description"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/testing/factory"
 )
@@ -226,6 +227,39 @@ func (s *UserSuite) TestRemoveUser(c *gc.C) {
 	c.Check(err, jc.Satisfies, errors.IsUserNotFound)
 }
 
+func (s *UserSuite) TestRemoveUserRemovesUserAccess(c *gc.C) {
+	user := s.Factory.MakeUser(c, &factory.UserParams{Password: "so sekrit"})
+
+	// Assert user exists and can authenticate.
+	c.Assert(user.PasswordValid("so sekrit"), jc.IsTrue)
+
+	s.State.SetUserAccess(user.UserTag(), s.State.ModelTag(), description.AdminAccess)
+	s.State.SetUserAccess(user.UserTag(), s.State.ControllerTag(), description.SuperuserAccess)
+
+	uam, err := s.State.UserAccess(user.UserTag(), s.State.ModelTag())
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(uam.Access, gc.Equals, description.AdminAccess)
+
+	uac, err := s.State.UserAccess(user.UserTag(), s.State.ControllerTag())
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(uac.Access, gc.Equals, description.SuperuserAccess)
+
+	// Look for the user.
+	u, err := s.State.User(user.UserTag())
+	c.Check(err, jc.ErrorIsNil)
+	c.Assert(u, jc.DeepEquals, user)
+
+	// Remove the user.
+	err = s.State.RemoveUser(user.UserTag())
+	c.Check(err, jc.ErrorIsNil)
+
+	uam, err = s.State.UserAccess(user.UserTag(), s.State.ModelTag())
+	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("model user %q not found", user.UserTag().Canonical()))
+
+	uac, err = s.State.UserAccess(user.UserTag(), s.State.ControllerTag())
+	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("controller user %q not found", user.UserTag().Canonical()))
+}
+
 func (s *UserSuite) TestDisable(c *gc.C) {
 	user := s.Factory.MakeUser(c, &factory.UserParams{Password: "a-password"})
 	c.Assert(user.IsDisabled(), jc.IsFalse)