WIP: Acls add write #5526

Merged
merged 1 commit into from Jun 21, 2016
Jump to file or symbol
Failed to load files and symbols.
+535 −207
Split
@@ -120,6 +120,8 @@ func ParseModelAccess(access string) (params.ModelAccessPermission, error) {
accessPermission = params.ModelReadAccess
case permission.ModelWriteAccess:
accessPermission = params.ModelWriteAccess
+ case permission.ModelAdminAccess:
+ accessPermission = params.ModelAdminAccess
default:
return fail, errors.Errorf("unsupported model access permission %v", modelAccess)
}
@@ -60,9 +60,9 @@ func (s *usermanagerSuite) TestAddUserWithModelAccess(c *gc.C) {
for i, u := range users {
modelUserTags[i] = u.UserTag()
if u.UserTag().Name() == "foobar" {
- c.Assert(u.ReadOnly(), jc.IsTrue)
+ c.Assert(u.IsReadOnly(), jc.IsTrue)
} else {
- c.Assert(u.ReadOnly(), jc.IsFalse)
+ c.Assert(u.IsReadOnly(), jc.IsFalse)
}
}
c.Assert(modelUserTags, jc.SameContents, []names.UserTag{
View
@@ -146,18 +146,18 @@ func (a *admin) doLogin(req params.LoginRequest, loginVersion int) (params.Login
}
var maybeUserInfo *params.AuthUserInfo
- var envUser *state.ModelUser
+ var modelUser *state.ModelUser
// Send back user info if user
if isUser && !serverOnlyLogin {
maybeUserInfo = &params.AuthUserInfo{
Identity: entity.Tag().String(),
LastConnection: lastConnection,
}
- envUser, err = a.root.state.ModelUser(entity.Tag().(names.UserTag))
+ modelUser, err = a.root.state.ModelUser(entity.Tag().(names.UserTag))
if err != nil {
return fail, errors.Annotatef(err, "missing ModelUser for logged in user %s", entity.Tag())
}
- maybeUserInfo.ReadOnly = envUser.ReadOnly()
+ maybeUserInfo.ReadOnly = modelUser.IsReadOnly()
if maybeUserInfo.ReadOnly {
logger.Debugf("model user %s is READ ONLY", entity.Tag())
}
@@ -201,8 +201,8 @@ func (a *admin) doLogin(req params.LoginRequest, loginVersion int) (params.Login
loginResult.Facades = facades
}
- if envUser != nil {
- authedApi = newClientAuthRoot(authedApi, envUser)
+ if modelUser != nil {
+ authedApi = newClientAuthRoot(authedApi, modelUser)
}
a.root.rpcConn.ServeFinder(authedApi, serverError)
@@ -83,8 +83,8 @@ func (s *serverSuite) TestModelUsersInfo(c *gc.C) {
localUser1 := s.makeLocalModelUser(c, "ralphdoe", "Ralph Doe")
localUser2 := s.makeLocalModelUser(c, "samsmith", "Sam Smith")
- remoteUser1 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "bobjohns@ubuntuone", DisplayName: "Bob Johns"})
- remoteUser2 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "nicshaw@idprovider", DisplayName: "Nic Shaw"})
+ remoteUser1 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "bobjohns@ubuntuone", DisplayName: "Bob Johns", Access: state.WriteAccess})
+ remoteUser2 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "nicshaw@idprovider", DisplayName: "Nic Shaw", Access: state.WriteAccess})
results, err := s.client.ModelUserInfo()
c.Assert(err, jc.ErrorIsNil)
@@ -98,21 +98,21 @@ func (s *serverSuite) TestModelUsersInfo(c *gc.C) {
&params.ModelUserInfo{
UserName: owner.UserName(),
DisplayName: owner.DisplayName(),
- Access: "write",
+ Access: "admin",
},
}, {
localUser1,
&params.ModelUserInfo{
UserName: "ralphdoe@local",
DisplayName: "Ralph Doe",
- Access: "write",
+ Access: "admin",
},
}, {
localUser2,
&params.ModelUserInfo{
UserName: "samsmith@local",
DisplayName: "Sam Smith",
- Access: "write",
+ Access: "admin",
},
}, {
remoteUser1,
@@ -34,14 +34,21 @@ func (r *clientAuthRoot) FindMethod(rootName string, version int, methodName str
if err != nil {
return nil, err
}
- if r.user.ReadOnly() {
+
+ // ReadOnly User
+ if r.user.IsReadOnly() {
canCall := isCallAllowableByReadOnlyUser(rootName, methodName) ||
isCallReadOnly(rootName, methodName)
if !canCall {
return nil, errors.Trace(common.ErrPerm)
}
}
+ // Check if our call requires higher access than the user has.
+ if doesCallRequireAdmin(rootName, methodName) && !r.user.IsAdmin() {
+ return nil, errors.Trace(common.ErrPerm)
+ }
+
return caller, nil
}
@@ -54,3 +61,12 @@ func isCallAllowableByReadOnlyUser(facade, _ /*method*/ string) bool {
// endpoint rather than a model oriented one.
return restrictedRootNames.Contains(facade)
}
+
+func doesCallRequireAdmin(facade, method string) bool {
+ // TODO(perrito666) This should filter adding users to controllers.
+ // TODO(perrito666) Add an exaustive list of facades/methods that are
+ // admin only and put them in an authoritative source to be re-used.
+ // TODO(perrito666) This is a stub, the idea is to maintain the current
+ // status of permissions until we decide what goes to admin only.
+ return false
+}
@@ -43,17 +43,17 @@ func (s *clientAuthRootSuite) AssertCallErrPerm(c *gc.C, client *clientAuthRoot,
}
func (s *clientAuthRootSuite) TestNormalUser(c *gc.C) {
- envUser := s.Factory.MakeModelUser(c, nil)
- client := newClientAuthRoot(&fakeFinder{}, envUser)
+ modelUser := s.Factory.MakeModelUser(c, nil)
+ client := newClientAuthRoot(&fakeFinder{}, modelUser)
s.AssertCallGood(c, client, "Application", 1, "Deploy")
s.AssertCallGood(c, client, "UserManager", 1, "UserInfo")
s.AssertCallNotImplemented(c, client, "Client", 1, "Unknown")
s.AssertCallNotImplemented(c, client, "Unknown", 1, "Method")
}
func (s *clientAuthRootSuite) TestReadOnlyUser(c *gc.C) {
- envUser := s.Factory.MakeModelUser(c, &factory.ModelUserParams{Access: state.ModelReadAccess})
- client := newClientAuthRoot(&fakeFinder{}, envUser)
+ modelUser := s.Factory.MakeModelUser(c, &factory.ModelUserParams{Access: state.ReadAccess})
+ client := newClientAuthRoot(&fakeFinder{}, modelUser)
// deploys are bad
s.AssertCallErrPerm(c, client, "Application", 1, "Deploy")
// read only commands are fine
@@ -16,20 +16,17 @@ import (
// ModelUser defines the subset of the state.ModelUser type
// that we require to convert to a params.ModelUserInfo.
type ModelUser interface {
- Access() state.ModelAccess
DisplayName() string
LastConnection() (time.Time, error)
UserName() string
UserTag() names.UserTag
+ IsReadOnly() bool
+ IsReadWrite() bool
+ IsAdmin() bool
}
// ModelUserInfo converts *state.ModelUser to params.ModelUserInfo.
func ModelUserInfo(user ModelUser) (params.ModelUserInfo, error) {
- access, err := StateToParamsModelAccess(user.Access())
- if err != nil {
- return params.ModelUserInfo{}, errors.Trace(err)
- }
-
var lastConn *time.Time
userLastConn, err := user.LastConnection()
if err == nil {
@@ -38,6 +35,14 @@ func ModelUserInfo(user ModelUser) (params.ModelUserInfo, error) {
return params.ModelUserInfo{}, errors.Trace(err)
}
+ access := params.ModelReadAccess
+ switch {
+ case user.IsAdmin():
+ access = params.ModelAdminAccess
+ case user.IsReadWrite():
+ access = params.ModelWriteAccess
+ }
+
userInfo := params.ModelUserInfo{
UserName: user.UserName(),
DisplayName: user.DisplayName(),
@@ -47,13 +52,15 @@ func ModelUserInfo(user ModelUser) (params.ModelUserInfo, error) {
return userInfo, nil
}
-// StateToParamsModelAccess converts state.ModelAccess to params.ModelAccessPermission.
-func StateToParamsModelAccess(stateAccess state.ModelAccess) (params.ModelAccessPermission, error) {
+// StateToParamsModelAccess converts state.Access to params.AccessPermission.
+func StateToParamsModelAccess(stateAccess state.Access) (params.ModelAccessPermission, error) {
switch stateAccess {
- case state.ModelReadAccess:
+ case state.ReadAccess:
return params.ModelReadAccess, nil
- case state.ModelAdminAccess:
+ case state.WriteAccess:
return params.ModelWriteAccess, nil
+ case state.AdminAccess:
+ return params.ModelAdminAccess, nil
}
return "", errors.Errorf("invalid model access permission %q", stateAccess)
}
@@ -57,15 +57,15 @@ func (s *modelInfoSuite) SetUpTest(c *gc.C) {
},
users: []*mockModelUser{{
userName: "admin",
- access: state.ModelAdminAccess,
+ access: state.AdminAccess,
}, {
userName: "bob@local",
displayName: "Bob",
- access: state.ModelReadAccess,
+ access: state.ReadAccess,
}, {
userName: "charlotte@local",
displayName: "Charlotte",
- access: state.ModelReadAccess,
+ access: state.ReadAccess,
}},
}
var err error
@@ -82,7 +82,8 @@ func (s *modelInfoSuite) setAPIUser(c *gc.C, user names.UserTag) {
func (s *modelInfoSuite) TestModelInfo(c *gc.C) {
s.st.model.users[1].SetErrors(
- nil, state.NeverConnectedError("never connected"),
+ state.NeverConnectedError("never connected"),
+ nil, nil, nil, nil,
)
info := s.getModelInfo(c)
c.Assert(info, jc.DeepEquals, params.ModelInfo{
@@ -102,7 +103,7 @@ func (s *modelInfoSuite) TestModelInfo(c *gc.C) {
Users: []params.ModelUserInfo{{
UserName: "admin",
LastConnection: &time.Time{},
- Access: params.ModelWriteAccess,
+ Access: params.ModelAdminAccess,
}, {
UserName: "bob@local",
DisplayName: "Bob",
@@ -353,14 +354,26 @@ type mockModelUser struct {
gitjujutesting.Stub
userName string
displayName string
- access state.ModelAccess
lastConnection time.Time
+ access state.Access
}
-func (u *mockModelUser) Access() state.ModelAccess {
- u.MethodCall(u, "Access")
+func (u *mockModelUser) IsAdmin() bool {
+ u.MethodCall(u, "IsAdmin")
u.PopNoErr()
- return u.access
+ return u.access == state.AdminAccess
+}
+
+func (u *mockModelUser) IsReadOnly() bool {
+ u.MethodCall(u, "IsReadOnly")
+ u.PopNoErr()
+ return u.access == state.ReadAccess
+}
+
+func (u *mockModelUser) IsReadWrite() bool {
+ u.MethodCall(u, "IsReadWrite")
+ u.PopNoErr()
+ return u.access == state.WriteAccess
}
func (u *mockModelUser) DisplayName() string {
@@ -425,34 +425,18 @@ func (m *ModelManagerAPI) ModifyModelAccess(args params.ModifyModelAccessRequest
// resolveStateAccess returns the state representation of the logical model
// access type.
-func resolveStateAccess(access permission.ModelAccess) (state.ModelAccess, error) {
- var fail state.ModelAccess
+func resolveStateAccess(access permission.ModelAccess) (state.Access, error) {
+ var fail state.Access
switch access {
case permission.ModelReadAccess:
- return state.ModelReadAccess, nil
+ return state.ReadAccess, nil
case permission.ModelWriteAccess:
- // TODO: Initially, we'll map "write" access to admin-level access.
- // Post Juju-2.0, support for more nuanced access will be added to the
- // permission business logic and state model.
- return state.ModelAdminAccess, nil
+ return state.WriteAccess, nil
}
logger.Errorf("invalid access permission: %+v", access)
return fail, errors.Errorf("invalid access permission")
}
-// isGreaterAccess returns whether the new access provides more permissions
-// than the current access.
-// TODO(cmars): If/when more access types are implemented in state,
-// the implementation of this function will certainly need to change, and it
-// should be abstracted away to juju/permission as pure business logic
-// instead of operating on state values.
-func isGreaterAccess(currentAccess, newAccess state.ModelAccess) bool {
- if currentAccess == state.ModelReadAccess && newAccess == state.ModelAdminAccess {
- return true
- }
- return false
-}
-
func userAuthorizedToChangeAccess(st Backend, userIsAdmin bool, userTag names.UserTag) error {
if userIsAdmin {
// Just confirm that the model that has been given is a valid model.
@@ -473,7 +457,7 @@ func userAuthorizedToChangeAccess(st Backend, userIsAdmin bool, userTag names.Us
}
return errors.Annotate(err, "could not retrieve user")
}
- if currentUser.Access() != state.ModelAdminAccess {
+ if !currentUser.IsAdmin() {
return common.ErrPerm
}
return nil
@@ -511,34 +495,42 @@ func ChangeModelAccess(accessor Backend, modelTag names.ModelTag, apiUser, targe
}
// Only set access if greater access is being granted.
- if isGreaterAccess(modelUser.Access(), stateAccess) {
+ if modelUser.IsGreaterAccess(stateAccess) {
err = modelUser.SetAccess(stateAccess)
if err != nil {
return errors.Annotate(err, "could not set model access for user")
}
} else {
- return errors.Errorf("user already has %q access", modelUser.Access())
+ return errors.Errorf("user already has %q access or greater", stateAccess)
}
return nil
}
return errors.Annotate(err, "could not grant model access")
case params.RevokeModelAccess:
- if stateAccess == state.ModelReadAccess {
+ switch stateAccess {
+ case state.ReadAccess:
// Revoking read access removes all access.
err := st.RemoveModelUser(targetUserTag)
return errors.Annotate(err, "could not revoke model access")
-
- } else if stateAccess == state.ModelAdminAccess {
- // Revoking admin access sets read-only.
+ case state.WriteAccess:
+ // Revoking write access sets read-only.
modelUser, err := st.ModelUser(targetUserTag)
if err != nil {
return errors.Annotate(err, "could not look up model access for user")
}
- err = modelUser.SetAccess(state.ModelReadAccess)
+ err = modelUser.SetAccess(state.ReadAccess)
return errors.Annotate(err, "could not set model access to read-only")
+ case state.AdminAccess:
+ // Revoking admin access sets read-write.
+ modelUser, err := st.ModelUser(targetUserTag)
+ if err != nil {
+ return errors.Annotate(err, "could not look up model access for user")
+ }
+ err = modelUser.SetAccess(state.WriteAccess)
+ return errors.Annotate(err, "could not set model access to read-write")
- } else {
+ default:
return errors.Errorf("don't know how to revoke %q access", stateAccess)
}
@@ -555,6 +547,8 @@ func FromModelAccessParam(paramAccess params.ModelAccessPermission) (permission.
return permission.ModelReadAccess, nil
case params.ModelWriteAccess:
return permission.ModelWriteAccess, nil
+ case params.ModelAdminAccess:
+ return permission.ModelAdminAccess, nil
}
return fail, errors.Errorf("invalid model access permission %q", paramAccess)
}
Oops, something went wrong.