Merge branch 'main' into feature/peer-approval

This commit is contained in:
pascal-fischer
2023-11-29 16:27:01 +01:00
committed by GitHub
70 changed files with 1095 additions and 346 deletions
+59 -5
View File
@@ -67,6 +67,7 @@ type AccountManager interface {
GetAccountByUserOrAccountID(userID, accountID, domain string) (*Account, error)
GetAccountFromToken(claims jwtclaims.AuthorizationClaims) (*Account, *User, error)
GetAccountFromPAT(pat string) (*Account, *User, *PersonalAccessToken, error)
DeleteAccount(accountID, userID string) error
MarkPATUsed(tokenID string) error
GetUser(claims jwtclaims.AuthorizationClaims) (*User, error)
ListUsers(accountID string) ([]*User, error)
@@ -978,14 +979,15 @@ func (am *DefaultAccountManager) newAccount(userID, domain string) (*Account, er
_, err := am.Store.GetAccount(accountId)
statusErr, _ := status.FromError(err)
if err == nil {
switch {
case err == nil:
log.Warnf("an account with ID already exists, retrying...")
continue
} else if statusErr.Type() == status.NotFound {
case statusErr.Type() == status.NotFound:
newAccount := newAccountWithId(accountId, userID, domain)
am.StoreEvent(userID, newAccount.Id, accountId, activity.AccountCreated, nil)
return newAccount, nil
} else {
default:
return nil, err
}
}
@@ -1031,6 +1033,57 @@ func (am *DefaultAccountManager) warmupIDPCache() error {
return nil
}
// DeleteAccount deletes an account and all its users from local store and from the remote IDP if the requester is an admin and account owner
func (am *DefaultAccountManager) DeleteAccount(accountID, userID string) error {
unlock := am.Store.AcquireAccountLock(accountID)
defer unlock()
account, err := am.Store.GetAccount(accountID)
if err != nil {
return err
}
user, err := account.FindUser(userID)
if err != nil {
return err
}
if !user.IsAdmin() {
return status.Errorf(status.PermissionDenied, "user is not allowed to delete account")
}
if user.Id != account.CreatedBy {
return status.Errorf(status.PermissionDenied, "user is not allowed to delete account. Only account owner can delete account")
}
for _, otherUser := range account.Users {
if otherUser.IsServiceUser {
continue
}
if otherUser.Id == userID {
continue
}
deleteUserErr := am.deleteRegularUser(account, userID, otherUser.Id)
if deleteUserErr != nil {
return deleteUserErr
}
}
err = am.deleteRegularUser(account, userID, userID)
if err != nil {
log.Errorf("failed deleting user %s. error: %s", userID, err)
return err
}
err = am.Store.DeleteAccount(account)
if err != nil {
log.Errorf("failed deleting account %s. error: %s", accountID, err)
return err
}
log.Debugf("account %s deleted", accountID)
return nil
}
// GetAccountByUserOrAccountID looks for an account by user or accountID, if no account is provided and
// userID doesn't have an account associated with it, one account is created
func (am *DefaultAccountManager) GetAccountByUserOrAccountID(userID, accountID, domain string) (*Account, error) {
@@ -1624,9 +1677,10 @@ func (am *DefaultAccountManager) GetAllConnectedPeers() (map[string]struct{}, er
return am.peersUpdateManager.GetAllConnectedPeers(), nil
}
var invalidDomainRegexp = regexp.MustCompile(`^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$`)
func isDomainValid(domain string) bool {
re := regexp.MustCompile(`^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$`)
return re.MatchString(domain)
return invalidDomainRegexp.MatchString(domain)
}
// GetDNSDomain returns the configured dnsDomain
+25
View File
@@ -902,6 +902,31 @@ func TestAccountManager_GetAccount(t *testing.T) {
}
}
func TestAccountManager_DeleteAccount(t *testing.T) {
manager, err := createManager(t)
if err != nil {
t.Fatal(err)
return
}
expectedId := "test_account"
userId := "account_creator"
account, err := createAccount(manager, expectedId, userId, "")
if err != nil {
t.Fatal(err)
}
err = manager.DeleteAccount(account.Id, userId)
if err != nil {
t.Fatal(err)
}
getAccount, err := manager.Store.GetAccount(account.Id)
if err == nil {
t.Fatal(fmt.Errorf("expected to get an error when trying to get deleted account, got %v", getAccount))
}
}
func TestAccountManager_AddPeer(t *testing.T) {
manager, err := createManager(t)
if err != nil {
+35
View File
@@ -352,6 +352,41 @@ func (s *FileStore) SaveAccount(account *Account) error {
return s.persist(s.storeFile)
}
func (s *FileStore) DeleteAccount(account *Account) error {
s.mux.Lock()
defer s.mux.Unlock()
if account.Id == "" {
return status.Errorf(status.InvalidArgument, "account id should not be empty")
}
for keyID := range account.SetupKeys {
delete(s.SetupKeyID2AccountID, strings.ToUpper(keyID))
}
// enforce peer to account index and delete peer to route indexes for rebuild
for _, peer := range account.Peers {
delete(s.PeerKeyID2AccountID, peer.Key)
delete(s.PeerID2AccountID, peer.ID)
}
for _, user := range account.Users {
for _, pat := range user.PATs {
delete(s.TokenID2UserID, pat.ID)
delete(s.HashedPAT2TokenID, pat.HashedToken)
}
delete(s.UserID2AccountID, user.Id)
}
if account.DomainCategory == PrivateCategory && account.IsDomainPrimaryAccount {
delete(s.PrivateDomain2AccountID, account.Domain)
}
delete(s.Accounts, account.Id)
return s.persist(s.storeFile)
}
// DeleteHashedPAT2TokenIDIndex removes an entry from the indexing map HashedPAT2TokenID
func (s *FileStore) DeleteHashedPAT2TokenIDIndex(hashedToken string) error {
s.mux.Lock()
+54
View File
@@ -122,6 +122,60 @@ func TestSaveAccount(t *testing.T) {
}
}
func TestDeleteAccount(t *testing.T) {
storeDir := t.TempDir()
storeFile := filepath.Join(storeDir, "store.json")
err := util.CopyFileContents("testdata/store.json", storeFile)
if err != nil {
t.Fatal(err)
}
store, err := NewFileStore(storeDir, nil)
if err != nil {
t.Fatal(err)
}
var account *Account
for _, a := range store.Accounts {
account = a
break
}
require.NotNil(t, account, "failed to restore a FileStore file and get at least one account")
err = store.DeleteAccount(account)
require.NoError(t, err, "failed to delete account, error: %v", err)
_, ok := store.Accounts[account.Id]
require.False(t, ok, "failed to delete account")
for id := range account.Users {
_, ok := store.UserID2AccountID[id]
assert.False(t, ok, "failed to delete UserID2AccountID index")
for _, pat := range account.Users[id].PATs {
_, ok := store.HashedPAT2TokenID[pat.HashedToken]
assert.False(t, ok, "failed to delete HashedPAT2TokenID index")
_, ok = store.TokenID2UserID[pat.ID]
assert.False(t, ok, "failed to delete TokenID2UserID index")
}
}
for _, p := range account.Peers {
_, ok := store.PeerKeyID2AccountID[p.Key]
assert.False(t, ok, "failed to delete PeerKeyID2AccountID index")
_, ok = store.PeerID2AccountID[p.ID]
assert.False(t, ok, "failed to delete PeerID2AccountID index")
}
for id := range account.SetupKeys {
_, ok := store.SetupKeyID2AccountID[id]
assert.False(t, ok, "failed to delete SetupKeyID2AccountID index")
}
_, ok = store.PrivateDomain2AccountID[account.Domain]
assert.False(t, ok, "failed to delete PrivateDomain2AccountID index")
}
func TestStore(t *testing.T) {
store := newStore(t)
@@ -103,6 +103,30 @@ func (h *AccountsHandler) UpdateAccount(w http.ResponseWriter, r *http.Request)
util.WriteJSONObject(w, &resp)
}
// DeleteAccount is a HTTP DELETE handler to delete an account
func (h *AccountsHandler) DeleteAccount(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodDelete {
util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w)
return
}
claims := h.claimsExtractor.FromRequestContext(r)
vars := mux.Vars(r)
targetAccountID := vars["accountId"]
if len(targetAccountID) == 0 {
util.WriteError(status.Errorf(status.InvalidArgument, "invalid account ID"), w)
return
}
err := h.accountManager.DeleteAccount(targetAccountID, claims.UserId)
if err != nil {
util.WriteError(err, w)
return
}
util.WriteJSONObject(w, emptyObject{})
}
func toAccountResponse(account *server.Account) *api.Account {
settings := api.AccountSettings{
PeerLoginExpiration: int(account.Settings.PeerLoginExpiration.Seconds()),
+26
View File
@@ -1091,6 +1091,32 @@ paths:
'500':
"$ref": "#/components/responses/internal_error"
/api/accounts/{accountId}:
delete:
summary: Delete an Account
description: Deletes an account and all its resources. Only administrators and account owners can delete accounts.
tags: [ Accounts ]
security:
- BearerAuth: [ ]
- TokenAuth: [ ]
parameters:
- in: path
name: accountId
required: true
schema:
type: string
description: The unique identifier of an account
responses:
'200':
description: Delete account status code
content: { }
'400':
"$ref": "#/components/responses/bad_request"
'401':
"$ref": "#/components/responses/requires_authentication"
'403':
"$ref": "#/components/responses/forbidden"
'500':
"$ref": "#/components/responses/internal_error"
put:
summary: Update an Account
description: Update information about an account
+2
View File
@@ -7,6 +7,7 @@ import (
"github.com/rs/cors"
"github.com/netbirdio/management-integrations/integrations"
s "github.com/netbirdio/netbird/management/server"
"github.com/netbirdio/netbird/management/server/http/middleware"
"github.com/netbirdio/netbird/management/server/jwtclaims"
@@ -105,6 +106,7 @@ func APIHandler(accountManager s.AccountManager, jwtValidator jwtclaims.JWTValid
func (apiHandler *apiHandler) addAccountsEndpoint() {
accountsHandler := NewAccountsHandler(apiHandler.AccountManager, apiHandler.AuthCfg)
apiHandler.Router.HandleFunc("/accounts/{accountId}", accountsHandler.UpdateAccount).Methods("PUT", "OPTIONS")
apiHandler.Router.HandleFunc("/accounts/{accountId}", accountsHandler.DeleteAccount).Methods("DELETE", "OPTIONS")
apiHandler.Router.HandleFunc("/accounts", accountsHandler.GetAllAccounts).Methods("GET", "OPTIONS")
}
@@ -33,6 +33,8 @@ func NewAccessControl(audience, userIDClaim string, getUser GetUser) *AccessCont
}
}
var tokenPathRegexp = regexp.MustCompile(`^.*/api/users/.*/tokens.*$`)
// Handler method of the middleware which forbids all modify requests for non admin users
// It also adds
func (a *AccessControl) Handler(h http.Handler) http.Handler {
@@ -55,13 +57,7 @@ func (a *AccessControl) Handler(h http.Handler) http.Handler {
switch r.Method {
case http.MethodDelete, http.MethodPost, http.MethodPatch, http.MethodPut:
ok, err := regexp.MatchString(`^.*/api/users/.*/tokens.*$`, r.URL.Path)
if err != nil {
log.Debugf("regex failed")
util.WriteError(status.Errorf(status.Internal, ""), w)
return
}
if ok {
if tokenPathRegexp.MatchString(r.URL.Path) {
log.Debugf("valid Path")
h.ServeHTTP(w, r)
return
+1 -1
View File
@@ -221,7 +221,7 @@ func toGroupsInfo(groups map[string]*server.Group, peerID string) []api.GroupMin
}
groupsChecked[group.ID] = struct{}{}
for _, pk := range group.Peers {
if pk != peerID {
if pk == peerID {
info := api.GroupMinimum{
Id: group.ID,
Name: group.Name,
+1 -1
View File
@@ -300,7 +300,7 @@ func toPolicyResponse(account *server.Account, policy *server.Policy) *api.Polic
Action: api.PolicyRuleAction(r.Action),
}
if len(r.Ports) != 0 {
portsCopy := r.Ports[:]
portsCopy := r.Ports
rule.Ports = &portsCopy
}
for _, gid := range r.Sources {
+5 -4
View File
@@ -192,13 +192,14 @@ func writeSuccess(w http.ResponseWriter, key *server.SetupKey) {
func toResponseBody(key *server.SetupKey) *api.SetupKey {
var state string
if key.IsExpired() {
switch {
case key.IsExpired():
state = "expired"
} else if key.IsRevoked() {
case key.IsRevoked():
state = "revoked"
} else if key.IsOverUsed() {
case key.IsOverUsed():
state = "overused"
} else {
default:
state = "valid"
}
+7 -2
View File
@@ -94,7 +94,7 @@ func (h *UsersHandler) UpdateUser(w http.ResponseWriter, r *http.Request) {
util.WriteJSONObject(w, toUserResponse(newUser, claims.UserId))
}
// DeleteUser is a DELETE request to delete a user (only works for service users right now)
// DeleteUser is a DELETE request to delete a user
func (h *UsersHandler) DeleteUser(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodDelete {
util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w)
@@ -155,9 +155,14 @@ func (h *UsersHandler) CreateUser(w http.ResponseWriter, r *http.Request) {
email = *req.Email
}
name := ""
if req.Name != nil {
name = *req.Name
}
newUser, err := h.accountManager.CreateUser(account.Id, user.Id, &server.UserInfo{
Email: email,
Name: *req.Name,
Name: name,
Role: req.Role,
AutoGroups: req.AutoGroups,
IsServiceUser: req.IsServiceUser,
+3 -5
View File
@@ -463,11 +463,9 @@ func (zp zitadelProfile) userData() *UserData {
if zp.Human != nil {
email = zp.Human.Email.Email
name = zp.Human.Profile.DisplayName
} else {
if len(zp.LoginNames) > 0 {
email = zp.LoginNames[0]
name = zp.LoginNames[0]
}
} else if len(zp.LoginNames) > 0 {
email = zp.LoginNames[0]
name = zp.LoginNames[0]
}
return &UserData{
+6 -12
View File
@@ -7,7 +7,6 @@ import (
"fmt"
"io"
"net/http"
"regexp"
"sort"
"strings"
"time"
@@ -201,14 +200,14 @@ func (w *Worker) generateProperties() properties {
expirationEnabled++
}
groups = groups + len(account.Groups)
routes = routes + len(account.Routes)
groups += len(account.Groups)
routes += len(account.Routes)
for _, route := range account.Routes {
if len(route.PeerGroups) > 0 {
routesWithRGGroups++
}
}
nameservers = nameservers + len(account.NameServerGroups)
nameservers += len(account.NameServerGroups)
for _, policy := range account.Policies {
for _, rule := range policy.Rules {
@@ -232,10 +231,10 @@ func (w *Worker) generateProperties() properties {
}
for _, key := range account.SetupKeys {
setupKeysUsage = setupKeysUsage + key.UsedTimes
setupKeysUsage += key.UsedTimes
if key.Ephemeral {
ephemeralPeersSKs++
ephemeralPeersSKUsage = ephemeralPeersSKUsage + key.UsedTimes
ephemeralPeersSKUsage += key.UsedTimes
}
}
@@ -381,15 +380,10 @@ func createPostRequest(ctx context.Context, endpoint string, payloadStr string)
}
func getMinMaxVersion(inputList []string) (string, string) {
reg, err := regexp.Compile(version.SemverRegexpRaw)
if err != nil {
return "", ""
}
versions := make([]*version.Version, 0)
for _, raw := range inputList {
if raw != "" && reg.MatchString(raw) {
if raw != "" && nbversion.SemverRegexp.MatchString(raw) {
v, err := version.NewVersion(raw)
if err == nil {
versions = append(versions, v)
@@ -68,6 +68,7 @@ type MockAccountManager struct {
ListNameServerGroupsFunc func(accountID string) ([]*nbdns.NameServerGroup, error)
CreateUserFunc func(accountID, userID string, key *server.UserInfo) (*server.UserInfo, error)
GetAccountFromTokenFunc func(claims jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error)
DeleteAccountFunc func(accountID, userID string) error
GetDNSDomainFunc func() string
StoreEventFunc func(initiatorID, targetID, accountID string, activityID activity.Activity, meta map[string]any)
GetEventsFunc func(accountID, userID string) ([]*activity.Event, error)
@@ -157,6 +158,14 @@ func (am *MockAccountManager) GetAccountFromPAT(pat string) (*server.Account, *s
return nil, nil, nil, status.Errorf(codes.Unimplemented, "method GetAccountFromPAT is not implemented")
}
// DeleteAccount mock implementation of DeleteAccount from server.AccountManager interface
func (am *MockAccountManager) DeleteAccount(accountID, userID string) error {
if am.DeleteAccountFunc != nil {
return am.DeleteAccountFunc(accountID, userID)
}
return status.Errorf(codes.Unimplemented, "method DeleteAccount is not implemented")
}
// MarkPATUsed mock implementation of MarkPATUsed from server.AccountManager interface
func (am *MockAccountManager) MarkPATUsed(pat string) error {
if am.MarkPATUsedFunc != nil {
+2 -1
View File
@@ -267,8 +267,9 @@ func validateGroups(list []string, groups map[string]*Group) error {
return nil
}
var domainMatcher = regexp.MustCompile(domainPattern)
func validateDomain(domain string) error {
domainMatcher := regexp.MustCompile(domainPattern)
if !domainMatcher.MatchString(domain) {
return errors.New("domain should consists of only letters, numbers, and hyphens with no leading, trailing hyphens, or spaces")
}
+1 -1
View File
@@ -67,7 +67,7 @@ func NewNetwork() *Network {
func (n *Network) IncSerial() {
n.mu.Lock()
defer n.mu.Unlock()
n.Serial = n.Serial + 1
n.Serial++
}
// CurrentSerial returns the Network.Serial of the network (latest state id)
+1 -1
View File
@@ -407,7 +407,7 @@ func (am *DefaultAccountManager) ListPolicies(accountID, userID string) ([]*Poli
return nil, status.Errorf(status.PermissionDenied, "Only Administrators can view policies")
}
return account.Policies[:], nil
return account.Policies, nil
}
func (am *DefaultAccountManager) deletePolicy(account *Account, policyID string) (*Policy, error) {
+1 -1
View File
@@ -137,7 +137,7 @@ func (key *SetupKey) HiddenCopy(length int) *SetupKey {
// IncrementUsage makes a copy of a key, increments the UsedTimes by 1 and sets LastUsed to now
func (key *SetupKey) IncrementUsage() *SetupKey {
c := key.Copy()
c.UsedTimes = c.UsedTimes + 1
c.UsedTimes++
c.LastUsed = time.Now().UTC()
return c
}
+32 -1
View File
@@ -204,6 +204,37 @@ func (s *SqliteStore) SaveAccount(account *Account) error {
return err
}
func (s *SqliteStore) DeleteAccount(account *Account) error {
start := time.Now()
err := s.db.Transaction(func(tx *gorm.DB) error {
result := tx.Select(clause.Associations).Delete(account.Policies, "account_id = ?", account.Id)
if result.Error != nil {
return result.Error
}
result = tx.Select(clause.Associations).Delete(account.UsersG, "account_id = ?", account.Id)
if result.Error != nil {
return result.Error
}
result = tx.Select(clause.Associations).Delete(account)
if result.Error != nil {
return result.Error
}
return nil
})
took := time.Since(start)
if s.metrics != nil {
s.metrics.StoreMetrics().CountPersistenceDuration(took)
}
log.Debugf("took %d ms to delete an account to the SQLite", took.Milliseconds())
return err
}
func (s *SqliteStore) SaveInstallationID(ID string) error {
installation := installation{InstallationIDValue: ID}
installation.ID = uint(s.installationPK)
@@ -338,7 +369,7 @@ func (s *SqliteStore) GetAccount(accountID string) (*Account, error) {
var rules []*PolicyRule
err := s.db.Model(&PolicyRule{}).Find(&rules, "policy_id = ?", policy.ID).Error
if err != nil {
return nil, status.Errorf(status.NotFound, "account not found")
return nil, status.Errorf(status.NotFound, "rule not found")
}
account.Policies[i].Rules = rules
}
+74
View File
@@ -100,6 +100,80 @@ func TestSqlite_SaveAccount(t *testing.T) {
}
}
func TestSqlite_DeleteAccount(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("The SQLite store is not properly supported by Windows yet")
}
store := newSqliteStore(t)
testUserID := "testuser"
user := NewAdminUser(testUserID)
user.PATs = map[string]*PersonalAccessToken{"testtoken": {
ID: "testtoken",
Name: "test token",
}}
account := newAccountWithId("account_id", testUserID, "")
setupKey := GenerateDefaultSetupKey()
account.SetupKeys[setupKey.Key] = setupKey
account.Peers["testpeer"] = &Peer{
Key: "peerkey",
SetupKey: "peerkeysetupkey",
IP: net.IP{127, 0, 0, 1},
Meta: PeerSystemMeta{},
Name: "peer name",
Status: &PeerStatus{Connected: true, LastSeen: time.Now().UTC()},
}
account.Users[testUserID] = user
err := store.SaveAccount(account)
require.NoError(t, err)
if len(store.GetAllAccounts()) != 1 {
t.Errorf("expecting 1 Accounts to be stored after SaveAccount()")
}
err = store.DeleteAccount(account)
require.NoError(t, err)
if len(store.GetAllAccounts()) != 0 {
t.Errorf("expecting 0 Accounts to be stored after DeleteAccount()")
}
_, err = store.GetAccountByPeerPubKey("peerkey")
require.Error(t, err, "expecting error after removing DeleteAccount when getting account by peer public key")
_, err = store.GetAccountByUser("testuser")
require.Error(t, err, "expecting error after removing DeleteAccount when getting account by user")
_, err = store.GetAccountByPeerID("testpeer")
require.Error(t, err, "expecting error after removing DeleteAccount when getting account by peer id")
_, err = store.GetAccountBySetupKey(setupKey.Key)
require.Error(t, err, "expecting error after removing DeleteAccount when getting account by setup key")
_, err = store.GetAccount(account.Id)
require.Error(t, err, "expecting error after removing DeleteAccount when getting account by id")
for _, policy := range account.Policies {
var rules []*PolicyRule
err = store.db.Model(&PolicyRule{}).Find(&rules, "policy_id = ?", policy.ID).Error
require.NoError(t, err, "expecting no error after removing DeleteAccount when searching for policy rules")
require.Len(t, rules, 0, "expecting no policy rules to be found after removing DeleteAccount")
}
for _, accountUser := range account.Users {
var pats []*PersonalAccessToken
err = store.db.Model(&PersonalAccessToken{}).Find(&pats, "user_id = ?", accountUser.Id).Error
require.NoError(t, err, "expecting no error after removing DeleteAccount when searching for personal access token")
require.Len(t, pats, 0, "expecting no personal access token to be found after removing DeleteAccount")
}
}
func TestSqlite_SavePeerStatus(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("The SQLite store is not properly supported by Windows yet")
+1
View File
@@ -15,6 +15,7 @@ import (
type Store interface {
GetAllAccounts() []*Account
GetAccount(accountID string) (*Account, error)
DeleteAccount(account *Account) error
GetAccountByUser(userID string) (*Account, error)
GetAccountByPeerPubKey(peerKey string) (*Account, error)
GetAccountByPeerID(peerID string) (*Account, error)
+8
View File
@@ -259,6 +259,14 @@ func (am *DefaultAccountManager) inviteNewUser(accountID, userID string, invite
return nil, fmt.Errorf("provided user update is nil")
}
switch {
case invite.Name == "":
return nil, status.Errorf(status.InvalidArgument, "name can't be empty")
case invite.Email == "":
return nil, status.Errorf(status.InvalidArgument, "email can't be empty")
default:
}
account, err := am.Store.GetAccount(accountID)
if err != nil {
return nil, status.Errorf(status.NotFound, "account %s doesn't exist", accountID)