Extends management user and group structure (#1268)

* extends user and group structure by introducing fields for issued and integration references

* Add integration checks to group management to prevent groups added by integration.

* Add integration checks to user management to prevent deleting user added by integration.

* Fix broken user update tests

* Initialize all user fields for testing

* Change a serializer option to embedded for IntegrationReference in user and group models

* Add issued field to user api response

* Add IntegrationReference to Group in update groups handler

* Set the default issued field for users in file store
This commit is contained in:
Bethuel Mmbaga 2023-11-01 13:04:17 +03:00 committed by GitHub
parent 6d4240a5ae
commit c38d65ef4c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 188 additions and 61 deletions

View File

@ -36,6 +36,7 @@ const (
UnknownCategory = "unknown"
GroupIssuedAPI = "api"
GroupIssuedJWT = "jwt"
GroupIssuedIntegration = "integration"
CacheExpirationMax = 7 * 24 * 3600 * time.Second // 7 days
CacheExpirationMin = 3 * 24 * 3600 * time.Second // 3 days
DefaultPeerLoginExpiration = 24 * time.Hour
@ -195,15 +196,17 @@ type Account struct {
}
type UserInfo struct {
ID string `json:"id"`
Email string `json:"email"`
Name string `json:"name"`
Role string `json:"role"`
AutoGroups []string `json:"auto_groups"`
Status string `json:"-"`
IsServiceUser bool `json:"is_service_user"`
IsBlocked bool `json:"is_blocked"`
LastLogin time.Time `json:"last_login"`
ID string `json:"id"`
Email string `json:"email"`
Name string `json:"name"`
Role string `json:"role"`
AutoGroups []string `json:"auto_groups"`
Status string `json:"-"`
IsServiceUser bool `json:"is_service_user"`
IsBlocked bool `json:"is_blocked"`
LastLogin time.Time `json:"last_login"`
Issued string `json:"issued"`
IntegrationReference IntegrationReference `json:"-"`
}
// getRoutesToSync returns the enabled routes for the peer ID and the routes

View File

@ -133,6 +133,11 @@ func restore(file string) (*FileStore, error) {
}
for _, user := range account.Users {
store.UserID2AccountID[user.Id] = accountID
if user.Issued == "" {
user.Issued = UserIssuedAPI
account.Users[user.Id] = user
}
for _, pat := range user.PATs {
store.TokenID2UserID[pat.ID] = user.Id
store.HashedPAT2TokenID[pat.HashedToken] = pat.ID

View File

@ -34,6 +34,8 @@ type Group struct {
// Peers list of the group
Peers []string `gorm:"serializer:json"`
IntegrationReference IntegrationReference `gorm:"embedded;embeddedPrefix:integration_ref_"`
}
// EventMeta returns activity event meta related to the group
@ -160,6 +162,11 @@ func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string)
return nil
}
// check integration link
if g.Issued == GroupIssuedIntegration {
return &GroupLinkError{GroupIssuedIntegration, g.IntegrationReference.String()}
}
// check route links
for _, r := range account.Routes {
for _, g := range r.Groups {

View File

@ -52,6 +52,11 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
"grp-for-users",
"user",
},
{
"integration",
"grp-for-integration",
"integration",
},
}
for _, testCase := range testCases {
@ -79,43 +84,51 @@ func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
domain := "example.com"
groupForRoute := &Group{
"grp-for-route",
"account-id",
"Group for route",
GroupIssuedAPI,
make([]string, 0),
ID: "grp-for-route",
AccountID: "account-id",
Name: "Group for route",
Issued: GroupIssuedAPI,
Peers: make([]string, 0),
}
groupForNameServerGroups := &Group{
"grp-for-name-server-grp",
"account-id",
"Group for name server groups",
GroupIssuedAPI,
make([]string, 0),
ID: "grp-for-name-server-grp",
AccountID: "account-id",
Name: "Group for name server groups",
Issued: GroupIssuedAPI,
Peers: make([]string, 0),
}
groupForPolicies := &Group{
"grp-for-policies",
"account-id",
"Group for policies",
GroupIssuedAPI,
make([]string, 0),
ID: "grp-for-policies",
AccountID: "account-id",
Name: "Group for policies",
Issued: GroupIssuedAPI,
Peers: make([]string, 0),
}
groupForSetupKeys := &Group{
"grp-for-keys",
"account-id",
"Group for setup keys",
GroupIssuedAPI,
make([]string, 0),
ID: "grp-for-keys",
AccountID: "account-id",
Name: "Group for setup keys",
Issued: GroupIssuedAPI,
Peers: make([]string, 0),
}
groupForUsers := &Group{
"grp-for-users",
"account-id",
"Group for users",
GroupIssuedAPI,
make([]string, 0),
ID: "grp-for-users",
AccountID: "account-id",
Name: "Group for users",
Issued: GroupIssuedAPI,
Peers: make([]string, 0),
}
groupForIntegration := &Group{
ID: "grp-for-integration",
AccountID: "account-id",
Name: "Group for users",
Issued: GroupIssuedIntegration,
Peers: make([]string, 0),
}
routeResource := &route.Route{
@ -164,6 +177,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
_ = am.SaveGroup(accountID, groupAdminUserID, groupForPolicies)
_ = am.SaveGroup(accountID, groupAdminUserID, groupForSetupKeys)
_ = am.SaveGroup(accountID, groupAdminUserID, groupForUsers)
_ = am.SaveGroup(accountID, groupAdminUserID, groupForIntegration)
return am.Store.GetAccount(account.Id)
}

View File

@ -125,6 +125,10 @@ components:
description: Is true if this user is blocked. Blocked users can't use the system
type: boolean
example: false
issued:
description: How user was issued by API or Integration
type: string
example: api
required:
- id
- email

View File

@ -791,6 +791,9 @@ type User struct {
// IsServiceUser Is true if this user is a service user
IsServiceUser *bool `json:"is_service_user,omitempty"`
// Issued How user was issued by API or Integration
Issued *string `json:"issued,omitempty"`
// LastLogin Last time this user performed a login to the dashboard
LastLogin *time.Time `json:"last_login,omitempty"`

View File

@ -107,10 +107,11 @@ func (h *GroupsHandler) UpdateGroup(w http.ResponseWriter, r *http.Request) {
peers = *req.Peers
}
group := server.Group{
ID: groupID,
Name: req.Name,
Peers: peers,
Issued: eg.Issued,
ID: groupID,
Name: req.Name,
Peers: peers,
Issued: eg.Issued,
IntegrationReference: eg.IntegrationReference,
}
if err := h.accountManager.SaveGroup(account.Id, user.Id, &group); err != nil {

View File

@ -54,6 +54,12 @@ func (h *UsersHandler) UpdateUser(w http.ResponseWriter, r *http.Request) {
return
}
existingUser, ok := account.Users[userID]
if !ok {
util.WriteError(status.Errorf(status.NotFound, "couldn't find user with ID %s", userID), w)
return
}
req := &api.PutApiUsersUserIdJSONRequestBody{}
err = json.NewDecoder(r.Body).Decode(&req)
if err != nil {
@ -73,10 +79,12 @@ func (h *UsersHandler) UpdateUser(w http.ResponseWriter, r *http.Request) {
}
newUser, err := h.accountManager.SaveUser(account.Id, user.Id, &server.User{
Id: userID,
Role: userRole,
AutoGroups: req.AutoGroups,
Blocked: req.IsBlocked,
Id: userID,
Role: userRole,
AutoGroups: req.AutoGroups,
Blocked: req.IsBlocked,
Issued: existingUser.Issued,
IntegrationReference: existingUser.IntegrationReference,
})
if err != nil {
@ -153,6 +161,7 @@ func (h *UsersHandler) CreateUser(w http.ResponseWriter, r *http.Request) {
Role: req.Role,
AutoGroups: req.AutoGroups,
IsServiceUser: req.IsServiceUser,
Issued: server.UserIssuedAPI,
})
if err != nil {
util.WriteError(err, w)
@ -271,5 +280,6 @@ func toUserResponse(user *server.UserInfo, currenUserID string) *api.User {
IsServiceUser: &user.IsServiceUser,
IsBlocked: user.IsBlocked,
LastLogin: &user.LastLogin,
Issued: &user.Issued,
}
}

View File

@ -33,18 +33,21 @@ var usersTestAccount = &server.Account{
Role: "admin",
IsServiceUser: false,
AutoGroups: []string{"group_1"},
Issued: server.UserIssuedAPI,
},
regularUserID: {
Id: regularUserID,
Role: "user",
IsServiceUser: false,
AutoGroups: []string{"group_1"},
Issued: server.UserIssuedAPI,
},
serviceUserID: {
Id: serviceUserID,
Role: "user",
IsServiceUser: true,
AutoGroups: []string{"group_1"},
Issued: server.UserIssuedAPI,
},
},
}
@ -64,6 +67,7 @@ func initUsersTestData() *UsersHandler {
Name: "",
Email: "",
IsServiceUser: v.IsServiceUser,
Issued: v.Issued,
})
}
return users, nil
@ -170,6 +174,7 @@ func TestGetUsers(t *testing.T) {
assert.Equal(t, v.ID, usersTestAccount.Users[v.ID].Id)
assert.Equal(t, v.Role, string(usersTestAccount.Users[v.ID].Role))
assert.Equal(t, v.IsServiceUser, usersTestAccount.Users[v.ID].IsServiceUser)
assert.Equal(t, v.Issued, usersTestAccount.Users[v.ID].Issued)
}
})
}

View File

@ -22,6 +22,9 @@ const (
UserStatusActive UserStatus = "active"
UserStatusDisabled UserStatus = "disabled"
UserStatusInvited UserStatus = "invited"
UserIssuedAPI = "api"
UserIssuedIntegration = "integration"
)
// StrRoleToUserRole returns UserRole for a given strRole or UserRoleUnknown if the specified role is unknown
@ -42,6 +45,16 @@ type UserStatus string
// UserRole is the role of a User
type UserRole string
// IntegrationReference holds the reference to a particular integration
type IntegrationReference struct {
ID int
IntegrationType string
}
func (ir IntegrationReference) String() string {
return fmt.Sprintf("%d:%s", ir.ID, ir.IntegrationType)
}
// User represents a user of the system
type User struct {
Id string `gorm:"primaryKey"`
@ -59,6 +72,11 @@ type User struct {
Blocked bool
// LastLogin is the last time the user logged in to IdP
LastLogin time.Time
// Issued of the user
Issued string `gorm:"default:api"`
IntegrationReference IntegrationReference `gorm:"embedded;embeddedPrefix:integration_ref_"`
}
// IsBlocked returns true if the user is blocked, false otherwise
@ -93,6 +111,7 @@ func (u *User) ToUserInfo(userData *idp.UserData) (*UserInfo, error) {
IsServiceUser: u.IsServiceUser,
IsBlocked: u.Blocked,
LastLogin: u.LastLogin,
Issued: u.Issued,
}, nil
}
if userData.ID != u.Id {
@ -114,6 +133,7 @@ func (u *User) ToUserInfo(userData *idp.UserData) (*UserInfo, error) {
IsServiceUser: u.IsServiceUser,
IsBlocked: u.Blocked,
LastLogin: u.LastLogin,
Issued: u.Issued,
}, nil
}
@ -126,37 +146,40 @@ func (u *User) Copy() *User {
pats[k] = v.Copy()
}
return &User{
Id: u.Id,
AccountID: u.AccountID,
Role: u.Role,
AutoGroups: autoGroups,
IsServiceUser: u.IsServiceUser,
ServiceUserName: u.ServiceUserName,
PATs: pats,
Blocked: u.Blocked,
LastLogin: u.LastLogin,
Id: u.Id,
AccountID: u.AccountID,
Role: u.Role,
AutoGroups: autoGroups,
IsServiceUser: u.IsServiceUser,
ServiceUserName: u.ServiceUserName,
PATs: pats,
Blocked: u.Blocked,
LastLogin: u.LastLogin,
Issued: u.Issued,
IntegrationReference: u.IntegrationReference,
}
}
// NewUser creates a new user
func NewUser(id string, role UserRole, isServiceUser bool, serviceUserName string, autoGroups []string) *User {
func NewUser(id string, role UserRole, isServiceUser bool, serviceUserName string, autoGroups []string, issued string) *User {
return &User{
Id: id,
Role: role,
IsServiceUser: isServiceUser,
ServiceUserName: serviceUserName,
AutoGroups: autoGroups,
Issued: issued,
}
}
// NewRegularUser creates a new user with role UserRoleUser
func NewRegularUser(id string) *User {
return NewUser(id, UserRoleUser, false, "", []string{})
return NewUser(id, UserRoleUser, false, "", []string{}, UserIssuedAPI)
}
// NewAdminUser creates a new user with role UserRoleAdmin
func NewAdminUser(id string) *User {
return NewUser(id, UserRoleAdmin, false, "", []string{})
return NewUser(id, UserRoleAdmin, false, "", []string{}, UserIssuedAPI)
}
// createServiceUser creates a new service user under the given account.
@ -178,7 +201,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs
}
newUserID := uuid.New().String()
newUser := NewUser(newUserID, role, true, serviceUserName, autoGroups)
newUser := NewUser(newUserID, role, true, serviceUserName, autoGroups, UserIssuedAPI)
log.Debugf("New User: %v", newUser)
account.Users[newUserID] = newUser
@ -199,6 +222,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs
Status: string(UserStatusActive),
IsServiceUser: true,
LastLogin: time.Time{},
Issued: UserIssuedAPI,
}, nil
}
@ -270,9 +294,11 @@ func (am *DefaultAccountManager) inviteNewUser(accountID, userID string, invite
role := StrRoleToUserRole(invite.Role)
newUser := &User{
Id: idpUser.ID,
Role: role,
AutoGroups: invite.AutoGroups,
Id: idpUser.ID,
Role: role,
AutoGroups: invite.AutoGroups,
Issued: invite.Issued,
IntegrationReference: invite.IntegrationReference,
}
account.Users[idpUser.ID] = newUser
@ -361,6 +387,10 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t
return status.Errorf(status.NotFound, "target user not found")
}
if targetUser.Issued == UserIssuedIntegration {
return status.Errorf(status.PermissionDenied, "only integration can delete this user")
}
// handle service user first and exit, no need to fetch extra data from IDP, etc
if targetUser.IsServiceUser {
am.deleteServiceUser(account, initiatorUserID, targetUser)

View File

@ -269,6 +269,11 @@ func TestUser_Copy(t *testing.T) {
},
Blocked: false,
LastLogin: time.Now(),
Issued: "test",
IntegrationReference: IntegrationReference{
ID: 0,
IntegrationType: "test",
},
}
err := validateStruct(user)
@ -453,12 +458,25 @@ func TestUser_DeleteUser_SelfDelete(t *testing.T) {
func TestUser_DeleteUser_regularUser(t *testing.T) {
store := newStore(t)
account := newAccountWithId(mockAccountID, mockUserID, "")
targetId := "user2"
account.Users[targetId] = &User{
Id: targetId,
IsServiceUser: true,
ServiceUserName: "user2username",
}
targetId = "user3"
account.Users[targetId] = &User{
Id: targetId,
IsServiceUser: false,
Issued: UserIssuedAPI,
}
targetId = "user4"
account.Users[targetId] = &User{
Id: targetId,
IsServiceUser: false,
Issued: UserIssuedIntegration,
}
err := store.SaveAccount(account)
if err != nil {
@ -470,10 +488,37 @@ func TestUser_DeleteUser_regularUser(t *testing.T) {
eventStore: &activity.InMemoryEventStore{},
}
err = am.DeleteUser(mockAccountID, mockUserID, targetId)
if err != nil {
t.Errorf("unexpected error: %s", err)
testCases := []struct {
name string
userID string
assertErrFunc assert.ErrorAssertionFunc
assertErrMessage string
}{
{
name: "Delete service user successfully ",
userID: "user2",
assertErrFunc: assert.NoError,
},
{
name: "Delete regular user successfully ",
userID: "user3",
assertErrFunc: assert.NoError,
},
{
name: "Delete integration regular user permission denied ",
userID: "user4",
assertErrFunc: assert.Error,
assertErrMessage: "only integration can delete this user",
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
err = am.DeleteUser(mockAccountID, mockUserID, testCase.userID)
testCase.assertErrFunc(t, err, testCase.assertErrMessage)
})
}
}
func TestDefaultAccountManager_GetUser(t *testing.T) {