From e30def175b5bdf525e6dbcc28c4ebcb5e457ac00 Mon Sep 17 00:00:00 2001
From: Pascal Fischer <pascal@netbird.io>
Date: Mon, 20 Mar 2023 16:14:55 +0100
Subject: [PATCH] switch PATs to map and add deletion
---
management/server/account.go | 2 +
management/server/account_test.go | 25 ++++++------
management/server/file_store.go | 18 ++++++++
management/server/file_store_test.go | 26 ++++++++++--
management/server/store.go | 2 +
management/server/testdata/store.json | 9 ++--
management/server/user.go | 47 ++++++++++++++++++---
management/server/user_test.go | 59 ++++++++++++++++++++++++++-
8 files changed, 162 insertions(+), 26 deletions(-)
diff --git a/management/server/account.go b/management/server/account.go
index d3f7df028..e998b8d5c 100644
--- a/management/server/account.go
+++ b/management/server/account.go
@@ -66,6 +66,8 @@ type AccountManager interface {
GetNetworkMap(peerID string) (*NetworkMap, error)
GetPeerNetwork(peerID string) (*Network, error)
AddPeer(setupKey, userID string, peer *Peer) (*Peer, *NetworkMap, error)
+ AddPATToUser(accountID string, userID string, pat *PersonalAccessToken) error
+ DeletePAT(accountID string, userID string, tokenID string) error
UpdatePeerSSHKey(peerID string, sshKey string) error
GetUsersFromAccount(accountID, userID string) ([]*UserInfo, error)
GetGroup(accountId, groupID string) (*Group, error)
diff --git a/management/server/account_test.go b/management/server/account_test.go
index fa932023a..d211973c2 100644
--- a/management/server/account_test.go
+++ b/management/server/account_test.go
@@ -473,20 +473,21 @@ func TestAccountManager_GetAccountFromPAT(t *testing.T) {
token := "nbp_9999EUDNdkeusjentDLSJEn1902u84390W6W"
hashedToken := sha256.Sum256([]byte(token))
- pat := PersonalAccessToken{
- ID: "tokenId",
- Description: "some Description",
- HashedToken: string(hashedToken[:]),
- ExpirationDate: time.Time{},
- CreatedBy: "testuser",
- CreatedAt: time.Time{},
- LastUsed: time.Time{},
- }
account.Users["someUser"] = &User{
Id: "someUser",
Role: "",
AutoGroups: nil,
- PATs: []PersonalAccessToken{pat},
+ PATs: map[string]*PersonalAccessToken{
+ "pat1": {
+ ID: "tokenId",
+ Description: "some Description",
+ HashedToken: string(hashedToken[:]),
+ ExpirationDate: time.Time{},
+ CreatedBy: "testuser",
+ CreatedAt: time.Time{},
+ LastUsed: time.Time{},
+ },
+ },
}
err := store.SaveAccount(account)
if err != nil {
@@ -1267,8 +1268,8 @@ func TestAccount_Copy(t *testing.T) {
Id: "user1",
Role: UserRoleAdmin,
AutoGroups: []string{"group1"},
- PATs: []PersonalAccessToken{
- {
+ PATs: map[string]*PersonalAccessToken{
+ "pat1": {
ID: "pat1",
Description: "First PAT",
HashedToken: "SoMeHaShEdToKeN",
diff --git a/management/server/file_store.go b/management/server/file_store.go
index 50bfec1d6..cd98ca6b8 100644
--- a/management/server/file_store.go
+++ b/management/server/file_store.go
@@ -288,6 +288,24 @@ func (s *FileStore) SaveAccount(account *Account) error {
return s.persist(s.storeFile)
}
+func (s *FileStore) DeleteHashedPAT2TokenIDIndex(hashedToken string) error {
+ s.mux.Lock()
+ defer s.mux.Unlock()
+
+ delete(s.HashedPAT2TokenID, hashedToken)
+
+ return s.persist(s.storeFile)
+}
+
+func (s *FileStore) DeleteTokenID2UserIDIndex(tokenID string) error {
+ s.mux.Lock()
+ defer s.mux.Unlock()
+
+ delete(s.TokenID2UserID, tokenID)
+
+ return s.persist(s.storeFile)
+}
+
// GetAccountByPrivateDomain returns account by private domain
func (s *FileStore) GetAccountByPrivateDomain(domain string) (*Account, error) {
s.mux.Lock()
diff --git a/management/server/file_store_test.go b/management/server/file_store_test.go
index 838af50fb..c2ccbf546 100644
--- a/management/server/file_store_test.go
+++ b/management/server/file_store_test.go
@@ -249,7 +249,7 @@ func TestRestore(t *testing.T) {
require.NotNil(t, account.SetupKeys["A2C8E62B-38F5-4553-B31E-DD66C696CEBB"], "failed to restore a FileStore file - missing Account SetupKey A2C8E62B-38F5-4553-B31E-DD66C696CEBB")
- require.Len(t, account.Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs, 1, "failed to restore a FileStore wrong PATs length")
+ require.NotNil(t, account.Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs["9dj38s35-63fb-11ec-90d6-0242ac120003"], "failed to restore a FileStore wrong PATs length")
require.Len(t, store.UserID2AccountID, 2, "failed to restore a FileStore wrong UserID2AccountID mapping length")
@@ -383,16 +383,34 @@ func TestFileStore_GetTokenIDByHashedToken(t *testing.T) {
t.Fatal(err)
}
- hashedToken := accounts.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"].Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs[0].HashedToken
+ hashedToken := accounts.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"].Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs["9dj38s35-63fb-11ec-90d6-0242ac120003"].HashedToken
tokenID, err := store.GetTokenIDByHashedToken(hashedToken)
if err != nil {
t.Fatal(err)
}
- expectedTokenID := accounts.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"].Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs[0].ID
+ expectedTokenID := accounts.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"].Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs["9dj38s35-63fb-11ec-90d6-0242ac120003"].ID
assert.Equal(t, expectedTokenID, tokenID)
}
+func TestFileStore_DeleteHashedPAT2TokenIDIndex(t *testing.T) {
+ store := newStore(t)
+ store.HashedPAT2TokenID["someHashedToken"] = "someTokenId"
+
+ store.DeleteHashedPAT2TokenIDIndex("someHashedToken")
+
+ assert.Empty(t, store.HashedPAT2TokenID["someHashedToken"])
+}
+
+func TestFileStore_DeleteTokenID2UserIDIndex(t *testing.T) {
+ store := newStore(t)
+ store.TokenID2UserID["someTokenId"] = "someUserId"
+
+ store.DeleteTokenID2UserIDIndex("someTokenId")
+
+ assert.Empty(t, store.TokenID2UserID["someTokenId"])
+}
+
func TestFileStore_GetTokenIDByHashedToken_Failure(t *testing.T) {
storeDir := t.TempDir()
storeFile := filepath.Join(storeDir, "store.json")
@@ -437,7 +455,7 @@ func TestFileStore_GetUserByTokenID(t *testing.T) {
t.Fatal(err)
}
- tokenID := accounts.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"].Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs[0].ID
+ tokenID := accounts.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"].Users["f4f6d672-63fb-11ec-90d6-0242ac120003"].PATs["9dj38s35-63fb-11ec-90d6-0242ac120003"].ID
user, err := store.GetUserByTokenID(tokenID)
if err != nil {
t.Fatal(err)
diff --git a/management/server/store.go b/management/server/store.go
index 3577a9f59..daad30eaa 100644
--- a/management/server/store.go
+++ b/management/server/store.go
@@ -11,6 +11,8 @@ type Store interface {
GetTokenIDByHashedToken(secret string) (string, error)
GetUserByTokenID(tokenID string) (*User, error)
SaveAccount(account *Account) error
+ DeleteHashedPAT2TokenIDIndex(hashedToken string) error
+ DeleteTokenID2UserIDIndex(tokenID string) error
GetInstallationID() string
SaveInstallationID(ID string) error
// AcquireAccountLock should attempt to acquire account lock and return a function that releases the lock
diff --git a/management/server/testdata/store.json b/management/server/testdata/store.json
index 7eed7f4c0..ecde766c3 100644
--- a/management/server/testdata/store.json
+++ b/management/server/testdata/store.json
@@ -29,13 +29,14 @@
"Users": {
"edafee4e-63fb-11ec-90d6-0242ac120003": {
"Id": "edafee4e-63fb-11ec-90d6-0242ac120003",
- "Role": "admin"
+ "Role": "admin",
+ "PATs": {}
},
"f4f6d672-63fb-11ec-90d6-0242ac120003": {
"Id": "f4f6d672-63fb-11ec-90d6-0242ac120003",
"Role": "user",
- "PATs":[
- {
+ "PATs": {
+ "9dj38s35-63fb-11ec-90d6-0242ac120003": {
"ID":"9dj38s35-63fb-11ec-90d6-0242ac120003",
"Description":"some Description",
"HashedToken":"SoMeHaShEdToKeN",
@@ -44,7 +45,7 @@
"CreatedAt":"2023-01-01T00:00:00Z",
"LastUsed":"2023-02-01T00:00:00Z"
}
- ]
+ }
}
}
}
diff --git a/management/server/user.go b/management/server/user.go
index fcebe006d..8047c624c 100644
--- a/management/server/user.go
+++ b/management/server/user.go
@@ -46,7 +46,7 @@ type User struct {
Role UserRole
// AutoGroups is a list of Group IDs to auto-assign to peers registered by this user
AutoGroups []string
- PATs []PersonalAccessToken
+ PATs map[string]*PersonalAccessToken
}
// IsAdmin returns true if user is an admin, false otherwise
@@ -94,8 +94,12 @@ func (u *User) toUserInfo(userData *idp.UserData) (*UserInfo, error) {
func (u *User) Copy() *User {
autoGroups := make([]string, len(u.AutoGroups))
copy(autoGroups, u.AutoGroups)
- pats := make([]PersonalAccessToken, len(u.PATs))
- copy(pats, u.PATs)
+ pats := make(map[string]*PersonalAccessToken, len(u.PATs))
+ for k, v := range u.PATs {
+ patCopy := new(PersonalAccessToken)
+ *patCopy = *v
+ pats[k] = patCopy
+ }
return &User{
Id: u.Id,
Role: u.Role,
@@ -190,7 +194,7 @@ func (am *DefaultAccountManager) CreateUser(accountID, userID string, invite *Us
}
// AddPATToUser takes the userID and the accountID the user belongs to and assigns a provided PersonalAccessToken to that user
-func (am *DefaultAccountManager) AddPATToUser(accountID string, userID string, pat PersonalAccessToken) error {
+func (am *DefaultAccountManager) AddPATToUser(accountID string, userID string, pat *PersonalAccessToken) error {
unlock := am.Store.AcquireAccountLock(accountID)
defer unlock()
@@ -204,7 +208,40 @@ func (am *DefaultAccountManager) AddPATToUser(accountID string, userID string, p
return status.Errorf(status.NotFound, "user not found")
}
- user.PATs = append(user.PATs, pat)
+ user.PATs[pat.ID] = pat
+
+ err = am.Store.SaveAccount(account)
+ return err
+}
+
+func (am *DefaultAccountManager) DeletePAT(accountID string, userID string, tokenID string) error {
+ unlock := am.Store.AcquireAccountLock(accountID)
+ defer unlock()
+
+ account, err := am.Store.GetAccount(accountID)
+ if err != nil {
+ return err
+ }
+
+ user := account.Users[userID]
+ if user == nil {
+ return status.Errorf(status.NotFound, "user not found")
+ }
+
+ pat := user.PATs["tokenID"]
+ if pat == nil {
+ return status.Errorf(status.NotFound, "PAT not found")
+ }
+
+ err = am.Store.DeleteTokenID2UserIDIndex(pat.ID)
+ if err != nil {
+ return err
+ }
+ err = am.Store.DeleteHashedPAT2TokenIDIndex(pat.HashedToken)
+ if err != nil {
+ return err
+ }
+ delete(user.PATs, tokenID)
err = am.Store.SaveAccount(account)
return err
diff --git a/management/server/user_test.go b/management/server/user_test.go
index 1c81827ac..b3cd1db9a 100644
--- a/management/server/user_test.go
+++ b/management/server/user_test.go
@@ -51,7 +51,7 @@ func TestUser_AddPATToUser(t *testing.T) {
LastUsed: time.Time{},
}
- err = am.AddPATToUser("account_id", "testuser", pat)
+ err = am.AddPATToUser("account_id", "testuser", &pat)
if err != nil {
t.Fatalf("Error when adding PAT to user: %s", err)
}
@@ -71,3 +71,60 @@ func TestUser_AddPATToUser(t *testing.T) {
}
assert.Equal(t, "testuser", userID)
}
+
+func TestUser_DeletePAT(t *testing.T) {
+ store := newStore(t)
+ account := newAccountWithId("account_id", "testuser", "")
+ 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()},
+ }
+ account.Users["user1"] = &User{
+ Id: "user1",
+ Role: "admin",
+ AutoGroups: nil,
+ PATs: map[string]*PersonalAccessToken{
+ "tokenID": {
+ ID: "tokenID",
+ Description: "some Description",
+ HashedToken: "SoMeHaShEdToKeN",
+ ExpirationDate: time.Now().AddDate(0, 0, 7),
+ CreatedBy: "user1",
+ CreatedAt: time.Now(),
+ LastUsed: time.Now(),
+ },
+ },
+ }
+ err := store.SaveAccount(account)
+ if err != nil {
+ t.Fatalf("Error when saving account: %s", err)
+ }
+
+ am := DefaultAccountManager{
+ Store: store,
+ cacheMux: sync.Mutex{},
+ cacheLoading: nil,
+ peersUpdateManager: nil,
+ idpManager: nil,
+ cacheManager: nil,
+ ctx: nil,
+ eventStore: nil,
+ singleAccountMode: false,
+ singleAccountModeDomain: "",
+ dnsDomain: "",
+ peerLoginExpiry: nil,
+ }
+
+ err = am.DeletePAT("account_id", "user1", "tokenID")
+ if err != nil {
+ t.Fatalf("Error when adding PAT to user: %s", err)
+ }
+
+ assert.Nil(t, store.Accounts["account_id"].Users["user1"].PATs["tokenID"])
+ assert.Empty(t, store.HashedPAT2TokenID["SoMeHaShEdToKeN"])
+ assert.Empty(t, store.TokenID2UserID["tokenID"])
+}