Remove short_term_phone_otp_rate_limiter_enabled configuration key

[mitchellhenke]

🛠 Summary of changes

Related to #10360. This has been tested and can be enabled everywhere.

[aduth]

Maybe we should have something which resets all rate limits between tests? Unclear if that would actually help for the reason this was added.

That, and/or use a fake phone generator in the phone factory setup.

[mitchellhenke]

I think some of them were sending too many SMS within the same test. ex:

identity-idp/spec/features/openid_connect/openid_connect_spec.rb

Lines 33 to 38 in 333c22c

	it 'succeeds in forcing login with prompt login and prior session' do
	user = oidc_end_client_secret_jwt(prompt: 'login')
	travel_to((IdentityConfig.store.sp_handoff_bounce_max_seconds + 1).seconds.from_now) do
	oidc_end_client_secret_jwt(prompt: 'login', user: user)
	end
	end

[aduth]

Hm, fair enough. Seems like we may want to revisit those at some point to make them more realistic to what we'd expect in the real-world.

I started poking at my previous idea of resetting the rate limiter, though I don't love the idea here of multiple calls to Redis between each test:

# spec/support/rate_limiter.rb

RSpec.configure do |config|
  config.before(:each) do
    REDIS_THROTTLE_POOL.with do |client|
      keys = client.call [:keys, 'throttle:*']
      client.call [:del, *keys]
    end
  end
end

[zachmargolis]

do we need the .call? I think we can:

RSpec.configure do |config|
  config.before(:each) do
    REDIS_THROTTLE_POOL.with do |client|
      keys = client.keys('throttle:*')
      client.del(*keys)
    end
  end
end

[aduth]

Stumbling my way through the solution led me to this issue, which seemed to imply that call is a bit safer when deleting many keys. Though I doubt we have so many matches that it'd become an issue.

redis/redis-rb#122

[mitchellhenke]

Haven't confirmed that it does or doesn't work, but I think we intend to reset the rate limit pool between tests:

identity-idp/spec/rails_helper.rb

Line 112 in 333c22c

REDIS_THROTTLE_POOL.with { |client| client.flushdb }

[aduth]

Ah, that would (likely) work too! As you mentioned, it's probably more to do with limits within a single test being exceeded.