From b05e30ac5aa25a6ab460c615cb70142ccdf33537 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 3 Apr 2023 12:44:55 +0200 Subject: [PATCH 1/9] do not use UTC for time to stay consistent --- management/server/personal_access_token.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index bdf34e9fd..68e4cda6b 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -48,7 +48,7 @@ func CreateNewPAT(name string, expirationInDays int, createdBy string) (*Persona if err != nil { return nil, err } - currentTime := time.Now().UTC() + currentTime := time.Now() return &PersonalAccessTokenGenerated{ PersonalAccessToken: PersonalAccessToken{ ID: xid.New().String(), From 489892553ac9c9cc8e3042ee7d3631a4bcbd8a77 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 3 Apr 2023 15:09:35 +0200 Subject: [PATCH 2/9] use UTC everywhere in server --- management/server/account.go | 2 +- management/server/account_test.go | 36 +++++++++---------- .../server/activity/sqlite/sqlite_test.go | 10 +++--- management/server/event.go | 10 +++--- management/server/event_test.go | 8 +++-- management/server/file_store.go | 6 ++-- management/server/file_store_test.go | 8 ++--- management/server/grpcserver.go | 2 +- management/server/http/events_handler_test.go | 24 ++++++------- .../server/http/middleware/auth_middleware.go | 2 +- .../http/middleware/auth_middleware_test.go | 6 ++-- management/server/http/pat_handler_test.go | 12 +++---- management/server/idp/auth0.go | 5 +-- management/server/idp/auth0_test.go | 16 +++++---- management/server/idp/keycloak.go | 5 +-- management/server/idp/keycloak_test.go | 9 ++--- management/server/management_test.go | 13 ++++--- management/server/metrics/selfhosted.go | 8 ++--- management/server/network.go | 16 +++++---- management/server/peer.go | 15 ++++---- management/server/peer_test.go | 6 ++-- management/server/setupkey.go | 22 ++++++------ management/server/setupkey_test.go | 26 +++++++------- management/server/turncredentials.go | 10 +++--- 24 files changed, 150 insertions(+), 127 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 78c9237b8..416584774 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1151,7 +1151,7 @@ func (am *DefaultAccountManager) MarkPATUsed(tokenID string) error { return fmt.Errorf("token not found") } - pat.LastUsed = time.Now() + pat.LastUsed = time.Now().UTC() return am.Store.SaveAccount(account) } diff --git a/management/server/account_test.go b/management/server/account_test.go index f21c93f0e..a8a1806a7 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -127,12 +127,12 @@ func TestAccount_GetPeerNetworkMap(t *testing.T) { Name: peerID1, DNSLabel: peerID1, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: false, LoginExpired: true, }, UserID: userID, - LastLogin: time.Now().Add(-time.Hour * 24 * 30 * 30), + LastLogin: time.Now().UTC().Add(-time.Hour * 24 * 30 * 30), }, "peer-2": { ID: peerID2, @@ -141,12 +141,12 @@ func TestAccount_GetPeerNetworkMap(t *testing.T) { Name: peerID2, DNSLabel: peerID2, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: false, LoginExpired: false, }, UserID: userID, - LastLogin: time.Now(), + LastLogin: time.Now().UTC(), LoginExpirationEnabled: true, }, }, @@ -165,12 +165,12 @@ func TestAccount_GetPeerNetworkMap(t *testing.T) { Name: peerID1, DNSLabel: peerID1, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: false, LoginExpired: true, }, UserID: userID, - LastLogin: time.Now().Add(-time.Hour * 24 * 30 * 30), + LastLogin: time.Now().UTC().Add(-time.Hour * 24 * 30 * 30), LoginExpirationEnabled: true, }, "peer-2": { @@ -180,12 +180,12 @@ func TestAccount_GetPeerNetworkMap(t *testing.T) { Name: peerID2, DNSLabel: peerID2, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: false, LoginExpired: true, }, UserID: userID, - LastLogin: time.Now().Add(-time.Hour * 24 * 30 * 30), + LastLogin: time.Now().UTC().Add(-time.Hour * 24 * 30 * 30), LoginExpirationEnabled: true, }, }, @@ -1288,10 +1288,10 @@ func TestAccount_Copy(t *testing.T) { ID: "pat1", Name: "First PAT", HashedToken: "SoMeHaShEdToKeN", - ExpirationDate: time.Now().AddDate(0, 0, 7), + ExpirationDate: time.Now().UTC().AddDate(0, 0, 7), CreatedBy: "user1", - CreatedAt: time.Now(), - LastUsed: time.Now(), + CreatedAt: time.Now().UTC(), + LastUsed: time.Now().UTC(), }, }, }, @@ -1569,22 +1569,22 @@ func TestAccount_GetExpiredPeers(t *testing.T) { ID: "peer-1", LoginExpirationEnabled: true, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: true, LoginExpired: false, }, - LastLogin: time.Now().Add(-30 * time.Minute), + LastLogin: time.Now().UTC().Add(-30 * time.Minute), UserID: userID, }, "peer-2": { ID: "peer-2", LoginExpirationEnabled: true, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: true, LoginExpired: false, }, - LastLogin: time.Now().Add(-2 * time.Hour), + LastLogin: time.Now().UTC().Add(-2 * time.Hour), UserID: userID, }, @@ -1592,11 +1592,11 @@ func TestAccount_GetExpiredPeers(t *testing.T) { ID: "peer-3", LoginExpirationEnabled: true, Status: &PeerStatus{ - LastSeen: time.Now(), + LastSeen: time.Now().UTC(), Connected: true, LoginExpired: false, }, - LastLogin: time.Now().Add(-1 * time.Hour), + LastLogin: time.Now().UTC().Add(-1 * time.Hour), UserID: userID, }, }, @@ -1797,7 +1797,7 @@ func TestAccount_GetNextPeerExpiration(t *testing.T) { LoginExpired: false, }, LoginExpirationEnabled: true, - LastLogin: time.Now(), + LastLogin: time.Now().UTC(), UserID: userID, }, "peer-2": { diff --git a/management/server/activity/sqlite/sqlite_test.go b/management/server/activity/sqlite/sqlite_test.go index f42c94d36..2ca9a1e64 100644 --- a/management/server/activity/sqlite/sqlite_test.go +++ b/management/server/activity/sqlite/sqlite_test.go @@ -2,10 +2,12 @@ package sqlite import ( "fmt" - "github.com/netbirdio/netbird/management/server/activity" - "github.com/stretchr/testify/assert" "testing" "time" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/management/server/activity" ) func TestNewSQLiteStore(t *testing.T) { @@ -15,13 +17,13 @@ func TestNewSQLiteStore(t *testing.T) { t.Fatal(err) return } - defer store.Close() //nolint + defer store.Close() //nolint accountID := "account_1" for i := 0; i < 10; i++ { _, err = store.Save(&activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.PeerAddedByUser, InitiatorID: "user_" + fmt.Sprint(i), TargetID: "peer_" + fmt.Sprint(i), diff --git a/management/server/event.go b/management/server/event.go index f9d61c84b..6ada88d6f 100644 --- a/management/server/event.go +++ b/management/server/event.go @@ -2,9 +2,11 @@ package server import ( "fmt" - "github.com/netbirdio/netbird/management/server/activity" - log "github.com/sirupsen/logrus" "time" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/management/server/activity" ) // GetEvents returns a list of activity events of an account @@ -39,7 +41,7 @@ func (am *DefaultAccountManager) storeEvent(initiatorID, targetID, accountID str go func() { _, err := am.eventStore.Save(&activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activityID, InitiatorID: initiatorID, TargetID: targetID, @@ -47,7 +49,7 @@ func (am *DefaultAccountManager) storeEvent(initiatorID, targetID, accountID str Meta: meta, }) if err != nil { - //todo add metric + // todo add metric log.Errorf("received an error while storing an activity event, error: %s", err) } }() diff --git a/management/server/event_test.go b/management/server/event_test.go index be6dd7005..320728eef 100644 --- a/management/server/event_test.go +++ b/management/server/event_test.go @@ -1,17 +1,19 @@ package server import ( - "github.com/netbirdio/netbird/management/server/activity" - "github.com/stretchr/testify/assert" "testing" "time" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/management/server/activity" ) func generateAndStoreEvents(t *testing.T, manager *DefaultAccountManager, typ activity.Activity, initiatorID, targetID, accountID string, count int) { for i := 0; i < count; i++ { _, err := manager.eventStore.Save(&activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: typ, InitiatorID: initiatorID, TargetID: targetID, diff --git a/management/server/file_store.go b/management/server/file_store.go index f79179841..1815cf6fe 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -173,7 +173,7 @@ func restore(file string) (*FileStore, error) { for key, peer := range account.Peers { // set LastLogin for the peers that were onboarded before the peer login expiration feature if peer.LastLogin.IsZero() { - peer.LastLogin = time.Now() + peer.LastLogin = time.Now().UTC() } if peer.ID != "" { continue @@ -227,7 +227,7 @@ func (s *FileStore) persist(file string) error { // AcquireGlobalLock acquires global lock across all the accounts and returns a function that releases the lock func (s *FileStore) AcquireGlobalLock() (unlock func()) { log.Debugf("acquiring global lock") - start := time.Now() + start := time.Now().UTC() s.globalAccountLock.Lock() unlock = func() { @@ -241,7 +241,7 @@ func (s *FileStore) AcquireGlobalLock() (unlock func()) { // AcquireAccountLock acquires account lock and returns a function that releases the lock func (s *FileStore) AcquireAccountLock(accountID string) (unlock func()) { log.Debugf("acquiring lock for account %s", accountID) - start := time.Now() + start := time.Now().UTC() value, _ := s.accountLocks.LoadOrStore(accountID, &sync.Mutex{}) mtx := value.(*sync.Mutex) mtx.Lock() diff --git a/management/server/file_store_test.go b/management/server/file_store_test.go index b2e9bff29..484ee987b 100644 --- a/management/server/file_store_test.go +++ b/management/server/file_store_test.go @@ -95,7 +95,7 @@ func TestSaveAccount(t *testing.T) { IP: net.IP{127, 0, 0, 1}, Meta: PeerSystemMeta{}, Name: "peer name", - Status: &PeerStatus{Connected: true, LastSeen: time.Now()}, + Status: &PeerStatus{Connected: true, LastSeen: time.Now().UTC()}, } // SaveAccount should trigger persist @@ -131,7 +131,7 @@ func TestStore(t *testing.T) { IP: net.IP{127, 0, 0, 1}, Meta: PeerSystemMeta{}, Name: "peer name", - Status: &PeerStatus{Connected: true, LastSeen: time.Now()}, + Status: &PeerStatus{Connected: true, LastSeen: time.Now().UTC()}, } account.Groups["all"] = &Group{ ID: "all", @@ -514,7 +514,7 @@ func TestFileStore_SavePeerStatus(t *testing.T) { } // save status of non-existing peer - newStatus := PeerStatus{Connected: true, LastSeen: time.Now()} + newStatus := PeerStatus{Connected: true, LastSeen: time.Now().UTC()} err = store.SavePeerStatus(account.Id, "non-existing-peer", newStatus) assert.Error(t, err) @@ -526,7 +526,7 @@ func TestFileStore_SavePeerStatus(t *testing.T) { IP: net.IP{127, 0, 0, 1}, Meta: PeerSystemMeta{}, Name: "peer name", - Status: &PeerStatus{Connected: false, LastSeen: time.Now()}, + Status: &PeerStatus{Connected: false, LastSeen: time.Now().UTC()}, } err = store.SaveAccount(account) diff --git a/management/server/grpcserver.go b/management/server/grpcserver.go index 0c8dad246..b92704435 100644 --- a/management/server/grpcserver.go +++ b/management/server/grpcserver.go @@ -98,7 +98,7 @@ func (s *GRPCServer) GetServerKey(ctx context.Context, req *proto.Empty) (*proto if s.appMetrics != nil { s.appMetrics.GRPCMetrics().CountGetKeyRequest() } - now := time.Now().Add(24 * time.Hour) + now := time.Now().UTC().Add(24 * time.Hour) secs := int64(now.Second()) nanos := int32(now.Nanosecond()) expiresAt := ×tamp.Timestamp{Seconds: secs, Nanos: nanos} diff --git a/management/server/http/events_handler_test.go b/management/server/http/events_handler_test.go index 015c25458..a77e44f45 100644 --- a/management/server/http/events_handler_test.go +++ b/management/server/http/events_handler_test.go @@ -54,7 +54,7 @@ func generateEvents(accountID, userID string) []*activity.Event { ID := uint64(1) events := make([]*activity.Event, 0) events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.PeerAddedByUser, ID: ID, InitiatorID: userID, @@ -64,7 +64,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.UserJoined, ID: ID, InitiatorID: userID, @@ -74,7 +74,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.GroupCreated, ID: ID, InitiatorID: userID, @@ -84,7 +84,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.SetupKeyUpdated, ID: ID, InitiatorID: userID, @@ -94,7 +94,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.SetupKeyUpdated, ID: ID, InitiatorID: userID, @@ -104,7 +104,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.SetupKeyRevoked, ID: ID, InitiatorID: userID, @@ -114,7 +114,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.SetupKeyOverused, ID: ID, InitiatorID: userID, @@ -124,7 +124,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.SetupKeyCreated, ID: ID, InitiatorID: userID, @@ -134,7 +134,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.RuleAdded, ID: ID, InitiatorID: userID, @@ -144,7 +144,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.RuleRemoved, ID: ID, InitiatorID: userID, @@ -154,7 +154,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.RuleUpdated, ID: ID, InitiatorID: userID, @@ -164,7 +164,7 @@ func generateEvents(accountID, userID string) []*activity.Event { }) ID++ events = append(events, &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), Activity: activity.PeerAddedWithSetupKey, ID: ID, InitiatorID: userID, diff --git a/management/server/http/middleware/auth_middleware.go b/management/server/http/middleware/auth_middleware.go index a8c81012a..09771deb4 100644 --- a/management/server/http/middleware/auth_middleware.go +++ b/management/server/http/middleware/auth_middleware.go @@ -117,7 +117,7 @@ func (m *AuthMiddleware) CheckPATFromRequest(w http.ResponseWriter, r *http.Requ if err != nil { return fmt.Errorf("invalid Token: %w", err) } - if time.Now().After(pat.ExpirationDate) { + if time.Now().UTC().After(pat.ExpirationDate) { return fmt.Errorf("token expired") } diff --git a/management/server/http/middleware/auth_middleware_test.go b/management/server/http/middleware/auth_middleware_test.go index 5a5558fa5..b041b12d5 100644 --- a/management/server/http/middleware/auth_middleware_test.go +++ b/management/server/http/middleware/auth_middleware_test.go @@ -34,10 +34,10 @@ var testAccount = &server.Account{ ID: tokenID, Name: "My first token", HashedToken: "someHash", - ExpirationDate: time.Now().AddDate(0, 0, 7), + ExpirationDate: time.Now().UTC().AddDate(0, 0, 7), CreatedBy: userID, - CreatedAt: time.Now(), - LastUsed: time.Now(), + CreatedAt: time.Now().UTC(), + LastUsed: time.Now().UTC(), }, }, }, diff --git a/management/server/http/pat_handler_test.go b/management/server/http/pat_handler_test.go index de79f1006..6a1ea8312 100644 --- a/management/server/http/pat_handler_test.go +++ b/management/server/http/pat_handler_test.go @@ -41,19 +41,19 @@ var testAccount = &server.Account{ ID: existingTokenID, Name: "My first token", HashedToken: "someHash", - ExpirationDate: time.Now().AddDate(0, 0, 7), + ExpirationDate: time.Now().UTC().AddDate(0, 0, 7), CreatedBy: existingUserID, - CreatedAt: time.Now(), - LastUsed: time.Now(), + CreatedAt: time.Now().UTC(), + LastUsed: time.Now().UTC(), }, "token2": { ID: "token2", Name: "My second token", HashedToken: "someOtherHash", - ExpirationDate: time.Now().AddDate(0, 0, 7), + ExpirationDate: time.Now().UTC().AddDate(0, 0, 7), CreatedBy: existingUserID, - CreatedAt: time.Now(), - LastUsed: time.Now(), + CreatedAt: time.Now().UTC(), + LastUsed: time.Now().UTC(), }, }, }, diff --git a/management/server/idp/auth0.go b/management/server/idp/auth0.go index a19117326..89158e164 100644 --- a/management/server/idp/auth0.go +++ b/management/server/idp/auth0.go @@ -6,7 +6,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/netbirdio/netbird/management/server/telemetry" "io" "net/http" "net/url" @@ -15,6 +14,8 @@ import ( "sync" "time" + "github.com/netbirdio/netbird/management/server/telemetry" + "github.com/golang-jwt/jwt" log "github.com/sirupsen/logrus" ) @@ -151,7 +152,7 @@ func NewAuth0Manager(config Auth0ClientConfig, appMetrics telemetry.AppMetrics) // jwtStillValid returns true if the token still valid and have enough time to be used and get a response from Auth0 func (c *Auth0Credentials) jwtStillValid() bool { - return !c.jwtToken.expiresInTime.IsZero() && time.Now().Add(5*time.Second).Before(c.jwtToken.expiresInTime) + return !c.jwtToken.expiresInTime.IsZero() && time.Now().UTC().Add(5*time.Second).Before(c.jwtToken.expiresInTime) } // requestJWTToken performs request to get jwt token diff --git a/management/server/idp/auth0_test.go b/management/server/idp/auth0_test.go index 58a6df8f6..ab86f3986 100644 --- a/management/server/idp/auth0_test.go +++ b/management/server/idp/auth0_test.go @@ -3,14 +3,16 @@ package idp import ( "encoding/json" "fmt" - "github.com/netbirdio/netbird/management/server/telemetry" - "github.com/stretchr/testify/require" "io" "net/http" "strings" "testing" "time" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/telemetry" + "github.com/golang-jwt/jwt" "github.com/stretchr/testify/assert" ) @@ -63,7 +65,7 @@ func (mc *mockAuth0Credentials) Authenticate() (JWTToken, error) { } func newTestJWT(t *testing.T, expInt int) string { - now := time.Now() + now := time.Now().UTC() token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ "iat": now.Unix(), "exp": now.Add(time.Duration(expInt) * time.Second).Unix(), @@ -207,13 +209,13 @@ func TestAuth0_JwtStillValid(t *testing.T) { } jwtStillValidTestCase1 := jwtStillValidTest{ name: "JWT still valid", - inputTime: time.Now().Add(10 * time.Second), + inputTime: time.Now().UTC().Add(10 * time.Second), expectedResult: true, message: "should be true", } jwtStillValidTestCase2 := jwtStillValidTest{ name: "JWT is invalid", - inputTime: time.Now(), + inputTime: time.Now().UTC(), expectedResult: false, message: "should be false", } @@ -249,9 +251,9 @@ func TestAuth0_Authenticate(t *testing.T) { authenticateTestCase1 := authenticateTest{ name: "Get Cached token", - inputExpireToken: time.Now().Add(30 * time.Second), + inputExpireToken: time.Now().UTC().Add(30 * time.Second), helper: JsonParser{}, - //expectedFuncExitErrDiff: fmt.Errorf("unable to get token, statusCode 400"), + // expectedFuncExitErrDiff: fmt.Errorf("unable to get token, statusCode 400"), expectedCode: 200, expectedToken: "", } diff --git a/management/server/idp/keycloak.go b/management/server/idp/keycloak.go index f9fc94ae7..8ae09fc99 100644 --- a/management/server/idp/keycloak.go +++ b/management/server/idp/keycloak.go @@ -13,8 +13,9 @@ import ( "time" "github.com/golang-jwt/jwt" - "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/management/server/telemetry" ) const ( @@ -118,7 +119,7 @@ func NewKeycloakManager(config KeycloakClientConfig, appMetrics telemetry.AppMet // jwtStillValid returns true if the token still valid and have enough time to be used and get a response from keycloak. func (kc *KeycloakCredentials) jwtStillValid() bool { - return !kc.jwtToken.expiresInTime.IsZero() && time.Now().Add(5*time.Second).Before(kc.jwtToken.expiresInTime) + return !kc.jwtToken.expiresInTime.IsZero() && time.Now().UTC().Add(5*time.Second).Before(kc.jwtToken.expiresInTime) } // requestJWTToken performs request to get jwt token. diff --git a/management/server/idp/keycloak_test.go b/management/server/idp/keycloak_test.go index 00acf81bd..6cff7f56a 100644 --- a/management/server/idp/keycloak_test.go +++ b/management/server/idp/keycloak_test.go @@ -7,9 +7,10 @@ import ( "testing" "time" - "github.com/netbirdio/netbird/management/server/telemetry" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/telemetry" ) func TestNewKeycloakManager(t *testing.T) { @@ -198,13 +199,13 @@ func TestKeycloakJwtStillValid(t *testing.T) { jwtStillValidTestCase1 := jwtStillValidTest{ name: "JWT still valid", - inputTime: time.Now().Add(10 * time.Second), + inputTime: time.Now().UTC().Add(10 * time.Second), expectedResult: true, message: "should be true", } jwtStillValidTestCase2 := jwtStillValidTest{ name: "JWT is invalid", - inputTime: time.Now(), + inputTime: time.Now().UTC(), expectedResult: false, message: "should be false", } @@ -239,7 +240,7 @@ func TestKeycloakAuthenticate(t *testing.T) { authenticateTestCase1 := authenticateTest{ name: "Get Cached token", - inputExpireToken: time.Now().Add(30 * time.Second), + inputExpireToken: time.Now().UTC().Add(30 * time.Second), helper: JsonParser{}, expectedFuncExitErrDiff: nil, expectedCode: 200, diff --git a/management/server/management_test.go b/management/server/management_test.go index d892fecf8..25ce4fb0d 100644 --- a/management/server/management_test.go +++ b/management/server/management_test.go @@ -12,20 +12,23 @@ import ( "github.com/netbirdio/netbird/management/server/activity" - server "github.com/netbirdio/netbird/management/server" "google.golang.org/grpc/credentials/insecure" + "github.com/netbirdio/netbird/management/server" + pb "github.com/golang/protobuf/proto" //nolint - "github.com/netbirdio/netbird/encryption" log "github.com/sirupsen/logrus" - mgmtProto "github.com/netbirdio/netbird/management/proto" - "github.com/netbirdio/netbird/util" + "github.com/netbirdio/netbird/encryption" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" "google.golang.org/grpc" "google.golang.org/grpc/keepalive" + + mgmtProto "github.com/netbirdio/netbird/management/proto" + "github.com/netbirdio/netbird/util" ) const ( @@ -368,7 +371,7 @@ var _ = Describe("Management service", func() { for i := 0; i < additionalPeers; i++ { key, _ := wgtypes.GenerateKey() loginPeerWithValidSetupKey(serverPubKey, key, client) - rand.Seed(time.Now().UnixNano()) + rand.Seed(time.Now().UTC().UnixNano()) n := rand.Intn(200) time.Sleep(time.Duration(n) * time.Millisecond) } diff --git a/management/server/metrics/selfhosted.go b/management/server/metrics/selfhosted.go index 2c7e943ac..751e07fba 100644 --- a/management/server/metrics/selfhosted.go +++ b/management/server/metrics/selfhosted.go @@ -67,7 +67,7 @@ type Worker struct { // NewWorker returns a metrics worker func NewWorker(ctx context.Context, id string, dataSource DataSource, connManager ConnManager) *Worker { - currentTime := time.Now() + currentTime := time.Now().UTC() return &Worker{ ctx: ctx, id: id, @@ -90,7 +90,7 @@ func (w *Worker) Run() { if err != nil { log.Error(err) } - w.lastRun = time.Now() + w.lastRun = time.Now().UTC() } } } @@ -149,7 +149,7 @@ func (w *Worker) generatePayload(apiKey string) pushPayload { DistinctID: w.id, Event: PayloadEvent, Properties: properties, - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), } } @@ -172,7 +172,7 @@ func (w *Worker) generateProperties() properties { peerActiveVersions []string osUIClients map[string]int ) - start := time.Now() + start := time.Now().UTC() metricsProperties := make(properties) osPeers = make(map[string]int) osUIClients = make(map[string]int) diff --git a/management/server/network.go b/management/server/network.go index c436a88b8..2fe99338f 100644 --- a/management/server/network.go +++ b/management/server/network.go @@ -1,15 +1,17 @@ package server import ( - "github.com/c-robinson/iplib" - nbdns "github.com/netbirdio/netbird/dns" - "github.com/netbirdio/netbird/management/server/status" - "github.com/netbirdio/netbird/route" - "github.com/rs/xid" "math/rand" "net" "sync" "time" + + "github.com/c-robinson/iplib" + "github.com/rs/xid" + + nbdns "github.com/netbirdio/netbird/dns" + "github.com/netbirdio/netbird/management/server/status" + "github.com/netbirdio/netbird/route" ) const ( @@ -48,7 +50,7 @@ func NewNetwork() *Network { n := iplib.NewNet4(net.ParseIP("100.64.0.0"), NetSize) sub, _ := n.Subnet(SubnetSize) - s := rand.NewSource(time.Now().Unix()) + s := rand.NewSource(time.Now().UTC().Unix()) r := rand.New(s) intn := r.Intn(len(sub)) @@ -99,7 +101,7 @@ func AllocatePeerIP(ipNet net.IPNet, takenIps []net.IP) (net.IP, error) { } // pick a random IP - s := rand.NewSource(time.Now().Unix()) + s := rand.NewSource(time.Now().UTC().Unix()) r := rand.New(s) intn := r.Intn(len(ips)) diff --git a/management/server/peer.go b/management/server/peer.go index 7b5ca539f..cb0112d37 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -6,9 +6,10 @@ import ( "strings" "time" + "github.com/rs/xid" + "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/status" - "github.com/rs/xid" log "github.com/sirupsen/logrus" @@ -143,7 +144,7 @@ func (p *Peer) LoginExpired(expiresIn time.Duration) (bool, time.Duration) { return false, 0 } expiresAt := p.LastLogin.Add(expiresIn) - now := time.Now() + now := time.Now().UTC() timeLeft := expiresAt.Sub(now) return timeLeft <= 0, timeLeft } @@ -245,7 +246,7 @@ func (am *DefaultAccountManager) MarkPeerConnected(peerPubKey string, connected oldStatus := peer.Status.Copy() newStatus := oldStatus - newStatus.LastSeen = time.Now() + newStatus.LastSeen = time.Now().UTC() newStatus.Connected = connected // whenever peer got connected that means that it logged in successfully if newStatus.Connected { @@ -477,7 +478,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* } opEvent := &activity.Event{ - Timestamp: time.Now(), + Timestamp: time.Now().UTC(), AccountID: account.Id, } @@ -524,10 +525,10 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* Name: peer.Meta.Hostname, DNSLabel: newLabel, UserID: userID, - Status: &PeerStatus{Connected: false, LastSeen: time.Now()}, + Status: &PeerStatus{Connected: false, LastSeen: time.Now().UTC()}, SSHEnabled: false, SSHKey: peer.SSHKey, - LastLogin: time.Now(), + LastLogin: time.Now().UTC(), LoginExpirationEnabled: addedByUser, } @@ -704,7 +705,7 @@ func updatePeerLastLogin(peer *Peer, account *Account) { // UpdateLastLogin and set login expired false func (p *Peer) UpdateLastLogin() *Peer { - p.LastLogin = time.Now() + p.LastLogin = time.Now().UTC() newStatus := p.Status.Copy() newStatus.LoginExpired = false p.Status = newStatus diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 6adfc55da..b10a66e12 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -21,7 +21,7 @@ func TestPeer_LoginExpired(t *testing.T) { { name: "Peer Login Expiration Disabled. Peer Login Should Not Expire", expirationEnabled: false, - lastLogin: time.Now().Add(-25 * time.Hour), + lastLogin: time.Now().UTC().Add(-25 * time.Hour), accountSettings: &Settings{ PeerLoginExpirationEnabled: true, PeerLoginExpiration: time.Hour, @@ -31,7 +31,7 @@ func TestPeer_LoginExpired(t *testing.T) { { name: "Peer Login Should Expire", expirationEnabled: true, - lastLogin: time.Now().Add(-25 * time.Hour), + lastLogin: time.Now().UTC().Add(-25 * time.Hour), accountSettings: &Settings{ PeerLoginExpirationEnabled: true, PeerLoginExpiration: time.Hour, @@ -41,7 +41,7 @@ func TestPeer_LoginExpired(t *testing.T) { { name: "Peer Login Should Not Expire", expirationEnabled: true, - lastLogin: time.Now(), + lastLogin: time.Now().UTC(), accountSettings: &Settings{ PeerLoginExpirationEnabled: true, PeerLoginExpiration: time.Hour, diff --git a/management/server/setupkey.go b/management/server/setupkey.go index 0ba6bff91..cce819056 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -1,15 +1,17 @@ package server import ( - "github.com/google/uuid" - "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/status" - log "github.com/sirupsen/logrus" "hash/fnv" "strconv" "strings" "time" "unicode/utf8" + + "github.com/google/uuid" + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/management/server/activity" + "github.com/netbirdio/netbird/management/server/status" ) const ( @@ -130,7 +132,7 @@ func (key *SetupKey) HiddenCopy(length int) *SetupKey { func (key *SetupKey) IncrementUsage() *SetupKey { c := key.Copy() c.UsedTimes = c.UsedTimes + 1 - c.LastUsed = time.Now() + c.LastUsed = time.Now().UTC() return c } @@ -146,7 +148,7 @@ func (key *SetupKey) IsRevoked() bool { // IsExpired if key was expired func (key *SetupKey) IsExpired() bool { - return time.Now().After(key.ExpiresAt) + return time.Now().UTC().After(key.ExpiresAt) } // IsOverUsed if the key was used too many times. SetupKey.UsageLimit == 0 indicates the unlimited usage. @@ -171,9 +173,9 @@ func GenerateSetupKey(name string, t SetupKeyType, validFor time.Duration, autoG Key: key, Name: name, Type: t, - CreatedAt: time.Now(), - ExpiresAt: time.Now().Add(validFor), - UpdatedAt: time.Now(), + CreatedAt: time.Now().UTC(), + ExpiresAt: time.Now().UTC().Add(validFor), + UpdatedAt: time.Now().UTC(), Revoked: false, UsedTimes: 0, AutoGroups: autoGroups, @@ -274,7 +276,7 @@ func (am *DefaultAccountManager) SaveSetupKey(accountID string, keyToSave *Setup newKey.Name = keyToSave.Name newKey.AutoGroups = keyToSave.AutoGroups newKey.Revoked = keyToSave.Revoked - newKey.UpdatedAt = time.Now() + newKey.UpdatedAt = time.Now().UTC() account.SetupKeys[newKey.Key] = newKey diff --git a/management/server/setupkey_test.go b/management/server/setupkey_test.go index d8ffc320c..a6f318ab9 100644 --- a/management/server/setupkey_test.go +++ b/management/server/setupkey_test.go @@ -2,12 +2,14 @@ package server import ( "fmt" - "github.com/google/uuid" - "github.com/netbirdio/netbird/management/server/activity" - "github.com/stretchr/testify/assert" "strconv" "testing" "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/management/server/activity" ) func TestDefaultAccountManager_SaveSetupKey(t *testing.T) { @@ -54,7 +56,7 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) { } assertKey(t, newKey, newKeyName, revoked, "reusable", 0, key.CreatedAt, key.ExpiresAt, - key.Id, time.Now(), autoGroups) + key.Id, time.Now().UTC(), autoGroups) // check the corresponding events that should have been generated ev := getEvent(t, account.Id, manager, activity.SetupKeyRevoked) @@ -108,10 +110,10 @@ func TestDefaultAccountManager_CreateSetupKey(t *testing.T) { expectedCreatedAt time.Time expectedUpdatedAt time.Time expectedExpiresAt time.Time - expectedFailure bool //indicates whether key creation should fail + expectedFailure bool // indicates whether key creation should fail } - now := time.Now() + now := time.Now().UTC() expiresIn := time.Hour testCase1 := testCase{ name: "Should Create Setup Key successfully", @@ -169,9 +171,9 @@ func TestGenerateDefaultSetupKey(t *testing.T) { expectedRevoke := false expectedType := "reusable" expectedUsedTimes := 0 - expectedCreatedAt := time.Now() - expectedUpdatedAt := time.Now() - expectedExpiresAt := time.Now().Add(24 * 30 * time.Hour) + expectedCreatedAt := time.Now().UTC() + expectedUpdatedAt := time.Now().UTC() + expectedExpiresAt := time.Now().UTC().Add(24 * 30 * time.Hour) var expectedAutoGroups []string key := GenerateDefaultSetupKey() @@ -186,9 +188,9 @@ func TestGenerateSetupKey(t *testing.T) { expectedRevoke := false expectedType := "one-off" expectedUsedTimes := 0 - expectedCreatedAt := time.Now() - expectedExpiresAt := time.Now().Add(time.Hour) - expectedUpdatedAt := time.Now() + expectedCreatedAt := time.Now().UTC() + expectedExpiresAt := time.Now().UTC().Add(time.Hour) + expectedUpdatedAt := time.Now().UTC() var expectedAutoGroups []string key := GenerateSetupKey(expectedName, SetupKeyOneOff, time.Hour, []string{}, SetupKeyUnlimitedUsage) diff --git a/management/server/turncredentials.go b/management/server/turncredentials.go index a9423ea71..8db885853 100644 --- a/management/server/turncredentials.go +++ b/management/server/turncredentials.go @@ -5,10 +5,12 @@ import ( "crypto/sha1" "encoding/base64" "fmt" - "github.com/netbirdio/netbird/management/proto" - log "github.com/sirupsen/logrus" "sync" "time" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/management/proto" ) // TURNCredentialsManager used to manage TURN credentials @@ -44,7 +46,7 @@ func NewTimeBasedAuthSecretsManager(updateManager *PeersUpdateManager, config *T func (m *TimeBasedAuthSecretsManager) GenerateCredentials() TURNCredentials { mac := hmac.New(sha1.New, []byte(m.config.Secret)) - timeAuth := time.Now().Add(m.config.CredentialsTTL.Duration).Unix() + timeAuth := time.Now().UTC().Add(m.config.CredentialsTTL.Duration).Unix() username := fmt.Sprint(timeAuth) @@ -88,7 +90,7 @@ func (m *TimeBasedAuthSecretsManager) SetupRefresh(peerID string) { log.Debugf("starting turn refresh for %s", peerID) go func() { - //we don't want to regenerate credentials right on expiration, so we do it slightly before (at 3/4 of TTL) + // we don't want to regenerate credentials right on expiration, so we do it slightly before (at 3/4 of TTL) ticker := time.NewTicker(m.config.CredentialsTTL.Duration / 4 * 3) for { From 1057cd211d9bf7a8dca40364954fa96f53f14278 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Wed, 5 Apr 2023 21:57:47 +0200 Subject: [PATCH 3/9] Add scope and id token environment variables (#785) --- .github/workflows/test-docker-compose-linux.yml | 3 +++ infrastructure_files/base.setup.env | 9 +++++++-- infrastructure_files/management.json.tmpl | 4 +++- infrastructure_files/setup.env.example | 5 ++++- infrastructure_files/tests/setup.env | 3 ++- 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test-docker-compose-linux.yml b/.github/workflows/test-docker-compose-linux.yml index c28e94a4f..3c9c944d6 100644 --- a/.github/workflows/test-docker-compose-linux.yml +++ b/.github/workflows/test-docker-compose-linux.yml @@ -62,6 +62,7 @@ jobs: CI_NETBIRD_TOKEN_SOURCE: "idToken" CI_NETBIRD_AUTH_USER_ID_CLAIM: "email" CI_NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE: "super" + CI_NETBIRD_AUTH_DEVICE_AUTH_SCOPE: "openid email" run: | grep AUTH_CLIENT_ID docker-compose.yml | grep $CI_NETBIRD_AUTH_CLIENT_ID @@ -76,6 +77,8 @@ jobs: grep NETBIRD_TOKEN_SOURCE docker-compose.yml | grep $CI_NETBIRD_TOKEN_SOURCE grep AuthUserIDClaim management.json | grep $CI_NETBIRD_AUTH_USER_ID_CLAIM grep -A 1 ProviderConfig management.json | grep Audience | grep $CI_NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE + grep Scope management.json | grep "$CI_NETBIRD_AUTH_DEVICE_AUTH_SCOPE" + grep UseIDToken management.json | grep false - name: run docker compose up working-directory: infrastructure_files diff --git a/infrastructure_files/base.setup.env b/infrastructure_files/base.setup.env index 8fa58ffc3..6d6b21238 100644 --- a/infrastructure_files/base.setup.env +++ b/infrastructure_files/base.setup.env @@ -34,9 +34,12 @@ SIGNAL_VOLUMESUFFIX="signal" LETSENCRYPT_VOLUMESUFFIX="letsencrypt" NETBIRD_AUTH_DEVICE_AUTH_PROVIDER="none" +NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE=${NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE:-$NETBIRD_AUTH_AUDIENCE} +NETBIRD_AUTH_DEVICE_AUTH_SCOPE=${NETBIRD_AUTH_DEVICE_AUTH_SCOPE:-openid} +NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN=${NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN:-false} + NETBIRD_DISABLE_ANONYMOUS_METRICS=${NETBIRD_DISABLE_ANONYMOUS_METRICS:-false} -NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE=${NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE:-$NETBIRD_AUTH_AUDIENCE} NETBIRD_TOKEN_SOURCE=${NETBIRD_TOKEN_SOURCE:-accessToken} # exports @@ -72,4 +75,6 @@ export NETBIRD_SIGNAL_PROTOCOL export NETBIRD_SIGNAL_PORT export NETBIRD_AUTH_USER_ID_CLAIM export NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE -export NETBIRD_TOKEN_SOURCE \ No newline at end of file +export NETBIRD_TOKEN_SOURCE +export NETBIRD_AUTH_DEVICE_AUTH_SCOPE +export NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN \ No newline at end of file diff --git a/infrastructure_files/management.json.tmpl b/infrastructure_files/management.json.tmpl index 19dcff898..a0a91b3d3 100644 --- a/infrastructure_files/management.json.tmpl +++ b/infrastructure_files/management.json.tmpl @@ -47,7 +47,9 @@ "Domain": "$NETBIRD_AUTH0_DOMAIN", "ClientID": "$NETBIRD_AUTH_DEVICE_AUTH_CLIENT_ID", "TokenEndpoint": "$NETBIRD_AUTH_TOKEN_ENDPOINT", - "DeviceAuthEndpoint": "$NETBIRD_AUTH_DEVICE_AUTH_ENDPOINT" + "DeviceAuthEndpoint": "$NETBIRD_AUTH_DEVICE_AUTH_ENDPOINT", + "Scope": "$NETBIRD_AUTH_DEVICE_AUTH_SCOPE", + "UseIDToken": $NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN } } } diff --git a/infrastructure_files/setup.env.example b/infrastructure_files/setup.env.example index 324174757..6781e050a 100644 --- a/infrastructure_files/setup.env.example +++ b/infrastructure_files/setup.env.example @@ -17,8 +17,11 @@ NETBIRD_AUTH_CLIENT_ID="" NETBIRD_USE_AUTH0="false" NETBIRD_AUTH_DEVICE_AUTH_PROVIDER="none" NETBIRD_AUTH_DEVICE_AUTH_CLIENT_ID="" -# Some IDPs requires different audience for device authorization flow, you can customize here +# Some IDPs requires different audience, scopes and to use id token for device authorization flow +# you can customize here: NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE +NETBIRD_AUTH_DEVICE_AUTH_SCOPE="openid" +NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN=false # if your IDP provider doesn't support fragmented URIs, configure custom # redirect and silent redirect URIs, these will be concatenated into your NETBIRD_DOMAIN domain. diff --git a/infrastructure_files/tests/setup.env b/infrastructure_files/tests/setup.env index 09164a135..cafcba246 100644 --- a/infrastructure_files/tests/setup.env +++ b/infrastructure_files/tests/setup.env @@ -15,4 +15,5 @@ NETBIRD_AUTH_REDIRECT_URI="/peers" NETBIRD_DISABLE_LETSENCRYPT=true NETBIRD_TOKEN_SOURCE="idToken" NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE="super" -NETBIRD_AUTH_USER_ID_CLAIM="email" \ No newline at end of file +NETBIRD_AUTH_USER_ID_CLAIM="email" +NETBIRD_AUTH_DEVICE_AUTH_SCOPE="openid email" \ No newline at end of file From 0aad9169e9d7ec5a1e7e82514b5f717b776fcd11 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Thu, 6 Apr 2023 18:15:55 +0200 Subject: [PATCH 4/9] Fix nil pointer exception (#790) Nil pointer exception fix. The error handling was in wrong order. --- client/internal/connect.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/internal/connect.go b/client/internal/connect.go index 47c63e6d0..1d0585647 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -82,12 +82,12 @@ func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status, log.Debugf("conecting to the Management service %s", config.ManagementURL.Host) mgmClient, err := mgm.NewClient(engineCtx, config.ManagementURL.Host, myPrivateKey, mgmTlsEnabled) - mgmNotifier := statusRecorderToMgmConnStateNotifier(statusRecorder) - mgmClient.SetConnStateListener(mgmNotifier) - if err != nil { return wrapErr(gstatus.Errorf(codes.FailedPrecondition, "failed connecting to Management Service : %s", err)) } + mgmNotifier := statusRecorderToMgmConnStateNotifier(statusRecorder) + mgmClient.SetConnStateListener(mgmNotifier) + log.Debugf("connected to the Management service %s", config.ManagementURL.Host) defer func() { err = mgmClient.Close() From 8f9826b2077713f2c9943837474caff2e7d2288e Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Fri, 7 Apr 2023 10:34:17 +0200 Subject: [PATCH 5/9] Fix export path for certificate files (#794) assign the value for NETBIRD_LETSENCRYPT_DOMAIN in the base.setup.env file --- infrastructure_files/base.setup.env | 2 ++ infrastructure_files/configure.sh | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/infrastructure_files/base.setup.env b/infrastructure_files/base.setup.env index 6d6b21238..3d3f92ef4 100644 --- a/infrastructure_files/base.setup.env +++ b/infrastructure_files/base.setup.env @@ -7,6 +7,7 @@ NETBIRD_MGMT_API_PORT=${NETBIRD_MGMT_API_PORT:-33073} # Management API endpoint address, used by the Dashboard NETBIRD_MGMT_API_ENDPOINT=https://$NETBIRD_DOMAIN:$NETBIRD_MGMT_API_PORT # Management Certficate file path. These are generated by the Dashboard container +NETBIRD_LETSENCRYPT_DOMAIN=$NETBIRD_DOMAIN NETBIRD_MGMT_API_CERT_FILE="/etc/letsencrypt/live/$NETBIRD_LETSENCRYPT_DOMAIN/fullchain.pem" # Management Certficate key file path. NETBIRD_MGMT_API_CERT_KEY_FILE="/etc/letsencrypt/live/$NETBIRD_LETSENCRYPT_DOMAIN/privkey.pem" @@ -53,6 +54,7 @@ export NETBIRD_AUTH_JWT_CERTS export NETBIRD_LETSENCRYPT_EMAIL export NETBIRD_MGMT_API_PORT export NETBIRD_MGMT_API_ENDPOINT +export NETBIRD_LETSENCRYPT_DOMAIN export NETBIRD_MGMT_API_CERT_FILE export NETBIRD_MGMT_API_CERT_KEY_FILE export NETBIRD_AUTH_DEVICE_AUTH_PROVIDER diff --git a/infrastructure_files/configure.sh b/infrastructure_files/configure.sh index 501098a57..872d46c4b 100755 --- a/infrastructure_files/configure.sh +++ b/infrastructure_files/configure.sh @@ -143,8 +143,6 @@ then unset NETBIRD_LETSENCRYPT_DOMAIN unset NETBIRD_MGMT_API_CERT_FILE unset NETBIRD_MGMT_API_CERT_KEY_FILE -else - export NETBIRD_LETSENCRYPT_DOMAIN="$NETBIRD_DOMAIN" fi env | grep NETBIRD From 6aba28ccb7b724e4705dcbd98edbcb131b2080ae Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 10 Apr 2023 10:54:23 +0200 Subject: [PATCH 6/9] remove UTC from some not store related operations --- client/cmd/status.go | 2 +- management/server/file_store.go | 4 ++-- management/server/grpcserver.go | 2 +- management/server/http/middleware/auth_middleware.go | 2 +- management/server/idp/auth0.go | 2 +- management/server/idp/auth0_test.go | 8 ++++---- management/server/idp/keycloak.go | 2 +- management/server/idp/keycloak_test.go | 6 +++--- management/server/management_test.go | 2 +- management/server/metrics/selfhosted.go | 8 ++++---- management/server/network.go | 4 ++-- management/server/peer.go | 2 +- management/server/setupkey.go | 2 +- management/server/turncredentials.go | 2 +- 14 files changed, 24 insertions(+), 24 deletions(-) diff --git a/client/cmd/status.go b/client/cmd/status.go index d090f4dda..119944d08 100644 --- a/client/cmd/status.go +++ b/client/cmd/status.go @@ -249,7 +249,7 @@ func mapPeers(peers []*proto.PeerState) peersStateOutput { IP: pbPeerState.GetIP(), PubKey: pbPeerState.GetPubKey(), Status: pbPeerState.GetConnStatus(), - LastStatusUpdate: timeLocal.UTC(), + LastStatusUpdate: timeLocal, ConnType: connType, Direct: pbPeerState.GetDirect(), IceCandidateType: iceCandidateType{ diff --git a/management/server/file_store.go b/management/server/file_store.go index 1815cf6fe..1765eb917 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -227,7 +227,7 @@ func (s *FileStore) persist(file string) error { // AcquireGlobalLock acquires global lock across all the accounts and returns a function that releases the lock func (s *FileStore) AcquireGlobalLock() (unlock func()) { log.Debugf("acquiring global lock") - start := time.Now().UTC() + start := time.Now() s.globalAccountLock.Lock() unlock = func() { @@ -241,7 +241,7 @@ func (s *FileStore) AcquireGlobalLock() (unlock func()) { // AcquireAccountLock acquires account lock and returns a function that releases the lock func (s *FileStore) AcquireAccountLock(accountID string) (unlock func()) { log.Debugf("acquiring lock for account %s", accountID) - start := time.Now().UTC() + start := time.Now() value, _ := s.accountLocks.LoadOrStore(accountID, &sync.Mutex{}) mtx := value.(*sync.Mutex) mtx.Lock() diff --git a/management/server/grpcserver.go b/management/server/grpcserver.go index b92704435..0c8dad246 100644 --- a/management/server/grpcserver.go +++ b/management/server/grpcserver.go @@ -98,7 +98,7 @@ func (s *GRPCServer) GetServerKey(ctx context.Context, req *proto.Empty) (*proto if s.appMetrics != nil { s.appMetrics.GRPCMetrics().CountGetKeyRequest() } - now := time.Now().UTC().Add(24 * time.Hour) + now := time.Now().Add(24 * time.Hour) secs := int64(now.Second()) nanos := int32(now.Nanosecond()) expiresAt := ×tamp.Timestamp{Seconds: secs, Nanos: nanos} diff --git a/management/server/http/middleware/auth_middleware.go b/management/server/http/middleware/auth_middleware.go index 09771deb4..a8c81012a 100644 --- a/management/server/http/middleware/auth_middleware.go +++ b/management/server/http/middleware/auth_middleware.go @@ -117,7 +117,7 @@ func (m *AuthMiddleware) CheckPATFromRequest(w http.ResponseWriter, r *http.Requ if err != nil { return fmt.Errorf("invalid Token: %w", err) } - if time.Now().UTC().After(pat.ExpirationDate) { + if time.Now().After(pat.ExpirationDate) { return fmt.Errorf("token expired") } diff --git a/management/server/idp/auth0.go b/management/server/idp/auth0.go index 89158e164..bb4d3e321 100644 --- a/management/server/idp/auth0.go +++ b/management/server/idp/auth0.go @@ -152,7 +152,7 @@ func NewAuth0Manager(config Auth0ClientConfig, appMetrics telemetry.AppMetrics) // jwtStillValid returns true if the token still valid and have enough time to be used and get a response from Auth0 func (c *Auth0Credentials) jwtStillValid() bool { - return !c.jwtToken.expiresInTime.IsZero() && time.Now().UTC().Add(5*time.Second).Before(c.jwtToken.expiresInTime) + return !c.jwtToken.expiresInTime.IsZero() && time.Now().Add(5*time.Second).Before(c.jwtToken.expiresInTime) } // requestJWTToken performs request to get jwt token diff --git a/management/server/idp/auth0_test.go b/management/server/idp/auth0_test.go index ab86f3986..75ff1d080 100644 --- a/management/server/idp/auth0_test.go +++ b/management/server/idp/auth0_test.go @@ -65,7 +65,7 @@ func (mc *mockAuth0Credentials) Authenticate() (JWTToken, error) { } func newTestJWT(t *testing.T, expInt int) string { - now := time.Now().UTC() + now := time.Now() token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ "iat": now.Unix(), "exp": now.Add(time.Duration(expInt) * time.Second).Unix(), @@ -209,13 +209,13 @@ func TestAuth0_JwtStillValid(t *testing.T) { } jwtStillValidTestCase1 := jwtStillValidTest{ name: "JWT still valid", - inputTime: time.Now().UTC().Add(10 * time.Second), + inputTime: time.Now().Add(10 * time.Second), expectedResult: true, message: "should be true", } jwtStillValidTestCase2 := jwtStillValidTest{ name: "JWT is invalid", - inputTime: time.Now().UTC(), + inputTime: time.Now(), expectedResult: false, message: "should be false", } @@ -251,7 +251,7 @@ func TestAuth0_Authenticate(t *testing.T) { authenticateTestCase1 := authenticateTest{ name: "Get Cached token", - inputExpireToken: time.Now().UTC().Add(30 * time.Second), + inputExpireToken: time.Now().Add(30 * time.Second), helper: JsonParser{}, // expectedFuncExitErrDiff: fmt.Errorf("unable to get token, statusCode 400"), expectedCode: 200, diff --git a/management/server/idp/keycloak.go b/management/server/idp/keycloak.go index 8ae09fc99..a0c004db2 100644 --- a/management/server/idp/keycloak.go +++ b/management/server/idp/keycloak.go @@ -119,7 +119,7 @@ func NewKeycloakManager(config KeycloakClientConfig, appMetrics telemetry.AppMet // jwtStillValid returns true if the token still valid and have enough time to be used and get a response from keycloak. func (kc *KeycloakCredentials) jwtStillValid() bool { - return !kc.jwtToken.expiresInTime.IsZero() && time.Now().UTC().Add(5*time.Second).Before(kc.jwtToken.expiresInTime) + return !kc.jwtToken.expiresInTime.IsZero() && time.Now().Add(5*time.Second).Before(kc.jwtToken.expiresInTime) } // requestJWTToken performs request to get jwt token. diff --git a/management/server/idp/keycloak_test.go b/management/server/idp/keycloak_test.go index 6cff7f56a..141937c62 100644 --- a/management/server/idp/keycloak_test.go +++ b/management/server/idp/keycloak_test.go @@ -199,13 +199,13 @@ func TestKeycloakJwtStillValid(t *testing.T) { jwtStillValidTestCase1 := jwtStillValidTest{ name: "JWT still valid", - inputTime: time.Now().UTC().Add(10 * time.Second), + inputTime: time.Now().Add(10 * time.Second), expectedResult: true, message: "should be true", } jwtStillValidTestCase2 := jwtStillValidTest{ name: "JWT is invalid", - inputTime: time.Now().UTC(), + inputTime: time.Now(), expectedResult: false, message: "should be false", } @@ -240,7 +240,7 @@ func TestKeycloakAuthenticate(t *testing.T) { authenticateTestCase1 := authenticateTest{ name: "Get Cached token", - inputExpireToken: time.Now().UTC().Add(30 * time.Second), + inputExpireToken: time.Now().Add(30 * time.Second), helper: JsonParser{}, expectedFuncExitErrDiff: nil, expectedCode: 200, diff --git a/management/server/management_test.go b/management/server/management_test.go index 25ce4fb0d..aaaba1799 100644 --- a/management/server/management_test.go +++ b/management/server/management_test.go @@ -371,7 +371,7 @@ var _ = Describe("Management service", func() { for i := 0; i < additionalPeers; i++ { key, _ := wgtypes.GenerateKey() loginPeerWithValidSetupKey(serverPubKey, key, client) - rand.Seed(time.Now().UTC().UnixNano()) + rand.Seed(time.Now().UnixNano()) n := rand.Intn(200) time.Sleep(time.Duration(n) * time.Millisecond) } diff --git a/management/server/metrics/selfhosted.go b/management/server/metrics/selfhosted.go index 751e07fba..2c7e943ac 100644 --- a/management/server/metrics/selfhosted.go +++ b/management/server/metrics/selfhosted.go @@ -67,7 +67,7 @@ type Worker struct { // NewWorker returns a metrics worker func NewWorker(ctx context.Context, id string, dataSource DataSource, connManager ConnManager) *Worker { - currentTime := time.Now().UTC() + currentTime := time.Now() return &Worker{ ctx: ctx, id: id, @@ -90,7 +90,7 @@ func (w *Worker) Run() { if err != nil { log.Error(err) } - w.lastRun = time.Now().UTC() + w.lastRun = time.Now() } } } @@ -149,7 +149,7 @@ func (w *Worker) generatePayload(apiKey string) pushPayload { DistinctID: w.id, Event: PayloadEvent, Properties: properties, - Timestamp: time.Now().UTC(), + Timestamp: time.Now(), } } @@ -172,7 +172,7 @@ func (w *Worker) generateProperties() properties { peerActiveVersions []string osUIClients map[string]int ) - start := time.Now().UTC() + start := time.Now() metricsProperties := make(properties) osPeers = make(map[string]int) osUIClients = make(map[string]int) diff --git a/management/server/network.go b/management/server/network.go index 2fe99338f..5237fa441 100644 --- a/management/server/network.go +++ b/management/server/network.go @@ -50,7 +50,7 @@ func NewNetwork() *Network { n := iplib.NewNet4(net.ParseIP("100.64.0.0"), NetSize) sub, _ := n.Subnet(SubnetSize) - s := rand.NewSource(time.Now().UTC().Unix()) + s := rand.NewSource(time.Now().Unix()) r := rand.New(s) intn := r.Intn(len(sub)) @@ -101,7 +101,7 @@ func AllocatePeerIP(ipNet net.IPNet, takenIps []net.IP) (net.IP, error) { } // pick a random IP - s := rand.NewSource(time.Now().UTC().Unix()) + s := rand.NewSource(time.Now().Unix()) r := rand.New(s) intn := r.Intn(len(ips)) diff --git a/management/server/peer.go b/management/server/peer.go index cb0112d37..b4b918083 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -144,7 +144,7 @@ func (p *Peer) LoginExpired(expiresIn time.Duration) (bool, time.Duration) { return false, 0 } expiresAt := p.LastLogin.Add(expiresIn) - now := time.Now().UTC() + now := time.Now() timeLeft := expiresAt.Sub(now) return timeLeft <= 0, timeLeft } diff --git a/management/server/setupkey.go b/management/server/setupkey.go index cce819056..bfba05839 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -148,7 +148,7 @@ func (key *SetupKey) IsRevoked() bool { // IsExpired if key was expired func (key *SetupKey) IsExpired() bool { - return time.Now().UTC().After(key.ExpiresAt) + return time.Now().After(key.ExpiresAt) } // IsOverUsed if the key was used too many times. SetupKey.UsageLimit == 0 indicates the unlimited usage. diff --git a/management/server/turncredentials.go b/management/server/turncredentials.go index 8db885853..1114aeeab 100644 --- a/management/server/turncredentials.go +++ b/management/server/turncredentials.go @@ -46,7 +46,7 @@ func NewTimeBasedAuthSecretsManager(updateManager *PeersUpdateManager, config *T func (m *TimeBasedAuthSecretsManager) GenerateCredentials() TURNCredentials { mac := hmac.New(sha1.New, []byte(m.config.Secret)) - timeAuth := time.Now().UTC().Add(m.config.CredentialsTTL.Duration).Unix() + timeAuth := time.Now().Add(m.config.CredentialsTTL.Duration).Unix() username := fmt.Sprint(timeAuth) From e197b89ac3c313e35a5a79fbf1a0bd9ec2e91ba6 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 10 Apr 2023 11:09:27 +0200 Subject: [PATCH 7/9] remove UTC from some not store related operations --- client/cmd/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cmd/status.go b/client/cmd/status.go index 119944d08..d090f4dda 100644 --- a/client/cmd/status.go +++ b/client/cmd/status.go @@ -249,7 +249,7 @@ func mapPeers(peers []*proto.PeerState) peersStateOutput { IP: pbPeerState.GetIP(), PubKey: pbPeerState.GetPubKey(), Status: pbPeerState.GetConnStatus(), - LastStatusUpdate: timeLocal, + LastStatusUpdate: timeLocal.UTC(), ConnType: connType, Direct: pbPeerState.GetDirect(), IceCandidateType: iceCandidateType{ From 306e02d32b1bd26f2aa6db69661246f79ccc60df Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 10 Apr 2023 18:22:25 +0200 Subject: [PATCH 8/9] Update calculate server state (#796) Refactored updateServerStates and calculateState added some checks to ensure we are not sending connecting on context canceled removed some state updates from the RunClient function --- client/internal/connect.go | 5 ++-- client/internal/peer/notifier.go | 35 +++++++++++---------------- client/internal/peer/notifier_test.go | 15 ++++++------ management/client/grpc.go | 19 +++++++++------ signal/client/grpc.go | 16 ++++++++---- 5 files changed, 46 insertions(+), 44 deletions(-) diff --git a/client/internal/connect.go b/client/internal/connect.go index 1d0585647..8f61e6403 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -59,9 +59,6 @@ func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status, return err } - statusRecorder.MarkManagementDisconnected() - - statusRecorder.ClientStart() defer statusRecorder.ClientStop() operation := func() error { // if context cancelled we not start new backoff cycle @@ -163,6 +160,8 @@ func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status, log.Print("Netbird engine started, my IP is: ", peerConfig.Address) state.Set(StatusConnected) + statusRecorder.ClientStart() + <-engineCtx.Done() statusRecorder.ClientTeardown() diff --git a/client/internal/peer/notifier.go b/client/internal/peer/notifier.go index b2d324c6c..527212198 100644 --- a/client/internal/peer/notifier.go +++ b/client/internal/peer/notifier.go @@ -15,7 +15,6 @@ type notifier struct { serverStateLock sync.Mutex listenersLock sync.Mutex listener Listener - currentServerState bool currentClientState bool lastNotification int } @@ -45,24 +44,14 @@ func (n *notifier) updateServerStates(mgmState bool, signalState bool) { n.serverStateLock.Lock() defer n.serverStateLock.Unlock() - var newState bool - if mgmState && signalState { - newState = true - } else { - newState = false - } + calculatedState := n.calculateState(mgmState, signalState) - if !n.isServerStateChanged(newState) { + if !n.isServerStateChanged(calculatedState) { return } - n.currentServerState = newState + n.lastNotification = calculatedState - if n.lastNotification == stateDisconnecting { - return - } - - n.lastNotification = n.calculateState(newState, n.currentClientState) n.notify(n.lastNotification) } @@ -70,7 +59,7 @@ func (n *notifier) clientStart() { n.serverStateLock.Lock() defer n.serverStateLock.Unlock() n.currentClientState = true - n.lastNotification = n.calculateState(n.currentServerState, true) + n.lastNotification = stateConnected n.notify(n.lastNotification) } @@ -78,7 +67,7 @@ func (n *notifier) clientStop() { n.serverStateLock.Lock() defer n.serverStateLock.Unlock() n.currentClientState = false - n.lastNotification = n.calculateState(n.currentServerState, false) + n.lastNotification = stateDisconnected n.notify(n.lastNotification) } @@ -90,8 +79,8 @@ func (n *notifier) clientTearDown() { n.notify(n.lastNotification) } -func (n *notifier) isServerStateChanged(newState bool) bool { - return n.currentServerState != newState +func (n *notifier) isServerStateChanged(newState int) bool { + return n.lastNotification != newState } func (n *notifier) notify(state int) { @@ -118,15 +107,19 @@ func (n *notifier) notifyListener(l Listener, state int) { }() } -func (n *notifier) calculateState(serverState bool, clientState bool) int { - if serverState && clientState { +func (n *notifier) calculateState(managementConn, signalConn bool) int { + if managementConn && signalConn { return stateConnected } - if !clientState { + if !managementConn && !signalConn { return stateDisconnected } + if n.lastNotification == stateDisconnecting { + return stateDisconnecting + } + return stateConnecting } diff --git a/client/internal/peer/notifier_test.go b/client/internal/peer/notifier_test.go index a9045ac34..bbdc00e13 100644 --- a/client/internal/peer/notifier_test.go +++ b/client/internal/peer/notifier_test.go @@ -47,25 +47,24 @@ func Test_notifier_serverState(t *testing.T) { type scenario struct { name string - expected bool + expected int mgmState bool signalState bool } scenarios := []scenario{ - {"connected", true, true, true}, - {"mgm down", false, false, true}, - {"signal down", false, true, false}, - {"disconnected", false, false, false}, + {"connected", stateConnected, true, true}, + {"mgm down", stateConnecting, false, true}, + {"signal down", stateConnecting, true, false}, + {"disconnected", stateDisconnected, false, false}, } for _, tt := range scenarios { t.Run(tt.name, func(t *testing.T) { n := newNotifier() n.updateServerStates(tt.mgmState, tt.signalState) - if n.currentServerState != tt.expected { - t.Errorf("invalid serverstate: %t, expected: %t", n.currentServerState, tt.expected) + if n.lastNotification != tt.expected { + t.Errorf("invalid serverstate: %d, expected: %d", n.lastNotification, tt.expected) } - }) } } diff --git a/management/client/grpc.go b/management/client/grpc.go index ad66aa3d6..2db070704 100644 --- a/management/client/grpc.go +++ b/management/client/grpc.go @@ -20,6 +20,7 @@ import ( "google.golang.org/grpc/keepalive" "github.com/cenkalti/backoff/v4" + "github.com/netbirdio/netbird/client/system" "github.com/netbirdio/netbird/encryption" "github.com/netbirdio/netbird/management/proto" @@ -144,15 +145,19 @@ func (c *GrpcClient) Sync(msgHandler func(msg *proto.SyncResponse) error) error // blocking until error err = c.receiveEvents(stream, *serverPubKey, msgHandler) if err != nil { - if s, ok := gstatus.FromError(err); ok && s.Code() == codes.PermissionDenied { + s, _ := gstatus.FromError(err) + switch s.Code() { + case codes.PermissionDenied: return backoff.Permanent(err) // unrecoverable error, propagate to the upper layer + case codes.Canceled: + log.Debugf("management connection context has been canceled, this usually indicates shutdown") + return nil + default: + backOff.Reset() // reset backoff counter after successful connection + c.notifyDisconnected() + log.Warnf("disconnected from the Management service but will retry silently. Reason: %v", err) + return err } - // we need this reset because after a successful connection and a consequent error, backoff lib doesn't - // reset times and next try will start with a long delay - backOff.Reset() - c.notifyDisconnected() - log.Warnf("disconnected from the Management service but will retry silently. Reason: %v", err) - return err } return nil diff --git a/signal/client/grpc.go b/signal/client/grpc.go index 5b0ae39ef..3c63fbe40 100644 --- a/signal/client/grpc.go +++ b/signal/client/grpc.go @@ -4,9 +4,11 @@ import ( "context" "crypto/tls" "fmt" + "io" + "sync" + "time" + "github.com/cenkalti/backoff/v4" - "github.com/netbirdio/netbird/encryption" - "github.com/netbirdio/netbird/signal/proto" log "github.com/sirupsen/logrus" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" "google.golang.org/grpc" @@ -17,9 +19,9 @@ import ( "google.golang.org/grpc/keepalive" "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" - "io" - "sync" - "time" + + "github.com/netbirdio/netbird/encryption" + "github.com/netbirdio/netbird/signal/proto" ) // ConnStateNotifier is a wrapper interface of the status recorder @@ -155,6 +157,10 @@ func (c *GrpcClient) Receive(msgHandler func(msg *proto.Message) error) error { // start receiving messages from the Signal stream (from other peers through signal) err = c.receive(stream, msgHandler) if err != nil { + if s, ok := status.FromError(err); ok && s.Code() == codes.Canceled { + log.Debugf("signal connection context has been canceled, this usually indicates shutdown") + return nil + } // we need this reset because after a successful connection and a consequent error, backoff lib doesn't // reset times and next try will start with a long delay backOff.Reset() From 251f2d7bc287c96162f53d82333a72fd0b8df2cc Mon Sep 17 00:00:00 2001 From: Misha Bragin Date: Tue, 11 Apr 2023 14:28:22 +0200 Subject: [PATCH 9/9] Pass newly generated ID to network map when adding peer (#800) --- management/server/peer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/peer.go b/management/server/peer.go index b4b918083..a223308b0 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -576,7 +576,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* return nil, nil, err } - networkMap := account.GetPeerNetworkMap(peer.ID, am.dnsDomain) + networkMap := account.GetPeerNetworkMap(newPeer.ID, am.dnsDomain) return newPeer, networkMap, nil }