mirror of
https://github.com/getAlby/hub.git
synced 2026-08-13 12:33:39 +02:00
fix: switch unlock rate limiter from per-IP to global (#2540)
The unlock endpoints were rate limited per client IP, which is derived from request headers and so is chosen by the caller. Switch to a single global rate limiter (one bucket for all callers) and apply it to every endpoint that verifies the unlock password: start, unlock, backup, mnemonic, apps, autoswap, unlock-password and auto-unlock. A small burst keeps unlocking and immediately performing an action working. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
3b3c37dd0c
commit
363c22f6d3
2 changed files with 107 additions and 8 deletions
|
|
@ -97,12 +97,22 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) {
|
|||
e.POST("/api/setup", httpSvc.setupHandler)
|
||||
e.POST("/api/restore", httpSvc.restoreBackupHandler)
|
||||
|
||||
// allow one unlock request per second
|
||||
unlockRateLimiter := middleware.RateLimiter(middleware.NewRateLimiterMemoryStore(1))
|
||||
// A single global rate limiter (one bucket for all callers, not per-IP)
|
||||
// shared by every endpoint that verifies the unlock password, to bound how
|
||||
// fast the password can be guessed.
|
||||
unlockRateLimiter := middleware.RateLimiterWithConfig(middleware.RateLimiterConfig{
|
||||
Store: middleware.NewRateLimiterMemoryStoreWithConfig(
|
||||
// burst of 2 so unlocking and then immediately acting is not blocked
|
||||
middleware.RateLimiterMemoryStoreConfig{Rate: 1, Burst: 2},
|
||||
),
|
||||
IdentifierExtractor: func(c echo.Context) (string, error) {
|
||||
return "", nil
|
||||
},
|
||||
})
|
||||
e.POST("/api/start", httpSvc.startHandler, unlockRateLimiter)
|
||||
e.POST("/api/unlock", httpSvc.unlockHandler, unlockRateLimiter)
|
||||
e.POST("/api/backup", httpSvc.createBackupHandler, unlockRateLimiter)
|
||||
e.GET("/logout", httpSvc.logoutHandler, unlockRateLimiter)
|
||||
e.GET("/logout", httpSvc.logoutHandler)
|
||||
|
||||
frontend.RegisterHandlers(e)
|
||||
|
||||
|
|
@ -157,17 +167,17 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) {
|
|||
fullAccessApiGroup.Use(httpSvc.requireFullAccess)
|
||||
|
||||
fullAccessApiGroup.POST("/event", httpSvc.eventHandler)
|
||||
fullAccessApiGroup.PATCH("/unlock-password", httpSvc.changeUnlockPasswordHandler)
|
||||
fullAccessApiGroup.PATCH("/auto-unlock", httpSvc.autoUnlockHandler)
|
||||
fullAccessApiGroup.PATCH("/unlock-password", httpSvc.changeUnlockPasswordHandler, unlockRateLimiter)
|
||||
fullAccessApiGroup.PATCH("/auto-unlock", httpSvc.autoUnlockHandler, unlockRateLimiter)
|
||||
fullAccessApiGroup.PATCH("/settings", httpSvc.updateSettingsHandler)
|
||||
fullAccessApiGroup.PATCH("/apps/:pubkey", httpSvc.appsUpdateHandler)
|
||||
fullAccessApiGroup.PATCH("/transactions/:id/labels", httpSvc.setTransactionUserLabelsHandler)
|
||||
fullAccessApiGroup.DELETE("/apps/:pubkey", httpSvc.appsDeleteHandler)
|
||||
fullAccessApiGroup.POST("/transfers", httpSvc.transfersHandler)
|
||||
fullAccessApiGroup.POST("/apps", httpSvc.appsCreateHandler)
|
||||
fullAccessApiGroup.POST("/apps", httpSvc.appsCreateHandler, unlockRateLimiter)
|
||||
fullAccessApiGroup.POST("/lightning-addresses", httpSvc.lightningAddressesCreateHandler)
|
||||
fullAccessApiGroup.DELETE("/lightning-addresses/:appId", httpSvc.lightningAddressesDeleteHandler)
|
||||
fullAccessApiGroup.POST("/mnemonic", httpSvc.mnemonicHandler)
|
||||
fullAccessApiGroup.POST("/mnemonic", httpSvc.mnemonicHandler, unlockRateLimiter)
|
||||
fullAccessApiGroup.PATCH("/backup-reminder", httpSvc.backupReminderHandler)
|
||||
fullAccessApiGroup.POST("/channels", httpSvc.openChannelHandler)
|
||||
fullAccessApiGroup.POST("/channels/rebalance", httpSvc.rebalanceChannelHandler)
|
||||
|
|
@ -192,7 +202,7 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) {
|
|||
fullAccessApiGroup.POST("/swaps/refund", httpSvc.refundSwapHandler)
|
||||
fullAccessApiGroup.GET("/swaps/mnemonic", httpSvc.swapMnemonicHandler)
|
||||
fullAccessApiGroup.GET("/log/:type", httpSvc.getLogOutputHandler)
|
||||
fullAccessApiGroup.POST("/autoswap", httpSvc.enableAutoSwapOutHandler)
|
||||
fullAccessApiGroup.POST("/autoswap", httpSvc.enableAutoSwapOutHandler, unlockRateLimiter)
|
||||
fullAccessApiGroup.DELETE("/autoswap", httpSvc.disableAutoSwapOutHandler)
|
||||
fullAccessApiGroup.POST("/node/alias", httpSvc.setNodeAliasHandler)
|
||||
|
||||
|
|
|
|||
|
|
@ -92,6 +92,95 @@ func TestUnlock_UnknownPermission(t *testing.T) {
|
|||
mockConfig.AssertNotCalled(t, "GetJWTSecret")
|
||||
}
|
||||
|
||||
// TestUnlock_RateLimited verifies that repeated requests to an unlock-password
|
||||
// endpoint are throttled with HTTP 429 once the limit is exceeded.
|
||||
func TestUnlock_RateLimited(t *testing.T) {
|
||||
e := echo.New()
|
||||
logger.Init(strconv.Itoa(int(logrus.DebugLevel)))
|
||||
mockSvc := mocks.NewMockService(t)
|
||||
gormDb, err := db.NewDB(t)
|
||||
require.NoError(t, err)
|
||||
defer db.CloseDB(gormDb)
|
||||
|
||||
mockEventPublisher := events.NewEventPublisher()
|
||||
|
||||
mockConfig := mocks.NewMockConfig(t)
|
||||
mockConfig.On("GetEnv").Return(&config.AppConfig{})
|
||||
mockConfig.On("CheckUnlockPassword", "wrong").Return(false)
|
||||
|
||||
mockSvc.On("GetDB").Return(gormDb)
|
||||
mockSvc.On("GetConfig").Return(mockConfig)
|
||||
mockSvc.On("GetKeys").Return(mocks.NewMockKeys(t))
|
||||
mockSvc.On("GetAlbySvc").Return(mocks.NewMockAlbyService(t))
|
||||
mockSvc.On("GetAlbyOAuthSvc").Return(mocks.NewMockAlbyOAuthService(t))
|
||||
|
||||
httpSvc := NewHttpService(mockSvc, mockEventPublisher)
|
||||
httpSvc.RegisterSharedRoutes(e)
|
||||
|
||||
jsonBody, _ := json.Marshal(api.UnlockRequest{UnlockPassword: "wrong", Permission: "full"})
|
||||
send := func() int {
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/unlock", bytes.NewBuffer(jsonBody))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
rec := httptest.NewRecorder()
|
||||
e.ServeHTTP(rec, req)
|
||||
return rec.Code
|
||||
}
|
||||
|
||||
// the burst of 2 is served (wrong password, so unauthorized)
|
||||
assert.Equal(t, http.StatusUnauthorized, send())
|
||||
assert.Equal(t, http.StatusUnauthorized, send())
|
||||
// the next request exceeds the limit and is rejected with 429
|
||||
assert.Equal(t, http.StatusTooManyRequests, send())
|
||||
}
|
||||
|
||||
// TestUnlock_RateLimitNotBypassedBySpoofedIP verifies that the unlock rate
|
||||
// limiter is global rather than per-IP: varying the X-Forwarded-For header per
|
||||
// request does not grant each request a fresh bucket.
|
||||
func TestUnlock_RateLimitNotBypassedBySpoofedIP(t *testing.T) {
|
||||
e := echo.New()
|
||||
logger.Init(strconv.Itoa(int(logrus.DebugLevel)))
|
||||
mockSvc := mocks.NewMockService(t)
|
||||
gormDb, err := db.NewDB(t)
|
||||
require.NoError(t, err)
|
||||
defer db.CloseDB(gormDb)
|
||||
|
||||
mockEventPublisher := events.NewEventPublisher()
|
||||
|
||||
mockConfig := mocks.NewMockConfig(t)
|
||||
mockConfig.On("GetEnv").Return(&config.AppConfig{})
|
||||
mockConfig.On("CheckUnlockPassword", "wrong").Return(false)
|
||||
|
||||
mockSvc.On("GetDB").Return(gormDb)
|
||||
mockSvc.On("GetConfig").Return(mockConfig)
|
||||
mockSvc.On("GetKeys").Return(mocks.NewMockKeys(t))
|
||||
mockSvc.On("GetAlbySvc").Return(mocks.NewMockAlbyService(t))
|
||||
mockSvc.On("GetAlbyOAuthSvc").Return(mocks.NewMockAlbyOAuthService(t))
|
||||
|
||||
httpSvc := NewHttpService(mockSvc, mockEventPublisher)
|
||||
httpSvc.RegisterSharedRoutes(e)
|
||||
|
||||
jsonBody, _ := json.Marshal(api.UnlockRequest{UnlockPassword: "wrong", Permission: "full"})
|
||||
|
||||
send := func(forwardedFor string) int {
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/unlock", bytes.NewBuffer(jsonBody))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("X-Forwarded-For", forwardedFor)
|
||||
rec := httptest.NewRecorder()
|
||||
e.ServeHTTP(rec, req)
|
||||
return rec.Code
|
||||
}
|
||||
|
||||
rateLimited := 0
|
||||
for i := 0; i < 12; i++ {
|
||||
// each request presents a distinct client address
|
||||
if send("10.0.0."+strconv.Itoa(i)) == http.StatusTooManyRequests {
|
||||
rateLimited++
|
||||
}
|
||||
}
|
||||
|
||||
assert.Positive(t, rateLimited, "spoofing X-Forwarded-For must not grant a fresh rate-limit bucket")
|
||||
}
|
||||
|
||||
func TestGetApps_NoToken(t *testing.T) {
|
||||
e := echo.New()
|
||||
logger.Init(strconv.Itoa(int(logrus.DebugLevel)))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue