change delete post authentication logic

this reorders the logic for checking authentication on post deletes to
first check for a provided edit token and after check for an access
token or auth'd user.
pull/117/head
Rob Loranger 6 years ago
parent c87b7ab39e
commit d58c142467
No known key found for this signature in database
GPG Key ID: D6F1633A4F0903B8
  1. 49
      posts.go

@ -14,6 +14,12 @@ import (
"database/sql" "database/sql"
"encoding/json" "encoding/json"
"fmt" "fmt"
"html/template"
"net/http"
"regexp"
"strings"
"time"
"github.com/gorilla/mux" "github.com/gorilla/mux"
"github.com/guregu/null" "github.com/guregu/null"
"github.com/guregu/null/zero" "github.com/guregu/null/zero"
@ -31,11 +37,6 @@ import (
"github.com/writeas/web-core/tags" "github.com/writeas/web-core/tags"
"github.com/writeas/writefreely/page" "github.com/writeas/writefreely/page"
"github.com/writeas/writefreely/parse" "github.com/writeas/writefreely/parse"
"html/template"
"net/http"
"regexp"
"strings"
"time"
) )
const ( const (
@ -737,7 +738,24 @@ func deletePost(app *app, w http.ResponseWriter, r *http.Request) error {
var collID sql.NullInt64 var collID sql.NullInt64
var coll *Collection var coll *Collection
var pp *PublicPost var pp *PublicPost
if accessToken != "" || u != nil { if editToken != "" {
// TODO: SELECT owner_id, as well, and return appropriate error if NULL instead of running two queries
var dummy int64
err = app.db.QueryRow("SELECT 1 FROM posts WHERE id = ?", friendlyID).Scan(&dummy)
switch {
case err == sql.ErrNoRows:
return impart.HTTPError{http.StatusNotFound, "Post not found."}
}
err = app.db.QueryRow("SELECT 1 FROM posts WHERE id = ? AND owner_id IS NULL", friendlyID).Scan(&dummy)
switch {
case err == sql.ErrNoRows:
// Post already has an owner. This could provide a bad experience
// for the user, but it's more important to ensure data isn't lost
// unexpectedly. So prevent deletion via token.
return impart.HTTPError{http.StatusConflict, "This post belongs to some user (hopefully yours). Please log in and delete it from that user's account."}
}
res, err = app.db.Exec("DELETE FROM posts WHERE id = ? AND modify_token = ? AND owner_id IS NULL", friendlyID, editToken)
} else if accessToken != "" || u != nil {
// Caller provided some way to authenticate; assume caller expects the // Caller provided some way to authenticate; assume caller expects the
// post to be deleted based on a specific post owner, thus we should // post to be deleted based on a specific post owner, thus we should
// return corresponding errors. // return corresponding errors.
@ -785,27 +803,8 @@ func deletePost(app *app, w http.ResponseWriter, r *http.Request) error {
res, err = t.Exec("DELETE FROM posts WHERE id = ? AND owner_id = ?", friendlyID, ownerID) res, err = t.Exec("DELETE FROM posts WHERE id = ? AND owner_id = ?", friendlyID, ownerID)
} }
} else { } else {
if editToken == "" {
return impart.HTTPError{http.StatusBadRequest, "No authenticated user or post token given."} return impart.HTTPError{http.StatusBadRequest, "No authenticated user or post token given."}
} }
// TODO: SELECT owner_id, as well, and return appropriate error if NULL instead of running two queries
var dummy int64
err = app.db.QueryRow("SELECT 1 FROM posts WHERE id = ?", friendlyID).Scan(&dummy)
switch {
case err == sql.ErrNoRows:
return impart.HTTPError{http.StatusNotFound, "Post not found."}
}
err = app.db.QueryRow("SELECT 1 FROM posts WHERE id = ? AND owner_id IS NULL", friendlyID).Scan(&dummy)
switch {
case err == sql.ErrNoRows:
// Post already has an owner. This could provide a bad experience
// for the user, but it's more important to ensure data isn't lost
// unexpectedly. So prevent deletion via token.
return impart.HTTPError{http.StatusConflict, "This post belongs to some user (hopefully yours). Please log in and delete it from that user's account."}
}
res, err = app.db.Exec("DELETE FROM posts WHERE id = ? AND modify_token = ? AND owner_id IS NULL", friendlyID, editToken)
}
if err != nil { if err != nil {
return err return err
} }

Loading…
Cancel
Save