Skip to content

Commit

Permalink
admin: Remove ban and unban
Browse files Browse the repository at this point in the history 
The api ban and unban endpoints do not change the user's ban score. The
server then bans or unbans upon connect based on the user's ban score.
This made the api endpoints obsolete and ineffective. Remove those
endpoints until and if an alternative can be discussed and implemented.
  • Loading branch information
@JoeGruffins
JoeGruffins authored Nov 8, 2021
1 parent f5835bd commit dfbbe9a
Show file tree
Hide file tree
Showing 7 changed files with 1 addition and 225 deletions.
55 changes: 0 additions & 55 deletions server/admin/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,61 +493,6 @@ func decodeAcctID(acctIDStr string) (account.AccountID, error) {
return acctID, nil
}

// apiBan is the handler for the '/account/{accountID}/ban?rule=RULE' API request.
func (s *Server) apiBan(w http.ResponseWriter, r *http.Request) {
acctIDStr := chi.URLParam(r, accountIDKey)
acctID, err := decodeAcctID(acctIDStr)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
ruleStr := r.URL.Query().Get(ruleKey)
if ruleStr == "" {
http.Error(w, "rule not specified", http.StatusBadRequest)
return
}
ruleInt, err := strconv.Atoi(ruleStr)
if err != nil {
http.Error(w, fmt.Sprintf("bad rule: %v", err), http.StatusBadRequest)
return
}
if !account.Rule(ruleInt).Punishable() {
msg := fmt.Sprintf("bad rule (%d): not known or not punishable", ruleInt)
http.Error(w, msg, http.StatusBadRequest)
return
}
// TODO: Allow operator supplied details to go with the ban.
if err := s.core.Penalize(acctID, account.Rule(ruleInt), ""); err != nil {
http.Error(w, fmt.Sprintf("failed to ban account: %v", err), http.StatusInternalServerError)
return
}
res := BanResult{
AccountID: acctIDStr,
BrokenRule: byte(ruleInt),
BanTime: APITime{time.Now()},
}
writeJSON(w, res)
}

// apiUnBan is the handler for the '/account/{accountID}/unban' API request.
func (s *Server) apiUnban(w http.ResponseWriter, r *http.Request) {
acctIDStr := chi.URLParam(r, accountIDKey)
acctID, err := decodeAcctID(acctIDStr)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
if err := s.core.Unban(acctID); err != nil {
http.Error(w, fmt.Sprintf("failed to unban account: %v", err), http.StatusInternalServerError)
return
}
res := UnbanResult{
AccountID: acctIDStr,
UnbanTime: APITime{time.Now()},
}
writeJSON(w, res)
}

// apiForgiveMatchFail is the handler for the '/account/{accountID}/forgive_match/{matchID}' API request.
func (s *Server) apiForgiveMatchFail(w http.ResponseWriter, r *http.Request) {
acctIDStr := chi.URLParam(r, accountIDKey)
Expand Down
4 changes: 0 additions & 4 deletions server/admin/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ type SvrCore interface {
MarketStatuses() map[string]*market.Status
SuspendMarket(name string, tSusp time.Time, persistBooks bool) (*market.SuspendEpoch, error)
ResumeMarket(name string, asSoonAs time.Time) (startEpoch int64, startTime time.Time, err error)
Penalize(aid account.AccountID, rule account.Rule, details string) error
Unban(aid account.AccountID) error
ForgiveMatchFail(aid account.AccountID, mid order.MatchID) (forgiven, unbanned bool, err error)
BookOrders(base, quote uint32) (orders []*order.LimitOrder, err error)
EpochOrders(base, quote uint32) (orders []order.Order, err error)
Expand Down Expand Up @@ -152,8 +150,6 @@ func NewServer(cfg *SrvConfig) (*Server, error) {
r.Get("/enabledataapi/{"+yesKey+"}", s.apiEnableDataAPI)
r.Route("/account/{"+accountIDKey+"}", func(rm chi.Router) {
rm.Get("/", s.apiAccountInfo)
rm.Get("/ban", s.apiBan)
rm.Get("/unban", s.apiUnban)
rm.Get("/forgive_match/{"+matchIDKey+"}", s.apiForgiveMatchFail)
rm.Post("/notify", s.apiNotify)
})
Expand Down
136 changes: 0 additions & 136 deletions server/admin/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1303,86 +1303,6 @@ func TestAccountInfo(t *testing.T) {
}
}

func TestBan(t *testing.T) {
core := new(TCore)
srv := &Server{
core: core,
}
mux := chi.NewRouter()
mux.Route("/account/{"+accountIDKey+"}/ban", func(rm chi.Router) {
rm.Get("/", srv.apiBan)
})
acctIDStr := "0a9912205b2cbab0c25c2de30bda9074de0ae23b065489a99199bad763f102cc"
tests := []struct {
name, acctID, rule string
penalizeErr error
wantCode int
}{{
name: "ok hex lower case",
acctID: acctIDStr,
rule: fmt.Sprint(int(account.MaxRule) - 1),
wantCode: http.StatusOK,
}, {
name: "ok hex upper case",
acctID: strings.ToUpper(acctIDStr),
rule: "1",
wantCode: http.StatusOK,
}, {
name: "account id not hex",
acctID: "nothex",
rule: "1",
wantCode: http.StatusBadRequest,
}, {
name: "account id wrong length",
acctID: acctIDStr[2:],
rule: "1",
wantCode: http.StatusBadRequest,
}, {
name: "no rule",
acctID: acctIDStr,
wantCode: http.StatusBadRequest,
}, {
name: "rule not integer",
acctID: acctIDStr,
rule: "not int",
wantCode: http.StatusBadRequest,
}, {
name: "rule NoRule",
acctID: acctIDStr,
rule: fmt.Sprint(account.NoRule),
wantCode: http.StatusBadRequest,
}, {
name: "unknown rule",
acctID: acctIDStr,
rule: fmt.Sprint(account.MaxRule),
wantCode: http.StatusBadRequest,
}, {
name: "core.Penalize error",
acctID: acctIDStr,
rule: "1",
penalizeErr: errors.New("error"),
wantCode: http.StatusInternalServerError,
}}
for _, test := range tests {
core.penalizeErr = test.penalizeErr
w := httptest.NewRecorder()
r, _ := http.NewRequest("GET", "https://localhost/account/"+test.acctID+"/ban?"+ruleKey+"="+test.rule, nil)
r.RemoteAddr = "localhost"

mux.ServeHTTP(w, r)

if w.Code != test.wantCode {
t.Fatalf("%q: apiBan returned code %d, expected %d", test.name, w.Code, test.wantCode)
}
if w.Code == http.StatusOK {
res := new(BanResult)
if err := json.Unmarshal(w.Body.Bytes(), res); err != nil {
t.Errorf("%q: unexpected response %v: %v", test.name, w.Body.String(), err)
}
}
}
}

func TestAPITimeMarshalJSON(t *testing.T) {
now := APITime{time.Now()}
b, err := json.Marshal(now)
Expand All @@ -1398,62 +1318,6 @@ func TestAPITimeMarshalJSON(t *testing.T) {
}
}

func TestUnban(t *testing.T) {
core := new(TCore)
srv := &Server{
core: core,
}
mux := chi.NewRouter()
mux.Route("/account/{"+accountIDKey+"}/unban", func(rm chi.Router) {
rm.Get("/", srv.apiUnban)
})
acctIDStr := "0a9912205b2cbab0c25c2de30bda9074de0ae23b065489a99199bad763f102cc"
tests := []struct {
name, acctID string
unbanErr error
wantCode int
}{{
name: "ok hex lower case",
acctID: acctIDStr,
wantCode: http.StatusOK,
}, {
name: "ok hex upper case",
acctID: strings.ToUpper(acctIDStr),
wantCode: http.StatusOK,
}, {
name: "account id not hex",
acctID: "nothex",
wantCode: http.StatusBadRequest,
}, {
name: "account id wrong length",
acctID: acctIDStr[2:],
wantCode: http.StatusBadRequest,
}, {
name: "core.Unban error",
acctID: acctIDStr,
unbanErr: errors.New("error"),
wantCode: http.StatusInternalServerError,
}}
for _, test := range tests {
core.unbanErr = test.unbanErr
w := httptest.NewRecorder()
r, _ := http.NewRequest("GET", "https://localhost/account/"+test.acctID+"/unban", nil)
r.RemoteAddr = "localhost"

mux.ServeHTTP(w, r)

if w.Code != test.wantCode {
t.Fatalf("%q: apiUnban returned code %d, expected %d", test.name, w.Code, test.wantCode)
}
if w.Code == http.StatusOK {
res := new(UnbanResult)
if err := json.Unmarshal(w.Body.Bytes(), res); err != nil {
t.Errorf("%q: unexpected response %v: %v", test.name, w.Body.String(), err)
}
}
}
}

func TestNotify(t *testing.T) {
core := new(TCore)
srv := &Server{
Expand Down
13 changes: 0 additions & 13 deletions server/admin/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,6 @@ func (at *APITime) UnmarshalJSON(b []byte) error {
return nil
}

// BanResult holds the result of a ban.
type BanResult struct {
AccountID string `json:"accountid"`
BrokenRule byte `json:"brokenrule"`
BanTime APITime `json:"bantime"`
}

// UnbanResult holds the result of an unban.
type UnbanResult struct {
AccountID string `json:"accountid"`
UnbanTime APITime `json:"unbantime"`
}

// ForgiveResult holds the result of a forgive_match.
type ForgiveResult struct {
AccountID string `json:"accountid"`
Expand Down
3 changes: 1 addition & 2 deletions server/auth/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -884,8 +884,7 @@ func (auth *AuthManager) Penalize(user account.AccountID, lastRule account.Rule,
}

// Unban forgives a user, allowing them to resume trading if their score permits
// it. This is primarily useful for reversing a manual ban. Use ForgiveMatchFail
// to forgive specific match negotiation failures.
// it. Use ForgiveMatchFail to forgive specific match negotiation failures.
func (auth *AuthManager) Unban(user account.AccountID) error {
client := auth.user(user)
if client != nil {
Expand Down
11 changes: 0 additions & 11 deletions server/dex/dex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -917,17 +917,6 @@ func (dm *DEX) AccountInfo(aid account.AccountID) (*db.Account, error) {
return dm.storage.AccountInfo(aid)
}

// Penalize bans an account by canceling the client's orders and setting their rule
// status to rule.
func (dm *DEX) Penalize(aid account.AccountID, rule account.Rule, details string) error {
return dm.authMgr.Penalize(aid, rule, details)
}

// Unban reverses a ban and allows a client to resume trading.
func (dm *DEX) Unban(aid account.AccountID) error {
return dm.authMgr.Unban(aid)
}

// ForgiveMatchFail forgives a user for a specific match failure, potentially
// allowing them to resume trading if their score becomes passing.
func (dm *DEX) ForgiveMatchFail(aid account.AccountID, mid order.MatchID) (forgiven, unbanned bool, err error) {
Expand Down
4 changes: 0 additions & 4 deletions spec/admin.mediawiki
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ The server will provide an HTTP API for performing various adminstrative tasks.
|-
| /account/{accountID} || GET || list information about a specific account
|-
| /account/{accountID}/unban || GET || clear an account's penalties and re-enable trading
|-
| /account/{accountID}/ban?rule=RULE || GET || ban an account for violating [[community.mediawiki/#Rules_of_Community_Conduct|a rule]]
|-
| /account/{accountID}/notify?timeout=TIMEOUT || POST || send a notification containing text in the request body to account. If not currently connected, the notification will be sent upon reconnect unless timeout duration has passed. default timeout is 72 hours. timeout should be of the form #h#m#s (i.e. "2h" or "5h30m"). Header Content-Type must be set to "text/plain"
|-
| /account/{accountID}/forgive_match/{matchID} || GET || forgive an account for a specific match failure
Expand Down

0 comments on commit dfbbe9a

Please sign in to comment.