Skip to content

Commit

Permalink
Personal access tokens need to compare the nickname
Browse files Browse the repository at this point in the history
Depending on how the user was created the username column can be very
different from the nickname returned by the ->name method. For example
"https://www.opensuse.org/openid/user/kraih" instead of the expected
"kraih".

https://progress.opensuse.org/issues/87698
  • Loading branch information
kraih committed Mar 9, 2021
1 parent 0b1531c commit 84ca0be
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 13 deletions.
5 changes: 3 additions & 2 deletions lib/OpenQA/Shared/Controller/Auth.pm
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,12 @@ sub _token_auth ($self, $reason, $userinfo) {
my $reject_msg = qq{Rejecting personal access token for user "$username" with ip "$ip"};
if (my $api_key = $self->schema->resultset('ApiKeys')->find({key => $key})) {
my $user = $api_key->user;
if ($user && secure_compare($user->username, $username)) {
my $name = $user->name;
if ($user && secure_compare($name, $username)) {
return ($user, undef) if secure_compare($api_key->secret, $secret);
$log->debug("$reject_msg, wrong secret");
}
else { $log->debug("$reject_msg, wrong username") }
else { $log->debug(qq{$reject_msg, wrong username, expected "$name"}) }
}
else { $log->debug("$reject_msg, wrong key") }
}
Expand Down
22 changes: 11 additions & 11 deletions t/api/03-auth.t
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ subtest 'personal access token' => sub {
$t->delete_ok('/api/v1/assets/1')->status_is(403)->json_is({error => 'no api key'});

# Valid access token
$t->$userinfo('arthur:ARTHURKEY01:EXCALIBUR')->delete_ok('/api/v1/assets/1')->status_is(404);
$t->$userinfo('artie:ARTHURKEY01:EXCALIBUR')->delete_ok('/api/v1/assets/1')->status_is(404);

# Invalid access token
$t->$userinfo('invalid:invalid')->delete_ok('/api/v1/assets/1')->status_is(403)
Expand All @@ -191,15 +191,15 @@ subtest 'personal access token' => sub {
->json_is({error => 'invalid personal access token'});

# Invalid key
$t->$userinfo('arthur:INVALID:EXCALIBUR')->delete_ok('/api/v1/assets/1')->status_is(403)
$t->$userinfo('artie:INVALID:EXCALIBUR')->delete_ok('/api/v1/assets/1')->status_is(403)
->json_is({error => 'invalid personal access token'});

# Invalid secret
$t->$userinfo('arthur:ARTHURKEY01:INVALID')->delete_ok('/api/v1/assets/1')->status_is(403)
$t->$userinfo('artie:ARTHURKEY01:INVALID')->delete_ok('/api/v1/assets/1')->status_is(403)
->json_is({error => 'invalid personal access token'});

# Valid access token (again)
$t->$userinfo('arthur:ARTHURKEY01:EXCALIBUR')->delete_ok('/api/v1/assets/1')->status_is(404);
$t->$userinfo('artie:ARTHURKEY01:EXCALIBUR')->delete_ok('/api/v1/assets/1')->status_is(404);
};

subtest 'personal access token (with reverse proxy)' => sub {
Expand All @@ -216,24 +216,24 @@ subtest 'personal access token (with reverse proxy)' => sub {
# Not HTTPS or localhost
local $ENV{MOJO_REVERSE_PROXY} = 1;
my $t = Test::Mojo->new('OpenQA::WebAPI');
$t->$forwarded('arthur:ARTHURKEY01:EXCALIBUR', '192.168.2.1', 'http')->delete_ok('/api/v1/assets/1')
->status_is(403)->json_is({error => 'personal access token can only be used via HTTPS or from localhost'});
$t->$forwarded('artie:ARTHURKEY01:EXCALIBUR', '192.168.2.1', 'http')->delete_ok('/api/v1/assets/1')->status_is(403)
->json_is({error => 'personal access token can only be used via HTTPS or from localhost'});

# HTTPS
$t->$forwarded('arthur:ARTHURKEY01:EXCALIBUR', '192.168.2.1', 'https')->delete_ok('/api/v1/assets/1')
$t->$forwarded('artie:ARTHURKEY01:EXCALIBUR', '192.168.2.1', 'https')->delete_ok('/api/v1/assets/1')
->status_is(404);

# localhost
$t->$forwarded('arthur:ARTHURKEY01:EXCALIBUR', '127.0.0.1', 'http')->delete_ok('/api/v1/assets/1')->status_is(404);
$t->$forwarded('artie:ARTHURKEY01:EXCALIBUR', '127.0.0.1', 'http')->delete_ok('/api/v1/assets/1')->status_is(404);

# localhost (IPv6)
$t->$forwarded('arthur:ARTHURKEY01:EXCALIBUR', '::1', 'http')->delete_ok('/api/v1/assets/1')->status_is(404);
$t->$forwarded('artie:ARTHURKEY01:EXCALIBUR', '::1', 'http')->delete_ok('/api/v1/assets/1')->status_is(404);

# HTTPS and localhost
$t->$forwarded('arthur:ARTHURKEY01:EXCALIBUR', '127.0.0.1', 'https')->delete_ok('/api/v1/assets/1')->status_is(404);
$t->$forwarded('artie:ARTHURKEY01:EXCALIBUR', '127.0.0.1', 'https')->delete_ok('/api/v1/assets/1')->status_is(404);

# HTTPS but invalid key
$t->$forwarded('arthur:INVALID:EXCALIBUR', '192.168.2.1', 'https')->delete_ok('/api/v1/assets/1')->status_is(403)
$t->$forwarded('artie:INVALID:EXCALIBUR', '192.168.2.1', 'https')->delete_ok('/api/v1/assets/1')->status_is(403)
->json_is({error => 'invalid personal access token'});
};

Expand Down

0 comments on commit 84ca0be

Please sign in to comment.