This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new e6eb7521c9 Make role name mutable (#6959)
e6eb7521c9 is described below
commit e6eb7521c949d128fc2d522e93b02ab7b1297ffa
Author: Eric Holguin <[email protected]>
AuthorDate: Fri Jul 15 12:06:22 2022 -0600
Make role name mutable (#6959)
* Make role name mutable, add parseDBerror functionality to create and
update
* Remove unused role function helper
* Add space validation for role name to match TP
* Increase role test coverage
* Add the space validation to v3
* Fix v4 tests
* Fix v3 tests
* Update tests
* Fix internal server error when v3 update and delete id doesnt exist
* Fix unit test
* remove space validation and tests
* Remove TP space validation for role names
* Address comments
---
traffic_ops/testing/api/v3/roles_test.go | 142 +++++++++++++++++++--
traffic_ops/testing/api/v3/tc-fixtures.json | 9 ++
traffic_ops/testing/api/v4/roles_test.go | 123 ++++++++++++++++--
traffic_ops/testing/api/v4/tc-fixtures.json | 8 ++
.../traffic_ops_golang/dbhelpers/db_helpers.go | 19 +--
traffic_ops/traffic_ops_golang/role/roles.go | 69 +++++-----
.../common/modules/form/role/FormRoleController.js | 2 +-
.../common/modules/form/role/form.role.tpl.html | 2 +-
8 files changed, 308 insertions(+), 66 deletions(-)
diff --git a/traffic_ops/testing/api/v3/roles_test.go
b/traffic_ops/testing/api/v3/roles_test.go
index 5a93ecd50f..f240367687 100644
--- a/traffic_ops/testing/api/v3/roles_test.go
+++ b/traffic_ops/testing/api/v3/roles_test.go
@@ -75,13 +75,50 @@ func TestRoles(t *testing.T) {
},
Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
+ "BAD REQUEST when MISSING NAME": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "description": "missing name",
+ "privLevel": 30,
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when MISSING DESCRIPTION": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "nodescription",
+ "privLevel": 30,
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when ROLE NAME ALREADY EXISTS": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "new_admin",
+ "description": "description",
+ "privLevel": 30,
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
},
"PUT": {
"OK when VALID request": {
- EndpointId: GetRoleID(t,
"another_role"),
+ EndpointId: GetRoleID(t,
"update_role"),
ClientSession: TOSession,
RequestBody: map[string]interface{}{
- "name": "another_role",
+ "name": "new_name",
"description": "new updated
description",
"privLevel": 30,
"capabilities": []string{
@@ -90,7 +127,75 @@ func TestRoles(t *testing.T) {
},
},
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-
validateRoleUpdateCreateFields("another_role",
map[string]interface{}{"Description": "new updated description"})),
+
validateRoleUpdateCreateFields("new_name", map[string]interface{}{"Name":
"new_name", "Description": "new updated description"})),
+ },
+ "BAD REQUEST when MISSING NAME": {
+ EndpointId: GetRoleID(t,
"another_role"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "description": "missing name",
+ "privLevel": 30,
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when MISSING DESCRIPTION": {
+ EndpointId: GetRoleID(t,
"another_role"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "noDescription",
+ "privLevel": 30,
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when ADMIN ROLE": {
+ EndpointId: GetRoleID(t, "admin"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "adminUpdated",
+ "privLevel": 30,
+ "description": "description",
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "NOT FOUND when ROLE DOESNT EXIST": {
+ EndpointId: func() int { return
9999999 },
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "doesntexist",
+ "privLevel": 30,
+ "description": "description",
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)),
+ },
+ "BAD REQUEST when ROLE NAME ALREADY EXISTS": {
+ EndpointId: GetRoleID(t,
"another_role"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "new_admin",
+ "privLevel": 30,
+ "description": "description",
+ "capabilities": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"PRECONDITION FAILED when updating with IMS &
IUS Headers": {
EndpointId: GetRoleID(t,
"another_role"),
@@ -193,9 +298,14 @@ func validateRoleFields(expectedResp
map[string]interface{}) utils.CkReqFunc {
for _, role := range roleResp {
switch field {
case "Name":
+ assert.RequireNotNil(t, role.Name,
"Expected Name to not be nil.")
assert.Equal(t, expected, *role.Name,
"Expected Name to be %v, but got %s", expected, *role.Name)
case "Description":
+ assert.RequireNotNil(t,
role.Description, "Expected Description to not be nil.")
assert.Equal(t, expected,
*role.Description, "Expected Description to be %v, but got %s", expected,
*role.Description)
+ case "PrivLevel":
+ assert.RequireNotNil(t, role.PrivLevel,
"Expected PrivLevel to not be nil.")
+ assert.Equal(t, expected,
*role.PrivLevel, "Expected PrivLevel to be %v, but got %d", expected,
*role.PrivLevel)
default:
t.Errorf("Expected field: %v, does not
exist in response", field)
}
@@ -219,6 +329,7 @@ func validateRoleSort() utils.CkReqFunc {
var roleNames []string
roleResp := resp.([]tc.Role)
for _, role := range roleResp {
+ assert.RequireNotNil(t, role.Name, "Expected Name to
not be nil.")
roleNames = append(roleNames, *role.Name)
}
assert.Equal(t, true, sort.StringsAreSorted(roleNames), "List
is not sorted by their names: %v", roleNames)
@@ -239,10 +350,12 @@ func validateRoleDescSort() utils.CkReqFunc {
assert.RequireEqual(t, len(roleAscResp), len(roleDescResp),
"Expected descending order response length: %d, to match ascending order
response length %d", len(roleAscResp), len(roleDescResp))
// Insert Role names to the front of a new list, so they are
now reversed to be in ascending order.
for _, role := range roleDescResp {
+ assert.RequireNotNil(t, role.Name, "Expected Name to
not be nil.")
descSortedList = append([]string{*role.Name},
descSortedList...)
}
// Insert Role names by appending to a new list, so they stay
in ascending order.
for _, role := range roleAscResp {
+ assert.RequireNotNil(t, role.Name, "Expected Name to
not be nil.")
ascSortedList = append(ascSortedList, *role.Name)
}
assert.Exactly(t, ascSortedList, descSortedList, "Role
responses are not equal after reversal: %v - %v", ascSortedList, descSortedList)
@@ -254,6 +367,8 @@ func GetRoleID(t *testing.T, name string) func() int {
role, _, _, err := TOSession.GetRoleByNameWithHdr(name, nil)
assert.RequireNoError(t, err, "Get Roles Request failed with
error:", err)
assert.RequireEqual(t, 1, len(role), "Expected response object
length 1, but got %d", len(role))
+ assert.RequireNotNil(t, role, "Expected role to not be nil.")
+ assert.RequireNotNil(t, role[0].ID, "Expected ID to not be
nil.")
return *role[0].ID
}
}
@@ -266,13 +381,20 @@ func CreateTestRoles(t *testing.T) {
}
func DeleteTestRoles(t *testing.T) {
- for _, r := range testData.Roles {
- roleID := GetRoleID(t, *r.Name)()
- _, _, _, err := TOSession.DeleteRoleByID(roleID)
- assert.NoError(t, err, "Expected no error while deleting role
%s, but got %v", *r.Name, err)
+ roles, _, _, err := TOSession.GetRolesWithHdr(nil)
+ assert.NoError(t, err, "Cannot get Roles: %v", err)
+ for _, role := range roles {
+ // Don't delete active roles created by test setup
+ assert.RequireNotNil(t, role.Name, "Expected Name to not be
nil.")
+ assert.RequireNotNil(t, role.ID, "Expected ID to not be nil.")
+ if *role.Name == "admin" || *role.Name == "disallowed" ||
*role.Name == "operations" || *role.Name == "portal" || *role.Name ==
"read-only" || *role.Name == "steering" || *role.Name == "federation" {
+ continue
+ }
+ _, _, _, err := TOSession.DeleteRoleByID(*role.ID)
+ assert.NoError(t, err, "Expected no error while deleting role
%s, but got %v", *role.Name, err)
// Retrieve the Role to see if it got deleted
- getRole, _, _, err := TOSession.GetRoleByIDWithHdr(roleID, nil)
- assert.NoError(t, err, "Error getting Role '%s' after deletion:
%v", *r.Name, err)
- assert.Equal(t, 0, len(getRole), "Expected Role '%s' to be
deleted, but it was found in Traffic Ops", *r.Name)
+ getRole, _, _, err := TOSession.GetRoleByIDWithHdr(*role.ID,
nil)
+ assert.NoError(t, err, "Error getting Role '%s' after deletion:
%v", *role.Name, err)
+ assert.Equal(t, 0, len(getRole), "Expected Role '%s' to be
deleted, but it was found in Traffic Ops", *role.Name)
}
}
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json
b/traffic_ops/testing/api/v3/tc-fixtures.json
index c10c8018bc..8ec7462391 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -2422,6 +2422,15 @@
"all-read",
"all-write"
]
+ },
+ {
+ "name": "update_role",
+ "description": "role for testing update",
+ "privLevel": 30,
+ "capabilities": [
+ "all-read",
+ "all-write"
+ ]
}
],
"servers": [
diff --git a/traffic_ops/testing/api/v4/roles_test.go
b/traffic_ops/testing/api/v4/roles_test.go
index ba87e20226..c5124b4705 100644
--- a/traffic_ops/testing/api/v4/roles_test.go
+++ b/traffic_ops/testing/api/v4/roles_test.go
@@ -61,12 +61,48 @@ func TestRoles(t *testing.T) {
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
validateRoleDescSort()),
},
},
+ "POST": {
+ "BAD REQUEST when MISSING NAME": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "description": "missing name",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when MISSING DESCRIPTION": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "noDescription",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when ROLE NAME ALREADY EXISTS": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "name": "new_admin",
+ "description": "description",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ },
"PUT": {
"OK when VALID request": {
ClientSession: TOSession,
- RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"another_role"}}},
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"update_role"}}},
RequestBody: map[string]interface{}{
- "name": "another_role",
+ "name": "new_name",
"description": "new updated
description",
"permissions": []string{
"all-read",
@@ -74,7 +110,70 @@ func TestRoles(t *testing.T) {
},
},
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-
validateRoleUpdateCreateFields("another_role",
map[string]interface{}{"Description": "new updated description"})),
+
validateRoleUpdateCreateFields("new_name", map[string]interface{}{"Name":
"new_name", "Description": "new updated description"})),
+ },
+ "BAD REQUEST when MISSING NAME": {
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"another_role"}}},
+ RequestBody: map[string]interface{}{
+ "description": "missing name",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when MISSING DESCRIPTION": {
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"another_role"}}},
+ RequestBody: map[string]interface{}{
+ "name": "noDescription",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when ADMIN ROLE": {
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"admin"}}},
+ RequestBody: map[string]interface{}{
+ "name": "adminUpdated",
+ "description": "description",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "NOT FOUND when ROLE DOESNT EXIST": {
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"doesntexist"}}},
+ RequestBody: map[string]interface{}{
+ "name": "doesntexist",
+ "description": "description",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)),
+ },
+ "BAD REQUEST when ROLE NAME ALREADY EXISTS": {
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name": {"another_role"}}},
+ RequestBody: map[string]interface{}{
+ "name": "new_admin",
+ "description": "description",
+ "permissions": []string{
+ "all-read",
+ "all-write",
+ },
+ },
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"PRECONDITION FAILED when updating with IMS &
IUS Headers": {
ClientSession: TOSession,
@@ -239,14 +338,20 @@ func CreateTestRoles(t *testing.T) {
}
func DeleteTestRoles(t *testing.T) {
- for _, r := range testData.Roles {
- _, _, err := TOSession.DeleteRole(r.Name,
client.NewRequestOptions())
- assert.NoError(t, err, "Expected no error while deleting role
%s, but got %v", r.Name, err)
+ roles, _, err := TOSession.GetRoles(client.RequestOptions{})
+ assert.NoError(t, err, "Cannot get Roles: %v - alerts: %+v", err,
roles.Alerts)
+ for _, role := range roles.Response {
+ // Don't delete active roles created by test setup
+ if role.Name == "admin" || role.Name == "disallowed" ||
role.Name == "operations" || role.Name == "portal" || role.Name == "read-only"
|| role.Name == "steering" || role.Name == "federation" {
+ continue
+ }
+ _, _, err := TOSession.DeleteRole(role.Name,
client.NewRequestOptions())
+ assert.NoError(t, err, "Expected no error while deleting role
%s, but got %v", role.Name, err)
// Retrieve the Role to see if it got deleted
opts := client.NewRequestOptions()
- opts.QueryParameters.Set("name", r.Name)
+ opts.QueryParameters.Set("name", role.Name)
getRole, _, err := TOSession.GetRoles(opts)
- assert.NoError(t, err, "Error getting Role '%s' after deletion:
%v - alerts: %+v", r.Name, err, getRole.Alerts)
- assert.Equal(t, 0, len(getRole.Response), "Expected Role '%s'
to be deleted, but it was found in Traffic Ops", r.Name)
+ assert.NoError(t, err, "Error getting Role '%s' after deletion:
%v - alerts: %+v", role.Name, err, getRole.Alerts)
+ assert.Equal(t, 0, len(getRole.Response), "Expected Role '%s'
to be deleted, but it was found in Traffic Ops", role.Name)
}
}
diff --git a/traffic_ops/testing/api/v4/tc-fixtures.json
b/traffic_ops/testing/api/v4/tc-fixtures.json
index 52bbf385a1..524c11f14d 100644
--- a/traffic_ops/testing/api/v4/tc-fixtures.json
+++ b/traffic_ops/testing/api/v4/tc-fixtures.json
@@ -2572,6 +2572,14 @@
"all-read",
"all-write"
]
+ },
+ {
+ "name": "update_role",
+ "description": "role for testing update",
+ "permissions": [
+ "all-read",
+ "all-write"
+ ]
}
],
"servers": [
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 7f6407748f..2be3f6cf88 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -542,6 +542,13 @@ func GetCapabilitiesFromRoleName(tx *sql.Tx, role string)
([]string, error) {
return caps, nil
}
+// RoleExists returns whether or not the role with the given roleName exists,
and any error that occurred.
+func RoleExists(tx *sql.Tx, roleID int) (bool, error) {
+ exists := false
+ err := tx.QueryRow(`SELECT EXISTS(SELECT * FROM role WHERE role.id =
$1)`, roleID).Scan(&exists)
+ return exists, err
+}
+
// GetDSNameFromID loads the DeliveryService's xml_id from the database, from
the ID. Returns whether the delivery service was found, and any error.
func GetDSNameFromID(tx *sql.Tx, id int) (tc.DeliveryServiceName, bool, error)
{
name := tc.DeliveryServiceName("")
@@ -1805,18 +1812,6 @@ func GetRoleIDFromName(tx *sql.Tx, roleName string)
(int, bool, error) {
return id, true, nil
}
-// GetRoleNameFromID returns the name of the role associated with the supplied
ID.
-func GetRoleNameFromID(tx *sql.Tx, roleID int) (string, bool, error) {
- var name string
- if err := tx.QueryRow(`SELECT name FROM role WHERE id = $1`,
roleID).Scan(&name); err != nil {
- if errors.Is(err, sql.ErrNoRows) {
- return name, false, nil
- }
- return name, false, fmt.Errorf("querying role name from ID:
%w", err)
- }
- return name, true, nil
-}
-
// GetCDNNameDomain returns the name and domain for a given CDN ID.
func GetCDNNameDomain(cdnID int, tx *sql.Tx) (string, string, error) {
q := `SELECT cdn.name, cdn.domain_name from cdn where cdn.id = $1`
diff --git a/traffic_ops/traffic_ops_golang/role/roles.go
b/traffic_ops/traffic_ops_golang/role/roles.go
index 3a0e3186f9..b5a4afd75d 100644
--- a/traffic_ops/traffic_ops_golang/role/roles.go
+++ b/traffic_ops/traffic_ops_golang/role/roles.go
@@ -58,19 +58,12 @@ type TORole struct {
PQCapabilities *pq.StringArray `json:"-" db:"capabilities"`
}
-func updateLegacyRoleQuery() string {
+func updateRoleQuery() string {
return `UPDATE
role SET
name=$1,
description=$2
-WHERE id=$3 RETURNING last_updated`
-}
-
-func updateRoleQuery() string {
- return `UPDATE
-role SET
-description=$1
-WHERE name=$2 RETURNING last_updated`
+WHERE name=$3 RETURNING last_updated`
}
func (v *TORole) GetLastUpdated() (*time.Time, bool, error) {
@@ -224,6 +217,13 @@ func (role *TORole) Read(h http.Header, useIMS bool)
([]interface{}, error, erro
}
func (role *TORole) Update(h http.Header) (error, error, int) {
+
+ if ok, err := dbhelpers.RoleExists(role.ReqInfo.Tx.Tx, *role.ID); err
!= nil {
+ return nil, fmt.Errorf("verifying Role exists: %w", err),
http.StatusInternalServerError
+ } else if !ok {
+ return errors.New("role not found"), nil, http.StatusNotFound
+ }
+
var isAdmin bool
if err := role.ReqInfo.Tx.Get(&isAdmin, isAdminQuery, role.ID); err !=
nil {
return nil, fmt.Errorf("checking if Role to be modified is
'%s': %w", tc.AdminRoleName, err), http.StatusInternalServerError
@@ -259,6 +259,13 @@ func (role *TORole) Update(h http.Header) (error, error,
int) {
}
func (role *TORole) Delete() (error, error, int) {
+
+ if ok, err := dbhelpers.RoleExists(role.ReqInfo.Tx.Tx, *role.ID); err
!= nil {
+ return nil, fmt.Errorf("verifying Role exists: %w", err),
http.StatusInternalServerError
+ } else if !ok {
+ return errors.New("role not found"), nil, http.StatusNotFound
+ }
+
assignedUsers := 0
if err := role.ReqInfo.Tx.Get(&assignedUsers, "SELECT COUNT(id) FROM
public.tm_user WHERE role=$1", role.ID); err != nil {
return nil, errors.New("role delete counting assigned users: "
+ err.Error()), http.StatusInternalServerError
@@ -332,12 +339,7 @@ func deleteQuery() string {
// Update will modify the role identified by the role name.
func Update(w http.ResponseWriter, r *http.Request) {
- var roleID int
- var roleDesc string
- var roleCapabilities []string
var roleV4 tc.RoleV4
- var ok bool
- var err error
inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"name"}, nil)
if userErr != nil || sysErr != nil {
@@ -352,24 +354,24 @@ func Update(w http.ResponseWriter, r *http.Request) {
return
}
- roleName := inf.Params["name"]
- if roleName == tc.AdminRoleName {
- api.HandleErr(w, r, tx, http.StatusBadRequest,
cannotModifyAdminError, nil)
+ if err := roleV4.Validate(); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
return
}
- if err := roleV4.Validate(); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ currentRoleName := inf.Params["name"]
+ if currentRoleName == tc.AdminRoleName {
+ api.HandleErr(w, r, tx, http.StatusBadRequest,
cannotModifyAdminError, nil)
return
}
+
missing := inf.User.MissingPermissions(roleV4.Permissions...)
if len(missing) != 0 {
api.HandleErr(w, r, tx, http.StatusForbidden,
fmt.Errorf("cannot request more than assigned permissions, current user needs
%s permissions", strings.Join(missing, ",")), nil)
return
}
- roleDesc = roleV4.Description
- roleCapabilities = roleV4.Permissions
- roleID, ok, err = dbhelpers.GetRoleIDFromName(tx, roleName)
+
+ roleID, ok, err := dbhelpers.GetRoleIDFromName(tx, currentRoleName)
if err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
return
@@ -378,7 +380,7 @@ func Update(w http.ResponseWriter, r *http.Request) {
return
}
- existingLastUpdated, found, err := api.GetLastUpdatedByName(inf.Tx,
roleName, "role")
+ existingLastUpdated, found, err := api.GetLastUpdatedByName(inf.Tx,
currentRoleName, "role")
if err == nil && found == false {
api.HandleErr(w, r, tx, http.StatusNotFound, errors.New("no
such role"), nil)
return
@@ -392,9 +394,10 @@ func Update(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, tx, http.StatusPreconditionFailed,
api.ResourceModifiedError, nil)
return
}
- rows, err := tx.Query(updateRoleQuery(), roleDesc, roleName)
+ rows, err := tx.Query(updateRoleQuery(), roleV4.Name,
roleV4.Description, currentRoleName)
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("updating role: %w", err))
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, fmt.Errorf("updating
role: %w", sysErr))
return
}
defer rows.Close()
@@ -411,26 +414,25 @@ func Update(w http.ResponseWriter, r *http.Request) {
roleV4.LastUpdated = &lastUpdated
}
- userErr, sysErr, errCode = deleteRoleCapabilityAssociations(inf.Tx,
roleName)
+ userErr, sysErr, errCode = deleteRoleCapabilityAssociations(inf.Tx,
roleV4.Name)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, tx, errCode, userErr, sysErr)
return
}
- userErr, sysErr, errCode = createRoleCapabilityAssociations(inf.Tx,
roleID, &roleCapabilities)
+ userErr, sysErr, errCode = createRoleCapabilityAssociations(inf.Tx,
roleID, &roleV4.Permissions)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, tx, errCode, userErr, sysErr)
return
}
alerts := tc.CreateAlerts(tc.SuccessLevel, "role was updated.")
var roleResponse interface{}
- capabilities := roleCapabilities
roleResponse = tc.RoleV4{
- Name: roleName,
- Permissions: capabilities,
- Description: roleDesc,
+ Name: roleV4.Name,
+ Permissions: roleV4.Permissions,
+ Description: roleV4.Description,
}
api.WriteAlertsObj(w, r, http.StatusOK, alerts, roleResponse)
- changeLogMsg := fmt.Sprintf("ROLE: %s, ID: %d, ACTION: Updated Role",
roleName, roleID)
+ changeLogMsg := fmt.Sprintf("ROLE: %s, ID: %d, ACTION: Updated Role",
roleV4.Name, roleID)
api.CreateChangeLogRawTx(api.ApiChange, changeLogMsg, inf.User, tx)
}
@@ -559,7 +561,8 @@ func Create(w http.ResponseWriter, r *http.Request) {
rows, err := tx.Query(createQuery(), roleName, roleDesc, privLevel)
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("creating role: %w", err))
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, fmt.Errorf("creating
role: %w", sysErr))
return
}
defer rows.Close()
diff --git
a/traffic_portal/app/src/common/modules/form/role/FormRoleController.js
b/traffic_portal/app/src/common/modules/form/role/FormRoleController.js
index dee50fadd4..c6010d09ba 100644
--- a/traffic_portal/app/src/common/modules/form/role/FormRoleController.js
+++ b/traffic_portal/app/src/common/modules/form/role/FormRoleController.js
@@ -46,7 +46,7 @@ var FormRoleController = function(roles, $scope, $location,
formUtils, locationU
if ($scope.role.name === "admin") {
_pattern = /^admin$/;
} else {
- _pattern = /^(?!admin$)\S+$/;
+ _pattern = /^(?!admin$).+$/;
}
}
return _pattern;
diff --git a/traffic_portal/app/src/common/modules/form/role/form.role.tpl.html
b/traffic_portal/app/src/common/modules/form/role/form.role.tpl.html
index 84fc0c48c8..1f0a5aeb33 100644
--- a/traffic_portal/app/src/common/modules/form/role/form.role.tpl.html
+++ b/traffic_portal/app/src/common/modules/form/role/form.role.tpl.html
@@ -37,7 +37,7 @@ under the License.
<div class="col-md-10 col-sm-10 col-xs-12">
<input name="name" id="name" type="text"
class="form-control" ng-model="role.name"
ng-readonly="!hasPropertyError(roleForm.name, 'pattern') && role.name ===
'admin'" ng-pattern="namePattern()" required autofocus/>
<small class="input-error"
ng-show="hasPropertyError(roleForm.name, 'required')">Required</small>
- <small class="input-error"
ng-show="hasPropertyError(roleForm.name, 'pattern')">No spaces, and cannot be
named "admin"</small>
+ <small class="input-error"
ng-show="hasPropertyError(roleForm.name, 'pattern')">Cannot be named
"admin"</small>
<span ng-show="hasError(roleForm.name)"
class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
