diff --git a/api/auth_middleware_test.go b/api/auth_middleware_test.go index 6ad08207..7800000d 100644 --- a/api/auth_middleware_test.go +++ b/api/auth_middleware_test.go @@ -318,7 +318,7 @@ func TestGetApiSignerWithApiAccessKey(t *testing.T) { // Same private key as TestGetApiSignerBasicAuth - derives to 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 testPrivateKey := "ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" - parentApiKey := "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" + parentApiKey := strings.ToLower("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266") apiAccessKey := "test-access-key-123" _, err := app.writePool.Exec(ctx, ` diff --git a/api/rate_limit_middleware.go b/api/rate_limit_middleware.go index 9128ec4d..9dc0fab7 100644 --- a/api/rate_limit_middleware.go +++ b/api/rate_limit_middleware.go @@ -33,12 +33,13 @@ type rpsState struct { data map[string][]int64 } -// normalizeAPIKeyForLookup prepends 0x when api_key is provided without it. +// normalizeAPIKeyForLookup canonicalizes api_key for exact primary-key lookup. func normalizeAPIKeyForLookup(apiKey string) string { if apiKey == "" { return "" } - if strings.HasPrefix(strings.ToLower(apiKey), "0x") { + apiKey = strings.ToLower(apiKey) + if strings.HasPrefix(apiKey, "0x") { return apiKey } return "0x" + apiKey @@ -239,7 +240,7 @@ func (rlm *RateLimitMiddleware) getLimits(ctx context.Context, identifier string err := rlm.writePool.QueryRow(ctx, ` SELECT COALESCE(rps, 10), COALESCE(rpm, 500000) FROM api_keys - WHERE LOWER(api_key) = LOWER($1) + WHERE api_key = $1 `, identifier).Scan(&rpsVal, &rpmVal) if err == pgx.ErrNoRows || err != nil { return 0, 0, true diff --git a/api/rate_limit_middleware_test.go b/api/rate_limit_middleware_test.go index ddac1a65..367f3ec9 100644 --- a/api/rate_limit_middleware_test.go +++ b/api/rate_limit_middleware_test.go @@ -65,14 +65,15 @@ func TestRateLimitMiddleware_NormalizesApiKeyWithout0xPrefix(t *testing.T) { return c.SendString("ok") }) - // Request omits 0x prefix but should still match stored 0x api_key row. - req1 := httptest.NewRequest("GET", "/test?api_key=6c1ef2e9c33e2ba1c0d352e41e06e8a3c7721c6f", nil) + // Request omits 0x prefix and uses mixed case, but should still match the + // stored canonical lowercase api_key row. + req1 := httptest.NewRequest("GET", "/test?api_key=6C1EF2E9C33E2BA1C0D352E41E06E8A3C7721C6F", nil) res1, err := testApp.Test(req1, -1) assert.NoError(t, err) assert.Equal(t, fiber.StatusOK, res1.StatusCode, "first request should succeed") // If normalization works, this hits app rps=1 and should be limited. - req2 := httptest.NewRequest("GET", "/test?api_key=6c1ef2e9c33e2ba1c0d352e41e06e8a3c7721c6f", nil) + req2 := httptest.NewRequest("GET", "/test?api_key=6C1EF2E9C33E2BA1C0D352E41E06E8A3C7721C6F", nil) res2, err := testApp.Test(req2, -1) assert.NoError(t, err) assert.Equal(t, fiber.StatusTooManyRequests, res2.StatusCode, "second request should be rate limited") diff --git a/api/request_helpers.go b/api/request_helpers.go index b20cadb0..b2a55eef 100644 --- a/api/request_helpers.go +++ b/api/request_helpers.go @@ -140,7 +140,7 @@ func (app *ApiServer) getSignerFromApiAccessKey(ctx context.Context, apiAccessKe err := app.pool.QueryRow(ctx, ` SELECT aak.api_key, ak.api_secret FROM api_access_keys aak - JOIN api_keys ak ON LOWER(ak.api_key) = LOWER(aak.api_key) + JOIN api_keys ak ON ak.api_key = aak.api_key WHERE aak.api_access_key = $1 AND aak.is_active = true `, apiAccessKey).Scan(&parentApiKey, &apiSecret) if err == nil && apiSecret != "" { @@ -181,10 +181,11 @@ func (app *ApiServer) getSignerFromOAuthToken(c *fiber.Ctx, token string) *Signe } // Look up api_secret for the client_id (developer app address = api_key) + clientID := strings.ToLower(entry.ClientID) var apiSecret string err := app.pool.QueryRow(c.Context(), ` - SELECT api_secret FROM api_keys WHERE LOWER(api_key) = LOWER($1) - `, entry.ClientID).Scan(&apiSecret) + SELECT api_secret FROM api_keys WHERE api_key = $1 + `, clientID).Scan(&apiSecret) if err != nil || apiSecret == "" { return nil } @@ -202,7 +203,7 @@ func (app *ApiServer) getSignerFromOAuthToken(c *fiber.Ctx, token string) *Signe } return &Signer{ - Address: strings.ToLower(entry.ClientID), + Address: clientID, PrivateKey: privateKey, } } diff --git a/api/v1_users_developer_apps.go b/api/v1_users_developer_apps.go index 38ef2dcb..0c8ba163 100644 --- a/api/v1_users_developer_apps.go +++ b/api/v1_users_developer_apps.go @@ -110,12 +110,12 @@ func (app *ApiServer) v1UsersDeveloperAppsWithMetrics(c *fiber.Ctx, userId int32 m.request_count_all_time, NOT EXISTS ( SELECT 1 FROM api_access_keys aak - WHERE LOWER(aak.api_key) = LOWER(da.address) AND aak.is_active = true + WHERE aak.api_key = LOWER(da.address) AND aak.is_active = true ) AS is_legacy, COALESCE( (SELECT json_agg(json_build_object('api_access_key', aak.api_access_key, 'is_active', aak.is_active)) FROM api_access_keys aak - WHERE LOWER(aak.api_key) = LOWER(da.address) AND aak.is_active = true), + WHERE aak.api_key = LOWER(da.address) AND aak.is_active = true), '[]'::json ) AS api_access_keys, oau.redirect_uris @@ -424,6 +424,7 @@ func (app *ApiServer) deleteV1UsersDeveloperApp(c *fiber.Ctx) error { if !strings.HasPrefix(address, "0x") { address = "0x" + address } + address = strings.ToLower(address) // Verify the app belongs to this user var ownerUserID int32 @@ -461,14 +462,14 @@ func (app *ApiServer) deleteV1UsersDeveloperApp(c *fiber.Ctx) error { // 2. Delete api_keys row // 3. Send ManageEntity transaction to delete the developer app on-chain if app.writePool != nil { - _, err = app.writePool.Exec(c.Context(), `DELETE FROM api_access_keys WHERE LOWER(api_key) = LOWER($1)`, address) + _, err = app.writePool.Exec(c.Context(), `DELETE FROM api_access_keys WHERE api_key = $1`, address) if err != nil { app.logger.Error("Failed to delete api_access_keys", zap.Error(err)) return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ "error": "Failed to delete developer app", }) } - _, err = app.writePool.Exec(c.Context(), `DELETE FROM api_keys WHERE LOWER(api_key) = LOWER($1)`, address) + _, err = app.writePool.Exec(c.Context(), `DELETE FROM api_keys WHERE api_key = $1`, address) if err != nil { app.logger.Error("Failed to delete api_keys", zap.Error(err)) return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ @@ -606,6 +607,7 @@ func (app *ApiServer) postV1UsersDeveloperAppAccessKeyDeactivate(c *fiber.Ctx) e if !strings.HasPrefix(address, "0x") { address = "0x" + address } + address = strings.ToLower(address) var body deactivateAccessKeyBody if err := c.BodyParser(&body); err != nil || strings.TrimSpace(body.ApiAccessKey) == "" { @@ -640,7 +642,7 @@ func (app *ApiServer) postV1UsersDeveloperAppAccessKeyDeactivate(c *fiber.Ctx) e result, err := app.writePool.Exec(c.Context(), ` UPDATE api_access_keys SET is_active = false - WHERE LOWER(api_key) = LOWER($1) AND api_access_key = $2 + WHERE api_key = $1 AND api_access_key = $2 `, address, apiAccessKey) if err != nil { app.logger.Error("Failed to deactivate api_access_key", zap.Error(err)) diff --git a/ddl/migrations/0218_canonicalize_api_keys.sql b/ddl/migrations/0218_canonicalize_api_keys.sql new file mode 100644 index 00000000..117a81cd --- /dev/null +++ b/ddl/migrations/0218_canonicalize_api_keys.sql @@ -0,0 +1,91 @@ +BEGIN; + +-- API keys are Ethereum-style app addresses. Keep them canonical lowercase so +-- lookups can use the existing api_keys primary key instead of LOWER(api_key). +-- +-- Production currently has case-variant duplicate groups, but those groups have +-- identical api_secret/rps/rpm values. Refuse to canonicalize if another +-- environment has conflicting duplicates that would need a product decision. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM api_keys + GROUP BY lower(api_key) + HAVING count(DISTINCT jsonb_build_array(api_secret, rps, rpm)) > 1 + ) THEN + RAISE EXCEPTION 'Cannot canonicalize api_keys: lower(api_key) duplicates have conflicting api_secret/rps/rpm values'; + END IF; +END $$; + +WITH ranked AS ( + SELECT + ctid, + row_number() OVER ( + PARTITION BY lower(api_key) + ORDER BY (api_key = lower(api_key)) DESC, created_at DESC, api_key ASC + ) AS row_number + FROM api_keys +) +DELETE FROM api_keys ak +USING ranked r +WHERE ak.ctid = r.ctid + AND r.row_number > 1; + +WITH ranked AS ( + SELECT + ctid, + row_number() OVER ( + PARTITION BY lower(api_key), api_access_key + ORDER BY is_active DESC, created_at DESC, api_key ASC + ) AS row_number + FROM api_access_keys +) +DELETE FROM api_access_keys aak +USING ranked r +WHERE aak.ctid = r.ctid + AND r.row_number > 1; + +UPDATE api_keys +SET api_key = lower(api_key) +WHERE api_key <> lower(api_key); + +UPDATE api_access_keys +SET api_key = lower(api_key) +WHERE api_key <> lower(api_key); + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conrelid = 'api_keys'::regclass + AND conname = 'api_keys_api_key_lowercase_check' + ) THEN + ALTER TABLE api_keys + ADD CONSTRAINT api_keys_api_key_lowercase_check + CHECK (api_key = lower(api_key)) NOT VALID; + END IF; +END $$; + +ALTER TABLE api_keys + VALIDATE CONSTRAINT api_keys_api_key_lowercase_check; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conrelid = 'api_access_keys'::regclass + AND conname = 'api_access_keys_api_key_lowercase_check' + ) THEN + ALTER TABLE api_access_keys + ADD CONSTRAINT api_access_keys_api_key_lowercase_check + CHECK (api_key = lower(api_key)) NOT VALID; + END IF; +END $$; + +ALTER TABLE api_access_keys + VALIDATE CONSTRAINT api_access_keys_api_key_lowercase_check; + +COMMIT;