From 97d12b94f687d78923bf068369b13d6d5b182b99 Mon Sep 17 00:00:00 2001
From: Krakinou <misterl56@hotmail.com>
Date: Tue, 4 Jun 2019 00:47:49 +0200
Subject: [PATCH] Correct settings update

---
 cps/templates/config_edit.html | 36 +++++++++++------------
 cps/ub.py                      | 50 +++++++++++++++++++++++++++++++
 cps/web.py                     | 54 ++++++++++++++++++----------------
 3 files changed, 97 insertions(+), 43 deletions(-)

diff --git a/cps/templates/config_edit.html b/cps/templates/config_edit.html
index fe2f41e6..e63b8566 100644
--- a/cps/templates/config_edit.html
+++ b/cps/templates/config_edit.html
@@ -164,58 +164,58 @@
     {% endif %}
     <div class="form-group">
       <input type="checkbox" id="config_use_ldap" name="config_use_ldap" data-control="ldap-settings" {% if content.config_use_ldap %}checked{% endif %}>
-      <label for="config_use_ldap">{{_('Use')}} LDAP Authentication</label>
+      <label for="config_use_ldap">{{_('Use LDAP Authentication')}}</label>
     </div>
     <div data-related="ldap-settings">
       <div class="form-group">
         <label for="config_ldap_provider_url">{{_('LDAP Server Host Name or IP Address')}}</label>
-        <input type="text" class="form-control" id="config_ldap_provider_url" name="config_ldap_provider_url" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_provider_url }}{% endif %}" autocomplete="off">
+        <input type="text" class="form-control" id="config_ldap_provider_url" name="config_ldap_provider_url" value="{% if content.config_ldap_provider_url != None %}{{ content.config_ldap_provider_url }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
         <label for="config_ldap_port">{{_('LDAP Server Port')}}</label>
-        <input type="text" class="form-control" id="config_ldap_port" name="config_ldap_port" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_port }}{% endif %}" autocomplete="off">
+        <input type="text" class="form-control" id="config_ldap_port" name="config_ldap_port" value="{% if content.config_ldap_port != None %}{{ content.config_ldap_port }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
         <label for="config_ldap_schema">{{_('LDAP schema (ldap or ldaps)')}}</label>
-        <input type="text" class="form-control" id="config_ldap_schema" name="config_ldap_schema" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_schema }}{% endif %}" autocomplete="off">
+        <input type="text" class="form-control" id="config_ldap_schema" name="config_ldap_schema" value="{% if content.config_ldap_schema != None %}{{ content.config_ldap_schema }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
         <label for="config_ldap_serv_username">{{_('LDAP Admin username')}}</label>
-        <input type="text" class="form-control" id="config_ldap_serv_username" name="config_ldap_serv_username" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_serv_username }}{% endif %}" autocomplete="off">
+        <input type="text" class="form-control" id="config_ldap_serv_username" name="config_ldap_serv_username" value="{% if content.config_ldap_serv_username != None %}{{ content.config_ldap_serv_username }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
         <label for="config_ldap_serv_password">{{_('LDAP Admin password')}}</label>
-        <input type="password" class="form-control" id="config_ldap_serv_password" name="config_ldap_serv_password" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_serv_password }}{% endif %}" autocomplete="off">
+        <input type="password" class="form-control" id="config_ldap_serv_password" name="config_ldap_serv_password" value="{% if content.config_ldap_serv_password != None %}{{ content.config_ldap_serv_password }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
-        <input type="checkbox" id="config_ldap_use_ssl" name="config_ldap_use_ssl" {% if content.config_use_ldap %}checked{% endif %}>
-        <label for="config_ldap_ssl">{{_('Use')}} LDAP Server Use SSL</label>
+        <input type="checkbox" id="config_ldap_use_ssl" name="config_ldap_use_ssl" {% if content.config_ldap_use_ssl %}checked{% endif %}>
+        <label for="config_ldap_ssl">{{_('LDAP Server use SSL')}}</label>
       </div>
       <div class="form-group">
-        <input type="checkbox" id="config_ldap_use_tls" name="config_ldap_use_tls" {% if content.config_use_ldap %}checked{% endif %}>
-        <label for="config_ldap_ssl">{{_('Use')}} LDAP Server Use TLS</label>
+        <input type="checkbox" id="config_ldap_use_tls" name="config_ldap_use_tls" {% if content.config_ldap_use_tls %}checked{% endif %}>
+        <label for="config_ldap_ssl">{{_('LDAP Server use TLS')}}</label>
       </div>
       <div class="form-group">
-        <input type="checkbox" id="config_ldap_require_cert" name="config_ldap_require_cert" data-control="ldap-cert-settings" {% if content.config_use_ldap %}checked{% endif %}>
-        <label for="config_ldap_ssl">{{_('Use')}} LDAP Server Certificate</label>
+        <input type="checkbox" id="config_ldap_require_cert" name="config_ldap_require_cert" data-control="ldap-cert-settings" {% if content.config_ldap_require_cert %}checked{% endif %}>
+        <label for="config_ldap_ssl">{{_('LDAP Server Certificate')}}</label>
       </div>
       <div data-related="ldap-cert-settings">
         <div class="form-group">
-          <label for="config_ldap_cert_path">{{_('LDAP Certificate Path')}}</label>
-          <input type="text" class="form-control" id="config_ldap_cert_path" name="config_ldap_cert_path" value="{% if content.config_use_ldap != None and content.config_ldap_require_cert !=None %}{{ content.config_ldap_cert_path }}{% endif %}" autocomplete="off">
+          <label for="config_ldap_cert_path">{{_('LDAP SSL Certificate Path')}}</label>
+          <input type="text" class="form-control" id="config_ldap_cert_path" name="config_ldap_cert_path" value="{% if content.config_ldap_cert_path != None and content.config_ldap_require_cert !=None %}{{ content.config_ldap_cert_path }}{% endif %}" autocomplete="off">
         </div>
       </div>
       <div class="form-group">
         <label for="config_ldap_dn">{{_('LDAP Distinguished Name (DN)')}}</label>
-        <input type="text" class="form-control" id="config_ldap_dn" name="config_ldap_dn" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_dn }}{% endif %}" autocomplete="off">
+        <input type="text" class="form-control" id="config_ldap_dn" name="config_ldap_dn" value="{% if content.config_ldap_dn != None %}{{ content.config_ldap_dn }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
         <label for="config_ldap_user_object">{{_('LDAP User object filter')}}</label>
-        <input type="text" class="form-control" id="config_ldap_user_object" name="config_ldap_user_object" value="{% if content.config_use_ldap != None %}{{ content.config_ldap_user_object }}{% endif %}" autocomplete="off">
+        <input type="text" class="form-control" id="config_ldap_user_object" name="config_ldap_user_object" value="{% if content.config_ldap_user_object != None %}{{ content.config_ldap_user_object }}{% endif %}" autocomplete="off">
       </div>
       <div class="form-group">
-        <input type="checkbox" id="config_ldap_openldap" name="config_ldap_openldap" {% if content.config_use_ldap %}checked{% endif %}>
-        <label for="config_ldap_openldap">{{_('Use')}} LDAP Server is OpenLDAP?</label>
+        <input type="checkbox" id="config_ldap_openldap" name="config_ldap_openldap" {% if content.config_ldap_openldap %}checked{% endif %}>
+        <label for="config_ldap_openldap">{{_('LDAP Server is OpenLDAP?')}}</label>
       </div>
     </div>
     </div>
diff --git a/cps/ub.py b/cps/ub.py
index f3aab574..e8f9947d 100644
--- a/cps/ub.py
+++ b/cps/ub.py
@@ -692,17 +692,67 @@ def migrate_Database():
     except exc.OperationalError:
         conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_use_ldap` INTEGER DEFAULT 0")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_provider_url)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_provider_url` String DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_port)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_port` INTEGER DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_schema)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_schema ` String DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_serv_username)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_serv_username` String DEFAULT ''")
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_serv_password` String DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_use_ssl)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_use_ssl` INTEGER DEFAULT 0")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_use_tls)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `cconfig_ldap_use_tls` INTEGER DEFAULT 0")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_require_cert)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_require_cert` INTEGER DEFAULT 0")
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_cert_path` String DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_dn)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_dn` String DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_user_object)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_user_object` String DEFAULT ''")
+        session.commit()
+    try:
+        session.query(exists().where(Settings.config_ldap_openldap)).scalar()
+    except exc.OperationalError:
+        conn = engine.connect()
         conn.execute("ALTER TABLE Settings ADD column `config_ldap_openldap` INTEGER DEFAULT 0")
         session.commit()
     try:
diff --git a/cps/web.py b/cps/web.py
index 59ba9540..9bed45e8 100644
--- a/cps/web.py
+++ b/cps/web.py
@@ -183,7 +183,7 @@ app.secret_key = os.getenv('SECRET_KEY', 'A0Zr98j/3yX R~XHH!jmN]LWX/,?RT')
 db.setup_db()
 
 if ldap_support and config.config_use_ldap:
-    app.confgi['LDAP_HOST'] = config.config_ldap_provider_url
+    app.config['LDAP_HOST'] = config.config_ldap_provider_url
     app.config['LDAP_PORT'] = config.config_ldap_port
     app.config['LDAP_SCHEMA'] = config.config_ldap_schema
     app.config['LDAP_USERNAME'] = config.config_ldap_serv_username
@@ -197,7 +197,7 @@ if ldap_support and config.config_use_ldap:
         app.config['LDAP_CERT_PATH'] = config.config_ldap_cert_path
     app.config['LDAP_BASE_DN'] = config.config_ldap_dn
     app.config['LDAP_USER_OBJECT_FILTER'] = config.config_ldap_user_object
-    if config.config_openldap:
+    if config.config_ldap_openldap:
         app.config['LDAP_OPENLDAP'] = True
 
 #    app.config['LDAP_BASE_DN'] = 'ou=users,dc=yunohost,dc=org'
@@ -3140,15 +3140,16 @@ def configuration_helper(origin):
             content.config_ebookconverter = int(to_save["config_ebookconverter"])
 
         #LDAP configuration,
+        content.config_use_ldap = 0
         if "config_use_ldap" in to_save and to_save["config_use_ldap"] == "on":
-            if not "config_ldap_provider_url" in to_save or not "config_ldap_dn" in to_save:
+            if not to_save["config_ldap_provider_url"] or not to_save["config_ldap_port"] or not to_save["config_ldap_dn"] or not to_save["config_ldap_user_object"]:
                 ub.session.commit()
-                flash(_(u'Please enter a LDAP provider and a DN'), category="error")
+                flash(_(u'Please enter a LDAP provider, port, DN and user object identifier'), category="error")
                 return render_title_template("config_edit.html", content=config, origin=origin,
                                              gdrive=gdriveutils.gdrive_support, gdriveError=gdriveError,
                                              goodreads=goodreads_support, title=_(u"Basic Configuration"),
                                              page="config")
-            elif not "config_ldap_serv_username" in to_save or not "config_ldap_serv_password" in to_save:
+            elif not to_save["config_ldap_serv_username"] or not to_save["config_ldap_serv_password"]:
                 ub.session.commit()
                 flash(_(u'Please enter a LDAP service account and password'), category="error")
                 return render_title_template("config_edit.html", content=config, origin=origin,
@@ -3162,29 +3163,32 @@ def configuration_helper(origin):
                 content.config_ldap_schema = to_save["config_ldap_schema"]
                 content.config_ldap_serv_username = to_save["config_ldap_serv_username"]
                 content.config_ldap_serv_password = to_save["config_ldap_serv_password"]
-                if content.config_ldap_use_ssl in to_save and to_save["config_ldap_use_ssl"] == "on":
-                    content.config_ldap_use_ssl = 1
-                if content.config_ldap_use_tls in to_save and to_save["config_ldap_use_tls"] == "on":
-                    content.config_ldap_use_tls = 1
-                if content.config_ldap_require_cert in to_save and to_save["config_ldap_require_cert"] == "on":
-                    content.config_ldap_require_cert = 1
-                    if "config_ldap_cert_path " in to_save:
-                        if content.config_ldap_cert_path  != to_save["config_ldap_cert_path "]:
-                            if os.path.isfile(to_save["config_ldap_cert_path "]) or to_save["config_ldap_cert_path "] is u"":
-                                content.config_certfile = to_save["config_ldap_cert_path "]
-                            else:
-                                ub.session.commit()
-                                flash(_(u'Certfile location is not valid, please enter correct path'), category="error")
-                                return render_title_template("config_edit.html", content=config, origin=origin,
-                                                 gdrive=gdriveutils.gdrive_support, gdriveError=gdriveError,
-                                                 goodreads=goodreads_support, title=_(u"Basic Configuration"),
-                                                 page="config")
                 content.config_ldap_dn = to_save["config_ldap_dn"]
                 content.config_ldap_user_object = to_save["config_ldap_user_object"]
-                if content.config_ldap_openldap in to_save and to_save["config_ldap_openldap"] == "on":
-                    content.config_ldap_openldap = 1
-                db_change = True
                 reboot_required = True
+        content.config_ldap_use_ssl = 0
+        content.config_ldap_use_tls = 0
+        content.config_ldap_require_cert = 0
+        content.config_ldap_openldap = 0
+        if "config_ldap_use_ssl" in to_save and to_save["config_ldap_use_ssl"] == "on":
+            content.config_ldap_use_ssl = 1
+        if "config_ldap_use_tls" in to_save and to_save["config_ldap_use_tls"] == "on":
+            content.config_ldap_use_tls = 1
+        if "config_ldap_require_cert" in to_save and to_save["config_ldap_require_cert"] == "on":
+            content.config_ldap_require_cert = 1
+        if "config_ldap_openldap" in to_save and to_save["config_ldap_openldap"] == "on":
+            content.config_ldap_openldap = 1
+        if "config_ldap_cert_path " in to_save:
+            if content.config_ldap_cert_path  != to_save["config_ldap_cert_path "]:
+                if os.path.isfile(to_save["config_ldap_cert_path "]) or to_save["config_ldap_cert_path "] is u"":
+                    content.config_certfile = to_save["config_ldap_cert_path "]
+                else:
+                    ub.session.commit()
+                    flash(_(u'Certfile location is not valid, please enter correct path'), category="error")
+                    return render_title_template("config_edit.html", content=config, origin=origin,
+                                        gdrive=gdriveutils.gdrive_support, gdriveError=gdriveError,
+                                        goodreads=goodreads_support, title=_(u"Basic Configuration"),
+                                        page="config")
 
         # Remote login configuration
         content.config_remote_login = ("config_remote_login" in to_save and to_save["config_remote_login"] == "on")