Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement stringer interface for UserID #289

Merged
merged 2 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@
//Users
case "getUser":
var config interface{}
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 470 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
config, err = proxmox.NewConfigUserFromApi(userId, c)
failError(err)
Expand All @@ -490,37 +490,37 @@
log.Printf("Error: Userid and Password required")
os.Exit(1)
}
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 493 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
err = proxmox.ConfigUser{
Password: proxmox.UserPassword(flag.Args()[2]),
User: userId,
}.UpdateUserPassword(c)
failError(err)
fmt.Printf("Password of User %s updated\n", userId.ToString())
fmt.Printf("Password of User %s updated\n", userId.String())

case "setUser":
var password proxmox.UserPassword
config, err := proxmox.NewConfigUserFromJson(GetConfig(*fConfigFile))
failError(err)
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 506 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
if len(flag.Args()) > 2 {
password = proxmox.UserPassword(flag.Args()[2])
}
failError(config.SetUser(userId, password, c))
log.Printf("User %s has been configured\n", userId.ToString())
log.Printf("User %s has been configured\n", userId.String())

case "deleteUser":
if len(flag.Args()) < 2 {
log.Printf("Error: userId required")
os.Exit(1)
}
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 519 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
err = proxmox.ConfigUser{User: userId}.DeleteUser(c)
failError(err)
fmt.Printf("User %s removed\n", userId.ToString())
fmt.Printf("User %s removed\n", userId.String())

//ACME Account
case "getAcmeAccountList":
Expand Down Expand Up @@ -926,9 +926,9 @@
failError(fmt.Errorf("error: invoke with <vmID> <node> <diskID [<forceRemoval: false|true>]"))
}

vmIdUnparsed := flag.Args()[1]

Check failure on line 929 in main.go

View workflow job for this annotation

GitHub Actions / audit

var vmIdUnparsed should be vmIDUnparsed
node := flag.Args()[2]
vmId, err := strconv.Atoi(vmIdUnparsed)

Check failure on line 931 in main.go

View workflow job for this annotation

GitHub Actions / audit

var vmId should be vmID
if err != nil {
failError(fmt.Errorf("failed to convert vmId: %s to a string, error: %+v", vmIdUnparsed, err))
}
Expand Down
2 changes: 1 addition & 1 deletion proxmox/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ func (c *Client) GetUserPermissions(id UserID, path string) (permissions []strin
if !existence {
return nil, fmt.Errorf("cannot get user (%s) permissions, the user does not exist", id)
}
permlist, err := c.GetItemList("/access/permissions?userid=" + id.ToString() + "&path=" + path)
permlist, err := c.GetItemList("/access/permissions?userid=" + id.String() + "&path=" + path)
failError(err)
data := permlist["data"].(map[string]interface{})
for pth, prm := range data {
Expand Down
6 changes: 3 additions & 3 deletions proxmox/config_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func (group GroupName) removeAllUsersFromGroupExcept(allUsers []interface{}, mem
}
var userInMembers bool
for _, ee := range *members {
if params["userid"] == ee.ToString() {
if params["userid"] == ee.String() {
userInMembers = true
break
}
Expand Down Expand Up @@ -324,7 +324,7 @@ func (group GroupName) usersToAddToGroup(allUsers []interface{}, members *[]User
continue
}
for ii, ee := range *members {
if params["userid"] == ee.ToString() {
if params["userid"] == ee.String() {
var groups []GroupName
if _, isSet := params["groups"]; isSet {
groups = GroupName("").csvToArray(params["groups"].(string))
Expand All @@ -350,7 +350,7 @@ func (group GroupName) usersToRemoveFromGroup(allUsers []interface{}, members *[
continue
}
for ii, ee := range *members {
if params["userid"] == ee.ToString() {
if params["userid"] == ee.String() {
var groups []GroupName
if _, isSet := params["groups"]; isSet {
groups = GroupName("").csvToArray(params["groups"].(string))
Expand Down
33 changes: 19 additions & 14 deletions proxmox/config_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ func (config ConfigUser) DeleteUser(client *Client) (err error) {
return
}
if !existence {
return fmt.Errorf("user (%s) could not be deleted, the user does not exist", config.User.ToString())
return fmt.Errorf("user (%s) could not be deleted, the user does not exist", config.User.String())
}
// Proxmox silently fails a user delete if the users does not exist
return client.Delete("/access/users/" + config.User.ToString())
return client.Delete("/access/users/" + config.User.String())
}

// Maps the struct to the API values proxmox understands
Expand All @@ -65,7 +65,7 @@ func (config ConfigUser) mapToApiValues(create bool) (params map[string]interfac
}
if create {
params["password"] = config.Password
params["userid"] = config.User.ToString()
params["userid"] = config.User.String()
}
return
}
Expand Down Expand Up @@ -150,7 +150,7 @@ func (config *ConfigUser) SetUser(userId UserID, password UserPassword, client *

func (config *ConfigUser) UpdateUser(client *Client) (err error) {
params := config.mapToApiValues(false)
err = client.Put(params, "/access/users/"+config.User.ToString())
err = client.Put(params, "/access/users/"+config.User.String())
if err != nil {
params, _ := json.Marshal(&params)
return fmt.Errorf("error updating User: %v, (params: %v)", err, string(params))
Expand All @@ -167,7 +167,7 @@ func (config ConfigUser) UpdateUserPassword(client *Client) (err error) {
return err
}
return client.Put(map[string]interface{}{
"userid": config.User.ToString(),
"userid": config.User.String(),
"password": config.Password,
}, "/access/password")
}
Expand Down Expand Up @@ -219,7 +219,7 @@ func (config ConfigUser) CreateApiToken(client *Client, token ApiToken) (value s
"comment": token.Comment,
"expire": token.Expire,
"privsep": token.Privsep,
}, "/access/users/"+config.User.ToString()+"/token/"+token.TokenId)
}, "/access/users/"+config.User.String()+"/token/"+token.TokenId)
if err != nil {
return
}
Expand All @@ -234,12 +234,12 @@ func (config ConfigUser) UpdateApiToken(client *Client, token ApiToken) (err err
"comment": token.Comment,
"expire": token.Expire,
"privsep": token.Privsep,
}, "/access/users/"+config.User.ToString()+"/token/"+token.TokenId)
}, "/access/users/"+config.User.String()+"/token/"+token.TokenId)
return
}

func (config ConfigUser) ListApiTokens(client *Client) (tokens *[]ApiToken, err error) {
status, err := client.GetItemListInterfaceArray("/access/users/" + config.User.ToString() + "/token")
status, err := client.GetItemListInterfaceArray("/access/users/" + config.User.String() + "/token")
if err != nil {
return
}
Expand All @@ -248,7 +248,7 @@ func (config ConfigUser) ListApiTokens(client *Client) (tokens *[]ApiToken, err
}

func (config ConfigUser) DeleteApiToken(client *Client, token ApiToken) (err error) {
err = client.Delete("/access/users/" + config.User.ToString() + "/token/" + token.TokenId)
err = client.Delete("/access/users/" + config.User.String() + "/token/" + token.TokenId)
return
}

Expand Down Expand Up @@ -333,13 +333,18 @@ func (UserID) mapToStruct(userId string) UserID {

// Converts the userID to "username@realm"
// Returns an empty string when either the Name or Realm is empty
func (id UserID) ToString() string {
func (id UserID) String() string {
if id.Name == "" || id.Realm == "" {
return ""
}
return id.Name + "@" + id.Realm
}

// deprecated use String() instead
func (id UserID) ToString() string {
return id.String()
}

// TODO improve when Name and Realm have their own types
func (id UserID) Validate() error {
if id.Name == "" {
Expand Down Expand Up @@ -369,9 +374,9 @@ func CheckUserExistence(userId UserID, client *Client) (existence bool, err erro
}
// This should be the case where you have an API Token with privilege separation but no permissions attached
if len(list) == 0 {
return false, fmt.Errorf("user %s has valid credentials but cannot retrieve user list, check privilege separation of api token", userId.ToString())
return false, fmt.Errorf("user %s has valid credentials but cannot retrieve user list, check privilege separation of api token", userId.String())
}
existence = ItemInKeyOfArray(list, "userid", userId.ToString())
existence = ItemInKeyOfArray(list, "userid", userId.String())
return
}

Expand Down Expand Up @@ -403,7 +408,7 @@ func listUsersFull(client *Client) ([]interface{}, error) {
}

func NewConfigUserFromApi(userId UserID, client *Client) (*ConfigUser, error) {
userConfig, err := client.GetItemConfigMapStringInterface("/access/users/"+userId.ToString(), "user", "CONFIG")
userConfig, err := client.GetItemConfigMapStringInterface("/access/users/"+userId.String(), "user", "CONFIG")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -451,5 +456,5 @@ func NewUserIDs(userIds string) (*[]UserID, error) {

// URL for updating users
func updateUser(user UserID, params map[string]interface{}, client *Client) error {
return client.Put(params, "/access/users/"+user.ToString())
return client.Put(params, "/access/users/"+user.String())
}
4 changes: 2 additions & 2 deletions proxmox/config_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ func Test_UserID_mapToStruct(t *testing.T) {
}

// TODO improve test when a validation function for the UserID exists
func Test_UserID_ToString(t *testing.T) {
func Test_UserID_String(t *testing.T) {
testData := []struct {
input UserID
Output string
Expand All @@ -499,7 +499,7 @@ func Test_UserID_ToString(t *testing.T) {
{input: UserID{}},
}
for _, e := range testData {
require.Equal(t, e.Output, e.input.ToString())
require.Equal(t, e.Output, e.input.String())
}
}

Expand Down
10 changes: 5 additions & 5 deletions test/cli/Users/user_sub_tests/user_sub_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@ import (
func Cleanup(t *testing.T, user proxmox.UserID) {
Test := cliTest.Test{
ReqErr: true,
ErrContains: user.ToString(),
Args: []string{"-i", "delete", "user", user.ToString()},
ErrContains: user.String(),
Args: []string{"-i", "delete", "user", user.String()},
}
Test.StandardTest(t)
}

// Default DELETE test for User
func Delete(t *testing.T, user proxmox.UserID) {
Test := cliTest.Test{
Contains: []string{user.ToString()},
Args: []string{"-i", "delete", "user", user.ToString()},
Contains: []string{user.String()},
Args: []string{"-i", "delete", "user", user.String()},
}
Test.StandardTest(t)
}

// Default SET test for User
func Set(t *testing.T, user proxmox.ConfigUser) {
userID := user.User.ToString()
userID := user.User.String()
user.User = proxmox.UserID{}
Test := cliTest.Test{
InputJson: user,
Expand Down
Loading