From da75a76d41408b263b653e6c3d1bb0721329e1f5 Mon Sep 17 00:00:00 2001 From: pascal-fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Fri, 18 Aug 2023 19:23:11 +0200 Subject: [PATCH] Adding dashboard login activity (#1092) For better auditing this PR adds a dashboard login event to the management service. For that the user object was extended with a field for last login that is not actively saved to the database but kept in memory until next write. The information about the last login can be extracted from the JWT claims nb_last_login. This timestamp will be stored and compared on each API request. If the value changes we generate an event to inform about a login. --- management/server/account.go | 17 ++++++------ management/server/activity/codes.go | 3 +++ management/server/file_store.go | 20 ++++++++++++++ management/server/http/api/openapi.yml | 5 ++++ management/server/http/api/types.gen.go | 3 +++ management/server/http/users_handler.go | 1 + management/server/jwtclaims/claims.go | 3 +++ management/server/jwtclaims/extractor.go | 18 +++++++++++++ management/server/jwtclaims/extractor_test.go | 11 ++++++++ management/server/store.go | 3 +++ management/server/updatechannel.go | 10 ++++--- management/server/user.go | 26 +++++++++++++++++++ management/server/user_test.go | 3 ++- 13 files changed, 110 insertions(+), 13 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index dc8f4d0fd..d9b73f713 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -189,14 +189,15 @@ type Account struct { } type UserInfo struct { - ID string `json:"id"` - Email string `json:"email"` - Name string `json:"name"` - Role string `json:"role"` - AutoGroups []string `json:"auto_groups"` - Status string `json:"-"` - IsServiceUser bool `json:"is_service_user"` - IsBlocked bool `json:"is_blocked"` + ID string `json:"id"` + Email string `json:"email"` + Name string `json:"name"` + Role string `json:"role"` + AutoGroups []string `json:"auto_groups"` + Status string `json:"-"` + IsServiceUser bool `json:"is_service_user"` + IsBlocked bool `json:"is_blocked"` + LastLogin time.Time `json:"last_login"` } // getRoutesToSync returns the enabled routes for the peer ID and the routes diff --git a/management/server/activity/codes.go b/management/server/activity/codes.go index 7c6b55218..4de667ded 100644 --- a/management/server/activity/codes.go +++ b/management/server/activity/codes.go @@ -110,6 +110,8 @@ const ( UserLoggedInPeer // PeerLoginExpired indicates that the user peer login has been expired and peer disconnected PeerLoginExpired + // DashboardLogin indicates that the user logged in to the dashboard + DashboardLogin ) var activityMap = map[Activity]Code{ @@ -163,6 +165,7 @@ var activityMap = map[Activity]Code{ GroupDeleted: {"Group deleted", "group.delete"}, UserLoggedInPeer: {"User logged in peer", "user.peer.login"}, PeerLoginExpired: {"Peer login expired", "peer.login.expire"}, + DashboardLogin: {"Dashboard login", "dashboard.login"}, } // StringCode returns a string code of the activity diff --git a/management/server/file_store.go b/management/server/file_store.go index 0e95e3a05..ecd02ba99 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -570,6 +570,26 @@ func (s *FileStore) SavePeerStatus(accountID, peerID string, peerStatus PeerStat return nil } +// SaveUserLastLogin stores the last login time for a user in memory. It doesn't attempt to persist data to speed up things. +func (s *FileStore) SaveUserLastLogin(accountID, userID string, lastLogin time.Time) error { + s.mux.Lock() + defer s.mux.Unlock() + + account, err := s.getAccount(accountID) + if err != nil { + return err + } + + peer := account.Users[userID] + if peer == nil { + return status.Errorf(status.NotFound, "user %s not found", userID) + } + + peer.LastLogin = lastLogin + + return nil +} + // Close the FileStore persisting data to disk func (s *FileStore) Close() error { s.mux.Lock() diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 1fb54c4f7..a09b9f6a6 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -100,6 +100,11 @@ components: type: string enum: [ "active","invited","blocked" ] example: active + last_login: + description: Last time this user performed a login to the dashboard + type: string + format: date-time + example: 2023-05-05T09:00:35.477782Z auto_groups: description: Groups to auto-assign to peers registered by this user type: array diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 93d371a17..5b629cc0e 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -767,6 +767,9 @@ type User struct { // IsServiceUser Is true if this user is a service user IsServiceUser *bool `json:"is_service_user,omitempty"` + // LastLogin Last time this user performed a login to the dashboard + LastLogin *time.Time `json:"last_login,omitempty"` + // Name User's name from idp provider Name string `json:"name"` diff --git a/management/server/http/users_handler.go b/management/server/http/users_handler.go index 45b2a7618..d215e1510 100644 --- a/management/server/http/users_handler.go +++ b/management/server/http/users_handler.go @@ -270,5 +270,6 @@ func toUserResponse(user *server.UserInfo, currenUserID string) *api.User { IsCurrent: &isCurrent, IsServiceUser: &user.IsServiceUser, IsBlocked: user.IsBlocked, + LastLogin: &user.LastLogin, } } diff --git a/management/server/jwtclaims/claims.go b/management/server/jwtclaims/claims.go index 946c0b8be..1fa00b2fe 100644 --- a/management/server/jwtclaims/claims.go +++ b/management/server/jwtclaims/claims.go @@ -1,6 +1,8 @@ package jwtclaims import ( + "time" + "github.com/golang-jwt/jwt" ) @@ -10,6 +12,7 @@ type AuthorizationClaims struct { AccountId string Domain string DomainCategory string + LastLogin time.Time Raw jwt.MapClaims } diff --git a/management/server/jwtclaims/extractor.go b/management/server/jwtclaims/extractor.go index 466856d77..42a41f140 100644 --- a/management/server/jwtclaims/extractor.go +++ b/management/server/jwtclaims/extractor.go @@ -2,6 +2,7 @@ package jwtclaims import ( "net/http" + "time" "github.com/golang-jwt/jwt" ) @@ -17,6 +18,8 @@ const ( DomainCategorySuffix = "wt_account_domain_category" // UserIDClaim claim for the user id UserIDClaim = "sub" + // LastLoginSuffix claim for the last login + LastLoginSuffix = "nb_last_login" ) // ExtractClaims Extract function type @@ -93,9 +96,24 @@ func (c *ClaimsExtractor) FromToken(token *jwt.Token) AuthorizationClaims { if ok { jwtClaims.DomainCategory = domainCategoryClaim.(string) } + LastLoginClaimString, ok := claims[c.authAudience+LastLoginSuffix] + if ok { + jwtClaims.LastLogin = parseTime(LastLoginClaimString.(string)) + } return jwtClaims } +func parseTime(timeString string) time.Time { + if timeString == "" { + return time.Time{} + } + parsedTime, err := time.Parse(time.RFC3339, timeString) + if err != nil { + return time.Time{} + } + return parsedTime +} + // fromRequestContext extracts claims from the request context previously filled by the JWT token (after auth) func (c *ClaimsExtractor) fromRequestContext(r *http.Request) AuthorizationClaims { if r.Context().Value(TokenUserProperty) == nil { diff --git a/management/server/jwtclaims/extractor_test.go b/management/server/jwtclaims/extractor_test.go index 9bececac6..f7eeb82e5 100644 --- a/management/server/jwtclaims/extractor_test.go +++ b/management/server/jwtclaims/extractor_test.go @@ -4,12 +4,15 @@ import ( "context" "net/http" "testing" + "time" "github.com/golang-jwt/jwt" "github.com/stretchr/testify/require" ) func newTestRequestWithJWT(t *testing.T, claims AuthorizationClaims, audiance string) *http.Request { + const layout = "2006-01-02T15:04:05.999Z" + claimMaps := jwt.MapClaims{} if claims.UserId != "" { claimMaps[UserIDClaim] = claims.UserId @@ -23,6 +26,9 @@ func newTestRequestWithJWT(t *testing.T, claims AuthorizationClaims, audiance st if claims.DomainCategory != "" { claimMaps[audiance+DomainCategorySuffix] = claims.DomainCategory } + if claims.LastLogin != (time.Time{}) { + claimMaps[audiance+LastLoginSuffix] = claims.LastLogin.Format(layout) + } token := jwt.NewWithClaims(jwt.SigningMethodHS256, claimMaps) r, err := http.NewRequest(http.MethodGet, "http://localhost", nil) require.NoError(t, err, "creating testing request failed") @@ -40,6 +46,9 @@ func TestExtractClaimsFromRequestContext(t *testing.T) { expectedMSG string } + const layout = "2006-01-02T15:04:05.999Z" + lastLogin, _ := time.Parse(layout, "2023-08-17T09:30:40.465Z") + testCase1 := test{ name: "All Claim Fields", inputAudiance: "https://login/", @@ -47,11 +56,13 @@ func TestExtractClaimsFromRequestContext(t *testing.T) { UserId: "test", Domain: "test.com", AccountId: "testAcc", + LastLogin: lastLogin, DomainCategory: "public", Raw: jwt.MapClaims{ "https://login/wt_account_domain": "test.com", "https://login/wt_account_domain_category": "public", "https://login/wt_account_id": "testAcc", + "https://login/nb_last_login": lastLogin.Format(layout), "sub": "test", }, }, diff --git a/management/server/store.go b/management/server/store.go index daad30eaa..9ebe41235 100644 --- a/management/server/store.go +++ b/management/server/store.go @@ -1,5 +1,7 @@ package server +import "time" + type Store interface { GetAllAccounts() []*Account GetAccount(accountID string) (*Account, error) @@ -20,6 +22,7 @@ type Store interface { // AcquireGlobalLock should attempt to acquire a global lock and return a function that releases the lock AcquireGlobalLock() func() SavePeerStatus(accountID, peerID string, status PeerStatus) error + SaveUserLastLogin(accountID, userID string, lastLogin time.Time) error // Close should close the store persisting all unsaved data. Close() error } diff --git a/management/server/updatechannel.go b/management/server/updatechannel.go index 6cc10ad24..744386547 100644 --- a/management/server/updatechannel.go +++ b/management/server/updatechannel.go @@ -1,9 +1,11 @@ package server import ( - "github.com/netbirdio/netbird/management/proto" - log "github.com/sirupsen/logrus" "sync" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/management/proto" ) const channelBufferSize = 100 @@ -33,7 +35,7 @@ func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) er if channel, ok := p.peerChannels[peerID]; ok { select { case channel <- update: - log.Infof("update was sent to channel for peer %s", peerID) + log.Debugf("update was sent to channel for peer %s", peerID) default: log.Warnf("channel for peer %s is %d full", peerID, len(channel)) } @@ -52,7 +54,7 @@ func (p *PeersUpdateManager) CreateChannel(peerID string) chan *UpdateMessage { delete(p.peerChannels, peerID) close(channel) } - //mbragin: todo shouldn't it be more? or configurable? + // mbragin: todo shouldn't it be more? or configurable? channel := make(chan *UpdateMessage, channelBufferSize) p.peerChannels[peerID] = channel diff --git a/management/server/user.go b/management/server/user.go index 19cffb840..3d0c0313e 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -3,6 +3,7 @@ package server import ( "fmt" "strings" + "time" "github.com/google/uuid" log "github.com/sirupsen/logrus" @@ -53,6 +54,8 @@ type User struct { PATs map[string]*PersonalAccessToken // Blocked indicates whether the user is blocked. Blocked users can't use the system. Blocked bool + // LastLogin is the last time the user logged in to IdP + LastLogin time.Time } // IsBlocked returns true if the user is blocked, false otherwise @@ -60,6 +63,10 @@ func (u *User) IsBlocked() bool { return u.Blocked } +func (u *User) LastDashboardLoginChanged(LastLogin time.Time) bool { + return LastLogin.After(u.LastLogin) && !u.LastLogin.IsZero() +} + // IsAdmin returns true if the user is an admin, false otherwise func (u *User) IsAdmin() bool { return u.Role == UserRoleAdmin @@ -82,6 +89,7 @@ func (u *User) ToUserInfo(userData *idp.UserData) (*UserInfo, error) { Status: string(UserStatusActive), IsServiceUser: u.IsServiceUser, IsBlocked: u.Blocked, + LastLogin: u.LastLogin, }, nil } if userData.ID != u.Id { @@ -102,6 +110,7 @@ func (u *User) ToUserInfo(userData *idp.UserData) (*UserInfo, error) { Status: string(userStatus), IsServiceUser: u.IsServiceUser, IsBlocked: u.Blocked, + LastLogin: u.LastLogin, }, nil } @@ -123,6 +132,7 @@ func (u *User) Copy() *User { ServiceUserName: u.ServiceUserName, PATs: pats, Blocked: u.Blocked, + LastLogin: u.LastLogin, } } @@ -186,6 +196,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs AutoGroups: newUser.AutoGroups, Status: string(UserStatusActive), IsServiceUser: true, + LastLogin: time.Time{}, }, nil } @@ -280,6 +291,21 @@ func (am *DefaultAccountManager) GetUser(claims jwtclaims.AuthorizationClaims) ( if !ok { return nil, status.Errorf(status.NotFound, "user not found") } + + // this code should be outside of the am.GetAccountFromToken(claims) because this method is called also by the gRPC + // server when user authenticates a device. And we need to separate the Dashboard login event from the Device login event. + unlock := am.Store.AcquireAccountLock(account.Id) + newLogin := user.LastDashboardLoginChanged(claims.LastLogin) + err = am.Store.SaveUserLastLogin(account.Id, claims.UserId, claims.LastLogin) + unlock() + if newLogin { + meta := map[string]any{"timestamp": claims.LastLogin} + am.storeEvent(claims.UserId, claims.UserId, account.Id, activity.DashboardLogin, meta) + if err != nil { + log.Errorf("failed saving user last login: %v", err) + } + } + return user, nil } diff --git a/management/server/user_test.go b/management/server/user_test.go index d6226f76d..b07154663 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -266,7 +266,8 @@ func TestUser_Copy(t *testing.T) { LastUsed: time.Now(), }, }, - Blocked: false, + Blocked: false, + LastLogin: time.Now(), } err := validateStruct(user)