From 52f080eba9cb0e3b7adf32d0b9d3e95f38485827 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Thu, 18 Apr 2019 11:31:19 +0200
Subject: [PATCH 3/3] actesweb: use atomic_write (#32413)

---
 passerelle/apps/actesweb/models.py | 41 ++++++++++++++----------------
 tests/test_actesweb.py             |  9 ++++---
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/passerelle/apps/actesweb/models.py b/passerelle/apps/actesweb/models.py
index aa9167b2..e332ad19 100644
--- a/passerelle/apps/actesweb/models.py
+++ b/passerelle/apps/actesweb/models.py
@@ -20,7 +20,9 @@ import os
 import stat
 import tempfile
 import contextlib
+import errno
 
+from django.conf import settings
 from django.core.files.storage import default_storage
 from django.template.loader import get_template
 from django.utils import six
@@ -32,6 +34,7 @@ from passerelle.base.models import BaseResource
 from passerelle.utils.api import endpoint
 from passerelle.utils.jsonresponse import APIError
 from passerelle.utils.conversion import ensure_encoding
+from passerelle.utils.files import atomic_write
 
 
 @contextlib.contextmanager
@@ -51,8 +54,18 @@ class ActesWeb(BaseResource):
 
     @property
     def basepath(self):
-        return os.path.join(
+        path = os.path.join(
             default_storage.path('actesweb'), self.slug)
+        if not os.path.exists(path):
+            try:
+                os.makedirs(path)
+            except OSError as e:
+                if e.errno != errno.EEXIST:
+                    raise
+            if settings.FILE_UPLOAD_DIRECTORY_PERMISSIONS:
+                # user and group must be able to delete/move files, others can read/access
+                os.chmod(path, 0o775)
+        return path
 
     @endpoint(perm='can_access', methods=['post'], description=_('Create demand'))
     def create(self, request, *args, **kwargs):
@@ -67,32 +80,16 @@ class ActesWeb(BaseResource):
         template_name = 'actesweb/demand.txt'
         demand_content = get_template(template_name).render(payload)
         application_id = payload['application_id']
-        # create tmp dir
-        tmp_dir = os.path.join(self.basepath, 'tmp')
-        if not os.path.exists(tmp_dir):
-            if default_storage.directory_permissions_mode:
-                d_umask = os.umask(0)
-                try:
-                    os.makedirs(tmp_dir, mode=default_storage.directory_permissions_mode)
-                except OSError:
-                    pass
-                finally:
-                    os.umask(d_umask)
-            else:
-                os.makedirs(tmp_dir)
 
         # ensure demand_content can be encoded to latin15
         demand_content = ensure_encoding(demand_content, 'iso-8859-15')
 
         filename = '%s.DEM' % now().strftime('%Y-%m-%d_%H-%M-%S_%f')
         filepath = os.path.join(self.basepath, filename)
-        with named_tempfile(dir=tmp_dir, suffix='.DEM', delete=False) as tpf:
-            tpf.write(demand_content)
-            tpf.flush()
-            os.fsync(tpf.file.fileno())
-            tempfile_name = tpf.name
-        os.rename(tempfile_name, filepath)
-        # set read only permission for owner and group
-        os.chmod(filepath, stat.S_IRUSR|stat.S_IRGRP|stat.S_IWGRP)
+        tmp_dir = os.path.join(self.basepath, 'tmp')
+        with atomic_write(filepath, dir=tmp_dir) as fd:
+            fd.write(demand_content.encode('iso-8859-15'))
+        # user can read/write, group can read, others can't
+        os.chmod(filepath, 0o640)
         demand_id = '%s_%s' % (application_id, os.path.basename(filepath))
         return {'data': {'demand_id': demand_id}}
diff --git a/tests/test_actesweb.py b/tests/test_actesweb.py
index 0d48d645..60c0d0da 100644
--- a/tests/test_actesweb.py
+++ b/tests/test_actesweb.py
@@ -158,13 +158,14 @@ def test_demand_creation(app, payload, actesweb):
         response = app.post_json(url, params=payload['death'])
         demand_id = response.json['data']['demand_id']
         demfile = get_demand_filepath(actesweb, demand_id)
-        # make sure only owner can read file
+        # make sure only owner and group can read file, others can't
         assert bool(os.stat(demfile).st_mode & stat.S_IRUSR)
-        # make sure group can read and write (move) file
         assert bool(os.stat(demfile).st_mode & stat.S_IRGRP)
-        assert bool(os.stat(demfile).st_mode & stat.S_IWGRP)
-        # and no others
         assert not bool(os.stat(demfile).st_mode & stat.S_IRWXO)
+        # make sure group can delete/move file
+        assert bool(os.stat(os.path.dirname(demfile)).st_mode & stat.S_IWGRP)
+        # and no others
+        assert not bool(os.stat(os.path.dirname(demfile)).st_mode & stat.S_IWOTH)
         assert_file_content_values(
             demfile, dict(
                 DEMANDEUR_CIVILITE="Madame",
-- 
2.20.1