From d66afffc1570130a72ba58c443f7c9bcc8312fbb Mon Sep 17 00:00:00 2001 From: Philippe Pittoli Date: Wed, 14 Jun 2023 01:46:38 +0200 Subject: [PATCH] Errors now have dedicated messages. --- src/requests/admin.cr | 6 ++--- src/requests/delete.cr | 4 +-- src/requests/moduser.cr | 4 +-- src/requests/password.cr | 4 +-- src/requests/permissions.cr | 8 +++--- src/requests/profile.cr | 4 +-- src/requests/register.cr | 18 +++++++------ src/requests/search.cr | 2 +- src/requests/users.cr | 6 ++--- src/responses/errors.cr | 54 +++++++++++++++++++++++++++++++++++++ 10 files changed, 83 insertions(+), 27 deletions(-) diff --git a/src/requests/admin.cr b/src/requests/admin.cr index 82e799c..57a49f0 100644 --- a/src/requests/admin.cr +++ b/src/requests/admin.cr @@ -11,18 +11,18 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? logged_user.assert_permission("authd", "*", User::PermissionLevel::Admin) if authd.users_per_login.get? @login - return Response::Error.new "login already used" + return Response::ErrorAlreadyUsedLogin.new end # No verification of the user's informations when an admin adds it. # No mail address verification. if authd.configuration.require_email && @email.nil? - return Response::Error.new "email required" + return Response::ErrorMailRequired.new end password_hash = authd.hash_password @password diff --git a/src/requests/delete.cr b/src/requests/delete.cr index 9a4fc7f..ce08795 100644 --- a/src/requests/delete.cr +++ b/src/requests/delete.cr @@ -8,7 +8,7 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? user_to_delete = if u = @user logged_user.assert_permission("authd", "*", User::PermissionLevel::Admin) @@ -16,7 +16,7 @@ class AuthD::Request else logged_user end - return Response::Error.new "unknown user" if user_to_delete.nil? + return Response::ErrorUserNotFound.new if user_to_delete.nil? # User or admin is now verified: let's proceed with the user deletion. authd.users_per_login.delete user_to_delete.login diff --git a/src/requests/moduser.cr b/src/requests/moduser.cr index 7a0d88d..b61c475 100644 --- a/src/requests/moduser.cr +++ b/src/requests/moduser.cr @@ -10,7 +10,7 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? user = if u = @user logged_user.assert_permission("authd", "*", User::PermissionLevel::Edit) @@ -18,7 +18,7 @@ class AuthD::Request else logged_user end - return Response::Error.new "user not found" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? # Only an admin can uprank or downrank someone. if admin = @admin diff --git a/src/requests/password.cr b/src/requests/password.cr index 1611200..bccff42 100644 --- a/src/requests/password.cr +++ b/src/requests/password.cr @@ -9,7 +9,7 @@ class AuthD::Request user = authd.user? @user # This is a way for an attacker to know what are the valid logins. # Not sure I care enough to fix this. - return Response::Error.new "user not found" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? # Create a new random key for password renewal. user.password_renew_key = UUID.random.to_s @@ -58,7 +58,7 @@ class AuthD::Request user = authd.user? @user # This is a way for an attacker to know what are the valid logins. # Not sure I care enough to fix this. - return Response::Error.new "user not found" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? if user.password_renew_key == @password_renew_key user.password_hash = authd.hash_password @new_password diff --git a/src/requests/permissions.cr b/src/requests/permissions.cr index d45c52c..13caaf6 100644 --- a/src/requests/permissions.cr +++ b/src/requests/permissions.cr @@ -9,7 +9,7 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? user = if u = @user logged_user.assert_permission("authd", "*", User::PermissionLevel::Read) @@ -17,7 +17,7 @@ class AuthD::Request else logged_user end - return Response::Error.new "no such user" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? service_permissions = user.permissions[@service]? resource_permissions = if service_permissions.nil? @@ -44,7 +44,7 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? logged_user.assert_permission("authd", "*", User::PermissionLevel::Admin) user = if u = @user @@ -52,7 +52,7 @@ class AuthD::Request else logged_user end - return Response::Error.new "no such user" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? service_permissions = user.permissions[@service]? diff --git a/src/requests/profile.cr b/src/requests/profile.cr index 216cfd8..579a81f 100644 --- a/src/requests/profile.cr +++ b/src/requests/profile.cr @@ -10,7 +10,7 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? user = if u = @user logged_user.assert_permission("authd", "*", User::PermissionLevel::Edit) @@ -18,7 +18,7 @@ class AuthD::Request else logged_user end - return Response::Error.new "user not found" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? new_profile_entries = user.profile || Hash(String, JSON::Any).new diff --git a/src/requests/register.cr b/src/requests/register.cr index 0314c22..94aff82 100644 --- a/src/requests/register.cr +++ b/src/requests/register.cr @@ -9,16 +9,20 @@ class AuthD::Request end def handle(authd : AuthD::Service, fd : Int32) - if ! authd.configuration.registrations - return Response::Error.new "registrations not allowed" + unless authd.configuration.registrations + return Response::ErrorRegistrationsClosed.new end if authd.users_per_login.get? @login - return Response::Error.new "login already used" + return Response::ErrorAlreadyUsedLogin.new end + acceptable_login_regex = "[a-zA-Z][a-zA-Z0-9 _-']+" + pattern = Regex.new acceptable_login_regex, Regex::Options::IGNORE_CASE + return Response::ErrorInvalidLoginFormat.new unless pattern =~ @login + if authd.configuration.require_email && @email.nil? - return Response::Error.new "email required" + return Response::ErrorMailRequired.new end if ! @email.nil? @@ -28,14 +32,12 @@ class AuthD::Request email = result["email"]? if email.nil? - return Response::Error.new "invalid email format" + return Response::ErrorInvalidEmailFormat.new end end # In this case we should not accept its registration. - if @password.size < 20 - return Response::Error.new "password too short (< 20 characters)" - end + return Response::ErrorPasswordTooShort.new if @password.size < 20 uid = authd.new_uid password = authd.hash_password @password diff --git a/src/requests/search.cr b/src/requests/search.cr index 0a5c947..a4e9caf 100644 --- a/src/requests/search.cr +++ b/src/requests/search.cr @@ -11,7 +11,7 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user_full? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? logged_user.assert_permission("authd", "*", User::PermissionLevel::Read) users = authd.users.to_a diff --git a/src/requests/users.cr b/src/requests/users.cr index 9c50976..c2fb417 100644 --- a/src/requests/users.cr +++ b/src/requests/users.cr @@ -10,7 +10,7 @@ class AuthD::Request user = authd.user? @user # This is a way for an attacker to know what are the valid logins. # Not sure I care enough to fix this. - return Response::Error.new "user not found" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? if user.contact.activation_key.nil? return Response::Error.new "user already validated" @@ -38,12 +38,12 @@ class AuthD::Request def handle(authd : AuthD::Service, fd : Int32) logged_user = authd.get_logged_user? fd - return Response::Error.new "you must be logged" if logged_user.nil? + return Response::ErrorMustBeAuthenticated.new if logged_user.nil? user = authd.user? @user # This is a way for an attacker to know what are the valid logins. # Not sure I care enough to fix this. - return Response::Error.new "user not found" if user.nil? + return Response::ErrorUserNotFound.new if user.nil? Response::User.new user.to_public end diff --git a/src/responses/errors.cr b/src/responses/errors.cr index a8fcd02..7a735eb 100644 --- a/src/responses/errors.cr +++ b/src/responses/errors.cr @@ -5,4 +5,58 @@ class AuthD::Response end end AuthD.responses << Error + + IPC::JSON.message ErrorMustBeAuthenticated, 20 do + def initialize() + end + end + AuthD.responses << ErrorMustBeAuthenticated + + IPC::JSON.message ErrorAlreadyUsedLogin, 21 do + def initialize() + end + end + AuthD.responses << ErrorAlreadyUsedLogin + + IPC::JSON.message ErrorMailRequired, 22 do + def initialize() + end + end + AuthD.responses << ErrorMailRequired + + IPC::JSON.message ErrorUserNotFound, 23 do + def initialize() + end + end + AuthD.responses << ErrorUserNotFound + + IPC::JSON.message ErrorPasswordTooShort, 24 do + def initialize() + end + end + AuthD.responses << ErrorPasswordTooShort + + IPC::JSON.message ErrorInvalidCredentials, 25 do + def initialize() + end + end + AuthD.responses << ErrorInvalidCredentials + + IPC::JSON.message ErrorRegistrationsClosed, 26 do + def initialize() + end + end + AuthD.responses << ErrorRegistrationsClosed + + IPC::JSON.message ErrorInvalidLoginFormat, 27 do + def initialize() + end + end + AuthD.responses << ErrorInvalidLoginFormat + + IPC::JSON.message ErrorInvalidEmailFormat, 28 do + def initialize() + end + end + AuthD.responses << ErrorInvalidEmailFormat end