From 06a5d742a45f546f9c84a9e8ad76c9ce55d1f69d Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Tue, 3 Jan 2023 23:55:56 -0800 Subject: [PATCH 1/8] fixes bug in which request is made to delete users that have already been deleted but exist in the aggregated_user_stats table --- cmd/src/users.go | 1 + cmd/src/users_prune.go | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cmd/src/users.go b/cmd/src/users.go index 20c2810120..d02304e005 100644 --- a/cmd/src/users.go +++ b/cmd/src/users.go @@ -97,4 +97,5 @@ type SiteUser struct { Email string SiteAdmin bool LastActiveAt string + DeletedAt string } diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 7c2d7c3325..df16b603d4 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -73,10 +73,12 @@ query getInactiveUsers { site { users { nodes { + id username email siteAdmin lastActiveAt + deletedAt } } } @@ -105,6 +107,10 @@ query getInactiveUsers { if user.Username == currentUserResult.Data.CurrentUser.Username { continue } + // filter out soft deleted users returned by site graphql endpoint + if user.DeletedAt != "" { + continue + } if !hasLastActive && !*removeNoLastActive { continue } @@ -152,9 +158,9 @@ query getInactiveUsers { }) } -// computes days since last usage from current day and time and UsageStatistics.LastActiveTime, uses time.Parse +// computes days since last usage from current day and time and aggregated_user_statistics.lastActiveAt, uses time.Parse func computeDaysSinceLastUse(user SiteUser) (timeDiff int, hasLastActive bool, _ error) { - // handle for null LastActiveAt returned from + // handle for null LastActiveAt, users who have never been active if user.LastActiveAt == "" { hasLastActive = false return 0, hasLastActive, nil From b14a437c26331ac4c3851ba945e6274b8245bf24 Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Wed, 11 Jan 2023 01:34:37 -0800 Subject: [PATCH 2/8] implemented some pagination -- totalCount of nodes is always 0 -- needing to fix that for break condition --- cmd/src/users_prune.go | 68 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index df16b603d4..c37373f91a 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -50,6 +50,7 @@ Examples: ctx := context.Background() client := cfg.apiClient(apiFlags, flagSet.Output()) + // get current user so as not to delete issuer of the prune request currentUserQuery := ` query getCurrentUser { currentUser { @@ -67,12 +68,34 @@ query getCurrentUser { if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(currentUserQuery, nil).DoRaw(context.Background(), ¤tUserResult); err != nil || !ok { return err } + + // get total users to paginate over + totalUsersQuery := ` +query getTotalUsers { + site { + users { + totalCount + } + } +}` + var totalUsers struct { + Site struct { + Users struct { + TotalCount float64 + } + } + } + if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(totalUsersQuery, nil).DoRaw(context.Background(), &totalUsers); err != nil || !ok { + return err + } + fmt.Printf("Total Users: %v\n\n", totalUsers) + // get 100 site users getInactiveUsersQuery := ` -query getInactiveUsers { + query getInactiveUsers($limit: Int $offset: Int) { site { users { - nodes { + nodes (limit: $limit offset: $offset) { id username email @@ -84,21 +107,48 @@ query getInactiveUsers { } } ` - - var usersResult struct { + + // paginate through users + var aggregatedUsers struct { Site struct { Users struct { Nodes []SiteUser } } } + fmt.Printf("Agg Users: %v\n\n", len(aggregatedUsers.Site.Users.Nodes)) - if ok, err := client.NewRequest(getInactiveUsersQuery, nil).Do(ctx, &usersResult); err != nil || !ok { - return err + offset := 0 + const limit int = 10 + + for len(aggregatedUsers.Site.Users.Nodes) < 56 { + pagVars := map[string]interface{}{ + "offset": offset, + "limit": limit, + } + + var usersResult struct { + Site struct { + Users struct { + Nodes []SiteUser + } + TotalCount float64 + } + } + if ok, err := client.NewRequest(getInactiveUsersQuery, pagVars).Do(ctx, &usersResult); err != nil || !ok { + return err + } + fmt.Printf("\nSite Users: %v\nAggUsers: %v\n\n", usersResult.Site.Users.Nodes, len(aggregatedUsers.Site.Users.Nodes)) + offset = offset + len(usersResult.Site.Users.Nodes) + fmt.Printf("offset: %v\npagVars: %v", offset, pagVars) + aggregatedUsers.Site.Users.Nodes = append(aggregatedUsers.Site.Users.Nodes, usersResult.Site.Users.Nodes...) } + fmt.Printf("\nEXITED AGGREGATOR AT %v TOTAL USERS\n", len(aggregatedUsers.Site.Users.Nodes)) + + // filter users for deletion usersToDelete := make([]UserToDelete, 0) - for _, user := range usersResult.Site.Users.Nodes { + for _, user := range aggregatedUsers.Site.Users.Nodes { daysSinceLastUse, hasLastActive, err := computeDaysSinceLastUse(user) if err != nil { return err @@ -120,9 +170,9 @@ query getInactiveUsers { if daysSinceLastUse <= *daysToDelete && hasLastActive { continue } - deleteUser := UserToDelete{user, daysSinceLastUse} + userToDelete := UserToDelete{user, daysSinceLastUse} - usersToDelete = append(usersToDelete, deleteUser) + usersToDelete = append(usersToDelete, userToDelete) } if *skipConfirmation { From b7e0481c3309e1d6b157a166bebb78fe925ddfb0 Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Wed, 11 Jan 2023 02:06:30 -0800 Subject: [PATCH 3/8] fixed totalCount issue -- switched DoRaw methods to Do --- cmd/src/users_prune.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index c37373f91a..4da9f8ae71 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -59,13 +59,11 @@ query getCurrentUser { } ` var currentUserResult struct { - Data struct { - CurrentUser struct { - Username string - } + CurrentUser struct { + Username string } } - if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(currentUserQuery, nil).DoRaw(context.Background(), ¤tUserResult); err != nil || !ok { + if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(currentUserQuery, nil).Do(context.Background(), ¤tUserResult); err != nil || !ok { return err } @@ -85,7 +83,7 @@ query getTotalUsers { } } } - if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(totalUsersQuery, nil).DoRaw(context.Background(), &totalUsers); err != nil || !ok { + if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(totalUsersQuery, nil).Do(context.Background(), &totalUsers); err != nil || !ok { return err } fmt.Printf("Total Users: %v\n\n", totalUsers) @@ -119,9 +117,9 @@ query getTotalUsers { fmt.Printf("Agg Users: %v\n\n", len(aggregatedUsers.Site.Users.Nodes)) offset := 0 - const limit int = 10 + const limit int = 100 - for len(aggregatedUsers.Site.Users.Nodes) < 56 { + for len(aggregatedUsers.Site.Users.Nodes) < int(totalUsers.Site.Users.TotalCount) { pagVars := map[string]interface{}{ "offset": offset, "limit": limit, @@ -154,7 +152,7 @@ query getTotalUsers { return err } // never remove user issuing command - if user.Username == currentUserResult.Data.CurrentUser.Username { + if user.Username == currentUserResult.CurrentUser.Username { continue } // filter out soft deleted users returned by site graphql endpoint From 0736971906b6629b1d2883994aded03cd57415f7 Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Wed, 11 Jan 2023 14:46:11 -0800 Subject: [PATCH 4/8] remove development print statements; hide users table behind flag; improve comments --- cmd/src/users_prune.go | 51 ++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 4da9f8ae71..9e43e93d5e 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -35,6 +35,7 @@ Examples: removeAdmin = flagSet.Bool("remove-admin", false, "prune admin accounts") removeNoLastActive = flagSet.Bool("remove-null-users", false, "removes users with no last active value") skipConfirmation = flagSet.Bool("force", false, "skips user confirmation step allowing programmatic use") + displayUsersToDelete = flagSet.Bool("display-users", false, "display table of users to be deleted by prune") apiFlags = api.NewFlags(flagSet) ) @@ -86,7 +87,6 @@ query getTotalUsers { if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(totalUsersQuery, nil).Do(context.Background(), &totalUsers); err != nil || !ok { return err } - fmt.Printf("Total Users: %v\n\n", totalUsers) // get 100 site users getInactiveUsersQuery := ` @@ -114,11 +114,12 @@ query getTotalUsers { } } } - fmt.Printf("Agg Users: %v\n\n", len(aggregatedUsers.Site.Users.Nodes)) - + + // pagination variables, limit set to maximum possible users returned per request offset := 0 const limit int = 100 - + + // paginate requests until all site users have been checked -- this includes soft deleted users for len(aggregatedUsers.Site.Users.Nodes) < int(totalUsers.Site.Users.TotalCount) { pagVars := map[string]interface{}{ "offset": offset, @@ -136,12 +137,9 @@ query getTotalUsers { if ok, err := client.NewRequest(getInactiveUsersQuery, pagVars).Do(ctx, &usersResult); err != nil || !ok { return err } - fmt.Printf("\nSite Users: %v\nAggUsers: %v\n\n", usersResult.Site.Users.Nodes, len(aggregatedUsers.Site.Users.Nodes)) offset = offset + len(usersResult.Site.Users.Nodes) - fmt.Printf("offset: %v\npagVars: %v", offset, pagVars) aggregatedUsers.Site.Users.Nodes = append(aggregatedUsers.Site.Users.Nodes, usersResult.Site.Users.Nodes...) } - fmt.Printf("\nEXITED AGGREGATOR AT %v TOTAL USERS\n", len(aggregatedUsers.Site.Users.Nodes)) // filter users for deletion @@ -159,17 +157,20 @@ query getTotalUsers { if user.DeletedAt != "" { continue } + // don't remove users with no last active value unless option flag is set if !hasLastActive && !*removeNoLastActive { continue } + // don't remove admins unless option flag is set if !*removeAdmin && user.SiteAdmin { continue } + // remove users who have been inactive for longer than the threshold set by the -days flag if daysSinceLastUse <= *daysToDelete && hasLastActive { continue } + // serialize user to print in table as part of confirmUserRemoval, add to delete slice userToDelete := UserToDelete{user, daysSinceLastUse} - usersToDelete = append(usersToDelete, userToDelete) } @@ -183,7 +184,7 @@ query getTotalUsers { } // confirm and remove users - if confirmed, _ := confirmUserRemoval(usersToDelete); !confirmed { + if confirmed, _ := confirmUserRemoval(usersToDelete, int(totalUsers.Site.Users.TotalCount), *daysToDelete, *displayUsersToDelete); !confirmed { fmt.Println("Aborting removal") return nil } else { @@ -244,25 +245,27 @@ type UserToDelete struct { } // Verify user wants to remove users with table of users and a command prompt for [y/N] -func confirmUserRemoval(usersToRemove []UserToDelete) (bool, error) { - fmt.Printf("Users to remove from instance at %s\n", cfg.Endpoint) - t := table.NewWriter() - t.SetOutputMirror(os.Stdout) - t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"}) - for _, user := range usersToRemove { - if user.User.Email != "" { - t.AppendRow([]interface{}{user.User.Username, user.User.Email, user.DaysSinceLastUse}) - t.AppendSeparator() - } else { - t.AppendRow([]interface{}{user.User.Username, "", user.DaysSinceLastUse}) - t.AppendSeparator() +func confirmUserRemoval(usersToDelete []UserToDelete, totalUsers int, daysThreshold int, displayUsers bool) (bool, error) { + if displayUsers { + fmt.Printf("Users to remove from %s\n", cfg.Endpoint) + t := table.NewWriter() + t.SetOutputMirror(os.Stdout) + t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"}) + for _, user := range usersToDelete { + if user.User.Email != "" { + t.AppendRow([]interface{}{user.User.Username, user.User.Email, user.DaysSinceLastUse}) + t.AppendSeparator() + } else { + t.AppendRow([]interface{}{user.User.Username, "", user.DaysSinceLastUse}) + t.AppendSeparator() + } } + t.SetStyle(table.StyleRounded) + t.Render() } - t.SetStyle(table.StyleRounded) - t.Render() input := "" for strings.ToLower(input) != "y" && strings.ToLower(input) != "n" { - fmt.Printf("Do you wish to proceed with user removal [y/N]: ") + fmt.Printf("%v of %v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), totalUsers, daysThreshold, cfg.Endpoint) if _, err := fmt.Scanln(&input); err != nil { return false, err } From 95afa77d461d4e8ae63fa81692197f014affd900 Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Wed, 11 Jan 2023 15:00:03 -0800 Subject: [PATCH 5/8] gofmt --- cmd/src/users_prune.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 9e43e93d5e..03656a1aeb 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -31,12 +31,12 @@ Examples: fmt.Println(usage) } var ( - daysToDelete = flagSet.Int("days", 60, "Days threshold on which to remove users, must be 60 days or greater and defaults to this value ") - removeAdmin = flagSet.Bool("remove-admin", false, "prune admin accounts") - removeNoLastActive = flagSet.Bool("remove-null-users", false, "removes users with no last active value") - skipConfirmation = flagSet.Bool("force", false, "skips user confirmation step allowing programmatic use") + daysToDelete = flagSet.Int("days", 60, "Days threshold on which to remove users, must be 60 days or greater and defaults to this value ") + removeAdmin = flagSet.Bool("remove-admin", false, "prune admin accounts") + removeNoLastActive = flagSet.Bool("remove-null-users", false, "removes users with no last active value") + skipConfirmation = flagSet.Bool("force", false, "skips user confirmation step allowing programmatic use") displayUsersToDelete = flagSet.Bool("display-users", false, "display table of users to be deleted by prune") - apiFlags = api.NewFlags(flagSet) + apiFlags = api.NewFlags(flagSet) ) handler := func(args []string) error { @@ -67,7 +67,7 @@ query getCurrentUser { if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(currentUserQuery, nil).Do(context.Background(), ¤tUserResult); err != nil || !ok { return err } - + // get total users to paginate over totalUsersQuery := ` query getTotalUsers { @@ -105,7 +105,7 @@ query getTotalUsers { } } ` - + // paginate through users var aggregatedUsers struct { Site struct { @@ -114,16 +114,16 @@ query getTotalUsers { } } } - + // pagination variables, limit set to maximum possible users returned per request offset := 0 const limit int = 100 - + // paginate requests until all site users have been checked -- this includes soft deleted users - for len(aggregatedUsers.Site.Users.Nodes) < int(totalUsers.Site.Users.TotalCount) { + for len(aggregatedUsers.Site.Users.Nodes) < int(totalUsers.Site.Users.TotalCount) { pagVars := map[string]interface{}{ "offset": offset, - "limit": limit, + "limit": limit, } var usersResult struct { @@ -141,7 +141,6 @@ query getTotalUsers { aggregatedUsers.Site.Users.Nodes = append(aggregatedUsers.Site.Users.Nodes, usersResult.Site.Users.Nodes...) } - // filter users for deletion usersToDelete := make([]UserToDelete, 0) for _, user := range aggregatedUsers.Site.Users.Nodes { From b7e1074206000f1d49d25d093dd5d49869fb4d6d Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Wed, 11 Jan 2023 15:08:41 -0800 Subject: [PATCH 6/8] added a few more comments --- cmd/src/users_prune.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 03656a1aeb..d6f8a0bd4b 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -137,7 +137,9 @@ query getTotalUsers { if ok, err := client.NewRequest(getInactiveUsersQuery, pagVars).Do(ctx, &usersResult); err != nil || !ok { return err } + // increment graphql request offset by the length of the last user set returned offset = offset + len(usersResult.Site.Users.Nodes) + // append graphql user results to aggregated users to be processed against user removal conditions aggregatedUsers.Site.Users.Nodes = append(aggregatedUsers.Site.Users.Nodes, usersResult.Site.Users.Nodes...) } From 75e42cc1b1a57ae5d17f7a5c97ee010bd7762ed5 Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Fri, 13 Jan 2023 00:24:10 -0800 Subject: [PATCH 7/8] style improvements --- cmd/src/users_prune.go | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index d6f8a0bd4b..6fcc23625f 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -107,20 +107,13 @@ query getTotalUsers { ` // paginate through users - var aggregatedUsers struct { - Site struct { - Users struct { - Nodes []SiteUser - } - } - } - + var aggregatedUsers []SiteUser // pagination variables, limit set to maximum possible users returned per request offset := 0 const limit int = 100 // paginate requests until all site users have been checked -- this includes soft deleted users - for len(aggregatedUsers.Site.Users.Nodes) < int(totalUsers.Site.Users.TotalCount) { + for len(aggregatedUsers) < int(totalUsers.Site.Users.TotalCount) { pagVars := map[string]interface{}{ "offset": offset, "limit": limit, @@ -140,16 +133,12 @@ query getTotalUsers { // increment graphql request offset by the length of the last user set returned offset = offset + len(usersResult.Site.Users.Nodes) // append graphql user results to aggregated users to be processed against user removal conditions - aggregatedUsers.Site.Users.Nodes = append(aggregatedUsers.Site.Users.Nodes, usersResult.Site.Users.Nodes...) + aggregatedUsers = append(aggregatedUsers, usersResult.Site.Users.Nodes...) } // filter users for deletion usersToDelete := make([]UserToDelete, 0) - for _, user := range aggregatedUsers.Site.Users.Nodes { - daysSinceLastUse, hasLastActive, err := computeDaysSinceLastUse(user) - if err != nil { - return err - } + for _, user := range aggregatedUsers { // never remove user issuing command if user.Username == currentUserResult.CurrentUser.Username { continue @@ -158,6 +147,11 @@ query getTotalUsers { if user.DeletedAt != "" { continue } + //compute days since last use + daysSinceLastUse, hasLastActive, err := computeDaysSinceLastUse(user) + if err != nil { + return err + } // don't remove users with no last active value unless option flag is set if !hasLastActive && !*removeNoLastActive { continue @@ -266,7 +260,7 @@ func confirmUserRemoval(usersToDelete []UserToDelete, totalUsers int, daysThresh } input := "" for strings.ToLower(input) != "y" && strings.ToLower(input) != "n" { - fmt.Printf("%v of %v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), totalUsers, daysThreshold, cfg.Endpoint) + fmt.Printf("%v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), daysThreshold, cfg.Endpoint) if _, err := fmt.Scanln(&input); err != nil { return false, err } From 89f58e7d222fcf69d65c0ba427986d52aaf2f4b3 Mon Sep 17 00:00:00 2001 From: DaedalusG Date: Fri, 13 Jan 2023 23:52:24 -0800 Subject: [PATCH 8/8] better graphQL querry formatting for single use queries --- cmd/src/users_prune.go | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 6fcc23625f..e35555feb6 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -52,13 +52,7 @@ Examples: client := cfg.apiClient(apiFlags, flagSet.Output()) // get current user so as not to delete issuer of the prune request - currentUserQuery := ` -query getCurrentUser { - currentUser { - username - } -} -` + currentUserQuery := `query getCurrentUser { currentUser { username }}` var currentUserResult struct { CurrentUser struct { Username string @@ -69,14 +63,7 @@ query getCurrentUser { } // get total users to paginate over - totalUsersQuery := ` -query getTotalUsers { - site { - users { - totalCount - } - } -}` + totalUsersQuery := `query getTotalUsers { site { users { totalCount }}}` var totalUsers struct { Site struct { Users struct { @@ -220,11 +207,7 @@ func computeDaysSinceLastUse(user SiteUser) (timeDiff int, hasLastActive bool, _ // Issue graphQL api request to remove user func removeUser(user SiteUser, client api.Client, ctx context.Context) error { - query := `mutation DeleteUser($user: ID!) { - deleteUser(user: $user) { - alwaysNil - } -}` + query := `mutation DeleteUser($user: ID!) { deleteUser(user: $user) { alwaysNil }}` vars := map[string]interface{}{ "user": user.ID, }