Skip to content
Permalink
Browse files

address PR comments

- update error messages to be correct
- move suspended message into template and include for other pages
- check suspended status on all relevant pages and show message if
logged in user is suspended.
- fix possible nil pointer error
- remove changes to db schema files
- add version comment to migration
- add UserStatus type with UserActive and UserSuspended
- change database table to use status column instead of suspended
- update toggle suspended handler to be toggle status in prep for
possible future inclusion of further user statuses
  • Loading branch information
robjloranger committed Oct 25, 2019
1 parent 9873fc4 commit f85f0751a3a7480bc29450cf42a4360c31ff3083
@@ -750,14 +750,20 @@ func viewArticles(app *App, u *User, w http.ResponseWriter, r *http.Request) err
log.Error("unable to fetch collections: %v", err)
}

suspended, err := app.db.IsUserSuspended(u.ID)
if err != nil {
log.Error("view articles: %v", err)
}
d := struct {
*UserPage
AnonymousPosts *[]PublicPost
Collections *[]Collection
Suspended bool
}{
UserPage: NewUserPage(app, r, u, u.Username+"'s Posts", f),
AnonymousPosts: p,
Collections: c,
Suspended: suspended,
}
d.UserPage.SetMessaging(u)
w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")
@@ -779,18 +785,25 @@ func viewCollections(app *App, u *User, w http.ResponseWriter, r *http.Request)
uc, _ := app.db.GetUserCollectionCount(u.ID)
// TODO: handle any errors

suspended, err := app.db.IsUserSuspended(u.ID)
if err != nil {
log.Error("view collections %v", err)
return fmt.Errorf("view collections: %v", err)
}
d := struct {
*UserPage
Collections *[]Collection

UsedCollections, TotalCollections int

NewBlogsDisabled bool
Suspended bool
}{
UserPage: NewUserPage(app, r, u, u.Username+"'s Blogs", f),
Collections: c,
UsedCollections: int(uc),
NewBlogsDisabled: !app.cfg.App.CanCreateBlogs(uc),
Suspended: suspended,
}
d.UserPage.SetMessaging(u)
showUserPage(w, "collections", d)
@@ -808,13 +821,20 @@ func viewEditCollection(app *App, u *User, w http.ResponseWriter, r *http.Reques
return ErrCollectionNotFound
}

suspended, err := app.db.IsUserSuspended(u.ID)
if err != nil {
log.Error("view edit collection %v", err)
return fmt.Errorf("view edit collection: %v", err)
}
flashes, _ := getSessionFlashes(app, w, r, nil)
obj := struct {
*UserPage
*Collection
Suspended bool
}{
UserPage: NewUserPage(app, r, u, "Edit "+c.DisplayTitle(), flashes),
Collection: c,
Suspended: suspended,
}

showUserPage(w, "collection", obj)
@@ -976,17 +996,24 @@ func viewStats(app *App, u *User, w http.ResponseWriter, r *http.Request) error
titleStats = c.DisplayTitle() + " "
}

suspended, err := app.db.IsUserSuspended(u.ID)
if err != nil {
log.Error("view stats: %v", err)
return err
}
obj := struct {
*UserPage
VisitsBlog string
Collection *Collection
TopPosts *[]PublicPost
APFollowers int
Suspended bool
}{
UserPage: NewUserPage(app, r, u, titleStats+"Stats", flashes),
VisitsBlog: alias,
Collection: c,
TopPosts: topPosts,
Suspended: suspended,
}
if app.cfg.App.Federation {
folls, err := app.db.GetAPFollowers(c)
@@ -1026,7 +1053,7 @@ func viewSettings(app *App, u *User, w http.ResponseWriter, r *http.Request) err
Email: fullUser.EmailClear(app.keys),
HasPass: passIsSet,
IsLogOut: r.FormValue("logout") == "1",
Suspended: fullUser.Suspended,
Suspended: fullUser.Status == UserSuspended,
}

showUserPage(w, "settings", obj)
@@ -82,7 +82,7 @@ func handleFetchCollectionActivities(app *App, w http.ResponseWriter, r *http.Re
}
suspended, err := app.db.IsUserSuspended(c.OwnerID)
if err != nil {
log.Error("fetch collection inbox: get owner: %v", err)
log.Error("fetch collection activities: %v", err)
return ErrInternalGeneral
}
if suspended {
@@ -115,7 +115,7 @@ func handleFetchCollectionOutbox(app *App, w http.ResponseWriter, r *http.Reques
}
suspended, err := app.db.IsUserSuspended(c.OwnerID)
if err != nil {
log.Error("fetch collection inbox: get owner: %v", err)
log.Error("fetch collection outbox: %v", err)
return ErrInternalGeneral
}
if suspended {
@@ -176,7 +176,7 @@ func handleFetchCollectionFollowers(app *App, w http.ResponseWriter, r *http.Req
}
suspended, err := app.db.IsUserSuspended(c.OwnerID)
if err != nil {
log.Error("fetch collection inbox: get owner: %v", err)
log.Error("fetch collection followers: %v", err)
return ErrInternalGeneral
}
if suspended {
@@ -230,7 +230,7 @@ func handleFetchCollectionFollowing(app *App, w http.ResponseWriter, r *http.Req
}
suspended, err := app.db.IsUserSuspended(c.OwnerID)
if err != nil {
log.Error("fetch collection inbox: get owner: %v", err)
log.Error("fetch collection following: %v", err)
return ErrInternalGeneral
}
if suspended {
@@ -272,7 +272,7 @@ func handleFetchCollectionInbox(app *App, w http.ResponseWriter, r *http.Request
}
suspended, err := app.db.IsUserSuspended(c.OwnerID)
if err != nil {
log.Error("fetch collection inbox: get owner: %v", err)
log.Error("fetch collection inbox: %v", err)
return ErrInternalGeneral
}
if suspended {
@@ -230,28 +230,27 @@ func handleViewAdminUser(app *App, u *User, w http.ResponseWriter, r *http.Reque
return nil
}

func handleAdminToggleUserSuspended(app *App, u *User, w http.ResponseWriter, r *http.Request) error {
func handleAdminToggleUserStatus(app *App, u *User, w http.ResponseWriter, r *http.Request) error {
vars := mux.Vars(r)
username := vars["username"]
if username == "" {
return impart.HTTPError{http.StatusFound, "/admin/users"}
}

userToToggle, err := app.db.GetUserForAuth(username)
user, err := app.db.GetUserForAuth(username)
if err != nil {
log.Error("failed to get user: %v", err)
return impart.HTTPError{http.StatusInternalServerError, fmt.Sprintf("Could not get user from username: %v", err)}
}
if userToToggle.Suspended {
err = app.db.SetUserSuspended(userToToggle.ID, false)
if user.Status == UserSuspended {
err = app.db.SetUserStatus(user.ID, UserActive)
} else {
err = app.db.SetUserSuspended(userToToggle.ID, true)
err = app.db.SetUserStatus(user.ID, UserSuspended)
}
if err != nil {
log.Error("toggle user suspended: %v", err)
return impart.HTTPError{http.StatusInternalServerError, fmt.Sprintf("Could not toggle user suspended: %v")}
return impart.HTTPError{http.StatusInternalServerError, fmt.Sprintf("Could not toggle user status: %v")}
}
// TODO: invalidate sessions
return impart.HTTPError{http.StatusFound, fmt.Sprintf("/admin/user/%s#status", username)}
}

@@ -71,6 +71,7 @@ type (
CurrentPage int
TotalPages int
Format *CollectionFormat
Suspended bool
}
SubmittedCollection struct {
// Data used for updating a given collection
@@ -398,7 +399,7 @@ func newCollection(app *App, w http.ResponseWriter, r *http.Request) error {
}
suspended, err := app.db.IsUserSuspended(userID)
if err != nil {
log.Error("new collection: get user: %v", err)
log.Error("new collection: %v", err)
return ErrInternalGeneral
}
if suspended {
@@ -486,6 +487,7 @@ func fetchCollection(app *App, w http.ResponseWriter, r *http.Request) error {
res.Owner = u
}
}
// TODO: check suspended
app.db.GetPostsCount(res, isCollOwner)
// Strip non-public information
res.Collection.ForPublic()
@@ -738,14 +740,10 @@ func handleViewCollection(app *App, w http.ResponseWriter, r *http.Request) erro

suspended, err := app.db.IsUserSuspended(c.OwnerID)
if err != nil {
log.Error("view collection: get owner: %v", err)
log.Error("view collection: %v", err)
return ErrInternalGeneral
}

if suspended {
return ErrCollectionNotFound
}

// Serve ActivityStreams data now, if requested
if strings.Contains(r.Header.Get("Accept"), "application/activity+json") {
ac := c.PersonObject()
@@ -802,6 +800,10 @@ func handleViewCollection(app *App, w http.ResponseWriter, r *http.Request) erro
log.Error("Error getting user for collection: %v", err)
}
}
if !isOwner && suspended {
return ErrCollectionNotFound
}
displayPage.Suspended = isOwner && suspended
displayPage.Owner = owner
coll.Owner = displayPage.Owner

@@ -853,10 +855,6 @@ func handleViewCollectionTag(app *App, w http.ResponseWriter, r *http.Request) e
return err
}

if u.Suspended {
return ErrCollectionNotFound
}

page := getCollectionPage(vars)

c, err := processCollectionPermissions(app, cr, u, w, r)
@@ -908,6 +906,10 @@ func handleViewCollectionTag(app *App, w http.ResponseWriter, r *http.Request) e
log.Error("Error getting user for collection: %v", err)
}
}
if !isOwner && u.Status == UserSuspended {
return ErrCollectionNotFound
}
displayPage.Suspended = u.Status == UserSuspended
displayPage.Owner = owner
coll.Owner = displayPage.Owner
// Add more data
@@ -946,7 +948,7 @@ func existingCollection(app *App, w http.ResponseWriter, r *http.Request) error
collAlias := vars["alias"]
isWeb := r.FormValue("web") == "1"

var u *User
u := &User{}
if reqJSON && !isWeb {
// Ensure an access token was given
accessToken := r.Header.Get("Authorization")
@@ -963,7 +965,7 @@ func existingCollection(app *App, w http.ResponseWriter, r *http.Request) error

suspended, err := app.db.IsUserSuspended(u.ID)
if err != nil {
log.Error("existing collection: get user suspended status: %v", err)
log.Error("existing collection: %v", err)
return ErrInternalGeneral
}

@@ -296,7 +296,7 @@ func (db *datastore) CreateCollection(cfg *config.Config, alias, title string, u
func (db *datastore) GetUserByID(id int64) (*User, error) {
u := &User{ID: id}

err := db.QueryRow("SELECT username, password, email, created, suspended FROM users WHERE id = ?", id).Scan(&u.Username, &u.HashedPass, &u.Email, &u.Created, &u.Suspended)
err := db.QueryRow("SELECT username, password, email, created, status FROM users WHERE id = ?", id).Scan(&u.Username, &u.HashedPass, &u.Email, &u.Created, &u.Status)
switch {
case err == sql.ErrNoRows:
return nil, ErrUserNotFound
@@ -313,16 +313,16 @@ func (db *datastore) GetUserByID(id int64) (*User, error) {
func (db *datastore) IsUserSuspended(id int64) (bool, error) {
u := &User{ID: id}

err := db.QueryRow("SELECT suspended FROM users WHERE id = ?", id).Scan(&u.Suspended)
err := db.QueryRow("SELECT status FROM users WHERE id = ?", id).Scan(&u.Status)
switch {
case err == sql.ErrNoRows:
return false, ErrUserNotFound
return false, fmt.Errorf("is user suspended: %v", ErrUserNotFound)
case err != nil:
log.Error("Couldn't SELECT user password: %v", err)
return false, err
return false, fmt.Errorf("is user suspended: %v", err)
}

return u.Suspended, nil
return u.Status == UserSuspended, nil
}

// DoesUserNeedAuth returns true if the user hasn't provided any methods for
@@ -364,7 +364,7 @@ func (db *datastore) IsUserPassSet(id int64) (bool, error) {
func (db *datastore) GetUserForAuth(username string) (*User, error) {
u := &User{Username: username}

err := db.QueryRow("SELECT id, password, email, created, suspended FROM users WHERE username = ?", username).Scan(&u.ID, &u.HashedPass, &u.Email, &u.Created, &u.Suspended)
err := db.QueryRow("SELECT id, password, email, created, status FROM users WHERE username = ?", username).Scan(&u.ID, &u.HashedPass, &u.Email, &u.Created, &u.Status)
switch {
case err == sql.ErrNoRows:
// Check if they've entered the wrong, unnormalized username
@@ -387,7 +387,7 @@ func (db *datastore) GetUserForAuth(username string) (*User, error) {
func (db *datastore) GetUserForAuthByID(userID int64) (*User, error) {
u := &User{ID: userID}

err := db.QueryRow("SELECT id, password, email, created, suspended FROM users WHERE id = ?", u.ID).Scan(&u.ID, &u.HashedPass, &u.Email, &u.Created, &u.Suspended)
err := db.QueryRow("SELECT id, password, email, created, status FROM users WHERE id = ?", u.ID).Scan(&u.ID, &u.HashedPass, &u.Email, &u.Created, &u.Status)
switch {
case err == sql.ErrNoRows:
return nil, ErrUserNotFound
@@ -1650,7 +1650,7 @@ func (db *datastore) GetTotalCollections() (collCount int64, err error) {
SELECT COUNT(*)
FROM collections c
LEFT JOIN users u ON u.id = c.owner_id
WHERE u.suspended = 0`).Scan(&collCount)
WHERE u.status = 0`).Scan(&collCount)
if err != nil {
log.Error("Unable to fetch collections count: %v", err)
}
@@ -1662,7 +1662,7 @@ func (db *datastore) GetTotalPosts() (postCount int64, err error) {
SELECT COUNT(*)
FROM posts p
LEFT JOIN users u ON u.id = p.owner_id
WHERE u.Suspended = 0`).Scan(&postCount)
WHERE u.status = 0`).Scan(&postCount)
if err != nil {
log.Error("Unable to fetch posts count: %v", err)
}
@@ -2384,7 +2384,7 @@ func (db *datastore) GetAllUsers(page uint) (*[]User, error) {
limitStr = fmt.Sprintf("%d, %d", (page-1)*adminUsersPerPage, adminUsersPerPage)
}

rows, err := db.Query("SELECT id, username, created, suspended FROM users ORDER BY created DESC LIMIT " + limitStr)
rows, err := db.Query("SELECT id, username, created, status FROM users ORDER BY created DESC LIMIT " + limitStr)
if err != nil {
log.Error("Failed selecting from users: %v", err)
return nil, impart.HTTPError{http.StatusInternalServerError, "Couldn't retrieve all users."}
@@ -2394,7 +2394,7 @@ func (db *datastore) GetAllUsers(page uint) (*[]User, error) {
users := []User{}
for rows.Next() {
u := User{}
err = rows.Scan(&u.ID, &u.Username, &u.Created, &u.Suspended)
err = rows.Scan(&u.ID, &u.Username, &u.Created, &u.Status)
if err != nil {
log.Error("Failed scanning GetAllUsers() row: %v", err)
break
@@ -2431,10 +2431,11 @@ func (db *datastore) GetUserLastPostTime(id int64) (*time.Time, error) {
return &t, nil
}

func (db *datastore) SetUserSuspended(id int64, suspend bool) error {
_, err := db.Exec("UPDATE users SET suspended = ? WHERE id = ?", suspend, id)
// SetUserStatus changes a user's status in the database. see Users.UserStatus
func (db *datastore) SetUserStatus(id int64, status UserStatus) error {
_, err := db.Exec("UPDATE users SET status = ? WHERE id = ?", status, id)
if err != nil {
return fmt.Errorf("failed to update user suspended status: %v", err)
return fmt.Errorf("failed to update user status: %v", err)
}
return nil
}
@@ -78,7 +78,7 @@ func handleCreateUserInvite(app *App, u *User, w http.ResponseWriter, r *http.Re
muVal := r.FormValue("uses")
expVal := r.FormValue("expires")

if u.Suspended {
if u.Status == UserSuspended {
return ErrUserSuspended
}

@@ -58,7 +58,7 @@ func (m *migration) Migrate(db *datastore) error {
var migrations = []Migration{
New("support user invites", supportUserInvites), // -> V1 (v0.8.0)
New("support dynamic instance pages", supportInstancePages), // V1 -> V2 (v0.9.0)
New("support users suspension", supportUserSuspension), // V2 -> V3 ()
New("support users suspension", supportUserStatus), // V2 -> V3 (v0.11.0)
}

// CurrentVer returns the current migration version the application is on

0 comments on commit f85f075

Please sign in to comment.
You can’t perform that action at this time.