Skip to content

Commit

Permalink
fix: support setting agent username and group in user APIs. (#5055)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ioga
ioga authored Sep 20, 2022
1 parent 9fc9865 commit 2bca637
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 25 deletions.
82 changes: 71 additions & 11 deletions e2e_tests/tests/cluster/test_users.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,18 +183,44 @@ def extract_id_and_owner_from_exp_list(output: str) -> List[Tuple[int, str]]:

@pytest.mark.e2e_cpu
def test_post_user_api(clean_auth: None) -> None:
master_url = conf.make_master_url()
authentication.cli_auth = authentication.Authentication(
conf.make_master_url(), requested_user="admin", password="", try_reauth=True
)
new_username = get_random_string()

sess = exp.determined_test_session(admin=True)

user = bindings.v1User(active=True, admin=False, username=new_username)
body = bindings.v1PostUserRequest(password="", user=user)
resp = bindings.post_PostUser(
api.Session(master_url, "admin", authentication.cli_auth, None), body=body
)
resp = bindings.post_PostUser(sess, body=body)
assert resp and resp.user
assert resp.to_json()["user"]["username"] == new_username
assert resp.user.agentUserGroup is None

user = bindings.v1User(
active=True,
admin=False,
username=get_random_string(),
agentUserGroup=bindings.v1AgentUserGroup(
agentUid=1000, agentGid=1001, agentUser="username", agentGroup="groupname"
),
)
resp = bindings.post_PostUser(sess, body=bindings.v1PostUserRequest(user=user))
assert resp and resp.user and resp.user.agentUserGroup
assert resp.user.agentUserGroup.agentUser == "username"
assert resp.user.agentUserGroup.agentGroup == "groupname"
assert resp.user.agentUserGroup.agentUid == 1000
assert resp.user.agentUserGroup.agentGid == 1001

user = bindings.v1User(
active=True,
admin=False,
username=get_random_string(),
agentUserGroup=bindings.v1AgentUserGroup(
agentUid=1000,
agentGid=1001,
),
)

with pytest.raises(errors.APIException):
bindings.post_PostUser(sess, body=bindings.v1PostUserRequest(user=user))


@pytest.mark.e2e_cpu
Expand Down Expand Up @@ -896,6 +922,13 @@ def test_experiment_delete() -> None:
pytest.fail("experiment didn't delete after timeout")


def _fetch_user_by_username(sess: api.Session, username: str) -> bindings.v1User:
# Get API bindings object for the created test user
all_users = bindings.get_GetUsers(sess).users
assert all_users is not None
return next(u for u in all_users if u.username == username)


@pytest.mark.e2e_cpu
@pytest.mark.e2e_cpu_postgres
def test_change_displayname(clean_auth: None) -> None:
Expand All @@ -909,10 +942,7 @@ def test_change_displayname(clean_auth: None) -> None:
)
sess = api.Session(master_url, original_name, authentication.cli_auth, certs.cli_cert)

# Get API bindings object for the created test user
all_users = bindings.get_GetUsers(sess).users
assert all_users is not None
current_user = list(filter(lambda u: u.username == original_name, all_users))[0]
current_user = _fetch_user_by_username(sess, original_name)
assert current_user is not None and current_user.id

# Rename user using display name
Expand All @@ -935,3 +965,33 @@ def test_change_displayname(clean_auth: None) -> None:
modded_user = bindings.get_GetUser(sess, userId=current_user.id).user
assert modded_user is not None
assert modded_user.displayName == ""


@pytest.mark.e2e_cpu
def test_patch_agentusergroup(clean_auth: None) -> None:
test_user_credentials = create_test_user(ADMIN_CREDENTIALS, False)
test_username = test_user_credentials.username

# Patch - normal.
sess = exp.determined_test_session(admin=True)
patch_user = bindings.v1PatchUser(
agentUserGroup=bindings.v1AgentUserGroup(
agentGid=1000, agentUid=1000, agentUser="username", agentGroup="groupname"
)
)
test_user = _fetch_user_by_username(sess, test_username)
assert test_user.id
bindings.patch_PatchUser(sess, body=patch_user, userId=test_user.id)
patched_user = bindings.get_GetUser(sess, userId=test_user.id).user
assert patched_user is not None and patched_user.agentUserGroup is not None
assert patched_user.agentUserGroup.agentUser == "username"
assert patched_user.agentUserGroup.agentGroup == "groupname"

# Patch - missing username/groupname.
patch_user = bindings.v1PatchUser(
agentUserGroup=bindings.v1AgentUserGroup(agentGid=1000, agentUid=1000)
)
test_user = _fetch_user_by_username(sess, test_username)
assert test_user.id
with pytest.raises(errors.APIException):
bindings.patch_PatchUser(sess, body=patch_user, userId=test_user.id)
9 changes: 6 additions & 3 deletions e2e_tests/tests/experiment/experiment.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,14 @@ def experiment_first_trial(exp_id: int) -> int:
return trial_id


def determined_test_session() -> api.Session:
def determined_test_session(admin: bool = False) -> api.Session:
murl = conf.make_master_url()
certs.cli_cert = certs.default_load(murl)
authentication.cli_auth = authentication.Authentication(murl, try_reauth=True)
return api.Session(murl, "determined", authentication.cli_auth, certs.cli_cert)
username = "admin" if admin else "determined"
authentication.cli_auth = authentication.Authentication(
murl, requested_user=username, password="", try_reauth=True
)
return api.Session(murl, username, authentication.cli_auth, certs.cli_cert)


def experiment_config_json(experiment_id: int) -> Dict[str, Any]:
Expand Down
8 changes: 8 additions & 0 deletions harness/determined/common/api/bindings.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,22 +478,30 @@ def __init__(
self,
*,
agentGid: "typing.Optional[int]" = None,
agentGroup: "typing.Optional[str]" = None,
agentUid: "typing.Optional[int]" = None,
agentUser: "typing.Optional[str]" = None,
):
self.agentUid = agentUid
self.agentGid = agentGid
self.agentUser = agentUser
self.agentGroup = agentGroup

@classmethod
def from_json(cls, obj: Json) -> "v1AgentUserGroup":
return cls(
agentUid=obj.get("agentUid", None),
agentGid=obj.get("agentGid", None),
agentUser=obj.get("agentUser", None),
agentGroup=obj.get("agentGroup", None),
)

def to_json(self) -> typing.Any:
return {
"agentUid": self.agentUid if self.agentUid is not None else None,
"agentGid": self.agentGid if self.agentGid is not None else None,
"agentUser": self.agentUser if self.agentUser is not None else None,
"agentGroup": self.agentGroup if self.agentGroup is not None else None,
}

class v1AggregateQueueStats:
Expand Down
33 changes: 26 additions & 7 deletions master/internal/api_user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ var errUserNotFound = status.Error(codes.NotFound, "user not found")

func toProtoUserFromFullUser(user model.FullUser) *userv1.User {
var agentUserGroup *userv1.AgentUserGroup
if user.AgentUID.Valid || user.AgentGID.Valid {
if user.AgentUID.Valid || user.AgentGID.Valid || user.AgentUser.Valid || user.AgentGroup.Valid {
agentUserGroup = &userv1.AgentUserGroup{
AgentUid: int32(user.AgentUID.ValueOrZero()),
AgentGid: int32(user.AgentGID.ValueOrZero()),
AgentUid: int32(user.AgentUID.ValueOrZero()),
AgentGid: int32(user.AgentGID.ValueOrZero()),
AgentUser: user.AgentUser.ValueOrZero(),
AgentGroup: user.AgentGroup.ValueOrZero(),
}
}
displayNameString := user.DisplayName.ValueOrZero()
Expand Down Expand Up @@ -166,8 +168,16 @@ func (a *apiServer) PostUser(
var agentUserGroup *model.AgentUserGroup
if req.User.AgentUserGroup != nil {
agentUserGroup = &model.AgentUserGroup{
UID: int(req.User.AgentUserGroup.AgentUid),
GID: int(req.User.AgentUserGroup.AgentGid),
UID: int(req.User.AgentUserGroup.AgentUid),
GID: int(req.User.AgentUserGroup.AgentGid),
User: req.User.AgentUserGroup.AgentUser,
Group: req.User.AgentUserGroup.AgentGroup,
}
if agentUserGroup.User == "" || agentUserGroup.Group == "" {
return nil, status.Error(
codes.InvalidArgument,
"AgentUser and AgentGroup names cannot be empty",
)
}
}

Expand Down Expand Up @@ -334,8 +344,10 @@ func (a *apiServer) PatchUser(
var ug *model.AgentUserGroup
if pug := req.User.AgentUserGroup; pug != nil {
ug = &model.AgentUserGroup{
UID: int(req.User.AgentUserGroup.AgentUid),
GID: int(req.User.AgentUserGroup.AgentGid),
UID: int(req.User.AgentUserGroup.AgentUid),
GID: int(req.User.AgentUserGroup.AgentGid),
User: req.User.AgentUserGroup.AgentUser,
Group: req.User.AgentUserGroup.AgentGroup,
}
if err = user.AuthZProvider.Get().
CanSetUsersAgentUserGroup(*curUser, targetUser, *ug); err != nil {
Expand All @@ -347,6 +359,13 @@ func (a *apiServer) PatchUser(
}
return nil, status.Error(codes.PermissionDenied, err.Error())
}

if ug.User == "" || ug.Group == "" {
return nil, status.Error(
codes.InvalidArgument,
"AgentUser and AgentGroup names cannot be empty",
)
}
}

switch err = a.m.db.UpdateUser(&targetUser, toUpdate, ug); {
Expand Down
18 changes: 14 additions & 4 deletions master/internal/api_user_intg_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,23 @@ func TestAuthzPostUser(t *testing.T) {
expectedErr := status.Error(codes.PermissionDenied, "canCreateUserError")
authzUsers.On("CanCreateUser", curUser,
model.User{Username: "admin", Admin: true},
&model.AgentUserGroup{UID: 5, GID: 6}).Return(fmt.Errorf("canCreateUserError")).Once()
&model.AgentUserGroup{
UID: 5,
GID: 6,
User: "five",
Group: "six",
}).Return(fmt.Errorf("canCreateUserError")).Once()

_, err := api.PostUser(ctx, &apiv1.PostUserRequest{
User: &userv1.User{
Username: "admin",
Admin: true,
AgentUserGroup: &userv1.AgentUserGroup{AgentUid: 5, AgentGid: 6},
Username: "admin",
Admin: true,
AgentUserGroup: &userv1.AgentUserGroup{
AgentUid: 5,
AgentGid: 6,
AgentUser: "five",
AgentGroup: "six",
},
},
})
require.Equal(t, expectedErr.Error(), err.Error())
Expand Down
4 changes: 4 additions & 0 deletions proto/src/determined/user/v1/user.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ message AgentUserGroup {
int32 agent_uid = 1;
// The group id on the agent.
int32 agent_gid = 2;
// User name.
string agent_user = 3;
// Group name.
string agent_group = 4;
}

// UserWebSetting represents user web setting.
Expand Down
12 changes: 12 additions & 0 deletions webui/react/src/services/api-ts-sdk/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,18 @@ export interface V1AgentUserGroup {
* @memberof V1AgentUserGroup
*/
agentGid?: number;
/**
* User name.
* @type {string}
* @memberof V1AgentUserGroup
*/
agentUser?: string;
/**
* Group name.
* @type {string}
* @memberof V1AgentUserGroup
*/
agentGroup?: string;
}

/**
Expand Down

0 comments on commit 2bca637

Please sign in to comment.