Improve error handling for non-redirect errors
Change-Id: I7b8b62e52b08ed92edf02c770fd908be47917505
diff --git a/Changes b/Changes
index ad2ee66..df62487 100755
--- a/Changes
+++ b/Changes
@@ -1,4 +1,4 @@
-0.48 2023-01-30
+0.48 2023-01-31
- Added support for NKJP tagset in annotation
assistant. (diewald)
- Remove deprecated 'auth_support' (since 0.31)
@@ -23,6 +23,8 @@
- Make scope a requirement for OAuth authorizations.
(diewald)
- Improve security of OAuth redirects. (diewald)
+ - Improve error handling for non-redirect error responses.
+ (diewald)
WARNING: Mojolicious 9.31 is a security update -
updating is highly recommended.
diff --git a/lib/Kalamar/Plugin/Auth.pm b/lib/Kalamar/Plugin/Auth.pm
index 58896d9..75d231f 100644
--- a/lib/Kalamar/Plugin/Auth.pm
+++ b/lib/Kalamar/Plugin/Auth.pm
@@ -1306,6 +1306,10 @@
sub {
my $tx = shift;
+ unless (ref($tx)) {
+ return Mojo::Promise->reject('Something went wrong');
+ };
+
# Check for location header with code in redirects
my $loc;
foreach (@{$tx->redirects}) {
@@ -1328,6 +1332,14 @@
if (my $err = $url->query->param('error_description')) {
return Mojo::Promise->reject($err);
};
+ }
+
+ $c->stash(redirect_uri => undef);
+
+ # Maybe json
+ my $json = $tx->res->json;
+ if ($json && $json->{error_description}) {
+ return Mojo::Promise->reject($json->{error_description});
};
# No location code
@@ -1337,9 +1349,20 @@
sub {
my $err_msg = shift;
my $url = $c->stash('redirect_uri');
- if ($err_msg) {
- $url = $url->query([error_description => $err_msg]);
+
+ # Redirect!
+ if ($url) {
+ if ($err_msg) {
+ $url = $url->query([error_description => $err_msg]);
+ };
+ }
+
+ # Do not redirect!
+ else {
+ $c->notify(error => $err_msg);
+ $url = $c->url_for('oauth-settings');
};
+
return Mojo::Promise->resolve($url);
}
)->then(
diff --git a/t/plugin/auth-oauth.t b/t/plugin/auth-oauth.t
index a2cc73c..67ddedd 100644
--- a/t/plugin/auth-oauth.t
+++ b/t/plugin/auth-oauth.t
@@ -955,6 +955,23 @@
$t->post_ok(Mojo::URL->new('/settings/oauth/authorize')->query({
client_id => 'xyz',
+ state => 'abcde',
+ scope => 'search match',
+ redirect_uri_server => 'http://example.com/',
+ redirect_uri => $t->app->close_redirect_to('http://wrong'),
+ csrf_token => $csrf,
+}))
+ ->status_is(302)
+ ->header_is('location', '/settings/oauth')
+ ->tx->res->headers->header('location')
+ ;
+
+$t->get_ok('/settings/oauth')
+ ->text_is('div.notify-error', 'Invalid redirect URI')
+ ;
+
+$t->post_ok(Mojo::URL->new('/settings/oauth/authorize')->query({
+ client_id => 'xyz',
state => 'fail',
scope => 'search match',
# redirect_uri_server => 'http://example.com/',
diff --git a/t/server/mock.pl b/t/server/mock.pl
index ab0c34d..e615306 100644
--- a/t/server/mock.pl
+++ b/t/server/mock.pl
@@ -697,6 +697,14 @@
return $c->rendered;
};
+ if (index($redirect_uri,'http://wrong') >= 0) {
+ return $c->render(
+ code => 400,
+ content_type => 'text/plain',
+ text => '{"error_description":"Invalid redirect URI","state":"ZMwDGTZ2RY","error":"invalid_request"}'
+ );
+ };
+
return $c->redirect_to(
Mojo::URL->new($redirect_uri)->query({
code => $tokens{auth_token_1},
@@ -713,7 +721,13 @@
scope => 'match_info search openid'
})
);
- }
+ };
+
+ return $c->render(
+ code => 400,
+ content_type => 'text/plain',
+ content => 'Unknown'
+ );
};