From f819b178c5c1cb8adda0182c610e5c52fad8bea7 Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Sun, 1 Nov 2015 21:35:57 -0500 Subject: [PATCH] Overhaul everything to get consistent error reports. --- bin/mailshears | 44 +++++++++++++++++---- doc/TODO | 41 ++++++-------------- lib/common/agendav_plugin.rb | 17 +++------ lib/common/davical_plugin.rb | 7 ++-- lib/common/domain.rb | 23 +++++++++++ lib/common/dovecot_plugin.rb | 55 +++++++++------------------ lib/common/errors.rb | 10 ++++- lib/common/plugin.rb | 32 ++++++++++++---- lib/common/postfixadmin_plugin.rb | 14 ++++--- lib/common/roundcube_plugin.rb | 7 ++-- lib/common/user.rb | 63 +++++++++++++++++++++++++++++++ lib/mv/mv_dummy_runner.rb | 10 +++-- lib/mv/mv_plugin.rb | 2 +- lib/mv/mv_runner.rb | 58 +++++++++++++++++----------- lib/mv/plugins/agendav.rb | 13 ++++--- lib/mv/plugins/davical.rb | 18 +++++---- lib/mv/plugins/dovecot.rb | 57 ++++++++++++++++------------ lib/mv/plugins/postfixadmin.rb | 29 +++++++------- lib/mv/plugins/roundcube.rb | 14 ++++--- lib/prune/prune_dummy_runner.rb | 18 +++------ lib/prune/prune_runner.rb | 21 +++-------- lib/rm/plugins/agendav.rb | 4 +- lib/rm/plugins/davical.rb | 4 +- lib/rm/plugins/dovecot.rb | 12 +++++- lib/rm/plugins/postfixadmin.rb | 10 ++--- lib/rm/plugins/roundcube.rb | 2 +- lib/rm/rm_dummy_runner.rb | 17 ++++++--- lib/rm/rm_plugin.rb | 18 +++++++-- lib/rm/rm_runner.rb | 51 +++++++++++-------------- 29 files changed, 403 insertions(+), 268 deletions(-) create mode 100644 lib/common/domain.rb create mode 100644 lib/common/user.rb diff --git a/bin/mailshears b/bin/mailshears index 4d69f8f..2c91f17 100755 --- a/bin/mailshears +++ b/bin/mailshears @@ -30,16 +30,27 @@ end require 'mailshears' # Since we removed both the executable name and the mode name (if it -# existed) from ARGV, what remains should be the required -# arguments. -if (mode == :prune and (ARGV.length() != 0)) or - (mode == :rm and (ARGV.length() < 1)) or - (mode == :mv and (ARGV.length() != 2)) then - puts "ERROR: missing (or extra) command-line arguments." +# existed) from ARGV, what remains should be the required arguments. +# Figure out if we have the wrong number of arguments, and store the +# associated error message in args_error_message if we do. +args_error_message = nil +if mode == :prune and ARGV.length() != 0 then + args_error_message = "ERROR: prune mode takes no additional arguments." +elsif mode == :rm and ARGV.length() < 1 then + args_error_message = "ERROR: rm mode takes two or more user arguments." +elsif mode == :mv and ARGV.length() != 2 then + args_error_message = "ERROR: mv mode takes exactly two user arguments." +end + +# If we got the wrong number of arguments, we'll have some error +# message. Report it and quit. +if not args_error_message.nil? then + STDERR.puts args_error_message puts "Usage: #{usage}" Kernel.exit(ExitCodes::BAD_COMMAND_LINE) end + cfg = Configuration.new() # Load each of the plugins that we'll need. @@ -84,15 +95,32 @@ else plugin_module = PrunePlugin end - # Buffer the output so that we can avoid printing the informational # header when no plugins produce output. require 'stringio' output_buffer = StringIO.new() $stdout = output_buffer +# Parse the remaining arguments as User/Domain objects. If we get some +# other argument that isn't one of those, it's an error. +parsed_args = [] +ARGV.each do |arg| + begin + u = User.new(arg) + parsed_args << u + rescue InvalidUserError + begin + d = Domain.new(arg) + parsed_args << d + rescue InvalidDomainError + STDERR.puts "ERROR: invalid user/domain argument #{arg}" + Kernel.exit(ExitCodes::BAD_COMMAND_LINE) + end + end +end + begin - plugin_module.run(cfg, *ARGV) + plugin_module.run(cfg, *parsed_args) ensure # Now restore stdout, and print the header plus whatever the plugins # produced if they produced anything. If they didn't produce any diff --git a/doc/TODO b/doc/TODO index 8482260..3a2ebfe 100644 --- a/doc/TODO +++ b/doc/TODO @@ -1,29 +1,12 @@ -* Error reporting sucks, and when a domain or user doesn't exist we - should be able to say so. The describe_domain/user functions - should also work better. For plugins that don't implement domains, - we can return a (count of?) list of users, or fall back to the - user deletion descriptions. - -* Error reporting is inconsistent. We know what goes wrong, and then - errors bubble up, but where do they get reported? Are they fatal or - informational? Do we want later plugins to run of earlier ones - failed? - -* Implement "mv". - - Design: - - This should only work from a domain that exists in postfixadmin to a - domain that already exists in postfixadmin. If the target domain - does not exist, we should error out as soon as possible. - - Once we're sure that the target domain does exist, we can hand off - the "mv" operation to the plugins. Some of them might have nothing - to do -- that's fine. - - MAKE SURE WE DON'T OVERWRITE AN EXISTING USER! - -* Add convenience methods (e.g. user_exists, domain_exists) that we - can use in the tests to check results. - -* Potentially add OpenDKIM support. +* There is essentially no error handling. We report errors, but we + don't fail when we see one. The main reason for this is that we + don't know when each plugin will be run. If the first plugin + encounters an error, we could quit right there. But what if the + third one fails after the first two succeed? We would need some kind + of rollback mechanism. + + For "mv", a rollback is conceivable. But with "rm", there's no going + back. Maybe relying on the user to interpret the output and go + fix stuff himself is the best we can do? + +* Add OpenDKIM support. diff --git a/lib/common/agendav_plugin.rb b/lib/common/agendav_plugin.rb index ae9b11c..fc19d3d 100644 --- a/lib/common/agendav_plugin.rb +++ b/lib/common/agendav_plugin.rb @@ -1,4 +1,5 @@ require 'common/plugin' +require 'common/user' module AgendavPlugin # Code that all Agendav plugins (Prune, Rm, Mv...) will @@ -18,17 +19,11 @@ module AgendavPlugin def describe_domain(domain) - # AgenDAV doesn't have a concept of domains. - return domain + return domain.to_s() end - def describe_user(user) - if self.user_exists(user) - return "Username: #{user}" - else - return 'User not found' - end + return user.to_s() end @@ -37,7 +32,7 @@ module AgendavPlugin # Produce a list of AgenDAV users. This is public because it's # useful in testing. # - usernames = [] + users = [] # Just assume PostgreSQL for now. begin @@ -54,7 +49,7 @@ module AgendavPlugin sql_query += '(SELECT user_from FROM shared);' connection.query(sql_query) do |result| - usernames = result.field_values('username') + users = result.field_values('username') end connection.close() @@ -63,7 +58,7 @@ module AgendavPlugin raise DatabaseError.new(e) end - return usernames + return users.map{ |u| User.new(u) } end end diff --git a/lib/common/davical_plugin.rb b/lib/common/davical_plugin.rb index 8c9646f..557c1dd 100644 --- a/lib/common/davical_plugin.rb +++ b/lib/common/davical_plugin.rb @@ -1,4 +1,5 @@ require 'common/plugin' +require 'common/user' module DavicalPlugin # Code that all Davical plugins (Prune, Rm, Mv...) will share. That @@ -18,7 +19,7 @@ module DavicalPlugin def describe_domain(domain) # DAViCal doesn't have a concept of domains. - return domain + return domain.to_s() end @@ -62,7 +63,7 @@ module DavicalPlugin raise DatabaseError.new(e) end - return usernames + return usernames.map{ |u| User.new(u) } end @@ -85,7 +86,7 @@ module DavicalPlugin sql_query += " ON principal.user_no = usr.user_no) " sql_query += "WHERE usr.username = $1;" - connection.query(sql_query, [user]) do |result| + connection.query(sql_query, [user.to_s()]) do |result| if result.num_tuples > 0 principal_id = result[0]['principal_id'] end diff --git a/lib/common/domain.rb b/lib/common/domain.rb new file mode 100644 index 0000000..5fb345b --- /dev/null +++ b/lib/common/domain.rb @@ -0,0 +1,23 @@ +require 'common/errors' + +class Domain + + @domain = nil + + def initialize(domain) + if domain.empty? then + msg = "domain cannot be empty" + raise InvalidDomainError.new(msg) + end + + @domain = domain + end + + def to_s() + return @domain + end + + def ==(other) + return self.to_s() == other.to_s() + end +end diff --git a/lib/common/dovecot_plugin.rb b/lib/common/dovecot_plugin.rb index dfd8749..4718483 100644 --- a/lib/common/dovecot_plugin.rb +++ b/lib/common/dovecot_plugin.rb @@ -1,5 +1,8 @@ -require 'common/plugin' +require 'common/domain' require 'common/filesystem' +require 'common/plugin' +require 'common/user' + module DovecotPlugin # Code that all Dovecot plugins (Prune, Rm, Mv...) will @@ -29,55 +32,31 @@ module DovecotPlugin end + def domain_exists(domain) + domains = list_domains() + return domains.include?(domain) + end + + protected; def get_domain_path(domain) # Return the filesystem path for the given domain. # That is, the directory where its mail is stored. # Only works if the domain directory exists! - domain_path = File.join(@domain_root, domain) - - if File.directory?(domain_path) - return domain_path - else - raise NonexistentDomainError.new(domain) - end + return File.join(@domain_root, domain.to_s()) end - def get_user_path(user, existence_check = true) - # Return the filesystem path of this user's mailbox. With - # existence_check = true, it only works if the user exists! - # We always require that the domain exists. - if not user.include?('@') - msg = "#{user}: Users must contain an '@' symbol." - raise InvalidUserError.new(msg) - end - - user_parts = user.split('@') - local_part = user_parts[0] - domain_part = user_parts[1] - - begin - domain_path = get_domain_path(domain_part) - rescue NonexistentDomainError - raise NonexistentUserError.new(user) - end - - user_path = File.join(domain_path, local_part) - - return user_path if not existence_check - - if File.directory?(user_path) - return user_path - else - raise NonexistentUserError.new(user) - end + def get_user_path(user) + # Return the filesystem path of this user's mailbox. + domain_path = get_domain_path(user.domain()) + return File.join(domain_path, user.localpart()) end def list_domains() - return Filesystem.get_subdirs(@domain_root) + return Filesystem.get_subdirs(@domain_root).map{ |d| Domain.new(d) } end def list_domains_users(domains) @@ -92,7 +71,7 @@ module DovecotPlugin usernames = Filesystem.get_subdirs(domain_path) usernames.each do |username| - users << "#{username}@#{domain}" + users << User.new("#{username}@#{domain}") end rescue NonexistentDomainError # Party hard. diff --git a/lib/common/errors.rb b/lib/common/errors.rb index bcae691..7a2fbba 100644 --- a/lib/common/errors.rb +++ b/lib/common/errors.rb @@ -8,12 +8,20 @@ end class InvalidUserError < StandardError end +# Domain is syntactically invalid. +class InvalidDomainError < StandardError +end + # Used to indicate that an user does not exist. class NonexistentUserError < StandardError end - # Used to indicate that a domain does not exist. class NonexistentDomainError < StandardError end + + +# When you try to rename a user on top of one that already exists. +class UserAlreadyExistsError < StandardError +end diff --git a/lib/common/plugin.rb b/lib/common/plugin.rb index 272f0df..c6f750b 100644 --- a/lib/common/plugin.rb +++ b/lib/common/plugin.rb @@ -1,3 +1,6 @@ +require 'common/domain' +require 'common/user' + # All plugins should include this module. It defines the basic # operations that all plugins are supposed to support. module Plugin @@ -26,7 +29,7 @@ module Plugin end def dummy_runner() - # The RummyRunner associated with this plugin. + # The DummyRunner associated with this plugin. raise NotImplementedError end @@ -48,6 +51,18 @@ module Plugin end end + def describe(target) + # A generic version of describe_user/describe_domain that + # dispatches base on the class of the target. + if target.is_a?(User) + return describe_user(target) + elsif target.is_a?(Domain) + return describe_domain(target) + else + raise NotImplementedError + end + end + def describe_domain(domain) # Provide a "description" of the domain. This is output along # with the domain name and can be anything of use to the system @@ -67,27 +82,28 @@ module Plugin raise NotImplementedError end - def user_exists(username) + def user_exists(user) # Does the given username exist for this plugin? We use a naive # implementation here based on list_users() which is required to # exist above. Plugins can override this with something fast. users = list_users() - return users.include?(username) + return users.include?(user) end def list_domains_users(domains) - # Get all usernames belonging to the given domains. If a username - # ends in @example.com, it belongs to the domain example.com + # Get all users belonging to the given domains. If a user has + # domainpart "example.com" then it belongs to the domain + # "example.com". # # This uses a naive loop, but relies only on the existence of a # list_users() method which is guaranteed above. More efficient # implementations can usually be made within the plugin. domains_users = [] - usernames = list_users(); + users = list_users(); domains.each do |d| - matches = usernames.select do |username| - username =~ /@#{d}$/ + matches = users.select do |user| + user.domainpart() == d.to_s() end domains_users += matches diff --git a/lib/common/postfixadmin_plugin.rb b/lib/common/postfixadmin_plugin.rb index 97d59f2..60880d9 100644 --- a/lib/common/postfixadmin_plugin.rb +++ b/lib/common/postfixadmin_plugin.rb @@ -1,4 +1,6 @@ +require 'common/domain' require 'common/plugin' +require 'common/user' require 'pg' module PostfixadminPlugin @@ -19,13 +21,13 @@ module PostfixadminPlugin def describe_user(user) # There's no other unique identifier in PostfixAdmin - return user + return user.to_s() end def describe_domain(domain) # There's no other unique identifier in PostfixAdmin - return domain + return domain.to_s() end @@ -53,7 +55,7 @@ module PostfixadminPlugin raise DatabaseError.new(e) end - return domains + return domains.map{ |d| Domain.new(d) } end @@ -89,7 +91,7 @@ module PostfixadminPlugin raise DatabaseError.new(e) end - return users + return users.map{ |u| User.new(u) } end @@ -108,7 +110,7 @@ module PostfixadminPlugin sql_query = 'SELECT username FROM mailbox WHERE domain IN $1;' - connection.query(sql_query, [domains]) do |result| + connection.query(sql_query, domains.map{|d| d.to_s()}) do |result| usernames = result.field_values('username') end @@ -118,7 +120,7 @@ module PostfixadminPlugin raise DatabaseError.new(e) end - return usernames + return usernames.map{ |u| User.new(u) } end diff --git a/lib/common/roundcube_plugin.rb b/lib/common/roundcube_plugin.rb index 1f9805a..b1c74a6 100644 --- a/lib/common/roundcube_plugin.rb +++ b/lib/common/roundcube_plugin.rb @@ -1,4 +1,5 @@ require 'common/plugin' +require 'common/user' module RoundcubePlugin # Code that all Roundcube plugins (Prune, Rm, Mv...) will share. @@ -18,7 +19,7 @@ module RoundcubePlugin def describe_domain(domain) # Roundcube doesn't have a concept of domains. - return domain + return domain.to_s() end def describe_user(user) @@ -58,7 +59,7 @@ module RoundcubePlugin raise DatabaseError.new(e) end - return usernames + return usernames.map{ |u| User.new(u) } end protected; @@ -77,7 +78,7 @@ module RoundcubePlugin sql_query = "SELECT user_id FROM users WHERE username = $1;" - connection.query(sql_query, [user]) do |result| + connection.query(sql_query, [user.to_s()]) do |result| if result.num_tuples > 0 user_id = result[0]['user_id'] end diff --git a/lib/common/user.rb b/lib/common/user.rb new file mode 100644 index 0000000..bb239d1 --- /dev/null +++ b/lib/common/user.rb @@ -0,0 +1,63 @@ +require 'common/domain' +require 'common/errors' + +class User + # A class representing a syntactically valid user; that is, an email + # address. Once constructed, you can be sure that it's valid. + + @localpart = nil + @domain = nil + + def domain() + return @domain + end + + def domainpart() + return @domain.to_s() + end + + def initialize(username) + # Initialize this user object, but throw an error if either the + # localpart or domainpart are invalid. The one argument is an + # email address string. + if not username.is_a?(String) + msg = 'username must be a String ' + msg += "but a #{username.class.to_s()} was given" + raise InvalidUserError.new(msg) + end + + parts = username.split('@') + + if parts.length() < 2 then + msg = "the username #{username} does not contain an '@' symbol" + raise InvalidUserError.new(msg) + end + + localpart = parts[0] + + if localpart.length() > 64 then + msg = "the local part of #{username} cannot have more than 64 characters" + raise InvalidUserError(msg) + end + + if localpart.empty? then + msg = "the local part of #{username} cannot be empty" + raise InvalidUserError.new(msg) + end + + @localpart = localpart + @domain = Domain.new(parts[1]) + end + + def localpart() + return @localpart + end + + def to_s() + return @localpart + '@' + @domain.to_s() + end + + def ==(other) + return self.to_s() == other.to_s() + end +end diff --git a/lib/mv/mv_dummy_runner.rb b/lib/mv/mv_dummy_runner.rb index 11f2ff5..662abff 100644 --- a/lib/mv/mv_dummy_runner.rb +++ b/lib/mv/mv_dummy_runner.rb @@ -4,11 +4,13 @@ class MvDummyRunner include Runner def run(plugin, src, dst) - if src.include?('@') then - puts "Would move user: #{src} to #{dst}" - else - raise NotImplementedError.new('Only users can be moved.') + + if src.is_a?(Domain) or dst.is_a?(Domain) then + msg = 'only users can be moved' + raise NotImplementedError.new(msg) end + + puts "Would move user #{src.to_s()} to #{dst.to_s()}." end end diff --git a/lib/mv/mv_plugin.rb b/lib/mv/mv_plugin.rb index 29f697c..089e6c7 100644 --- a/lib/mv/mv_plugin.rb +++ b/lib/mv/mv_plugin.rb @@ -13,7 +13,7 @@ module MvPlugin return MvDummyRunner end - def mv_user(from, to) + def mv_user(src, dst) # Rename the given user. raise NotImplementedError end diff --git a/lib/mv/mv_runner.rb b/lib/mv/mv_runner.rb index ee762e1..6da6f6d 100644 --- a/lib/mv/mv_runner.rb +++ b/lib/mv/mv_runner.rb @@ -1,3 +1,4 @@ +require 'common/domain' require 'common/errors' require 'common/runner' @@ -5,35 +6,50 @@ class MvRunner include Runner def run(plugin, src, dst) - # Why think too hard? An user has an @, a domain doesn't. - if not src.include?('@') then - # We only support moving users right now, and the destination - # domain must already exist. - raise NotImplementedError.new('Only users can be moved.') - end - # Handle this once so we don't have to do it in every plugin. - if not dst.include?('@') then - msg = "the destination user #{dst} is not valid" - raise InvalidUserError.new(msg) + if src.is_a?(Domain) or dst.is_a?(Domain) then + msg = 'only users can be moved' + raise NotImplementedError.new(msg) end begin - src_description = plugin.describe_user(src) + src_description = plugin.describe(src) plugin.mv_user(src, dst) - msg = "Moved user #{src} (#{src_description}) " - msg += "to #{dst}." + dst_description = plugin.describe(dst) + + msg = "Moved user #{src} " + + # Only append the extra description if it's useful. + if not src_description.nil? and + not src_description.empty? and + not src_description == src.to_s() then + msg += "(#{src_description}) " + end + + msg += "to #{dst}" + + # Only append the extra description if it's useful. + if not dst_description.nil? and + not dst_description.empty? and + not dst_description == dst.to_s() then + msg += " (#{dst_description})" + end + + msg += "." report(plugin, msg) - rescue InvalidUserError => e - report(plugin, e.message) + rescue NonexistentUserError => e - report(plugin, e.message) + # This means that the SOURCE user didn't exist, since a + # nonexistent destination user is perfectly expected. + report(plugin, "Source user #{src.to_s()} not found.") rescue NonexistentDomainError => e - report(plugin, e.message) - rescue StandardError => e - msg = "There was an error (#{e.to_s}) moving the user: #{e.message}" - report(plugin, msg) - Kernel.exit(ExitCodes::DATABASE_ERROR) + # This could mean that the source domain doesn't exist, but in + # that case, we just report that the source user doesn't + # exist. So a nonexistent domain refers to a nonexistent + # DESTINATION domain. + report(plugin, "Destination domain #{dst.domainpart()} not found.") + rescue UserAlreadyExistsError => e + report(plugin, "Destination user #{dst.to_s()} already exists.") end end diff --git a/lib/mv/plugins/agendav.rb b/lib/mv/plugins/agendav.rb index 9c508c9..0324e6d 100644 --- a/lib/mv/plugins/agendav.rb +++ b/lib/mv/plugins/agendav.rb @@ -8,12 +8,15 @@ class AgendavMv include AgendavPlugin include MvPlugin + def mv_user(src, dst) + # It's obviously an error if the source user does not exist. It + # would also be an error if the destination domain didn't exist; + # however, Agendav doesn't know about domains, so we let that slide. + raise NonexistentUserError.new(src.to_s()) if not user_exists(src) - def mv_domain(from, to) - # AgenDAV doesn't have a concept of domains. - end + # And it's an error if the destination user exists already. + raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) - def mv_user(from, to) sql_queries = ['UPDATE prefs SET username = $1 WHERE username = $2;'] sql_queries << 'UPDATE shared SET user_from = $1 WHERE user_from = $2;' sql_queries << 'UPDATE shared SET user_which = $1 WHERE user_which = $2;' @@ -28,7 +31,7 @@ class AgendavMv @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [to, from]) + connection.query(sql_query, [dst.to_s(), src.to_s()]) end connection.close() diff --git a/lib/mv/plugins/davical.rb b/lib/mv/plugins/davical.rb index a7cfe80..bd66df8 100644 --- a/lib/mv/plugins/davical.rb +++ b/lib/mv/plugins/davical.rb @@ -12,15 +12,19 @@ class DavicalMv include MvPlugin - def mv_domain(from, to) - # DAViCal doesn't have a concept of domains. - end - - - def mv_user(from, to) + def mv_user(src, dst) # Switch the given usernames. DAViCal uses foreign keys properly # and only supports postgres, so we let the ON UPDATE CASCADE # trigger handle most of the work. + + # It's obviously an error if the source user does not exist. It + # would also be an error if the destination domain didn't exist; + # however, DAViCal doesn't know about domains, so we let that slide. + raise NonexistentUserError.new(src.to_s()) if not user_exists(src) + + # And it's an error if the destination user exists already. + raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) + sql_queries = ['UPDATE usr SET username = $1 WHERE username = $2'] begin @@ -33,7 +37,7 @@ class DavicalMv @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [to, from]) + connection.query(sql_query, [dst.to_s(), src.to_s()]) end connection.close() diff --git a/lib/mv/plugins/dovecot.rb b/lib/mv/plugins/dovecot.rb index 9cdfc36..bd3084e 100644 --- a/lib/mv/plugins/dovecot.rb +++ b/lib/mv/plugins/dovecot.rb @@ -9,36 +9,43 @@ class DovecotMv include DovecotPlugin include MvPlugin - def mv_user(user_from, user_to) - # Move the user's mail directory from one domain (that should - # exist) to another domain (that should exist). + def mv_user(src, dst) + # It's obviously an error if the source user does not exist. + raise NonexistentUserError.new(src.to_s()) if not user_exists(src) + + # And it's an error if the destination user exists already. + raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) + + # But is it an error if the target domain does not exist? That's a + # bit subtle... The domain may exist in the database, but if it + # has not received any mail yet, then its directory won't exist + # on-disk. + # + # There are two possible "oops" scenarios resulting from the fact + # that we may run either the Postfixadmin move first or the + # Dovecot move first. If we move the user in the database, we + # definitely want to move him on disk (that is, we should create + # the directory here). But if we move him on disk first, then we + # don't know if the database move will fail! We don't want to move + # his mail files if he won't get moved in the database. # - # This is actually a little subtle, because none of the - # directories in question have to exist. If I create two new - # domains in postfixadmin and one mailbox but never deliver any - # mail, it should be possible to move the mailbox to the other - # domain. No directories will exist until mail is delivered, - # though. - - # get_user_path() will fail if there's no mailbox to move. It will - # also fail on an invalid user -- oh well! - begin - from_path = self.get_user_path(user_from) - rescue NonexistentUserError - # Do nothing, there's no source mailbox... - return + # Faced with two equally-bad (but easy-to-fix) options, we do the + # simplest thing and fail if the destination domain directory + # doesn't exist. If nothing else, this is at least consistent. + if not self.domain_exists(dst.domain()) then + raise NonexistentDomainError.new(dst.domainpart()) end # We may need to create the target domain directory, even if the # domain exists in the database. - domain_to = user_to.split('@')[1] - domain_dir = self.get_domain_path(domain_to) - FileUtils.mkdir_p(domain_dir) - - # The from_path exists or else we would have returned earlier. The - # parent of to_path exists because we just created it. - to_path = self.get_user_path(user_to, false) - FileUtils.mv(from_path, to_path) + FileUtils.mkdir_p(self.get_domain_path(dst.domain())) + + # The parent of dst_path exists because we just created it.The + # source path should exist too, because the "source user" does, + # and, well, how did we determine that? + src_path = self.get_user_path(src) + dst_path = self.get_user_path(dst) + FileUtils.mv(src_path, dst_path) end end diff --git a/lib/mv/plugins/postfixadmin.rb b/lib/mv/plugins/postfixadmin.rb index 3b6d188..338be05 100644 --- a/lib/mv/plugins/postfixadmin.rb +++ b/lib/mv/plugins/postfixadmin.rb @@ -8,20 +8,19 @@ class PostfixadminMv include PostfixadminPlugin include MvPlugin - def mv_user(user_from, user_to) - if not user_exists(user_from) - raise NonexistentUserError.new("source user #{user_from} does not exist") - end - - user_to_parts = user_to.split('@') - localpart_to = user_to_parts[0] - domain_to = user_to_parts[1] + def mv_user(src, dst) + # It's obviously an error if the source user does not exist. + raise NonexistentUserError.new(src.to_s()) if not user_exists(src) - if not domain_exists(domain_to) - msg = "destination domain #{domain_to} does not exist" - raise NonexistentDomainError.new(msg) + # And if the destination domain does not exist. + if not domain_exists(dst.domain()) + raise NonexistentDomainError.new(dst.domain.to_s()) end + # But the destination *user* should *not* exist. + # And it's an error if the destination user exists already. + raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) + mailbox_query = 'UPDATE mailbox SET ' mailbox_query += ' username=$1,' mailbox_query += ' domain=$2,' @@ -51,10 +50,10 @@ class PostfixadminMv sql_queries.each do |sql_query| varchar = 1043 # from pg_type.h - params = [{:value => user_to, :type => varchar}, - {:value => domain_to, :type => varchar}, - {:value => localpart_to, :type => varchar}, - {:value => user_from, :type => varchar}] + params = [{:value => dst.to_s(), :type => varchar}, + {:value => dst.domainpart(), :type => varchar}, + {:value => dst.localpart(), :type => varchar}, + {:value => src.to_s(), :type => varchar}] connection.query(sql_query, params) end diff --git a/lib/mv/plugins/roundcube.rb b/lib/mv/plugins/roundcube.rb index bac8344..4b3ebcb 100644 --- a/lib/mv/plugins/roundcube.rb +++ b/lib/mv/plugins/roundcube.rb @@ -9,11 +9,15 @@ class RoundcubeMv include MvPlugin - def mv_domain(from, to) - # Roundcube doesn't have a concept of domains. - end + def mv_user(src, dst) + # It's obviously an error if the source user does not exist. It + # would also be an error if the destination domain didn't exist; + # however, Roundcube doesn't know about domains, so we let that slide. + raise NonexistentUserError.new(src.to_s()) if not user_exists(src) + + # And it's an error if the destination user exists already. + raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) - def mv_user(from, to) sql_queries = ['UPDATE users SET username = $1 WHERE username = $2;'] begin @@ -26,7 +30,7 @@ class RoundcubeMv @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [to, from]) + connection.query(sql_query, [dst.to_s(), src.to_s()]) end connection.close() diff --git a/lib/prune/prune_dummy_runner.rb b/lib/prune/prune_dummy_runner.rb index 6cb789f..64c2606 100644 --- a/lib/prune/prune_dummy_runner.rb +++ b/lib/prune/prune_dummy_runner.rb @@ -1,7 +1,6 @@ require 'common/runner' - -# This is always needed, regardless of which plugin is running. require 'prune/plugins/postfixadmin' +require 'rm/rm_dummy_runner' class PruneDummyRunner include Runner @@ -16,18 +15,11 @@ class PruneDummyRunner db_users = pfa.list_users() db_domains = pfa.list_domains() - leftover_users = plugin.get_leftover_users(db_users) - leftover_domains = plugin.get_leftover_domains(db_domains) - - leftover_users.each do |user| - user_description = plugin.describe_user(user) - report(plugin, "Would remove user: #{user} (#{user_description})") - end + leftovers = plugin.get_leftover_users(db_users) + leftovers += plugin.get_leftover_domains(db_domains) - leftover_domains.each do |domain| - domain_description = plugin.describe_domain(domain) - report(plugin, "Would remove domain: #{domain} (#{domain_description})") - end + rm_dummy_runner = RmDummyRunner.new() + rm_dummy_runner.run(plugin, *leftovers) end end diff --git a/lib/prune/prune_runner.rb b/lib/prune/prune_runner.rb index 12a02d7..8ae8ad2 100644 --- a/lib/prune/prune_runner.rb +++ b/lib/prune/prune_runner.rb @@ -1,7 +1,6 @@ require 'common/runner' - -# This is always needed, regardless of which plugin is running. require 'prune/plugins/postfixadmin' +require 'rm/rm_runner' class PruneRunner include Runner @@ -16,21 +15,11 @@ class PruneRunner db_users = pfa.list_users() db_domains = pfa.list_domains() - leftover_users = plugin.get_leftover_users(db_users) - leftover_domains = plugin.get_leftover_domains(db_domains) - - leftover_users.each do |user| - user_description = plugin.describe_user(user) - plugin.delete_user(user) - report(plugin, "Removed user: #{user} (#{user_description})") - end - - leftover_domains.each do |domain| - domain_description = plugin.describe_domain(domain) - plugin.delete_domain(domain) - report(plugin, "Removed domain: #{domain} (#{domain_description})") - end + leftovers = plugin.get_leftover_users(db_users) + leftovers += plugin.get_leftover_domains(db_domains) + rm_runner = RmRunner.new() + rm_runner.run(plugin, *leftovers) end end diff --git a/lib/rm/plugins/agendav.rb b/lib/rm/plugins/agendav.rb index 0f4e790..bebab61 100644 --- a/lib/rm/plugins/agendav.rb +++ b/lib/rm/plugins/agendav.rb @@ -12,7 +12,7 @@ class AgendavRm def delete_user(user) # Delete the given username and any records in other tables # belonging to it. - raise NonexistentUserError.new(user) if not user_exists(user) + raise NonexistentUserError.new(user.to_s()) if not user_exists(user) sql_queries = ['DELETE FROM prefs WHERE username = $1;'] sql_queries << 'DELETE FROM shared WHERE user_from = $1;' @@ -27,7 +27,7 @@ class AgendavRm @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [user]) + connection.query(sql_query, [user.to_s()]) end connection.close() diff --git a/lib/rm/plugins/davical.rb b/lib/rm/plugins/davical.rb index f7d116e..3526dcf 100644 --- a/lib/rm/plugins/davical.rb +++ b/lib/rm/plugins/davical.rb @@ -16,7 +16,7 @@ class DavicalRm # Delete the given username. DAViCal uses foreign keys properly # and only supports postgres, so we let the ON DELETE CASCADE # trigger handle most of the work. - raise NonexistentUserError.new(user) if not user_exists(user) + raise NonexistentUserError.new(user.to_s()) if not user_exists(user) sql_queries = ['DELETE FROM usr WHERE username = $1'] @@ -30,7 +30,7 @@ class DavicalRm @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [user]) + connection.query(sql_query, [user.to_s()]) end connection.close() diff --git a/lib/rm/plugins/dovecot.rb b/lib/rm/plugins/dovecot.rb index 837a7fd..0ced5be 100644 --- a/lib/rm/plugins/dovecot.rb +++ b/lib/rm/plugins/dovecot.rb @@ -11,14 +11,22 @@ class DovecotRm def delete_domain(domain) - # Will raise an exception if the path doesn't exist. domain_path = self.get_domain_path(domain) + + if not File.directory?(domain_path) + raise NonexistentDomainError.new(domain.to_s()) + end + FileUtils.rm_r(domain_path) end def delete_user(user) - # Will raise an exception if the path doesn't exist. user_path = self.get_user_path(user) + + if not File.directory?(user_path) + raise NonexistentUserError.new(user.to_s()) + end + FileUtils.rm_r(user_path) end diff --git a/lib/rm/plugins/postfixadmin.rb b/lib/rm/plugins/postfixadmin.rb index 94526bc..eddf939 100644 --- a/lib/rm/plugins/postfixadmin.rb +++ b/lib/rm/plugins/postfixadmin.rb @@ -10,7 +10,7 @@ class PostfixadminRm def delete_user(user) - raise NonexistentUserError.new(user) if not user_exists(user) + raise NonexistentUserError.new(user.to_s()) if not user_exists(user) sql_queries = ['DELETE FROM alias WHERE address = $1;'] # Wipe out any aliases pointed at our user. @@ -33,7 +33,7 @@ class PostfixadminRm @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [user]) + connection.query(sql_query, [user.to_s()]) end # The earlier alias update query will leave things like @@ -53,7 +53,7 @@ class PostfixadminRm def delete_domain(domain) - raise NonexistentDomainError.new(domain) if not domain_exists(domain) + raise NonexistentDomainError.new(domain.to_s()) if not domain_exists(domain) sql_queries = ['DELETE FROM domain_admins WHERE domain = $1;'] sql_queries << 'DELETE FROM alias WHERE domain = $1;' @@ -73,7 +73,7 @@ class PostfixadminRm @db_pass) sql_queries.each do |sql_query| - connection.query(sql_query, [domain]) + connection.query(sql_query, [domain.to_s()]) end connection.close() @@ -102,7 +102,7 @@ class PostfixadminRm @db_pass) sql_query = 'SELECT COUNT(domain) as count FROM domain WHERE domain = $1;' - connection.query(sql_query, [domain]) do |result| + connection.query(sql_query, [domain.to_s()]) do |result| return false if result.ntuples() < 1 begin count = result.getvalue(0,0).to_i() diff --git a/lib/rm/plugins/roundcube.rb b/lib/rm/plugins/roundcube.rb index 23782d2..a315e77 100644 --- a/lib/rm/plugins/roundcube.rb +++ b/lib/rm/plugins/roundcube.rb @@ -11,7 +11,7 @@ class RoundcubeRm def delete_user(user) # Delete the given username and any records in other tables # belonging to it. - raise NonexistentUserError.new(user) if not user_exists(user) + raise NonexistentUserError.new(user.to_s()) if not user_exists(user) user_id = self.get_user_id(user) diff --git a/lib/rm/rm_dummy_runner.rb b/lib/rm/rm_dummy_runner.rb index 477608f..2ab25e8 100644 --- a/lib/rm/rm_dummy_runner.rb +++ b/lib/rm/rm_dummy_runner.rb @@ -5,13 +5,18 @@ class RmDummyRunner def run(plugin, *targets) targets.each do |target| - if target.include?('@') then - user_description = plugin.describe_user(target) - report(plugin, "Would remove user: #{target} (#{user_description})") - else - domain_description = plugin.describe_domain(target) - report(plugin, "Would remove domain: #{target} (#{domain_description})") + target_description = plugin.describe(target) + msg = "Would remove #{target.class.to_s().downcase()} #{target}" + + # Only append the extra description if it's useful. + if not target_description.nil? and + not target_description.empty? and + not target_description == target.to_s() then + msg += " (#{target_description})" end + msg += '.' + + report(plugin, msg) end end diff --git a/lib/rm/rm_plugin.rb b/lib/rm/rm_plugin.rb index fe938b1..fd0c541 100644 --- a/lib/rm/rm_plugin.rb +++ b/lib/rm/rm_plugin.rb @@ -15,15 +15,27 @@ module RmPlugin return RmDummyRunner end + def delete(target) + # A generic version of delete_user/delete_domain that + # dispatches base on the class of the target. + if target.is_a?(User) + return delete_user(target) + elsif target.is_a?(Domain) + return delete_domain(target) + else + raise NotImplementedError + end + end + def delete_domain(domain) # Delete the given domain. Some plugins don't have a concept of # domains, so just delete all users with a username that looks # like it's in the given domain. - usernames = list_domains_users([domain]) + users = list_domains_users([domain]) - raise NonexistentDomainError.new(domain) if usernames.empty? + raise NonexistentDomainError.new(domain.to_s()) if users.empty? - usernames.each do |u| + users.each do |u| delete_user(u) end end diff --git a/lib/rm/rm_runner.rb b/lib/rm/rm_runner.rb index 7142154..2df8410 100644 --- a/lib/rm/rm_runner.rb +++ b/lib/rm/rm_runner.rb @@ -6,37 +6,32 @@ class RmRunner def run(plugin, *targets) targets.each do |target| - # Why think too hard? An user has an @, a domain doesn't. - if target.include?('@') then - begin - user_description = plugin.describe_user(target) - plugin.delete_user(target) - report(plugin, "Removed user: #{target} (#{user_description})") + remove_target(plugin, target) + end + end + + private; + + def remove_target(plugin, target) + # Wrap the "remove this thing" operation so that it can be reused + # in the prine plugin. + target_description = plugin.describe(target) - rescue NonexistentUserError => e - report(plugin, "User not found: #{e.to_s}") - rescue StandardError => e - report(plugin, "There was an error removing the user: #{e.to_s}") - Kernel.exit(ExitCodes::DATABASE_ERROR) - end - else - begin - domain_description = plugin.describe_domain(target) - plugin.delete_domain(target) - report(plugin, "Removed domain: #{target} (#{domain_description})") + begin + plugin.delete(target) + msg = "Removed #{target.class.to_s().downcase()} #{target}" - rescue NonexistentUserError => e - # Can happen in the usernames.each... block. - report(plugin, "User not found: #{e.to_s}") - rescue NonexistentDomainError => e - report(plugin, "Domain not found: #{e.to_s}") - rescue StandardError => e - report(plugin, "There was an error removing the domain: #{e.to_s}") - Kernel.exit(ExitCodes::DATABASE_ERROR) - end + # Only append the extra description if it's useful. + if not target_description.nil? and + not target_description.empty? and + not target_description == target.to_s() then + msg += " (#{target_description})" end - end + msg += '.' + report(plugin, msg) + rescue NonexistentDomainError, NonexistentUserError => e + report(plugin, "#{target.class.to_s()} #{e.to_s} not found.") + end end - end -- 2.44.2