From bde018a3c7ca06f04699a40797598887f8ff07da Mon Sep 17 00:00:00 2001 From: Akshay Gaikwad Date: Thu, 3 Nov 2022 12:31:20 +0530 Subject: [PATCH] Use max select query for user last access time --- gen/openapi/proto/rpc/user/user.swagger.json | 8 ++-- internal/dao/common.go | 23 +++++++----- internal/models/kratossessions.go | 17 --------- pkg/service/user.go | 39 ++++++++++++-------- pkg/service/user_test.go | 18 ++++----- pkg/service/utils.go | 17 --------- proto/types/userpb/v3/user.pb.go | 11 +++--- proto/types/userpb/v3/user.proto | 2 +- 8 files changed, 56 insertions(+), 79 deletions(-) delete mode 100644 internal/models/kratossessions.go diff --git a/gen/openapi/proto/rpc/user/user.swagger.json b/gen/openapi/proto/rpc/user/user.swagger.json index 27a284a..d8b2638 100644 --- a/gen/openapi/proto/rpc/user/user.swagger.json +++ b/gen/openapi/proto/rpc/user/user.swagger.json @@ -243,7 +243,7 @@ }, { "name": "spec.lastLogin", - "description": "LastLogin. Last access date time in RFC3339 format. NA if not found", + "description": "LastLogin. Last access date time in RFC3339 format.", "in": "query", "required": false, "type": "string" @@ -478,7 +478,7 @@ }, { "name": "spec.lastLogin", - "description": "LastLogin. Last access date time in RFC3339 format. NA if not found", + "description": "LastLogin. Last access date time in RFC3339 format.", "in": "query", "required": false, "type": "string" @@ -894,7 +894,7 @@ }, { "name": "spec.lastLogin", - "description": "LastLogin. Last access date time in RFC3339 format. NA if not found", + "description": "LastLogin. Last access date time in RFC3339 format.", "in": "query", "required": false, "type": "string" @@ -1744,7 +1744,7 @@ }, "lastLogin": { "type": "string", - "description": "Last access date time in RFC3339 format. NA if not found", + "description": "Last access date time in RFC3339 format.", "title": "LastLogin", "readOnly": true } diff --git a/internal/dao/common.go b/internal/dao/common.go index a75a968..db814ca 100644 --- a/internal/dao/common.go +++ b/internal/dao/common.go @@ -3,9 +3,9 @@ package dao import ( "context" "fmt" + "time" "github.com/google/uuid" - "github.com/paralus/paralus/internal/models" bun "github.com/uptrace/bun" ) @@ -326,14 +326,19 @@ func GetUserIdByEmail(ctx context.Context, db bun.IDB, name string, entity inter return entity, nil } -// GetUserSessions fetches sessions of the user with userId. -func GetUserSessions(ctx context.Context, db bun.IDB, userId uuid.UUID) ([]models.KratosSessions, error) { - var sessions []models.KratosSessions - err := db.NewSelect().Model(&sessions). - Where("identity_id = ?", userId.String()). - Scan(ctx) +func GetUserLastAuthTime(ctx context.Context, db bun.IDB, userId uuid.UUID) (time.Time, error) { + var result time.Time + query := `select max(authenticated_at) from sessions where identity_id = ?` + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := db.QueryRowContext(ctx, query, userId).Scan(&result) if err != nil { - return nil, err + switch { + case err.Error() == `sql: Scan error on column index 0, name "max": unsupported Scan, storing driver.Value type into type *time.Time`: + return time.Time{}, nil + default: + return time.Time{}, err + } } - return sessions, nil + return result, nil } diff --git a/internal/models/kratossessions.go b/internal/models/kratossessions.go deleted file mode 100644 index 6449b94..0000000 --- a/internal/models/kratossessions.go +++ /dev/null @@ -1,17 +0,0 @@ -package models - -import ( - "time" - - "github.com/google/uuid" - "github.com/uptrace/bun" -) - -type KratosSessions struct { - bun.BaseModel `bun:"table:sessions,alias:sessions"` - - ID uuid.UUID `bun:"id,type:uuid,pk"` - AuthenticatedAt time.Time `bun:"authenticated_at,notnull"` - IdentityId uuid.UUID `bun:"identity_id,notnull"` - // Fill other columns of sessions table when necessary -} diff --git a/pkg/service/user.go b/pkg/service/user.go index 50aeac1..0c65c91 100644 --- a/pkg/service/user.go +++ b/pkg/service/user.go @@ -548,29 +548,19 @@ func (s *userService) GetByName(ctx context.Context, user *userv3.User) (*userv3 return &userv3.User{}, err } - err = s.updateLastLogin(ctx, user, usr.ID) + lastLogin, err := s.getUserLastLogin(ctx, usr.ID) if err != nil { return &userv3.User{}, err } + if lastLogin != "" { + user.GetSpec().LastLogin = lastLogin + } + return user, nil } return user, nil } -// updateLastLogin updates the last login field of the user provided. -func (s *userService) updateLastLogin(ctx context.Context, user *userv3.User, userId uuid.UUID) error { - lastLogin := "NA" - sessions, err := dao.GetUserSessions(ctx, s.db, userId) - if err != nil { - return err - } - if len(sessions) != 0 { - lastLogin = getLastLoginTime(sessions).Format(time.RFC3339) - } - user.GetSpec().LastLogin = lastLogin - return nil -} - func (s *userService) GetUserInfo(ctx context.Context, user *userv3.User) (*userv3.UserInfo, error) { username := "" if s.dev { @@ -911,10 +901,15 @@ func (s *userService) List(ctx context.Context, opts ...query.Option) (*userv3.U if err != nil { return userList, err } - err = s.updateLastLogin(ctx, user, usr.ID) + + lastLogin, err := s.getUserLastLogin(ctx, usr.ID) if err != nil { return userList, err } + if lastLogin != "" { + user.GetSpec().LastLogin = lastLogin + } + users = append(users, user) } @@ -1072,3 +1067,15 @@ func (s *userService) ForgotPassword(ctx context.Context, req *userrpcv3.UserFor return &userrpcv3.UserForgotPasswordResponse{}, fmt.Errorf("unable to generate recovery url") } } + +func (s *userService) getUserLastLogin(ctx context.Context, userId uuid.UUID) (string, error) { + var lastLogin string + authTime, err := dao.GetUserLastAuthTime(ctx, s.db, userId) + if err != nil { + return "", err + } + if !authTime.IsZero() { + lastLogin = authTime.Format(time.RFC3339) + } + return lastLogin, nil +} diff --git a/pkg/service/user_test.go b/pkg/service/user_test.go index bc240cb..f518afb 100644 --- a/pkg/service/user_test.go +++ b/pkg/service/user_test.go @@ -404,9 +404,9 @@ func TestUserGetByName(t *testing.T) { WithArgs().WillReturnRows(sqlmock.NewRows([]string{"role", "project"}).AddRow("role-"+ruuid, "project-"+pruuid)) mock.ExpectQuery(`SELECT authsrv_resourcerole.name as role, authsrv_project.name as project, namespace FROM "authsrv_projectaccountnamespacerole" JOIN authsrv_resourcerole ON authsrv_resourcerole.id=authsrv_projectaccountnamespacerole.role_id JOIN authsrv_project ON authsrv_project.id=authsrv_projectaccountnamespacerole.project_id WHERE .authsrv_projectaccountnamespacerole.account_id = '` + uuuid + `'`). WithArgs().WillReturnRows(sqlmock.NewRows([]string{"role", "project", "namespace"}).AddRow("role-"+ruuid, "project-"+pruuid, "ns")) - mock.ExpectQuery(`SELECT "sessions"."id", "sessions"."authenticated_at", "sessions"."identity_id".* FROM "sessions" WHERE .*`). - WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id", "authenticated_at", "identity_id"}). - AddRow(uuuid, authenticated, uuuid)) + mock.ExpectQuery(`select .* from sessions where .*`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"max"}). + AddRow(authenticated)) user := &userv3.User{ Metadata: &v3.Metadata{Partner: "partner-" + puuid, Organization: "org-" + ouuid, Name: "user-" + uuuid}, @@ -631,16 +631,16 @@ func TestUserList(t *testing.T) { guuid := addUsersGroupFetchExpectation(mock, uuuid1) addGroupRoleMappingsFetchExpectation(mock, guuid, pruuid) addUserRoleMappingsFetchExpectation(mock, uuuid1, pruuid) - mock.ExpectQuery(`SELECT "sessions"."id", "sessions"."authenticated_at", "sessions"."identity_id".* FROM "sessions" WHERE .*`). - WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id", "authenticated_at", "identity_id"}). - AddRow(uuuid1, authenticated, uuuid1)) + mock.ExpectQuery(`select .* from sessions where .*`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"max"}). + AddRow(authenticated)) guuid = addUsersGroupFetchExpectation(mock, uuuid2) addGroupRoleMappingsFetchExpectation(mock, guuid, pruuid) addUserRoleMappingsFetchExpectation(mock, uuuid2, pruuid) - mock.ExpectQuery(`SELECT "sessions"."id", "sessions"."authenticated_at", "sessions"."identity_id".* FROM "sessions" WHERE .*`). - WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id", "authenticated_at", "identity_id"}). - AddRow(uuuid1, authenticated, uuuid1)) + mock.ExpectQuery(`select .* from sessions where .*`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"max"}). + AddRow(authenticated)) qo := &commonv3.QueryOptions{ Q: tc.q, diff --git a/pkg/service/utils.go b/pkg/service/utils.go index 9523a1e..3dcdf91 100644 --- a/pkg/service/utils.go +++ b/pkg/service/utils.go @@ -2,11 +2,9 @@ package service import ( "context" - "time" "github.com/google/uuid" "github.com/paralus/paralus/internal/dao" - "github.com/paralus/paralus/internal/models" "github.com/paralus/paralus/pkg/common" commonv3 "github.com/paralus/paralus/proto/types/commonpb/v3" "github.com/uptrace/bun" @@ -35,18 +33,3 @@ func IsInternalRequest(ctx context.Context) bool { b, ok := v.(bool) return ok && b } - -// getLastLoginTime return latest authenticated time from sessions. -func getLastLoginTime(sessions []models.KratosSessions) time.Time { - var auths []int64 - for _, s := range sessions { - auths = append(auths, s.AuthenticatedAt.UnixMilli()) - } - latest := auths[0] - for _, auth := range auths { - if auth > latest { - latest = auth - } - } - return time.UnixMilli(latest) -} diff --git a/proto/types/userpb/v3/user.pb.go b/proto/types/userpb/v3/user.pb.go index a28d3fc..f0e0330 100644 --- a/proto/types/userpb/v3/user.pb.go +++ b/proto/types/userpb/v3/user.pb.go @@ -674,7 +674,7 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x74, 0x61, 0x74, 0x75, 0x73, 0x3a, 0x37, 0x92, 0x41, 0x34, 0x0a, 0x32, 0x2a, 0x04, 0x55, 0x73, 0x65, 0x72, 0x32, 0x04, 0x55, 0x73, 0x65, 0x72, 0xd2, 0x01, 0x0a, 0x61, 0x70, 0x69, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0xd2, 0x01, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0xd2, 0x01, 0x08, 0x6d, - 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xd2, 0x01, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0xce, + 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xd2, 0x01, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0xbe, 0x08, 0x0a, 0x08, 0x55, 0x73, 0x65, 0x72, 0x53, 0x70, 0x65, 0x63, 0x12, 0x44, 0x0a, 0x09, 0x66, 0x69, 0x72, 0x73, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x26, 0x92, 0x41, 0x23, 0x2a, 0x09, 0x46, 0x69, 0x72, 0x73, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x32, 0x16, @@ -733,13 +733,12 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x70, 0x20, 0x55, 0x52, 0x4c, 0x20, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x65, 0x64, 0x20, 0x61, 0x66, 0x74, 0x65, 0x72, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x63, 0x72, 0x65, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x40, 0x01, 0x48, 0x00, 0x52, 0x0b, 0x72, 0x65, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x79, - 0x55, 0x72, 0x6c, 0x88, 0x01, 0x01, 0x12, 0x68, 0x0a, 0x09, 0x6c, 0x61, 0x73, 0x74, 0x4c, 0x6f, - 0x67, 0x69, 0x6e, 0x18, 0x0c, 0x20, 0x01, 0x28, 0x09, 0x42, 0x4a, 0x92, 0x41, 0x47, 0x2a, 0x09, - 0x4c, 0x61, 0x73, 0x74, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x32, 0x38, 0x4c, 0x61, 0x73, 0x74, 0x20, + 0x55, 0x72, 0x6c, 0x88, 0x01, 0x01, 0x12, 0x58, 0x0a, 0x09, 0x6c, 0x61, 0x73, 0x74, 0x4c, 0x6f, + 0x67, 0x69, 0x6e, 0x18, 0x0c, 0x20, 0x01, 0x28, 0x09, 0x42, 0x3a, 0x92, 0x41, 0x37, 0x2a, 0x09, + 0x4c, 0x61, 0x73, 0x74, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x32, 0x28, 0x4c, 0x61, 0x73, 0x74, 0x20, 0x61, 0x63, 0x63, 0x65, 0x73, 0x73, 0x20, 0x64, 0x61, 0x74, 0x65, 0x20, 0x74, 0x69, 0x6d, 0x65, 0x20, 0x69, 0x6e, 0x20, 0x52, 0x46, 0x43, 0x33, 0x33, 0x33, 0x39, 0x20, 0x66, 0x6f, 0x72, 0x6d, - 0x61, 0x74, 0x2e, 0x20, 0x4e, 0x41, 0x20, 0x69, 0x66, 0x20, 0x6e, 0x6f, 0x74, 0x20, 0x66, 0x6f, - 0x75, 0x6e, 0x64, 0x40, 0x01, 0x52, 0x09, 0x6c, 0x61, 0x73, 0x74, 0x4c, 0x6f, 0x67, 0x69, 0x6e, + 0x61, 0x74, 0x2e, 0x40, 0x01, 0x52, 0x09, 0x6c, 0x61, 0x73, 0x74, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x3a, 0x2d, 0x92, 0x41, 0x2a, 0x0a, 0x28, 0x2a, 0x12, 0x55, 0x73, 0x65, 0x72, 0x20, 0x53, 0x70, 0x65, 0x63, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x32, 0x12, 0x55, 0x73, 0x65, 0x72, 0x20, 0x73, 0x70, 0x65, 0x63, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x42, diff --git a/proto/types/userpb/v3/user.proto b/proto/types/userpb/v3/user.proto index 7c450f8..b0ce90c 100644 --- a/proto/types/userpb/v3/user.proto +++ b/proto/types/userpb/v3/user.proto @@ -203,7 +203,7 @@ message UserSpec { string lastLogin = 12 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "LastLogin" - description : "Last access date time in RFC3339 format. NA if not found" + description : "Last access date time in RFC3339 format." read_only : true } ]; }