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

Commit

Permalink
Handle edge case when realm quota is first enabled (#765)
Browse files Browse the repository at this point in the history
* Handle edge case when realm quota is first enabled

If realm quota is enabled and then someone "takes" from the bucket before the modeler runs, then the realm gets the default quota (which is low). Worse, it gets the default TTL (60s), which also isn't correct.

* Oops
  • Loading branch information
sethvargo authored Oct 7, 2020
1 parent 077fdd5 commit ad5a212
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 8 deletions.
14 changes: 8 additions & 6 deletions cmd/server/assets/realmadmin/_form_abuse_prevention.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,16 @@
<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.
<small class="text-monospace">{{$quotaRemaining}}/{{$quotaLimit}}</small>.
This value is calculated and reset each day at <strong>11:59:59 UTC</strong>.
</div>

{{if gt $quotaRemaining $quotaLimit}}
If your remaining quota exceeds the maximum quota, it means a realm
{{if gt $quotaRemaining $quotaLimit}}
<div class="alert alert-warning" role="alert">
Your remaining quota exceeds the maximum quota, meaning a realm
administrator added a temporary burst.
{{end}}
</div>
</div>
{{end}}

<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 />
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/admin_server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type AdminAPIServerConfig struct {

CollisionRetryCount uint `env:"COLLISION_RETRY_COUNT,default=6"`
AllowedSymptomAge time.Duration `env:"ALLOWED_PAST_SYMPTOM_DAYS,default=336h"` // 336h is 14 days.
EnforceRealmQuotas bool `env:"ENFORCE_REALM_QUOTAS, default=false"`
EnforceRealmQuotas bool `env:"ENFORCE_REALM_QUOTAS, default=true"`

// For EN Express, the link will be
// https://[realm-region].[ENX_REDIRECT_DOMAIN]/v?c=[longcode]
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ type ServerConfig struct {
ServerName string `env:"SERVER_NAME,default=Diagnosis Verification Server"`
CollisionRetryCount uint `env:"COLLISION_RETRY_COUNT,default=6"`
AllowedSymptomAge time.Duration `env:"ALLOWED_PAST_SYMPTOM_DAYS,default=336h"` // 336h is 14 days.
EnforceRealmQuotas bool `env:"ENFORCE_REALM_QUOTAS, default=false"`
EnforceRealmQuotas bool `env:"ENFORCE_REALM_QUOTAS, default=true"`

AssetsPath string `env:"ASSETS_PATH,default=./cmd/server/assets"`

Expand Down
27 changes: 27 additions & 0 deletions pkg/controller/realmadmin/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,38 @@ func (c *Controller) HandleSettings() http.Handler {
}

// Abuse prevention
var abusePreventionJustEnabled bool
if form.AbusePrevention {
abusePreventionJustEnabled = !realm.AbusePreventionEnabled && form.AbusePreventionEnabled

realm.AbusePreventionEnabled = form.AbusePreventionEnabled
realm.AbusePreventionLimitFactor = form.AbusePreventionLimitFactor
}

// If abuse prevention was just enabled, create the initial bucket so
// enforcement works. We do this before actually saving the configuration on
// the realm to avoid a race where someone is issuing a code where abuse
// prevention has been enabled, but the quota has not been set. In that
// case, the quota would be the "default" quota for the limiter, which is
// not ideal or correct.
//
// Even if saving the realm fails, there's no harm in doing this early. It's
// an idempotent operation that TTLs out after a week anyway.
if abusePreventionJustEnabled {
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)
limit := uint64(realm.AbusePreventionEffectiveLimit())
ttl := 7 * 24 * time.Hour
if err := c.limiter.Set(ctx, key, limit, ttl); err != nil {
controller.InternalError(w, r, c.h, err)
return
}
}

// Save realm
if err := c.db.SaveRealm(realm, currentUser); err != nil {
flash.Error("Failed to update realm: %v", err)
Expand Down

0 comments on commit ad5a212

Please sign in to comment.