Check peer expiration after ACL check (#714)

Bug 1: When calculating the network map, peers added by a setup key
were falling under expiration logic while they shouldn't.

Bug 2: Peers HTTP API didn't return expired peers for non-admin users
because of the expired peer check in the ACL logic.

The fix applies peer expiration checks outside of the ACL logic.
This commit is contained in:
Misha Bragin 2023-03-02 12:45:10 +01:00 committed by GitHub
parent 1bda8fd563
commit fe22eb3b98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 20 additions and 7 deletions

View File

@ -1498,6 +1498,7 @@ func TestAccount_GetExpiredPeers(t *testing.T) {
LoginExpired: false,
},
LastLogin: time.Now().Add(-30 * time.Minute),
UserID: userID,
},
"peer-2": {
ID: "peer-2",
@ -1508,6 +1509,7 @@ func TestAccount_GetExpiredPeers(t *testing.T) {
LoginExpired: false,
},
LastLogin: time.Now().Add(-2 * time.Hour),
UserID: userID,
},
"peer-3": {
@ -1519,6 +1521,7 @@ func TestAccount_GetExpiredPeers(t *testing.T) {
LoginExpired: false,
},
LastLogin: time.Now().Add(-1 * time.Hour),
UserID: userID,
},
},
expectedPeers: map[string]struct{}{

View File

@ -108,11 +108,15 @@ func (p *Peer) MarkLoginExpired(expired bool) {
// Return true if a login has expired, false otherwise, and time left to expiration (negative when expired).
// Login expiration can be disabled/enabled on a Peer level via Peer.LoginExpirationEnabled property.
// Login expiration can also be disabled/enabled globally on the Account level via Settings.PeerLoginExpirationEnabled.
// Only peers added by interactive SSO login can be expired.
func (p *Peer) LoginExpired(expiresIn time.Duration) (bool, time.Duration) {
if !p.AddedWithSSOLogin() || !p.LoginExpirationEnabled {
return false, 0
}
expiresAt := p.LastLogin.Add(expiresIn)
now := time.Now()
timeLeft := expiresAt.Sub(now)
return p.LoginExpirationEnabled && (timeLeft <= 0), timeLeft
return timeLeft <= 0, timeLeft
}
// FQDN returns peers FQDN combined of the peer's DNS label and the system's DNS domain
@ -433,8 +437,17 @@ func (am *DefaultAccountManager) GetNetworkMap(peerID string) (*NetworkMap, erro
}
aclPeers := account.getPeersByACL(peerID)
// exclude expired peers
var peersToConnect []*Peer
for _, p := range aclPeers {
expired, _ := peer.LoginExpired(account.Settings.PeerLoginExpiration)
if expired {
continue
}
peersToConnect = append(peersToConnect, p)
}
// Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID.
routesUpdate := account.getRoutesToSync(peerID, aclPeers)
routesUpdate := account.getRoutesToSync(peerID, peersToConnect)
dnsManagementStatus := account.getPeerDNSManagementStatus(peerID)
dnsUpdate := nbdns.Config{
@ -452,7 +465,7 @@ func (am *DefaultAccountManager) GetNetworkMap(peerID string) (*NetworkMap, erro
}
return &NetworkMap{
Peers: aclPeers,
Peers: peersToConnect,
Network: account.Network.Copy(),
Routes: routesUpdate,
DNSConfig: dnsUpdate,
@ -800,10 +813,6 @@ func (a *Account) getPeersByACL(peerID string) []*Peer {
)
continue
}
expired, _ := peer.LoginExpired(a.Settings.PeerLoginExpiration)
if expired {
continue
}
// exclude original peer
if _, ok := peersSet[peer.ID]; peer.ID != peerID && !ok {
peersSet[peer.ID] = struct{}{}

View File

@ -55,6 +55,7 @@ func TestPeer_LoginExpired(t *testing.T) {
peer := &Peer{
LoginExpirationEnabled: c.expirationEnabled,
LastLogin: c.lastLogin,
UserID: userID,
}
expired, _ := peer.LoginExpired(c.accountSettings.PeerLoginExpiration)