Skip to content

Commit

Permalink
Bug 1911202 - Remove support for iprepd from the BMO code base as it …
Browse files Browse the repository at this point in the history
…is not being used since the move to GCP
  • Loading branch information
dklawren authored Aug 30, 2024
1 parent b483e5d commit 3d826bc
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 193 deletions.
31 changes: 1 addition & 30 deletions Bugzilla.pm
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ sub datadog {

sub check_rate_limit {
my ($class, $name, $identifier, $throw_error) = @_;
$identifier ||= remote_ip();
$throw_error //= sub { ThrowUserError("rate_limit") };
my $params = Bugzilla->params;
if ($params->{rate_limit_active}) {
Expand Down Expand Up @@ -796,36 +797,6 @@ sub check_rate_limit {
return 1;
}

sub iprepd_report {
my ($class, $name) = @_;
my $params = Bugzilla->params;

return 0 if !$params->{iprepd_base_url} || !$params->{iprepd_client_secret};

# Send information about this event to the iprepd API if active
my $ip = remote_ip();
my $ua = mojo_user_agent({request_timeout => 5});
my $payload = {object => $ip, type => "ip", violation => $name};

# We should also audit this so it is recorded in the logs
Bugzilla->audit(sprintf 'iprepd: violation %s from ip address %s', $name, $ip);

try {
my $tx = $ua->put(
$params->{iprepd_base_url} . '/violations/type/ip/' . $ip,
{'Authorization' => 'APIKey ' . $params->{iprepd_client_secret}} => json =>
$payload
);
my $res = $tx->result;
die $res->message . ' ' . $res->body unless $res->is_success;
}
catch {
WARN("IPREPD ERROR: $_");
};

return 1;
}

sub markdown {
require Bugzilla::Markdown;
state $markdown = Bugzilla::Markdown->new;
Expand Down
6 changes: 3 additions & 3 deletions Bugzilla/App/Plugin/Login.pm
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ sub register {
|| $api_key->revoked
)
{
Bugzilla->iprepd_report('bmo.api_key_mismatch');
Bugzilla->check_rate_limit('api_key_mismatch');
}
else {
$api_key->update_last_used($remote_ip);
$user_id = $api_key->user_id;
}
}
else {
Bugzilla->iprepd_report('bmo.api_key_mismatch');
Bugzilla->check_rate_limit('api_key_mismatch');
}
}

Expand All @@ -109,7 +109,7 @@ sub register {
$user_id = $user->id;
}
else {
Bugzilla->iprepd_report('bmo.api_key_mismatch');
Bugzilla->check_rate_limit('api_key_mismatch');
}
}
}
Expand Down
1 change: 0 additions & 1 deletion Bugzilla/Auth.pm
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ sub _handle_login_result {
# to find account names by brute force)
elsif ($fail_code == AUTH_LOGINFAILED or $fail_code == AUTH_NO_SUCH_USER) {
my $remaining_attempts = MAX_LOGIN_ATTEMPTS - ($result->{failure_count} || 0);
Bugzilla->iprepd_report('bmo.username_password_mismatch', remote_ip());
ThrowUserError("invalid_username_or_password",
{remaining => $remaining_attempts});
}
Expand Down
4 changes: 2 additions & 2 deletions Bugzilla/Auth/Login/APIKey.pm
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ sub get_login_info {
if (!$api_key or $api_key->api_key ne $api_key_text) {

# The second part checks the correct capitalization. Silly MySQL
Bugzilla->iprepd_report('bmo.api_key_mismatch', $remote_ip);
Bugzilla->check_rate_limit('api_key_mismatch');
ThrowUserError("api_key_not_valid");
}
elsif ($api_key->sticky
&& $api_key->last_used_ip
&& $api_key->last_used_ip ne $remote_ip)
{
Bugzilla->iprepd_report('bmo.api_key_mismatch');
Bugzilla->check_rate_limit('api_key_mismatch');
ThrowUserError("api_key_not_valid");
}
elsif ($api_key->revoked) {
Expand Down
2 changes: 1 addition & 1 deletion Bugzilla/Auth/Login/CGI.pm
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ sub get_login_info {
# If the web browser doesn't accept cookies, we have no way to
# make sure that the authentication request comes from the user.
elsif ($login && $password) {
Bugzilla->iprepd_report('bmo.token_mismatch');
Bugzilla->check_rate_limit('token_mismatch');
ThrowUserError('auth_untrusted_request', {login => $login});
}

Expand Down
4 changes: 2 additions & 2 deletions Bugzilla/Auth/Login/Cookie.pm
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ sub get_login_info {
|| $token_type ne 'api_token'
|| $user_id != $token_user_id)
{
Bugzilla->iprepd_report('bmo.token_mismatch');
Bugzilla->check_rate_limit('token_mismatch');
ThrowUserError('auth_invalid_token', {token => $api_token});
}
$is_internal = 1;
Expand Down Expand Up @@ -104,7 +104,7 @@ sub get_login_info {
if (i_am_webservice() && !$is_internal) {
my $user = Bugzilla::User->new({id => $user_id, cache => 1});
if ($user->settings->{api_key_only}->{value} eq 'on') {
Bugzilla->iprepd_report('bmo.token_mismatch');
Bugzilla->check_rate_limit('token_mismatch');
ThrowUserError('invalid_cookies_or_token');
}
}
Expand Down
32 changes: 14 additions & 18 deletions Bugzilla/Config/Admin.pm
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ sub get_param_list {
updater => \&update_rate_limit_rules,
},

{
name => 'iprepd_base_url',
type => 't',
default => '',
},

{
name => 'iprepd_client_secret',
type => 't',
default => '',
},

{name => 'log_user_requests', type => 'b', default => 0},

{name => 'block_user_agent', type => 't', default => ''},
Expand All @@ -71,12 +59,20 @@ sub get_param_list {

sub default_rate_limit_rules {
return encode_json({
get_bug => [75, 60],
show_bug => [75, 60],
github => [10, 60],
get_attachments => [75, 60],
get_comments => [75, 60],
webpage_errors => [75, 60],
get_bug => [75, 60],
show_bug => [75, 60],
github => [10, 60],
get_attachments => [75, 60],
get_comments => [75, 60],
webpage_errors => [75, 60],
token_mismatch => [5, 60],
github => [5, 60],
create_account => [5, 60],
email_change => [5, 60],
password_reset => [5, 60],
cancel_token => [5, 60],
api_key_mismatch => [5, 60],
mfa_mismatch => [5, 60],
});
}

Expand Down
4 changes: 2 additions & 2 deletions Bugzilla/MFA/TOTP.pm
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ sub check {
my $code = $params->{code};
return if $self->_auth()->verify($code, 1);

Bugzilla->iprepd_report('bmo.mfa_mismatch', remote_ip());
Bugzilla->check_rate_limit('mfa_mismatch', $self->{user}->id);

if ($params->{mfa_action} && $params->{mfa_action} eq 'enable') {
ThrowUserError('mfa_totp_bad_enrollment_code');
}
else {
Bugzilla->check_rate_limit('mfa_mismatch', $self->{user}->id);
ThrowUserError('mfa_bad_code');
}
}
Expand Down
10 changes: 5 additions & 5 deletions Bugzilla/Token.pm
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ sub issue_new_user_account_token {
my $vars = {};

# Report this event for possible rate limiting
Bugzilla->iprepd_report('bmo.create_account');
Bugzilla->check_rate_limit('create_account');

# Is there already a pending request for this login name? If yes, do not throw
# an error because the user may have lost their email with the token inside.
Expand Down Expand Up @@ -110,7 +110,7 @@ sub IssueEmailChangeToken {
my $old_email = $user->login;

# Report this event for possible rate limiting
Bugzilla->iprepd_report('bmo.email_change');
Bugzilla->check_rate_limit('email_change');

my ($token, $token_ts)
= _create_token($user->id, 'emailold', $old_email . ":" . $new_email);
Expand Down Expand Up @@ -155,7 +155,7 @@ sub IssuePasswordToken {
my $dbh = Bugzilla->dbh;

# Report this event for possible rate limiting
Bugzilla->iprepd_report('bmo.password_reset');
Bugzilla->check_rate_limit('password_reset');

my $too_soon = $dbh->selectrow_array(
'SELECT 1 FROM tokens
Expand Down Expand Up @@ -348,7 +348,7 @@ sub Cancel {
$vars ||= {};

# Report this event for possible rate limiting
Bugzilla->iprepd_report('bmo.cancel_token');
Bugzilla->check_rate_limit('cancel_token');

# Get information about the token being canceled.
my ($db_token, $issuedate, $tokentype, $eventdata, $userid)
Expand All @@ -363,7 +363,7 @@ sub Cancel {
# Some DBs such as MySQL are case-insensitive by default so we do
# a quick comparison to make sure the tokens are indeed the same.
unless (defined $db_token && $db_token eq $token) {
Bugzilla->iprepd_report('bmo.token_mismatch', remote_ip());
Bugzilla->check_rate_limit('token_mismatch');
ThrowCodeError("cancel_token_does_not_exist");
}

Expand Down
15 changes: 0 additions & 15 deletions external_test_api.pl
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,6 @@ sub startup {
my ($self) = @_;
my $r = $self->routes;

# Mock the IPrepD API violations endpoint
$r->put(
'/violations/type/ip/*ip' => sub {
my $c = shift;
$c->app->defaults->{last_violation} = $c->req->json;
$c->render(text => 'OK ', status => 200);
}
);
$r->get(
'/violations/last' => sub {
my $c = shift;
$c->render(json => $c->app->defaults->{last_violation}, status => 200);
}
);

# Webhook endpoints for testing
$r->post(
'/webhooks/test/noauth' => sub {
Expand Down
108 changes: 2 additions & 106 deletions qa/t/3_test_rate_limit.t
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ set_parameters(
'rate_limit_rules' => {
type => 'text',
value =>
'{"get_attachments":[5,100],"get_comments":[5,100],"get_bug":[5,100],"show_bug":[5,100],"github":[5,100],"webpage_errors":[5,100], "token":[5,100], "api_key":[5,100], "username_password":[3,100]}'
},
'iprepd_base_url' => {type => 'text', value => 'http://externalapi.test:8001'},
'iprepd_client_secret' => {type => 'text', value => 'iprepd_client_secret'},
'{"get_attachments":[5,100],"get_comments":[5,100],"get_bug":[5,100],"show_bug":[5,100],"github":[5,100],"webpage_errors":[5,100]}'
}
}
}
);
Expand Down Expand Up @@ -92,108 +90,6 @@ $t->get_ok($config->{browser_url} . "/rest/bug/$bug_id")->status_is(400)

clear_memcache(); # So we can connect again

# IPREPD REPORTING TESTS

# Incorrect username and/or password sends report to iprepd

# Enter incorrect username and password 4 times where the
# 4th should be rate limited
$sel->open_ok('/login', undef, 'Go to the home page');
$sel->title_is('Log in to Bugzilla');
$sel->type_ok(
'Bugzilla_login',
$config->{admin_user_login},
'Enter admin login name'
);
$sel->type_ok('Bugzilla_password', 'bad_password', 'Enter bad admin password');
$sel->click_ok('log_in', undef, 'Submit credentials');
$sel->title_is('Invalid Username Or Password',
'Username or password error page displayed');

# Check that iprepd was properly notified
$t->get_ok('http://externalapi.test:8001/violations/last')->status_is(200)
->json_is('/violation' => 'bmo.username_password_mismatch')
->json_has('/object')->json_is('/type', 'ip');

# Invalid api keys via REST API should send report to iprepd

# Use a bad api key gives the normal API key error.
$t->post_ok($config->{browser_url}
. '/rest/bug' => {'X-Bugzilla-API-Key' => 'bad_key'} => json => {})
->status_is(400)
->json_like('/message', qr/The API key you specified is invalid/);

# Check that iprepd was properly notified
$t->get_ok('http://externalapi.test:8001/violations/last')->status_is(200)
->json_is('/violation' => 'bmo.api_key_mismatch')->json_has('/object')
->json_is('/type', 'ip');

# Token errors should send report to iprepd

# You should be able to use an invalid token 5 times but then fail the next
$t->get_ok(
$config->{browser_url} . '/token.cgi?t=bad_token&a=request_new_account')
->status_is(200)->content_like(qr/Token Does Not Exist/);

# Check that iprepd was properly notified
$t->get_ok('http://externalapi.test:8001/violations/last')->status_is(200)
->json_is('/violation' => 'bmo.token_mismatch')->json_has('/object')
->json_is('/type', 'ip');

# Check that iprepd is notified for new account emails
$sel->open_ok('/createaccount.cgi', undef, 'Go to the new account page');
$sel->type_ok('login', '[email protected]', 'Enter new account email');
$sel->check_ok('etiquette', 'Accept etiquette');
$sel->click_ok('//input[@value="Create Account"]', 'Submit');
$sel->title_like(qr/Request for new user account/,
'Request for new account sent');

# Check that iprepd was properly notified
$t->get_ok('http://externalapi.test:8001/violations/last')->status_is(200)
->json_is('/violation' => 'bmo.create_account')->json_has('/object')
->json_is('/type', 'ip');

# Check that iprepd is notified for password reset emails
$sel->open_ok('/home', 'Go to home page');
$sel->click_ok('forgot_link_top', 'Show password reset field');
$sel->type_ok(
'//input[@name="loginname"]',
$config->{admin_user_login},
'Enter email for password reset'
);
$sel->click_ok('forgot_button_top', 'Submit');
$sel->title_is('Request to Change Password', 'Request for password reset sent');

# Check that iprepd was properly notified
$t->get_ok('http://externalapi.test:8001/violations/last')->status_is(200)
->json_is('/violation' => 'bmo.password_reset')->json_has('/object')
->json_is('/type', 'ip');

# Check that iprepd is notified for password reset emails
log_in($sel, $config, 'unprivileged');
$sel->click_ok('header-account-menu-button');
$sel->click_ok("link=Preferences");
$sel->title_is("User Preferences");
$sel->click_ok("link=Account");
$sel->title_is("User Preferences");
$sel->type_ok(
'//input[@name="new_login_name"]',
'new-' . $config->{unprivileged_user_login},
'Enter new email'
);
$sel->type_ok(
'//input[@name="old_password"]',
$config->{unprivileged_user_passwd},
'Enter current password'
);
$sel->click_ok('update');
logout($sel);

# Check that iprepd was properly notified
$t->get_ok('http://externalapi.test:8001/violations/last')->status_is(200)
->json_is('/violation' => 'bmo.email_change')->json_has('/object')
->json_is('/type', 'ip');

# Turn rate limiting off
log_in($sel, $config, 'admin');
set_parameters($sel,
Expand Down
1 change: 0 additions & 1 deletion t/migrate-params.t
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,5 @@ __DATA__
'bitly_token' => 'BITLY_TOKEN',
'github_client_id' => 'GITHUB_CLIENT_ID',
'honeypot_api_key' => 'HONEYPOT_API_KEY',
'iprepd_client_secret' => 'IPREPD_CLIENT_SECRET',
'phabricator_api_key' => 'PHABRICATOR_API_KEY',
);
Loading

0 comments on commit 3d826bc

Please sign in to comment.