Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Attempt to fix realm quota alert #743

Merged
merged 1 commit into from
Oct 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cmd/server/assets/realmadmin/_form_abuse_prevention.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{{define "realmadmin/_form_abuse_prevention"}}

{{$realm := .realm}}
{{$quotaRemaining := .quotaRemaining}}
{{$quotaLimit := .quotaLimit}}

<form method="POST" action="/realm/settings#abuse-prevention" class="floating-form">
{{ .csrfField }}
Expand All @@ -27,6 +29,17 @@
</div>

<div id="abuse-prevention-configuration" class="collapse{{if $realm.AbusePreventionEnabled}} show{{end}}">
<div class="alert alert-primary" role="alert">
Your current remaining daily quota is:
<small class="text-monospace">{{$quotaRemaining}}/{{$quotaLimit}}</small>. This value resets at
midnight UTC.

{{if gt $quotaRemaining $quotaLimit}}
If your remaining quota exceeds the maximum quota, it means a realm
administrator added a temporary burst.
{{end}}
</div>

<div class="form-label-group">
<input type="text" name="abuse_prevention_limit" id="abuse-prevention-limit" class="form-control" placeholder="Computed limit" value="{{$realm.AbusePreventionLimit}}" readonly />
<label for="abuse-prevention-limit">Computed limit</label>
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ require (
github.com/rakutentech/jwk-go v1.0.1
github.com/russross/blackfriday/v2 v2.0.1
github.com/sethvargo/go-envconfig v0.3.2
github.com/sethvargo/go-limiter v0.5.2
github.com/sethvargo/go-limiter v0.6.0
github.com/sethvargo/go-password v0.2.0
github.com/sethvargo/go-redisstore v0.2.1-opencensus
github.com/sethvargo/go-redisstore v0.3.0-opencensus
github.com/sethvargo/go-retry v0.1.0
github.com/sethvargo/go-signalcontext v0.1.0
github.com/sirupsen/logrus v1.7.0 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1153,12 +1153,12 @@ github.com/sethvargo/go-envconfig v0.3.2 h1:277Lb2iTpUZjUZu1qLoLa/aetwvtZbKh8wNW
github.com/sethvargo/go-envconfig v0.3.2/go.mod h1:XZ2JRR7vhlBEO5zMmOpLgUhgYltqYqq4d4tKagtPUv0=
github.com/sethvargo/go-gcpkms v0.1.0 h1:pyjDLqLwpk9pMjDSTilPpaUjgP1AfSjX9WGzitZwGUY=
github.com/sethvargo/go-gcpkms v0.1.0/go.mod h1:33BuvqUjsYk0bpMgn+WCclCYtMLOyaqtn5j0fCo4vvk=
github.com/sethvargo/go-limiter v0.5.2 h1:NIFp7xy3NyE2+mEHbengdLQF0C0STOpwF5Qw5JtayIs=
github.com/sethvargo/go-limiter v0.5.2/go.mod h1:C0kbSFbiriE5k2FFOe18M1YZbAR2Fiwf72uGu0CXCcU=
github.com/sethvargo/go-limiter v0.6.0 h1:186jmCdl1ItQUXbHFdTBrFSZztN6/bL9855C5jfMlKU=
github.com/sethvargo/go-limiter v0.6.0/go.mod h1:C0kbSFbiriE5k2FFOe18M1YZbAR2Fiwf72uGu0CXCcU=
github.com/sethvargo/go-password v0.2.0 h1:BTDl4CC/gjf/axHMaDQtw507ogrXLci6XRiLc7i/UHI=
github.com/sethvargo/go-password v0.2.0/go.mod h1:Ym4Mr9JXLBycr02MFuVQ/0JHidNetSgbzutTr3zsYXE=
github.com/sethvargo/go-redisstore v0.2.1-opencensus h1:EAwZAuZr5DJdLmEruJTj2zeBvmZsIXI7wqgMueuaxks=
github.com/sethvargo/go-redisstore v0.2.1-opencensus/go.mod h1:TMFAy7azG5hDd/Hb5ng2CDsawcxg1+oEuGhuVp7eycI=
github.com/sethvargo/go-redisstore v0.3.0-opencensus h1:H9W15fuJHwHmttV+G6oY94J/YjxhRz0E/1S5y7elxlg=
github.com/sethvargo/go-redisstore v0.3.0-opencensus/go.mod h1:byILvIz3sOqWiKLQmL7KUK0CzD3MWHajkksZH7V43yk=
github.com/sethvargo/go-retry v0.1.0 h1:8sPqlWannzcReEcYjHSNw9becsiYudcwTD7CasGjQaI=
github.com/sethvargo/go-retry v0.1.0/go.mod h1:JzIOdZqQDNpPkQDmcqgtteAcxFLtYpNF/zJCM1ysDg8=
github.com/sethvargo/go-signalcontext v0.1.0 h1:3IU7HOlmRXF0PSDf85C4nJ/zjYDjF+DS+LufcKfLvyk=
Expand Down
58 changes: 28 additions & 30 deletions pkg/controller/issueapi/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,31 +192,35 @@ func (c *Controller) HandleIssue() http.Handler {
}
}

// If we got this far, we're about to issue a code.
dig, err := digest.HMACUint(realm.ID, c.config.GetRateLimitConfig().HMACKey)
if err != nil {
controller.InternalError(w, r, c.h, err)
return
}
key := fmt.Sprintf("realm:quota:%s", dig)
limit, remaining, reset, ok, err := c.limiter.Take(ctx, key)
if err != nil {
logger.Errorw("failed to take from limiter", "error", err)
stats.Record(ctx, c.metrics.QuotaErrors.M(1))
c.h.RenderJSON(w, http.StatusInternalServerError, api.Errorf("failed to verify realm stats, please try again"))
return
}
if !ok {
logger.Warnw("realm has exceeded daily quota",
"realm", realm.ID,
"limit", limit,
"reset", reset)
stats.Record(ctx, c.metrics.QuotaExceeded.M(1))

if c.config.GetEnforceRealmQuotas() {
c.h.RenderJSON(w, http.StatusTooManyRequests, api.Errorf("exceeded realm quota"))
// If we got this far, we're about to issue a code - take from the limiter
// to ensure this is permitted.
if realm.AbusePreventionEnabled {
dig, err := digest.HMACUint(realm.ID, c.config.GetRateLimitConfig().HMACKey)
if err != nil {
controller.InternalError(w, r, c.h, err)
return
}
key := fmt.Sprintf("realm:quota:%s", dig)
limit, remaining, reset, ok, err := c.limiter.Take(ctx, key)
c.recordCapacity(ctx, limit, remaining)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this up - we were only recording capacity later, which I think would cause us to miss some data points.

if err != nil {
logger.Errorw("failed to take from limiter", "error", err)
stats.Record(ctx, c.metrics.QuotaErrors.M(1))
c.h.RenderJSON(w, http.StatusInternalServerError, api.Errorf("failed to verify realm stats, please try again"))
return
}
if !ok {
logger.Warnw("realm has exceeded daily quota",
"realm", realm.ID,
"limit", limit,
"reset", reset)
stats.Record(ctx, c.metrics.QuotaExceeded.M(1))

if c.config.GetEnforceRealmQuotas() {
c.h.RenderJSON(w, http.StatusTooManyRequests, api.Errorf("exceeded realm quota"))
return
}
}
}

now := time.Now().UTC()
Expand Down Expand Up @@ -251,8 +255,6 @@ func (c *Controller) HandleIssue() http.Handler {
return
}

c.recordCapacity(ctx, realm, remaining)

if request.Phone != "" && smsProvider != nil {
message := realm.BuildSMSText(code, longCode, c.config.GetENXRedirectDomain())
if err := smsProvider.SendSMS(ctx, request.Phone, message); err != nil {
Expand Down Expand Up @@ -296,13 +298,9 @@ func (c *Controller) getAuthorizationFromContext(r *http.Request) (*database.Aut
return authorizedApp, currentUser, nil
}

func (c *Controller) recordCapacity(ctx context.Context, realm *database.Realm, remaining uint64) {
if !realm.AbusePreventionEnabled {
return
}
func (c *Controller) recordCapacity(ctx context.Context, limit, remaining uint64) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to rely on the value that comes from the limiter, not the value on the realm. The enforcement happens based on what's in the limiter, so we should alert and record off of that.

stats.Record(ctx, c.metrics.RealmTokenRemaining.M(int64(remaining)))

limit := realm.AbusePreventionEffectiveLimit()
issued := uint64(limit) - remaining
stats.Record(ctx, c.metrics.RealmTokenIssued.M(int64(issued)))

Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/realmadmin/express.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (c *Controller) HandleDisableExpress() http.Handler {

if !realm.EnableENExpress {
flash.Error("Realm is not currently enrolled in EN Express.")
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, 0, 0)
return
}

Expand All @@ -56,7 +56,7 @@ func (c *Controller) HandleDisableExpress() http.Handler {
if err := c.db.SaveRealm(realm, currentUser); err != nil {
flash.Error("Failed to disable EN Express: %v", err)

c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, 0, 0)
return
}

Expand Down Expand Up @@ -90,7 +90,7 @@ func (c *Controller) HandleEnableExpress() http.Handler {

if realm.EnableENExpress {
flash.Error("Realm already has EN Express Enabled.")
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, 0, 0)
return
}

Expand All @@ -110,7 +110,7 @@ func (c *Controller) HandleEnableExpress() http.Handler {
// This will allow the user to correct other validation errors and then click "uprade" again.
realm.EnableENExpress = false
realm.SMSTextTemplate = enxSettings.SMSTextTemplate
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, 0, 0)
return
}

Expand Down
38 changes: 29 additions & 9 deletions pkg/controller/realmadmin/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,31 @@ func (c *Controller) HandleSettings() http.Handler {
return
}

var quotaLimit, quotaRemaining uint64
if realm.AbusePreventionEnabled {
dig, err := digest.HMACUint(realm.ID, c.config.RateLimit.HMACKey)
if err != nil {
controller.InternalError(w, r, c.h, err)
return
}
key := fmt.Sprintf("realm:quota:%s", dig)

quotaLimit, quotaRemaining, err = c.limiter.Get(ctx, key)
if err != nil {
controller.InternalError(w, r, c.h, err)
return
}
}

if r.Method == http.MethodGet {
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, quotaLimit, quotaRemaining)
return
}

var form FormData
if err := controller.BindForm(w, r, &form); err != nil {
flash.Error("Failed to process form: %v", err)
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, quotaLimit, quotaRemaining)
return
}

Expand Down Expand Up @@ -158,7 +174,7 @@ func (c *Controller) HandleSettings() http.Handler {
if err != nil {
realm.AddError("allowedCIDRsAdminAPI", err.Error())
flash.Error("Failed to update realm")
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, quotaLimit, quotaRemaining)
return
}
realm.AllowedCIDRsAdminAPI = allowedCIDRsAdminADPI
Expand All @@ -167,7 +183,7 @@ func (c *Controller) HandleSettings() http.Handler {
if err != nil {
realm.AddError("allowedCIDRsAPIServer", err.Error())
flash.Error("Failed to update realm")
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, quotaLimit, quotaRemaining)
return
}
realm.AllowedCIDRsAPIServer = allowedCIDRsAPIServer
Expand All @@ -176,7 +192,7 @@ func (c *Controller) HandleSettings() http.Handler {
if err != nil {
realm.AddError("allowedCIDRsServer", err.Error())
flash.Error("Failed to update realm")
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, quotaLimit, quotaRemaining)
return
}
realm.AllowedCIDRsServer = allowedCIDRsServer
Expand All @@ -191,7 +207,7 @@ func (c *Controller) HandleSettings() http.Handler {
// Save realm
if err := c.db.SaveRealm(realm, currentUser); err != nil {
flash.Error("Failed to update realm: %v", err)
c.renderSettings(ctx, w, r, realm, nil)
c.renderSettings(ctx, w, r, realm, nil, quotaLimit, quotaRemaining)
return
}

Expand All @@ -213,7 +229,7 @@ func (c *Controller) HandleSettings() http.Handler {

if err := c.db.SaveSMSConfig(smsConfig); err != nil {
flash.Error("Failed to update realm: %v", err)
c.renderSettings(ctx, w, r, realm, smsConfig)
c.renderSettings(ctx, w, r, realm, smsConfig, quotaLimit, quotaRemaining)
return
}
} else {
Expand All @@ -229,7 +245,7 @@ func (c *Controller) HandleSettings() http.Handler {

if err := c.db.SaveSMSConfig(smsConfig); err != nil {
flash.Error("Failed to update realm: %v", err)
c.renderSettings(ctx, w, r, realm, smsConfig)
c.renderSettings(ctx, w, r, realm, smsConfig, quotaLimit, quotaRemaining)
return
}
}
Expand All @@ -256,7 +272,7 @@ func (c *Controller) HandleSettings() http.Handler {
})
}

func (c *Controller) renderSettings(ctx context.Context, w http.ResponseWriter, r *http.Request, realm *database.Realm, smsConfig *database.SMSConfig) {
func (c *Controller) renderSettings(ctx context.Context, w http.ResponseWriter, r *http.Request, realm *database.Realm, smsConfig *database.SMSConfig, quotaLimit, quotaRemaining uint64) {
if smsConfig == nil {
var err error
smsConfig, err = realm.SMSConfig(c.db)
Expand Down Expand Up @@ -309,5 +325,9 @@ func (c *Controller) renderSettings(ctx context.Context, w http.ResponseWriter,
m["longCodeLengths"] = longCodeLengths
m["longCodeHours"] = longCodeHours
m["enxRedirectDomain"] = c.config.GetENXRedirectDomain()

m["quotaLimit"] = quotaLimit
m["quotaRemaining"] = quotaRemaining

c.h.RenderHTML(w, "realmadmin/edit", m)
}