Fix redirection handling for all authorization flows
Change-Id: Id59e7e524715317df7be5e3aac96b8ac6232219c
diff --git a/Changes b/Changes
index bd1d2fb..7a72374 100755
--- a/Changes
+++ b/Changes
@@ -1,10 +1,11 @@
-0.50 2023-04-12
+0.50 2023-05-08
- Improvement of NKJP-annotation helper. (diewald)
- Add redirect-uri to client view. (diewald)
- Added german translation for Cosmas-II. (hebasta)
- Workaround for failing utf8 test in some
environments (fixes #197). (diewald)
- Require at least conllu2korapxml v0.6.1. (diewald)
+ - Improve redirection on all authorization calls. (diewald)
0.49 2023-02-23
- Introduce conllu2korapxml command via plugin. (diewald)
diff --git a/lib/Kalamar/Plugin/Auth.pm b/lib/Kalamar/Plugin/Auth.pm
index 4e2c7bf..e65972b 100644
--- a/lib/Kalamar/Plugin/Auth.pm
+++ b/lib/Kalamar/Plugin/Auth.pm
@@ -283,6 +283,14 @@
my $client_id = $param->{client_id};
my $client_secret = $param->{client_secret};
+ my $no_redirect_ua = Mojo::UserAgent->new(
+ connect_timeout => 30,
+ inactivity_timeout => 30,
+ max_redirects => 0
+ );
+
+ $no_redirect_ua->server->app($app);
+
# Sets a requested token and returns
# an error, if it didn't work
@@ -410,6 +418,87 @@
);
+ # Issue new token and return a promise
+ $app->helper(
+ 'auth.new_token_p' => sub {
+ my $c = shift;
+ my %param = @_;
+
+ state $r_url = Mojo::URL->new($c->korap->api)->path('oauth2/authorize');
+
+ my $client_id = $param{'client_id'};
+
+ return $c->korap_request($no_redirect_ua, post => $r_url, { } => form => {
+ response_type => 'code',
+ client_id => $client_id,
+ redirect_uri => $param{'redirect_uri'},
+ state => $param{'state'},
+ scope => $param{'scope'},
+ })->then(
+ sub {
+ my $tx = shift;
+
+ unless (ref($tx)) {
+ return Mojo::Promise->reject('Something went wrong');
+ };
+
+ # Check for location header with code in redirects
+ my ($code, $scope, $loc, $name);
+
+ # Check for location header with code in current tx
+ # and in redirects.
+ # The loop should not be relevant though, as for now
+ # redirects are not allowed.
+ foreach ($tx, @{$tx->redirects}) {
+ $loc = $_->res->headers->header('Location');
+
+ next unless $loc;
+
+ my $url = Mojo::URL->new($loc);
+
+ if ($url->query->param('code')) {
+ my $q = $url->query;
+ $code = $q->param('code');
+ $scope = $q->param('scope');
+ $name = $q->param('name');
+ last;
+ } elsif (my $err = $url->query->param('error_description')) {
+ return Mojo::Promise->reject($err);
+ }
+ };
+
+ return Mojo::Promise->resolve(
+ $loc,
+ $client_id,
+ $param{'redirect_uri'},
+ $code,
+ $scope,
+ $name
+ ) if $loc;
+
+ # Failed redirect, but location set
+ if ($tx->res->headers->location) {
+ my $url = Mojo::URL->new($tx->res->headers->location);
+ 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
+ return Mojo::Promise->reject('no location response');
+ }
+ )
+ }
+ );
+
# Get a list of registered clients
$app->helper(
'auth.client_list_p' => sub {
@@ -1390,80 +1479,13 @@
return $c->redirect_to($url);
};
- state $r_url = Mojo::URL->new($c->korap->api)->path('oauth2/authorize');
$c->stash(redirect_uri => Mojo::URL->new($v->param('redirect_uri')));
- my $ua = Mojo::UserAgent->new(
- connect_timeout => 30,
- inactivity_timeout => 30,
- max_redirects => 0
- );
-
- $ua->server->app($app);
-
- return $c->korap_request($ua, post => $r_url, { } => form => {
- response_type => 'code',
+ return $c->auth->new_token_p(
client_id => $v->param('client_id'),
redirect_uri => $c->stash('redirect_uri'),
state => $v->param('state'),
scope => $v->param('scope'),
- })->then(
- sub {
- my $tx = shift;
-
- unless (ref($tx)) {
- return Mojo::Promise->reject('Something went wrong');
- };
-
- # Check for location header with code in redirects
- my $loc;
-
- # Look for code in location URL
- if ($loc = $tx->res->headers->header('Location')) {
- my $url = Mojo::URL->new($loc);
-
- if ($url->query->param('code')) {
- return Mojo::Promise->resolve($loc);
- } elsif (my $err = $url->query->param('error_description')) {
- return Mojo::Promise->reject($err);
- }
- };
-
- # Check for location header with code in redirects
- # This should be dead code tbh
- foreach (@{$tx->redirects}) {
- $loc = $_->res->headers->header('Location');
-
- my $url = Mojo::URL->new($loc);
-
- if ($url->query->param('code')) {
- last;
- } elsif (my $err = $url->query->param('error_description')) {
- return Mojo::Promise->reject($err);
- }
- };
-
- return Mojo::Promise->resolve($loc) if $loc;
-
- # Failed redirect, but location set
- if ($tx->res->headers->location) {
- my $url = Mojo::URL->new($tx->res->headers->location);
- 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
- return Mojo::Promise->reject('no location response');
- }
)->catch(
sub {
my $err_msg = shift;
@@ -1600,49 +1622,17 @@
};
# Get authorization token
- state $r_url = Mojo::URL->new($c->korap->api)->path('oauth2/authorize');
my $client_id = $c->stash('client_id');
my $name = $v->param('name');
my $redirect_url = $c->url_for->query({name => $name});
- return $c->korap_request(post => $r_url, {} => form => {
- response_type => 'code',
+ $c->auth->new_token_p(
client_id => $client_id,
redirect_uri => $redirect_url,
- # TODO: State
- })->then(
- sub {
- my $tx = shift;
-
- # Strip the token from the location header of the fake redirect
- # TODO: Alternatively redirect!
- my ($code, $scope, $loc, $name);
- foreach (@{$tx->redirects}) {
- $loc = $_->res->headers->header('Location');
- if (index($loc, 'code') > 0) {
- my $q = Mojo::URL->new($loc)->query;
- $code = $q->param('code');
- $scope = $q->param('scope');
- $name = $q->param('name');
- last;
- };
- };
-
- # Fine!
- if ($code) {
- return Mojo::Promise->resolve(
- $client_id,
- $redirect_url,
- $code,
- $scope,
- $name
- );
- };
- return Mojo::Promise->reject;
- }
+ # TODO: State, scope
)->then(
sub {
- my ($client_id, $redirect_url, $code, $scope, $name) = @_;
+ my ($loc, $client_id, $redirect_url, $code, $scope, $name) = @_;
# Get OAuth access token
state $r_url = Mojo::URL->new($c->korap->api)->path('oauth2/token');
diff --git a/t/plugin/auth-oauth.t b/t/plugin/auth-oauth.t
index 7709f4e..959e8f1 100644
--- a/t/plugin/auth-oauth.t
+++ b/t/plugin/auth-oauth.t
@@ -664,6 +664,24 @@
->header_is('Pragma','no-cache')
;
+$t->post_ok('/settings/oauth/307/token' => form => {
+ csrf_token => $csrf,
+ name => 'MyApp2',
+})
+ ->status_is(302)
+ ->header_is('Location','/settings/oauth/307')
+ ;
+
+$t->get_ok('/settings/oauth/fCBbQkA2NDA3MzM1Yw==')
+ ->status_is(200)
+ ->text_is('div.notify-success', 'New access token created')
+ ->status_is(200)
+ ->header_is('Cache-Control','max-age=0, no-cache, no-store, must-revalidate')
+ ->header_is('Expires','Thu, 01 Jan 1970 00:00:00 GMT')
+ ->header_is('Pragma','no-cache')
+ ;
+
+
$csrf = $t->get_ok('/settings/oauth/fCBbQkA2NDA3MzM1Yw==')
->status_is(200)
->attr_is('form.token-revoke', 'action', '/settings/oauth/fCBbQkA2NDA3MzM1Yw==/token/revoke')
diff --git a/t/server/mock.pl b/t/server/mock.pl
index af03832..7331c81 100644
--- a/t/server/mock.pl
+++ b/t/server/mock.pl
@@ -441,7 +441,7 @@
# Get auth_token_1
elsif ($grant_type eq 'authorization_code') {
- if ($c->param('code') eq $tokens{auth_token_1}) {
+ if ($c->param('code') && $c->param('code') eq $tokens{auth_token_1}) {
return $c->render(
status => 200,
json => {
@@ -763,13 +763,15 @@
}
elsif ($type eq 'code') {
+ my $loc = Mojo::URL->new($redirect_uri)->query({
+ code => $tokens{auth_token_1},
+ scope => 'match_info search openid'
+ });
- return $c->redirect_to(
- Mojo::URL->new($redirect_uri)->query({
- code => $tokens{auth_token_1},
- scope => 'match_info search openid'
- })
- );
+ my $res = $c->res;
+ $res->headers->location($loc);
+ return $c->rendered($client_id eq '307' ? 307 : 302);
+ # return $c->rendered(302);
};
return $c->render(