Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed the use of Integers in gitlab_rails Settings inside gitlab.rb #109

Merged
merged 1 commit into from
Mar 15, 2017

Conversation

rwuest
Copy link

@rwuest rwuest commented Jan 16, 2017

Within the rack_attack_git_basic_auth settings in gitlab_rails the support of integer values is needed.
At the moment all Integers will be interpreted as Strings like this:
gitlab_rails['rack_attack_git_basic_auth'] = {"bantime"=>"3600", "enabled"=>true, "findtime"=>"60", "ip_whitelist"=>["127.0.0.1", "195.65.222.171"], "maxretry"=>"10"}

And after the Puppet run with a gitlab reconfiguration gitlab breaks with the following error, as soon as rack_attack is used:
TypeError (String can't be coerced into Fixnum):
lib/gitlab/auth/ip_rate_limiter.rb:16:in reset!' lib/gitlab/auth.rb:51:in rate_limit!'
lib/gitlab/auth.rb:18:in find_for_git_client' app/controllers/projects/git_http_client_controller.rb:117:in handle_basic_authentication'
app/controllers/projects/git_http_client_controller.rb:31:in authenticate_user' lib/gitlab/request_profiler/middleware.rb:15:in call'
lib/gitlab/middleware/go.rb:16:in call' lib/gitlab/middleware/readonly_geo.rb:29:in call'

With the Fix in gitlab.rb.erb the rack_attack_git_basic_auth settings will be interpreted correctly.

@tobru
Copy link
Contributor

tobru commented Mar 15, 2017

Thanks a lot for this contribution which I'll merge now. One small hint: Please don't already set a version number in CHANGELOG and metadata.json, this will be done by the maintainer. However, adding a changelog entry is very much appreciated!

@tobru tobru merged commit 1102b0c into voxpupuli:master Mar 15, 2017
@rwuest
Copy link
Author

rwuest commented Mar 17, 2017

perfect, thanks for the hint regarding to the versioning, i won't do it in future ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants