From 7b81775603b8208c995cd1c4a15cd2a287009404 Mon Sep 17 00:00:00 2001
From: Guilhem Moulin <guilhem.moulin@fripost.org>
Date: Tue, 29 Jan 2013 02:37:58 +0100
Subject: Bug fixes.

---
 lib/Fripost/Schema/Domain.pm | 58 +++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 22 deletions(-)

(limited to 'lib/Fripost/Schema/Domain.pm')

diff --git a/lib/Fripost/Schema/Domain.pm b/lib/Fripost/Schema/Domain.pm
index 3caffb5..4cc12e4 100644
--- a/lib/Fripost/Schema/Domain.pm
+++ b/lib/Fripost/Schema/Domain.pm
@@ -18,11 +18,12 @@ use utf8;
 
 use parent 'Fripost::Schema';
 use Fripost::Schema::Util qw/softdie dn2mail email_valid domain_valid
-                             canonical_dn ldap_explode_dn ldap_error/;
+                             canonical_dn ldap_explode_dn ldap_error
+                             ldap_assert_absent/;
 use Fripost::Schema::Mail;
 use Net::IDN::Encode qw/domain_to_ascii domain_to_unicode email_to_unicode/;
 use URI::Escape::XS 'encodeURIComponent';
-use Encode;
+use Encode ();
 use Net::Domain::TLD 'tld_exists';
 use Net::DNS::Dig;
 use Net::Whois::Parser 'parse_whois';
@@ -280,7 +281,7 @@ sub _entries_to_domains {
                     if not @$keys or grep { $_ eq 'description' } @$keys;
             }
             else {
-                die "Missing translation for domain attribute ‘".$attr."’.";
+                die "Missing translation for domain attribute ‘".$attr."’";
             }
         }
 
@@ -327,7 +328,7 @@ sub _keys_to_attrs {
               );
     my %attrs;
     foreach my $k (@_) {
-        die "Missing translation for key ‘".$k."’."
+        die "Missing translation for key ‘".$k."’"
             unless exists $map{$k};
         if (ref $map{$k} eq 'ARRAY') {
             $attrs{$_} = 1 for @{$map{$k}};
@@ -410,7 +411,7 @@ sub list_owner_emails {
     my $domain;
 
     until ( tld_exists($tld) ) {
-        softdie ("‘".$domainname."’ has an Invalid TLD.", %options) // return
+        softdie ("‘".$domainname."’ has an Invalid TLD", %options) // return
             unless $tld =~ /\./;
         $domain = $tld;
 
@@ -495,25 +496,33 @@ sub add {
 
     my $domainname = $domain->{name};
     # Check domain validity.
-    &_is_valid($domain, %options) // return;
-    my $dn = $self->mail2dn( $domain->{name} );
-
-    # Search for an existing domain with the same name.
-    my $count = $self->search($domain->{name}, %options, '-count' => 1);
-    softdie ( "Domain ‘".$domainname."’ already exists.", %options ) // return
-        if not $options{'-append'} and $count;
-    # Stop here in dry-run mode.
-    return 1 if $options{'-dry-run'};
+    &_assert_valid($domain, %options) // return;
+
+    my $exists;
+    if ($options{'-dry-run'} or $options{'-append'}) {
+        # Search for an existing domain with the same name. We can't
+        # use our previously defined method here, since the current user
+        # may not have read access to the entry. There is a race
+        # condition since someone could modify the directory between
+        # this check and the actual insertion, but then the insertion
+        # would fail.
+        $exists = ldap_assert_absent( $self, $domain->{name}, undef, %options)
+            // return;
+        return 1 if $options{'-dry-run'};
+    }
 
     # Convert the domain into a LDAP entry, and remove keys to empty values.
     my %attrs = $self->_domain_to_entry (%$domain);
-    Fripost::Schema::Util::clean_ldap_entry( \%attrs );
+    Fripost::Schema::Util::ldap_clean_entry( \%attrs );
 
     my ($mesg, $token);
-    if ($options{'-append'} and $count) {
+    my $dn = $self->mail2dn( $domain->{name} );
+    if ($options{'-append'} and $exists) {
         # Replace single valued attributes; Add other attributes.
-        my %unique = ( fripostIsStatusActive => $attrs{fripostIsStatusActive} );
-        delete $attrs{$_} for (keys %unique);
+        my %unique;
+        foreach (qw/fripostIsStatusActive/) {
+            $unique{$_} = delete $attrs{$_} if exists $attrs{$_};
+        }
         $mesg = $self->ldap->modify( $dn, replace => \%unique, add => \%attrs );
     }
     else {
@@ -528,6 +537,10 @@ sub add {
         }
         # The default owner is the current user.
         $attrs{fripostOwner} //= [ $self->whoami ];
+        $options{'-die'} = { Net::LDAP::Constant::LDAP_ALREADY_EXISTS =>
+                                "‘".$domainname."’ exists"
+                           , Net::LDAP::Constant::LDAP_SUCCESS => 0 }
+            unless exists $options{'-die'};
         $mesg = $self->ldap->add( $dn, attrs => [ %attrs ] );
     }
 
@@ -594,7 +607,7 @@ sub _domain_to_entry {
                 [ map { $self->mail2dn($_) } @{$domain{postmaster}} ];
         }
         else {
-            die "Missing translation for domain key ‘".$key."’.";
+            die "Missing translation for domain key ‘".$key."’";
         }
     }
     return %entry;
@@ -681,7 +694,7 @@ sub replace {
     return if $options{'-error'} && ${$options{'-error'}};
 
     # Check domain validity.
-    &_is_valid($domain, %options) // return;
+    &_assert_valid($domain, %options, -replace => 1) // return;
     return 1 if $options{'-dry-run'};
 
     my %entry = $self->_domain_to_entry (%$domain);
@@ -693,11 +706,12 @@ sub replace {
 
 
 # Ensure that the given domain is valid.
-sub _is_valid {
+sub _assert_valid {
     my $d = shift;
     my %options = @_;
     eval {
-        Fripost::Schema::Util::must_attrs( $d, qw/name isActive/ );
+        Fripost::Schema::Util::must_attrs( $d, qw/name isActive/ )
+            unless $options{'-append'} or $options{'-replace'};
         $d->{name} = domain_valid( domain_to_ascii ($d->{name}) );
         $d->{catchAll} = [ map { email_valid($_, -prefix => 'fake') }
                                 @{$d->{catchAll}} ]
-- 
cgit v1.2.3