diff --git a/management/server/account.go b/management/server/account.go index e584a79c4..17d2f1486 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -92,6 +92,7 @@ type AccountManager interface { GetEvents(accountID, userID string) ([]*activity.Event, error) GetDNSSettings(accountID string, userID string) (*DNSSettings, error) SaveDNSSettings(accountID string, userID string, dnsSettingsToSave *DNSSettings) error + GetPeer(accountID, peerID, userID string) (*Peer, error) } type DefaultAccountManager struct { @@ -331,6 +332,19 @@ func (a *Account) FindPeerByPubKey(peerPubKey string) (*Peer, error) { return nil, status.Errorf(status.NotFound, "peer with the public key %s not found", peerPubKey) } +// FindUserPeers returns a list of peers that user owns (created) +func (a *Account) FindUserPeers(userID string) ([]*Peer, error) { + + peers := make([]*Peer, 0) + for _, peer := range a.Peers { + if peer.UserID == userID { + peers = append(peers, peer) + } + } + + return peers, nil +} + // FindUser looks for a given user in the Account or returns error if user wasn't found. func (a *Account) FindUser(userID string) (*User, error) { user := a.Users[userID] diff --git a/management/server/http/peers.go b/management/server/http/peers.go index e9d21832b..5d991c8ab 100644 --- a/management/server/http/peers.go +++ b/management/server/http/peers.go @@ -29,6 +29,16 @@ func NewPeers(accountManager server.AccountManager, authCfg AuthCfg) *Peers { } } +func (h *Peers) getPeer(account *server.Account, peerID, userID string, w http.ResponseWriter) { + peer, err := h.accountManager.GetPeer(account.Id, peerID, userID) + if err != nil { + util.WriteError(err, w) + return + } + + util.WriteJSONObject(w, toPeerResponse(peer, account, h.accountManager.GetDNSDomain())) +} + func (h *Peers) updatePeer(account *server.Account, user *server.User, peerID string, w http.ResponseWriter, r *http.Request) { req := &api.PutApiPeersIdJSONBody{} err := json.NewDecoder(r.Body).Decode(&req) @@ -77,6 +87,9 @@ func (h *Peers) HandlePeer(w http.ResponseWriter, r *http.Request) { case http.MethodPut: h.updatePeer(account, user, peerID, w, r) return + case http.MethodGet: + h.getPeer(account, peerID, user.Id, w) + return default: util.WriteError(status.Errorf(status.NotFound, "unknown METHOD"), w) } diff --git a/management/server/http/peers_test.go b/management/server/http/peers_test.go index 026fc668f..4671352fe 100644 --- a/management/server/http/peers_test.go +++ b/management/server/http/peers_test.go @@ -2,6 +2,7 @@ package http import ( "encoding/json" + "github.com/gorilla/mux" "io" "net" "net/http" @@ -17,9 +18,14 @@ import ( "github.com/netbirdio/netbird/management/server/mock_server" ) +const testPeerID = "test_peer" + func initTestMetaData(peers ...*server.Peer) *Peers { return &Peers{ accountManager: &mock_server.MockAccountManager{ + GetPeerFunc: func(accountID, peerID, userID string) (*server.Peer, error) { + return peers[0], nil + }, GetPeersFunc: func(accountID, userID string) ([]*server.Peer, error) { return peers, nil }, @@ -29,7 +35,7 @@ func initTestMetaData(peers ...*server.Peer) *Peers { Id: claims.AccountId, Domain: "hotmail.com", Peers: map[string]*server.Peer{ - "test_peer": peers[0], + peers[0].ID: peers[0], }, Users: map[string]*server.User{ "test_user": user, @@ -58,17 +64,27 @@ func TestGetPeers(t *testing.T) { requestType string requestPath string requestBody io.Reader + expectedArray bool }{ { name: "GetPeersMetaData", requestType: http.MethodGet, requestPath: "/api/peers/", expectedStatus: http.StatusOK, + expectedArray: true, + }, + { + name: "GetPeer", + requestType: http.MethodGet, + requestPath: "/api/peers/" + testPeerID, + expectedStatus: http.StatusOK, + expectedArray: false, }, } rr := httptest.NewRecorder() peer := &server.Peer{ + ID: testPeerID, Key: "key", SetupKey: "setupkey", IP: net.ParseIP("100.64.0.1"), @@ -89,11 +105,16 @@ func TestGetPeers(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { + + recorder := httptest.NewRecorder() req := httptest.NewRequest(tc.requestType, tc.requestPath, tc.requestBody) - p.GetPeers(rr, req) + router := mux.NewRouter() + router.HandleFunc("/api/peers/", p.GetPeers).Methods("GET") + router.HandleFunc("/api/peers/{id}", p.HandlePeer).Methods("GET") + router.ServeHTTP(recorder, req) - res := rr.Result() + res := recorder.Result() defer res.Body.Close() if status := rr.Code; status != tc.expectedStatus { @@ -106,13 +127,23 @@ func TestGetPeers(t *testing.T) { t.Fatalf("I don't know what I expected; %v", err) } - respBody := []*api.Peer{} - err = json.Unmarshal(content, &respBody) - if err != nil { - t.Fatalf("Sent content is not in correct json format; %v", err) + var got *api.Peer + if tc.expectedArray { + respBody := []*api.Peer{} + err = json.Unmarshal(content, &respBody) + if err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + got = respBody[0] + } else { + got = &api.Peer{} + err = json.Unmarshal(content, got) + if err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } } - got := respBody[0] assert.Equal(t, got.Name, peer.Name) assert.Equal(t, got.Version, peer.Meta.WtVersion) assert.Equal(t, got.Ip, peer.IP.String()) diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 7b56ce1a2..d5d68ef9f 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -20,7 +20,7 @@ type MockAccountManager struct { GetAccountByUserOrAccountIdFunc func(userId, accountId, domain string) (*server.Account, error) IsUserAdminFunc func(claims jwtclaims.AuthorizationClaims) (bool, error) AccountExistsFunc func(accountId string) (*bool, error) - GetPeerFunc func(peerKey string) (*server.Peer, error) + GetPeerByKeyFunc func(peerKey string) (*server.Peer, error) GetPeersFunc func(accountID, userID string) ([]*server.Peer, error) MarkPeerConnectedFunc func(peerKey string, connected bool) error DeletePeerFunc func(accountID, peerKey, userID string) (*server.Peer, error) @@ -64,8 +64,9 @@ type MockAccountManager struct { GetAccountFromTokenFunc func(claims jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) GetDNSDomainFunc func() string GetEventsFunc func(accountID, userID string) ([]*activity.Event, error) - GetDNSSettingsFunc func(accountID string, userID string) (*server.DNSSettings, error) - SaveDNSSettingsFunc func(accountID string, userID string, dnsSettingsToSave *server.DNSSettings) error + GetDNSSettingsFunc func(accountID, userID string) (*server.DNSSettings, error) + SaveDNSSettingsFunc func(accountID, userID string, dnsSettingsToSave *server.DNSSettings) error + GetPeerFunc func(accountID, peerID, userID string) (*server.Peer, error) } // GetUsersFromAccount mock implementation of GetUsersFromAccount from server.AccountManager interface @@ -142,10 +143,10 @@ func (am *MockAccountManager) AccountExists(accountId string) (*bool, error) { return nil, status.Errorf(codes.Unimplemented, "method AccountExists is not implemented") } -// GetPeer mock implementation of GetPeer from server.AccountManager interface +// GetPeerByKey mocks implementation of GetPeerByKey from server.AccountManager interface func (am *MockAccountManager) GetPeerByKey(peerKey string) (*server.Peer, error) { - if am.GetPeerFunc != nil { - return am.GetPeerFunc(peerKey) + if am.GetPeerByKeyFunc != nil { + return am.GetPeerByKeyFunc(peerKey) } return nil, status.Errorf(codes.Unimplemented, "method GetPeerByKey is not implemented") } @@ -517,3 +518,11 @@ func (am *MockAccountManager) SaveDNSSettings(accountID string, userID string, d } return status.Errorf(codes.Unimplemented, "method SaveDNSSettings is not implemented") } + +// GetPeer mocks GetPeer of the AccountManager interface +func (am *MockAccountManager) GetPeer(accountID, peerID, userID string) (*server.Peer, error) { + if am.GetPeerFunc != nil { + return am.GetPeerFunc(accountID, peerID, userID) + } + return nil, status.Errorf(codes.Unimplemented, "method GetPeer is not implemented") +} diff --git a/management/server/peer.go b/management/server/peer.go index ceafce978..db260f999 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -535,6 +535,51 @@ func (am *DefaultAccountManager) UpdatePeerSSHKey(peerID string, sshKey string) return am.updateAccountPeers(account) } +// GetPeer for a given accountID, peerID and userID error if not found. +func (am *DefaultAccountManager) GetPeer(accountID, peerID, userID string) (*Peer, error) { + + unlock := am.Store.AcquireAccountLock(accountID) + defer unlock() + + account, err := am.Store.GetAccount(accountID) + if err != nil { + return nil, err + } + + user, err := account.FindUser(userID) + if err != nil { + return nil, err + } + + peer := account.GetPeer(peerID) + if peer == nil { + return nil, status.Errorf(status.NotFound, "peer with %s not found under account %s", peerID, accountID) + } + + // if admin or user owns this peer, return peer + if user.IsAdmin() || peer.UserID == userID { + return peer, nil + } + + // it is also possible that user doesn't own the peer but some of his peers have access to it, + // this is a valid case, show the peer as well. + userPeers, err := account.FindUserPeers(userID) + if err != nil { + return nil, err + } + + for _, p := range userPeers { + aclPeers := account.getPeersByACL(p.ID) + for _, aclPeer := range aclPeers { + if aclPeer.ID == peerID { + return peer, nil + } + } + } + + return nil, status.Errorf(status.Internal, "user %s has no access to peer %s under account %s", userID, peerID, accountID) +} + // UpdatePeerMeta updates peer's system metadata func (am *DefaultAccountManager) UpdatePeerMeta(peerID string, meta PeerSystemMeta) error { diff --git a/management/server/peer_test.go b/management/server/peer_test.go index a0afdafe6..6afc7c4a8 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -1,6 +1,7 @@ package server import ( + "github.com/stretchr/testify/assert" "testing" "github.com/rs/xid" @@ -268,12 +269,7 @@ func TestAccountManager_GetPeerNetwork(t *testing.T) { t.Fatal(err) } - var setupKey *SetupKey - for _, key := range account.SetupKeys { - if key.Type == SetupKeyReusable { - setupKey = key - } - } + setupKey := getSetupKey(account, SetupKeyReusable) peerKey1, err := wgtypes.GeneratePrivateKey() if err != nil { @@ -319,3 +315,119 @@ func TestAccountManager_GetPeerNetwork(t *testing.T) { } } + +func TestDefaultAccountManager_GetPeer(t *testing.T) { + manager, err := createManager(t) + if err != nil { + t.Fatal(err) + return + } + + // account with an admin and a regular user + accountID := "test_account" + adminUser := "account_creator" + someUser := "some_user" + account := newAccountWithId(accountID, adminUser, "") + account.Users[someUser] = &User{ + Id: someUser, + Role: UserRoleUser, + } + err = manager.Store.SaveAccount(account) + if err != nil { + t.Fatal(err) + return + } + + // two peers one added by a regular user and one with a setup key + setupKey := getSetupKey(account, SetupKeyReusable) + peerKey1, err := wgtypes.GeneratePrivateKey() + if err != nil { + t.Fatal(err) + return + } + + peer1, err := manager.AddPeer("", someUser, &Peer{ + Key: peerKey1.PublicKey().String(), + Meta: PeerSystemMeta{}, + Name: "test-peer-2", + }) + + if err != nil { + t.Errorf("expecting peer to be added, got failure %v", err) + return + } + + peerKey2, err := wgtypes.GeneratePrivateKey() + if err != nil { + t.Fatal(err) + return + } + + // the second peer added with a setup key + peer2, err := manager.AddPeer(setupKey.Key, "", &Peer{ + Key: peerKey2.PublicKey().String(), + Meta: PeerSystemMeta{}, + Name: "test-peer-2", + }) + + if err != nil { + t.Fatal(err) + return + } + + // the user can see its own peer + peer, err := manager.GetPeer(accountID, peer1.ID, someUser) + if err != nil { + t.Fatal(err) + return + } + assert.NotNil(t, peer) + + // the user can see peer2 because peer1 of the user has access to peer2 due to the All group and the default rule 0 all-to-all access + peer, err = manager.GetPeer(accountID, peer2.ID, someUser) + if err != nil { + t.Fatal(err) + return + } + assert.NotNil(t, peer) + + // delete the all-to-all rule so that user's peer1 has no access to peer2 + for _, rule := range account.Rules { + err = manager.DeleteRule(accountID, rule.ID, adminUser) + if err != nil { + t.Fatal(err) + return + } + } + + // at this point the user can't see the details of peer2 + peer, err = manager.GetPeer(accountID, peer2.ID, someUser) //nolint + assert.Error(t, err) + + // admin users can always access all the peers + peer, err = manager.GetPeer(accountID, peer1.ID, adminUser) + if err != nil { + t.Fatal(err) + return + } + assert.NotNil(t, peer) + + peer, err = manager.GetPeer(accountID, peer2.ID, adminUser) + if err != nil { + t.Fatal(err) + return + } + assert.NotNil(t, peer) + +} + +func getSetupKey(account *Account, keyType SetupKeyType) *SetupKey { + + var setupKey *SetupKey + for _, key := range account.SetupKeys { + if key.Type == keyType { + setupKey = key + } + } + return setupKey +}