From 02b99c951b242b382be8a2f89d3895b19d74c53f Mon Sep 17 00:00:00 2001 From: dklawren Date: Thu, 24 Jan 2019 17:48:37 -0500 Subject: [PATCH] Bug 1511490 - BMO's oauth tokens should be use jwt --- Bugzilla/App/OAuth2/Clients.pm | 34 +- Bugzilla/App/Plugin/OAuth2.pm | 376 ++++++++---------- Bugzilla/DB/Schema.pm | 198 ++------- Bugzilla/Install/DB.pm | 60 +++ Bugzilla/Test/Util.pm | 13 +- t/mojo-oauth2.t | 63 ++- .../account/auth/confirm_scopes.html.tmpl | 2 +- .../admin/oauth/confirm-delete.html.tmpl | 2 +- .../en/default/admin/oauth/edit.html.tmpl | 2 +- 9 files changed, 334 insertions(+), 416 deletions(-) diff --git a/Bugzilla/App/OAuth2/Clients.pm b/Bugzilla/App/OAuth2/Clients.pm index 083e73d22..1dc53cc20 100644 --- a/Bugzilla/App/OAuth2/Clients.pm +++ b/Bugzilla/App/OAuth2/Clients.pm @@ -76,9 +76,13 @@ sub create { check_token_data($token, 'create_oauth_client'); - $dbh->do('INSERT INTO oauth2_client (id, description, secret) VALUES (?, ?, ?)', + $dbh->do('INSERT INTO oauth2_client (client_id, description, secret) VALUES (?, ?, ?)', undef, $id, $description, $secret); + my $client_data + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE client_id = ?', + undef, $id); + foreach my $scope_id (@scopes) { $scope_id = $dbh->selectrow_array('SELECT id FROM oauth2_scope WHERE id = ?', undef, $scope_id); @@ -86,8 +90,8 @@ sub create { ThrowCodeError('param_required', {param => 'scopes'}); } $dbh->do( - 'INSERT INTO oauth2_client_scope (client_id, scope_id, allowed) VALUES (?, ?, 1)', - undef, $id, $scope_id + 'INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', + undef, $client_data->{id}, $scope_id ); } @@ -111,12 +115,12 @@ sub delete { my $dbh = Bugzilla->dbh; my $vars = {}; - my $id = $self->param('id'); - my $client = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', + my $id = $self->param('id'); + my $client_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', undef, $id); if (!$self->param('deleteme')) { - $vars->{'client'} = $client; + $vars->{'client'} = $client_data; $vars->{'token'} = issue_session_token('delete_oauth_client'); $self->stash(%{$vars}); return $self->render( @@ -140,7 +144,7 @@ sub delete { $dbh->bz_commit_transaction; $vars->{'message'} = 'oauth_client_deleted'; - $vars->{'client'} = {description => $client->{description}}; + $vars->{'client'} = {description => $client_data->{description}}; $vars->{'clients'} = $clients; $self->stash(%{$vars}); return $self->render(template => 'admin/oauth/list', handler => 'bugzilla'); @@ -153,14 +157,14 @@ sub edit { my $vars = {}; my $id = $self->param('id'); - my $client = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', + my $client_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', undef, $id); my $client_scopes = $dbh->selectall_arrayref( 'SELECT scope_id FROM oauth2_client_scope WHERE client_id = ?', - undef, $id); - $client->{scopes} = [map { $_->[0] } @{$client_scopes}]; - $vars->{client} = $client; + undef, $client_data->{id}); + $client_data->{scopes} = [map { $_->[0] } @{$client_scopes}]; + $vars->{client} = $client_data; # All scopes my $all_scopes @@ -182,12 +186,12 @@ sub edit { my $active = $self->param('active'); my @scopes = $self->param('scopes'); - if ($description ne $client->{description}) { + if ($description ne $client_data->{description}) { $dbh->do('UPDATE oauth2_client SET description = ? WHERE id = ?', undef, $description, $id); } - if ($active ne $client->{active}) { + if ($active ne $client_data->{active}) { $dbh->do('UPDATE oauth2_client SET active = ? WHERE id = ?', undef, $active, $id); } @@ -195,8 +199,8 @@ sub edit { $dbh->do('DELETE FROM oauth2_client_scope WHERE client_id = ?', undef, $id); foreach my $scope_id (@scopes) { $dbh->do( - 'INSERT INTO oauth2_client_scope (client_id, scope_id, allowed) VALUES (?, ?, 1)', - undef, $id, $scope_id + 'INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', + undef, $client_data->{id}, $scope_id ); } diff --git a/Bugzilla/App/Plugin/OAuth2.pm b/Bugzilla/App/Plugin/OAuth2.pm index a3365dbac..3f2951de7 100644 --- a/Bugzilla/App/Plugin/OAuth2.pm +++ b/Bugzilla/App/Plugin/OAuth2.pm @@ -15,7 +15,14 @@ use Bugzilla::Logging; use Bugzilla::Util; use Bugzilla::Token; use DateTime; +use List::MoreUtils qw(any); +use Mojo::URL; use Mojo::Util qw(secure_compare); +use Try::Tiny; + +use constant TOKEN_TYPE_AUTH => 0; +use constant TOKEN_TYPE_ACCESS => 1; +use constant TOKEN_TYPE_REFRESH => 2; sub register { my ($self, $app, $conf) = @_; @@ -27,6 +34,13 @@ sub register { $conf->{verify_auth_code} = \&_verify_auth_code; $conf->{store_access_token} = \&_store_access_token; $conf->{verify_access_token} = \&_verify_access_token; + $conf->{jwt_secret} = Bugzilla->localconfig->{jwt_secret}; + $conf->{jwt_claims} = sub { + my $args = shift; + if (!$args->{user_id}) { + return (user_id => Bugzilla->user->id); + } + }; $app->helper( 'bugzilla.oauth' => sub { @@ -36,6 +50,7 @@ sub register { if ($oauth && $oauth->{user_id}) { my $user = Bugzilla::User->check({id => $oauth->{user_id}, cache => 1}); + return undef if !$user->is_enabled; Bugzilla->set_user($user); return $user; } @@ -54,7 +69,7 @@ sub _resource_owner_logged_in { $c->session->{override_login_target} = $c->url_for('current'); $c->session->{cgi_params} = $c->req->params->to_hash; - $c->bugzilla->login(LOGIN_REQUIRED) || return; + $c->bugzilla->login(LOGIN_REQUIRED) || return undef; delete $c->session->{override_login_target}; delete $c->session->{cgi_params}; @@ -73,14 +88,15 @@ sub _resource_owner_confirm_scopes { # access last time, we check [again] with the user for access if (!defined $is_allowed) { my $client - = Bugzilla->dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', + = Bugzilla->dbh->selectrow_hashref( + 'SELECT * FROM oauth2_client WHERE client_id = ?', undef, $client_id); my $vars = { client => $client, scopes => $scopes_ref, token => scalar issue_session_token('oauth_confirm_scopes') }; - $c->stash(%$vars); + $c->stash(%{$vars}); $c->render(template => 'account/auth/confirm_scopes', handler => 'bugzilla'); return undef; } @@ -94,8 +110,8 @@ sub _resource_owner_confirm_scopes { sub _verify_client { my (%args) = @_; - my ($c, $client_id, $scopes_ref) - = @args{qw/ mojo_controller client_id scopes /}; + my ($c, $client_id, $scopes_ref, $redirect_uri) + = @args{qw/ mojo_controller client_id scopes redirect_uri /}; my $dbh = Bugzilla->dbh; if (!@{$scopes_ref}) { @@ -103,9 +119,14 @@ sub _verify_client { return (0, 'invalid_scope'); } + if (!$ENV{MOJO_TEST} && Mojo::URL->new($redirect_uri)->scheme ne 'https') { + INFO("invalid_redirect_uri: $redirect_uri"); + return (0, 'invalid_redirect_uri'); + } + if ( my $client_data = $dbh->selectrow_hashref( - 'SELECT * FROM oauth2_client WHERE id = ?', + 'SELECT * FROM oauth2_client WHERE client_id = ?', undef, $client_id ) ) @@ -115,22 +136,15 @@ sub _verify_client { return (0, 'unauthorized_client'); } - foreach my $rqd_scope (@{$scopes_ref}) { - my $scope_allowed = $dbh->selectrow_array( - 'SELECT allowed FROM oauth2_client_scope - JOIN oauth2_scope ON oauth2_scope.id = oauth2_client_scope.scope_id - WHERE client_id = ? AND oauth2_scope.description = ?', undef, $client_id, - $rqd_scope + if ($scopes_ref) { + my $client_scopes = $dbh->selectcol_arrayref( + 'SELECT oauth2_scope.description FROM oauth2_scope + JOIN oauth2_client_scope ON oauth2_scope.id = oauth2_client_scope.scope_id + WHERE oauth2_client_scope.client_id = ?', undef, $client_data->{id} ); - if (defined $scope_allowed) { - if (!$scope_allowed) { - INFO("Client disallowed scope ($rqd_scope)"); - return (0, 'access_denied'); - } - } - else { - INFO("Client lacks scope ($rqd_scope)"); - return (0, 'invalid_scope'); + + foreach my $scope (@{$scopes_ref // []}) { + return (0, 'invalid_grant') if !_has_scope($scope, $client_scopes); } } @@ -141,41 +155,6 @@ sub _verify_client { return (0, 'unauthorized_client'); } -sub _store_auth_code { - my (%args) = @_; - my ($c, $auth_code, $client_id, $expires_in, $uri, $scopes_ref) - = @args{ - qw/ mojo_controller auth_code client_id expires_in redirect_uri scopes /}; - my $dbh = Bugzilla->dbh; - - my $user_id = Bugzilla->user->id; - - $dbh->do( - 'INSERT INTO oauth2_auth_code VALUES (?, ?, ?, ?, ?, 0)', - undef, - $auth_code, - $client_id, - Bugzilla->user->id, - DateTime->from_epoch(epoch => time + $expires_in), - $uri - ); - - foreach my $rqd_scope (@{$scopes_ref}) { - my $scope_id - = $dbh->selectrow_array('SELECT id FROM oauth2_scope WHERE description = ?', - undef, $rqd_scope); - if ($scope_id) { - $dbh->do('INSERT INTO oauth2_auth_code_scope VALUES (?, ?, 1)', - undef, $auth_code, $scope_id); - } - else { - ERROR("Unknown scope ($rqd_scope) in _store_auth_code"); - } - } - - return; -} - sub _verify_auth_code { my (%args) = @_; my ($c, $client_id, $client_secret, $auth_code, $uri) @@ -183,214 +162,170 @@ sub _verify_auth_code { my $dbh = Bugzilla->dbh; my $client_data - = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE client_id = ?', undef, $client_id); $client_data || return (0, 'unauthorized_client'); - my $auth_code_data = $dbh->selectrow_hashref( - 'SELECT expires, verified, redirect_uri, user_id FROM oauth2_auth_code WHERE client_id = ? AND auth_code = ?', - undef, $client_id, $auth_code - ); + my ($res, $jwt_claims) = _get_jwt_claims($auth_code, 'auth'); + return (0, 'invalid_jwt') unless $res; - if (!$auth_code_data - or $auth_code_data->{verified} - or ($uri ne $auth_code_data->{redirect_uri}) - or (datetime_from($auth_code_data->{expires})->epoch <= time) + my $jwt_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_jwt WHERE jti = ?', + undef, $jwt_claims->{jti}); + + if (!$jwt_data + or ($jwt_data->{type} ne TOKEN_TYPE_AUTH) + or ($jwt_claims->{user_id} != $jwt_data->{user_id}) + or ($uri ne $jwt_claims->{aud}) + or ($jwt_claims->{exp} <= time) or !secure_compare($client_secret, $client_data->{secret})) { - INFO('Auth code does not exist') if !$auth_code; INFO('Client secret does not match') if !secure_compare($client_secret, $client_data->{secret}); - if ($auth_code) { - INFO('Client secret does not match') - if ($uri && $auth_code_data->{redirect_uri} ne $uri); - INFO('Auth code expired') if ($auth_code_data->{expires} <= time); - - if ($auth_code_data->{verified}) { - - # the auth code has been used before - we must revoke the auth code - # and any associated access tokens (same client_id and user_id) - INFO( 'Auth code already used to get access token, ' - . 'revoking all associated access tokens'); - $dbh->do('DELETE FROM oauth2_auth_code WHERE auth_code = ?', undef, $auth_code); - $dbh->do('DELETE FROM oauth2_access_token WHERE client_id = ? AND user_id = ?', - undef, $client_id, $auth_code_data->{user_id}); - } + if ($jwt_data) { + INFO('Client redirect_uri does not match') + if ($uri && $jwt_claims->{aud} ne $uri); + INFO('Auth code expired') if ($jwt_claims->{exp} <= time); + $dbh->do('DELETE FROM oauth2_jwt WHERE client_id = ? AND user_id = ? AND type = ?', + undef, $client_data->{id}, $jwt_claims->{user_id}, TOKEN_TYPE_AUTH); } return (0, 'invalid_grant'); } - $dbh->do('UPDATE oauth2_auth_code SET verified = 1 WHERE auth_code = ?', - undef, $auth_code); + $dbh->do('DELETE FROM oauth2_jwt WHERE id = ?', + undef, $jwt_data->{id}); - # scopes are those that were requested in the authorization request, not - # those stored in the client (i.e. what the auth request restriced scopes - # to and not everything the client is capable of) - my $scope_descriptions = $dbh->selectcol_arrayref( - 'SELECT oauth2_scope.description FROM oauth2_scope - JOIN oauth2_auth_code_scope ON oauth2_scope.id = oauth2_auth_code_scope.scope_id - WHERE oauth2_auth_code_scope.auth_code = ?', undef, $auth_code - ); + return ($client_id, undef, $jwt_claims->{scopes}, $jwt_claims->{user_id}); +} + +sub _store_auth_code { + my (%args) = @_; + my ($c, $auth_code, $client_id, $expires_in, $uri, $scopes_ref) + = @args{ + qw/ mojo_controller auth_code client_id expires_in redirect_uri scopes /}; + my $dbh = Bugzilla->dbh; + + my $client_data + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE client_id = ?', + undef, $client_id); + + my ($res, $jwt_claims) = _get_jwt_claims($auth_code, 'auth'); + return (0, 'invalid_jwt') unless $res; - my %scope = map { $_ => 1 } @{$scope_descriptions}; + $dbh->do( + 'INSERT INTO oauth2_jwt (jti, client_id, user_id, type, expires) VALUES (?, ?, ?, ?, ?)', + undef, + $jwt_claims->{jti}, + $client_data->{id}, + $jwt_claims->{user_id}, + TOKEN_TYPE_AUTH, + DateTime->from_epoch(epoch => time + $expires_in), + ); - return ($client_id, undef, {%scope}, $auth_code_data->{user_id}); + return undef; } sub _store_access_token { my (%args) = @_; - my ($c, $client, $auth_code, $access_token, $refresh_token, $expires_in, + my ($c, $client_id, $auth_code, $access_token, $refresh_token, $expires_in, $scopes, $old_refresh_token) = @args{ qw/ mojo_controller client_id auth_code access_token refresh_token expires_in scopes old_refresh_token / }; my $dbh = Bugzilla->dbh; - my ($user_id); - if (!defined $auth_code && $old_refresh_token) { + my $client_data + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE client_id = ?', + undef, $client_id); + my $user_id; + if (!defined $auth_code && $old_refresh_token) { # must have generated an access token via a refresh token so revoke the # old access token and refresh token (also copy required data if missing) - my $prev_refresh_token - = $dbh->selectrow_hashref( - 'SELECT * FROM oauth2_refresh_token WHERE refresh_token = ?', - undef, $old_refresh_token); - my $prev_access_token - = $dbh->selectrow_hashref( - 'SELECT * FROM oauth2_access_token WHERE access_token = ?', - undef, $prev_refresh_token->{access_token}); - - # access tokens can be revoked, whilst refresh tokens can remain so we - # need to get the data from the refresh token as the access token may - # no longer exist at the point that the refresh token is used - my $scope_descriptions = $dbh->selectall_array( - 'SELECT oauth2_scope.description FROM oauth2_scope - JOIN oauth2_access_token_scope ON scope.id = oauth2_access_token_scope.scope_id - WHERE access_token = ?', undef, $old_refresh_token - ); - $scopes //= map { $_ => 1 } @{$scope_descriptions}; - - $user_id = $prev_refresh_token->{user_id}; + my ($res, $jwt_claims) = _get_jwt_claims($old_refresh_token, 'refresh'); + return (0, 'invalid_jwt') unless $res; + my $jwt_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_jwt WHERE jti = ?', undef, $jwt_claims->{jti}); + return (0, 'invalid_grant') if !$jwt_data; + $user_id = $jwt_claims->{user_id}; } else { - $user_id - = $dbh->selectrow_array( - 'SELECT user_id FROM oauth2_auth_code WHERE auth_code = ?', - undef, $auth_code); - } - - if (ref $client) { - $scopes //= $client->{scope}; - $user_id //= $client->{user_id}; - $client = $client->{client_id}; + my ($res, $jwt_claims) = _get_jwt_claims($auth_code, 'auth'); + return (0, 'invalid_jwt') unless $res; + $user_id = $jwt_claims->{user_id}; } - foreach my $token_type (qw/ access refresh /) { - my $table = "oauth2_${token_type}_token"; + my ($res, $jwt_claims) = _get_jwt_claims($access_token, 'access'); + return (0, 'invalid_jwt') unless $res; - # if the client has en existing access/refresh token we need to revoke it - $dbh->do("DELETE FROM $table WHERE client_id = ? AND user_id = ?", - undef, $client, $user_id); - } + # If the client has en existing access/refesh tokens, we need to revoke them + INFO('Revoking old access tokens (refresh)'); + $dbh->do('DELETE FROM oauth2_jwt WHERE client_id = ? AND user_id = ?', + undef, $client_data->{id}, $jwt_claims->{user_id}); $dbh->do( - 'INSERT INTO oauth2_access_token VALUES (?, ?, ?, ?, ?)', undef, - $access_token, $refresh_token, - $client, $user_id, - DateTime->from_epoch(epoch => time + $expires_in) + 'INSERT INTO oauth2_jwt (jti, client_id, user_id, type, expires) VALUES (?, ?, ?, ?, ?)', + undef, + $jwt_claims->{jti}, + $client_data->{id}, + $user_id, + TOKEN_TYPE_ACCESS, + DateTime->from_epoch(epoch => time + $expires_in), ); - $dbh->do('INSERT INTO oauth2_refresh_token VALUES (?, ?, ?, ?)', - undef, $refresh_token, $access_token, $client, $user_id); - - foreach my $rqd_scope (keys %{$scopes}) { - my $scope_id - = $dbh->selectrow_array('SELECT id FROM oauth2_scope WHERE description = ?', - undef, $rqd_scope); - if ($scope_id) { - foreach my $related (qw/ access_token refresh_token /) { - my $table = "oauth2_${related}_scope"; - $dbh->do( - "INSERT INTO $table VALUES (?, ?, ?)", - undef, $related eq 'access_token' ? $access_token : $refresh_token, - $scope_id, $scopes->{$rqd_scope} - ); - } - } - else { - ERROR("Unknown scope ($rqd_scope) in _store_access_token"); - } - } + ($res, $jwt_claims) = _get_jwt_claims($refresh_token, 'refresh'); + return (0, 'invalid_jwt') unless $res; - return; + $dbh->do( + 'INSERT INTO oauth2_jwt (jti, client_id, user_id, type) VALUES (?, ?, ?, ?)', + undef, + $jwt_claims->{jti}, + $client_data->{id}, + $user_id, + TOKEN_TYPE_REFRESH + ); + + return undef; } sub _verify_access_token { my (%args) = @_; - my ($c, $access_token, $scopes_ref) - = @args{qw/ mojo_controller access_token scope /}; + my ($c, $access_token, $scopes_ref, $is_refresh_token) + = @args{qw/ mojo_controller access_token scopes is_refresh_token /}; my $dbh = Bugzilla->dbh; - if ( - my $refresh_token_data = $dbh->selectrow_hashref( - 'SELECT * FROM oauth2_refresh_token WHERE access_token = ?', undef, - $access_token - ) - ) - { - foreach my $scope (@{$scopes_ref // []}) { - my $scope_allowed = $dbh->selectrow_array( - 'SELECT allowed FROM oauth2_refresh_token_scope - JOIN oauth2_scope ON oauth2_scope.id = oauth2_refresh_token_scope.scope_id - WHERE refresh_token = ? AND oauth2_scope.description = ?', undef, - $access_token, $scope - ); + my ($res, $jwt_claims) = _get_jwt_claims($access_token); + return (0, 'invalid_jwt') unless $res; - if (!defined $scope_allowed || !$scope_allowed) { - INFO("Refresh token doesn't have scope ($scope)"); - return (0, 'invalid_grant'); + my $jwt_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_jwt WHERE jti = ?', undef, $jwt_claims->{jti}); + + if ($jwt_data && $is_refresh_token) { + if ($scopes_ref) { + foreach my $scope (@{$scopes_ref // []}) { + return (0, 'invalid_grant') if !_has_scope($scope, $jwt_claims->{scopes}); } } - return { - client_id => $refresh_token_data->{client_id}, - user_id => $refresh_token_data->{user_id}, - }; + return ($jwt_claims, undef, $jwt_claims->{scopes}, $jwt_claims->{user_id}); } - elsif ( - my $access_token_data = $dbh->selectrow_hashref( - 'SELECT expires, client_id, user_id FROM oauth2_access_token WHERE access_token = ?', - undef, - $access_token - ) - ) - { - if (datetime_from($access_token_data->{expires})->epoch <= time) { + + if ($jwt_data) { + if ($jwt_claims->{exp} <= time) { INFO('Access token has expired'); - $dbh->do('DELETE FROM oauth2_access_token WHERE access_token = ?', - undef, $access_token); + $dbh->do('DELETE FROM oauth2_jwt WHERE id = ?', + undef, $jwt_data->{id}); return (0, 'invalid_grant'); } - - foreach my $scope (@{$scopes_ref // []}) { - my $scope_allowed = $dbh->selectrow_array( - 'SELECT allowed FROM oauth2_access_token_scope - JOIN oauth2_scope ON oauth2_access_token_scope.scope_id = oauth2_scope.id - WHERE scope.description = ? AND access_token = ?', undef, $scope, - $access_token - ); - if (!defined $scope_allowed || !$scope_allowed) { - INFO("Access token doesn't have scope ($scope)"); - return (0, 'invalid_grant'); + elsif ($scopes_ref) { + foreach my $scope (@{$scopes_ref // []}) { + if (!_has_scope($scope, $jwt_claims->{scopes})) { + INFO("Scope $scope not found"); + return (0, 'invalid_grant'); + } } } - return { - client_id => $access_token_data->{client_id}, - user_id => $access_token_data->{user_id}, - }; + return ($jwt_claims, undef, $jwt_claims->{scopes}, $jwt_claims->{user_id}); } else { INFO('Access token does not exist'); @@ -398,4 +333,31 @@ sub _verify_access_token { } } +sub _get_jwt_claims { + my ($jwt, $check_type) = @_; + my ($claims, $jwt_error); + + try { + $claims = Bugzilla->jwt->decode($jwt); + } + catch { + INFO("Error decoding JWT: $_"); + $jwt_error = 1; + }; + + return (0) if $jwt_error; + + if (defined $check_type && $check_type ne $claims->{type}) { + INFO("JWT not correct type: got: " . $claims->{type} . " expected: $check_type"); + return (0); + } + + return (1, $claims); +} + +sub _has_scope { + my ($scope, $available_scopes) = @_; + return any {$scope} @{$available_scopes // []}; +} + 1; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 60b61e135..ee96220cf 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -1785,189 +1785,65 @@ use constant ABSTRACT_SCHEMA => { oauth2_client => { FIELDS => [ - id => {TYPE => 'varchar(255)', NOTNULL => 1, PRIMARYKEY => 1}, + id => {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, + client_id => {TYPE => 'varchar(255)', NOTNULL => 1}, description => {TYPE => 'varchar(255)', NOTNULL => 1}, secret => {TYPE => 'varchar(255)', NOTNULL => 1}, active => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'}, - last_modified => {TYPE => 'DATETIME'} - ] + last_modified => {TYPE => 'DATETIME'}, + ], }, oauth2_scope => { FIELDS => [ - id => {TYPE => 'INT3', NOTNULL => 1, PRIMARYKEY => 1}, - description => {TYPE => 'varchar(255)', NOTNULL => 1} - ] + id => {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, + description => {TYPE => 'varchar(255)', NOTNULL => 1}, + ], }, oauth2_client_scope => { FIELDS => [ + id => {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, client_id => { - TYPE => 'varchar(255)', + TYPE => 'INT4', NOTNULL => 1, REFERENCES => { TABLE => 'oauth2_client', COLUMN => 'id', UPDATE => 'CASCADE', - DELETE => 'CASCADE' + DELETE => 'CASCADE', } }, scope_id => { - TYPE => 'INT3', + TYPE => 'INT4', NOTNULL => 1, REFERENCES => { TABLE => 'oauth2_scope', COLUMN => 'id', UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } + DELETE => 'CASCADE', + }, }, - allowed => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'} ], INDEXES => [ oauth2_client_scope_idx => {FIELDS => ['client_id', 'scope_id'], TYPE => 'UNIQUE'}, - ] - }, - - oauth2_auth_code => { - FIELDS => [ - auth_code => {TYPE => 'varchar(255)', NOTNULL => 1, PRIMARYKEY => 1}, - client_id => { - TYPE => 'varchar(255)', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_client', - COLUMN => 'id', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - user_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - expires => {TYPE => 'DATETIME', NOTNULL => 1}, - redirect_uri => {TYPE => 'TINYTEXT', NOTNULL => 1}, - verified => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}, - ] - }, - - oauth2_auth_code_scope => { - FIELDS => [ - auth_code => { - TYPE => 'varchar(255)', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_auth_code', - COLUMN => 'auth_code', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - scope_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_scope', - COLUMN => 'id', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - allowed => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}, ], - INDEXES => [ - oauth2_auth_code_scope_idx => - {FIELDS => ['auth_code', 'scope_id'], TYPE => 'UNIQUE'}, - ] }, - oauth2_access_token => { + oauth2_jwt => { FIELDS => [ - access_token => {TYPE => 'varchar(255)', NOTNULL => 1, PRIMARYKEY => 1}, - refresh_token => {TYPE => 'varchar(255)'}, - client_id => { - TYPE => 'varchar(255)', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_client', - COLUMN => 'id', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - user_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - expires => {TYPE => 'DATETIME', NOTNULL => 1}, - ] - }, - - oauth2_access_token_scope => { - FIELDS => [ - access_token => { - TYPE => 'varchar(255)', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_access_token', - COLUMN => 'access_token', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - scope_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_scope', - COLUMN => 'id', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - allowed => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}, - ], - INDEXES => [ - oauth2_access_token_scope_idx => - {FIELDS => ['access_token', 'scope_id'], TYPE => 'UNIQUE'} - ] - }, - - oauth2_refresh_token => { - FIELDS => [ - refresh_token => {TYPE => 'varchar(255)', NOTNULL => 1, PRIMARYKEY => 1}, - access_token => { - TYPE => 'varchar(255)', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_access_token', - COLUMN => 'access_token', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, + id => {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, + jti => {TYPE => 'varchar(255)', NOTNULL => 1}, + type => {TYPE => 'INT2', NOTNULL => 1}, client_id => { - TYPE => 'varchar(255)', + TYPE => 'INT4', NOTNULL => 1, REFERENCES => { TABLE => 'oauth2_client', COLUMN => 'id', UPDATE => 'CASCADE', - DELETE => 'CASCADE' + DELETE => 'CASCADE', } }, user_id => { @@ -1977,40 +1853,14 @@ use constant ABSTRACT_SCHEMA => { TABLE => 'profiles', COLUMN => 'userid', UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - } - ] - }, - - oauth2_refresh_token_scope => { - FIELDS => [ - refresh_token => { - TYPE => 'varchar(255)', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_refresh_token', - COLUMN => 'refresh_token', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } + DELETE => 'CASCADE', + }, }, - scope_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'oauth2_scope', - COLUMN => 'id', - UPDATE => 'CASCADE', - DELETE => 'CASCADE' - } - }, - allowed => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}, + expires => {TYPE => 'DATETIME'}, ], INDEXES => [ - oauth2_refresh_token_scope_idx => - {FIELDS => ['refresh_token', 'scope_id'], TYPE => 'UNIQUE'} - ] + oauth2_jwt_jti_type_idx => {FIELDS => [qw(jti type)], TYPE => 'UNIQUE'}, + ], } }; diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 0558aacfd..f9142fa4a 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -780,6 +780,8 @@ sub update_table_definitions { $dbh->bz_add_column('products', 'bug_description_template', {TYPE => 'MEDIUMTEXT'}); + _add_oauth2_jwt_support(); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -4227,6 +4229,64 @@ sub _populate_oauth2_scopes { $dbh->do("INSERT INTO oauth2_scope (id, description) VALUES (1, 'user:read')"); } +sub _add_oauth2_jwt_support { + my $dbh = Bugzilla->dbh; + + # Return if we have already made these changes + return if $dbh->bz_column_info('oauth2_client', 'client_id'); + + print "Updating OAuth2 tables for JWT support...\n"; + + # Some tables need to be dropped completely + foreach my $table ( + qw/ oauth2_refresh_token_scope oauth2_refresh_token + oauth2_access_token_scope oauth2_access_token + oauth2_auth_code_scope oauth2_auth_code / + ) + { + $dbh->bz_drop_table($table); + } + + # Drop foreign keys. THey will be recreated later. + $dbh->bz_drop_fk('oauth2_client_scope', 'client_id'); + $dbh->bz_drop_fk('oauth2_client_scope', 'scope_id'); + + # client id should no longer be the primary key for the clients table + $dbh->bz_rename_column('oauth2_client', 'id', 'client_id'); + $dbh->bz_alter_column('oauth2_client', 'client_id', + {TYPE => 'varchar(255)', NOTNULL => 1}); + + # scope table needs INTSERIAL (INT4) + $dbh->bz_alter_column('oauth2_scope', 'id', + {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}); + + # oauth2_client_scope.allowed is unncessary so we drop it + $dbh->bz_drop_column('oauth2_client_scope', 'allowed'); + + # Update old non-id string columns to new id column + $dbh->bz_alter_column('oauth2_client_scope', 'scope_id', + {TYPE => 'INT4', NOTNULL => 1}); + + # Add primary key columns to the tables that require it + foreach my $table (qw/oauth2_client oauth2_client_scope/) { + $dbh->bz_add_column($table, 'id', + {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}); + } + + # Last changes needed for the oauth2_client_scope table by + # populating the client_id table with integers instead of keys. + $dbh->bz_add_column('oauth2_client_scope', 'client_id_new', {TYPE => 'INT4'}); + $dbh->do( + 'UPDATE oauth2_client_scope AS oacs, + (SELECT * FROM oauth2_client) AS oac + SET oacs.client_id_new = oac.id' + ); + $dbh->bz_drop_column('oauth2_client_scope', 'client_id'); + $dbh->bz_rename_column('oauth2_client_scope', 'client_id_new', 'client_id'); + $dbh->bz_alter_column('oauth2_client_scope', 'client_id', + {TYPE => 'INT4', NOTNULL => 1}); +} + 1; __END__ diff --git a/Bugzilla/Test/Util.pm b/Bugzilla/Test/Util.pm index 8ce98ffe6..ed46ec104 100644 --- a/Bugzilla/Test/Util.pm +++ b/Bugzilla/Test/Util.pm @@ -50,9 +50,13 @@ sub create_oauth_client { my $id = generate_random_password(20); my $secret = generate_random_password(40); - $dbh->do('INSERT INTO oauth2_client (id, description, secret) VALUES (?, ?, ?)', + $dbh->do('INSERT INTO oauth2_client (client_id, description, secret) VALUES (?, ?, ?)', undef, $id, $description, $secret); + my $client_data + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE client_id = ?', + undef, $id); + foreach my $scope (@{$scopes}) { my $scope_id = $dbh->selectrow_array('SELECT id FROM oauth2_scope WHERE description = ?', @@ -61,13 +65,12 @@ sub create_oauth_client { die "Scope $scope not found"; } $dbh->do( - 'INSERT INTO oauth2_client_scope (client_id, scope_id, allowed) VALUES (?, ?, 1)', - undef, $id, $scope_id + 'INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', + undef, $client_data->{id}, $scope_id ); } - return $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', - undef, $id); + return $client_data; } sub issue_api_key { diff --git a/t/mojo-oauth2.t b/t/mojo-oauth2.t index 575c7e8b6..a5cb3defd 100644 --- a/t/mojo-oauth2.t +++ b/t/mojo-oauth2.t @@ -13,6 +13,7 @@ use lib qw( . lib local/lib/perl5 ); BEGIN { $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; + $ENV{MOJO_TEST} = 1; } use Bugzilla::Test::MockDB; @@ -28,11 +29,12 @@ my $referer = Bugzilla->localconfig->{urlbase}; my $stash = {}; # Create user to use as OAuth2 resource owner -create_user($oauth_login, $oauth_password); +my $oauth_user = create_user($oauth_login, $oauth_password); # Create a new OAuth2 client used for testing my $oauth_client = create_oauth_client('Shiny New OAuth Client', ['user:read']); -ok $oauth_client->{id}, 'New client id (' . $oauth_client->{id} . ')'; +ok $oauth_client->{client_id}, + 'New client id (' . $oauth_client->{client_id} . ')'; ok $oauth_client->{secret}, 'New client secret (' . $oauth_client->{secret} . ')'; @@ -48,7 +50,7 @@ $t->app->hook(after_dispatch => sub { $stash = shift->stash }); # User should be logged out so /oauth/authorize should redirect to a login screen $t->get_ok( '/oauth/authorize' => {Referer => $referer} => form => { - client_id => $oauth_client->{id}, + client_id => $oauth_client->{client_id}, response_type => 'code', state => 'state', scope => 'user:read', @@ -67,7 +69,7 @@ $t->post_ok( Bugzilla_password => $oauth_password, Bugzilla_restrictlogin => 1, GoAheadAndLogIn => 1, - client_id => $oauth_client->{id}, + client_id => $oauth_client->{client_id}, response_type => 'code', state => 'state', scope => 'user:read', @@ -84,13 +86,13 @@ ok $csrf_token, "Get csrf token ($csrf_token)"; # URI specified in the redirect_uri value. In this case a simple text page. $t->get_ok( '/oauth/authorize' => {Referer => $referer} => form => { - "oauth_confirm_" . $oauth_client->{id} => 1, - token => $csrf_token, - client_id => $oauth_client->{id}, - response_type => 'code', - state => 'state', - scope => 'user:read', - redirect_uri => '/oauth/redirect' + "oauth_confirm_" . $oauth_client->{client_id} => 1, + token => $csrf_token, + client_id => $oauth_client->{client_id}, + response_type => 'code', + state => 'state', + scope => 'user:read', + redirect_uri => '/oauth/redirect' } )->status_is(200)->content_is('Redirect Success!'); @@ -107,7 +109,7 @@ ok $auth_code, "Get auth code ($auth_code)"; # end user. $t->post_ok( '/oauth/access_token' => {Referer => $referer} => form => { - client_id => $oauth_client->{id}, + client_id => $oauth_client->{client_id}, client_secret => $oauth_client->{secret}, code => $auth_code, grant_type => 'authorization_code', @@ -130,6 +132,43 @@ $t->get_ok('/api/user/profile' => {Authorization => 'Bearer ' . $access_data->{access_token}})->status_is(200) ->json_is('/login' => $oauth_login); +# Should be able to use the refresh token to get a new access token +$t->post_ok( + '/oauth/access_token' => {Referer => $referer} => form => { + client_id => $oauth_client->{client_id}, + client_secret => $oauth_client->{secret}, + refresh_token => $access_data->{refresh_token}, + grant_type => 'refresh_token', + redirect_uri => '/oauth/redirect', + } +)->status_is(200)->json_has('access_token', 'Has access token') + ->json_has('refresh_token', 'Has refresh token') + ->json_has('token_type', 'Has token type'); + +$access_data = $t->tx->res->json; + +$t->get_ok('/api/user/profile' => + {Authorization => 'Bearer ' . $access_data->{access_token}})->status_is(200) + ->json_is('/login' => $oauth_login); + +# API should fail if user is disabled +$oauth_user->set_disabledtext('DISABLED'); +$oauth_user->update(); +$t->get_ok('/api/user/profile' => + {Authorization => 'Bearer ' . $access_data->{access_token}}) + ->status_is(401); + +# Should get an error if we try to re-use the same auth code again +$t->post_ok( + '/oauth/access_token' => {Referer => $referer} => form => { + client_id => $oauth_client->{client_id}, + client_secret => $oauth_client->{secret}, + code => $auth_code, + grant_type => 'authorization_code', + redirect_uri => '/oauth/redirect', + } +)->status_is(400)->json_is('/error' => 'invalid_grant'); + done_testing; sub _setup_routes { diff --git a/template/en/default/account/auth/confirm_scopes.html.tmpl b/template/en/default/account/auth/confirm_scopes.html.tmpl index 76b51e1f8..0005ffc2c 100644 --- a/template/en/default/account/auth/confirm_scopes.html.tmpl +++ b/template/en/default/account/auth/confirm_scopes.html.tmpl @@ -31,7 +31,7 @@ Scopes:
- + [% FOREACH field = c.req.params.names %] diff --git a/template/en/default/admin/oauth/confirm-delete.html.tmpl b/template/en/default/admin/oauth/confirm-delete.html.tmpl index 64bae7ab4..286fa119a 100644 --- a/template/en/default/admin/oauth/confirm-delete.html.tmpl +++ b/template/en/default/admin/oauth/confirm-delete.html.tmpl @@ -13,7 +13,7 @@ Client ID - [% client.id FILTER html %] + [% client.client_id FILTER html %] diff --git a/template/en/default/admin/oauth/edit.html.tmpl b/template/en/default/admin/oauth/edit.html.tmpl index e25ee82dc..09ceadf92 100644 --- a/template/en/default/admin/oauth/edit.html.tmpl +++ b/template/en/default/admin/oauth/edit.html.tmpl @@ -12,7 +12,7 @@ - [% client.id FILTER html %] + [% client.client_id FILTER html %] -- 2.47.3