Make scope a requirement when authorizing a client
Change-Id: Id0ea27d90afdca08f76021bf17f1fda125147268
diff --git a/Changes b/Changes
index c30b2c8..f41203a 100755
--- a/Changes
+++ b/Changes
@@ -1,4 +1,4 @@
-0.48 2023-01-27
+0.48 2023-01-30
- Added support for NKJP tagset in annotation
assistant. (diewald)
- Remove deprecated 'auth_support' (since 0.31)
@@ -20,6 +20,8 @@
- Replaced list with info API request for client information
in OAuth registration flow. (diewald)
- Fix tour localization to be overwritable. (diewald)
+ - Make scope a requirement for OAuth authorizations.
+ (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 322019b..bd3c388 100644
--- a/lib/Kalamar/Plugin/Auth.pm
+++ b/lib/Kalamar/Plugin/Auth.pm
@@ -85,6 +85,8 @@
logoutFail => 'Abmeldung fehlgeschlagen',
authenticationFail => 'Nicht authentifiziert',
csrfFail => 'Fehlerhafter CSRF Token',
+ scopeFail => 'Scope nicht definiert',
+ clientIDFail => 'Client ID nicht definiert',
invalidChar => 'Ungültiges Zeichen in Anfrage',
openRedirectFail => 'Weiterleitungsfehler',
tokenExpired => 'Zugriffstoken abgelaufen',
@@ -141,6 +143,8 @@
logoutFail => 'Logout failed',
authenticationFail => 'Not authenticated',
csrfFail => 'Bad CSRF token',
+ scopeFail => 'Scope required',
+ clientIDFail => 'Client ID required',
invalidChar => 'Invalid character in request',
openRedirectFail => 'Redirect failure',
tokenExpired => 'Access token expired',
@@ -1130,13 +1134,22 @@
my $v = $c->validation;
$v->required('client_id');
- $v->optional('scope');
+ $v->required('scope');
$v->optional('state');
$v->optional('redirect_uri');
# Redirect with error
if ($v->has_error) {
- $c->notify(error => $c->loc('Auth_paramError'));
+
+ if ($v->has_error('client_id')) {
+ $c->notify(error => $c->loc('Auth_clientIDFail'));
+ }
+ elsif ($v->has_error('scope')) {
+ $c->notify(error => $c->loc('Auth_scopeFail'));
+ }
+ else {
+ $c->notify(error => $c->loc('Auth_paramError'));
+ };
return $c->redirect_to('oauth-settings');
};
@@ -1196,7 +1209,7 @@
my $v = $c->validation;
$v->csrf_protect;
$v->required('client_id');
- $v->optional('scope');
+ $v->required('scope');
$v->optional('state');
$v->optional('redirect_uri');
@@ -1207,7 +1220,13 @@
if ($v->has_error) {
my $url = Mojo::URL->new($v->param('redirect_uri_server') || $c->url_for('index'));
- if ($v->has_error('csrf_token')) {
+ if ($v->has_error('client_id')) {
+ $url->query([error_description => $c->loc('Auth_clientIDFail')]);
+ }
+ elsif ($v->has_error('scope')) {
+ $url->query([error_description => $c->loc('Auth_scopeFail')]);
+ }
+ elsif ($v->has_error('csrf_token')) {
$url->query([error_description => $c->loc('Auth_csrfFail')]);
}
else {
diff --git a/t/plugin/auth-oauth.t b/t/plugin/auth-oauth.t
index b07645e..ba1f95d 100644
--- a/t/plugin/auth-oauth.t
+++ b/t/plugin/auth-oauth.t
@@ -769,7 +769,16 @@
;
$t->get_ok('/settings/oauth/')
- ->text_is('div.notify-error', 'Some fields are invalid')
+ ->text_is('div.notify-error', 'Client ID required')
+ ;
+
+$t->get_ok(Mojo::URL->new('/settings/oauth/authorize?client_id=xyz'))
+ ->status_is(302)
+ ->header_is('location','/settings/oauth')
+ ;
+
+$t->get_ok('/settings/oauth/')
+ ->text_is('div.notify-error', 'Scope required')
;
# OAuth client authorization flow
@@ -779,6 +788,16 @@
;
$t->get_ok('/settings/oauth/')
+ ->text_is('div.notify-error', 'Scope required')
+ ;
+
+# OAuth client authorization flow
+$t->get_ok(Mojo::URL->new('/settings/oauth/authorize?client_id=abc&scope=match'))
+ ->status_is(302)
+ ->header_is('location','/settings/oauth')
+ ;
+
+$t->get_ok('/settings/oauth/')
->text_is('div.notify-error', 'Unknown client with abc.')
;