Merge lp:~akretion-team/openerp-connector-magento/7-htaccess-auth-support-dbl into lp:~openerp-connector-core-editors/openerp-connector-magento/7.0

[David BEAL (ak)]

allow to import products with their images when authentification is required like htaccess,
e.g. location https://myuser:<email address hidden>

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Interesting. But why put the user and password in the location if 1. they need to be parsed/extracted afterwards, 2. they are used only for the urlopen() of the images but not for the API.

I would prefer to have 2 additional fields for that.

That would also avoid to have the password leaked in the log file each time we log the URL.

Thanks

review: Needs Fixing

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

(maybe I missed the use case / need to have them in the url, in that case please let me know!)

[David BEAL (ak) (davidbeal)]

yes, you missed the use case => I'm lazy, specially with xml view

;-)

but you are right.

fields name for htaccess could be : restricted_access_user and restricted_access_password

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

> yes, you missed the use case => I'm lazy, specially with xml view
>
> ;-)

You really dislike the xml views so, considering the parsing you needed to write ;-)

>
> but you are right.
>
> fields name for htaccess could be : restricted_access_user and
> restricted_access_password

Sounds good

[lp:~akretion-team/openerp-connector-magento/7-htaccess-auth-support-dbl]

966. By David BEAL (ak) on 2014-04-01

[FIX] replace _get_auth_parameters() by fields in magento.backend

967. By David BEAL (ak) on 2014-04-01

[IMP] complete htaccess case management, drop unused historic line

[David BEAL (ak) (davidbeal)]

seems to be ok for final review

[lp:~akretion-team/openerp-connector-magento/7-htaccess-auth-support-dbl]

968. By David BEAL (ak) on 2014-04-02

[IMP] add 'full url' option on the backend

[David BEAL (ak) (davidbeal)]

Please, thanks to give your feedback

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Thanks for the changes.

restricted_access_username and restricted_access_password should be mandatory when display_restricted_access is true.

The name of this authentication method is HTTP Auth Basic or
Basic Access Authentication http://en.wikipedia.org/wiki/Basic_access_authentication so I think it would be more accurate to use these terms in the tooltips. Upon reflection, the column names would be better named auth_basic, auth_basic_username and auth_basic_password.

I had some difficulties to figure out what full_url means. I think it would be simpler do understand if it was 'use_custom_api_path' and the labels / tooltips would say something like "The default API path is '/index.php/api/xmlrpc'. Check this box if you use a custom API path, in that case, the location has to be completed with the custom API path'.

Do you mind to change that? If you want, I can make the changes before merging, in that case I would just propose you an edited branch but I won't be able to do it the next few days.

[lp:~akretion-team/openerp-connector-magento/7-htaccess-auth-support-dbl]

969. By David BEAL (ak) on 2014-04-03

[FIX] fields name, tool tip

[David BEAL (ak) (davidbeal)]

Hi guewen,

changes are done.

could you integrate it in next release ?

thank you

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Thanks! I think it will be yes.

[lp:~akretion-team/openerp-connector-magento/7-htaccess-auth-support-dbl]

970. By David BEAL (ak) on 2014-04-03

[FIX] tool tip

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hi David,

I tested your branch, and I needed to rebase it from the next-release branch because it had conflict.

So I pushed my branch ready for next-rebase and made a proposal here https://code.launchpad.net/~camptocamp/openerp-connector-magento/7-htaccess-auth-support-dbl-next-release-gbr/+merge/214184 so you can comment.

I made some changes too, improving the placement of the fields in the view, and reorganizing the code in the BackendAdapter (the infos should be stored in the MagentoLocation object).

Can you check my branch and test for your use case?

Thanks

[Sébastien BEAU - http://www.akretion.com (sebastien.beau)]

I change the state of this merge as rejected as a new merge have been done based on this work

Unmerged revisions

970. By David BEAL (ak) on 2014-04-03

[FIX] tool tip

969. By David BEAL (ak) on 2014-04-03

[FIX] fields name, tool tip

968. By David BEAL (ak) on 2014-04-02

[IMP] add 'full url' option on the backend

967. By David BEAL (ak) on 2014-04-01

[IMP] complete htaccess case management, drop unused historic line

966. By David BEAL (ak) on 2014-04-01

[FIX] replace _get_auth_parameters() by fields in magento.backend

965. By David BEAL (ak) on 2014-04-01

[FIX] allow to import products with their images when authentif is required like htaccess, e.g. https://myuser:<email address hidden>

=== modified file 'magentoerpconnect/magento_model.py'
--- magentoerpconnect/magento_model.py	2013-08-15 08:01:46 +0000
+++ magentoerpconnect/magento_model.py	2014-04-03 16:28:05 +0000
@@ -60,7 +60,6 @@
         return [('1.7', '1.7')]
 
     def _get_stock_field_id(self, cr, uid, context=None):
-        stock_field = 'virtual_available'
         field_ids = self.pool.get('ir.model.fields').search(
             cr, uid,
             [('model', '=', 'product.product'),
@@ -73,9 +72,33 @@
             _select_versions,
             string='Version',
             required=True),
-        'location': fields.char('Location', required=True),
-        'username': fields.char('Username'),
-        'password': fields.char('Password'),
+        'location': fields.char(
+            'Location',
+            required=True,
+            help="Url to magento application"),
+        'use_custom_api_path': fields.boolean(
+            'Use Custom Api Path',
+            help="The default API path is '/index.php/api/xmlrpc'. "
+            "Check this box if you use a custom API path, in that case, "
+            "the location has to be completed with the custom API path "),
+        'username': fields.char(
+            'Username',
+            help="Webservice user"),
+        'password': fields.char(
+            'Password',
+            help="Webservice password"),
+        'display_auth_basic': fields.boolean(
+            'Display HTTP Auth Basic',
+            help="Display Basic Access Authentication is "
+            "required by some web servers installation \n(see "
+            "http://en.wikipedia.org/wiki/Basic_access_authentication)"),
+        'auth_basic_username': fields.char(
+            'Basic Access Auth. Username',
+            help="Web server side username : \n"
+            "some web servers may requires basic access authentication"),
+        'auth_basic_password': fields.char(
+            'Basic Access Auth. Password',
+            help="Basic access authentication password web server side "),
         'sale_prefix': fields.char(
             'Sale Prefix',
             help="A prefix put before the name of imported sales orders.\n"
@@ -122,6 +145,8 @@
 
     _defaults = {
         'product_stock_field_id': _get_stock_field_id,
+        'use_custom_api_path': False,
+        'display_auth_basic': False,
     }
 
     _sql_constraints = [

=== modified file 'magentoerpconnect/magento_model_view.xml'
--- magentoerpconnect/magento_model_view.xml	2014-02-07 11:39:12 +0000
+++ magentoerpconnect/magento_model_view.xml	2014-04-03 16:28:05 +0000
@@ -115,6 +115,12 @@
                                     <field name="product_stock_field_id" widget="selection"
                                         domain="[('model', 'in', ['product.product', 'product.template']), ('ttype', '=', 'float')]"/>
                                     <field name="catalog_price_tax_included"/>
+                                    <field name="use_custom_api_path"/>
+                                    <field name="display_auth_basic"/>
+                                    <field name="auth_basic_username" colspan="2"
+                                           attrs="{'invisible': [('display_auth_basic', '=', False)], 'required': [('display_auth_basic', '=', True)]}"/>
+                                    <field name="auth_basic_password" colspan="2"
+                                           attrs="{'invisible': [('display_auth_basic', '=', False)], 'required': [('display_auth_basic', '=', True)]}"/>
                                     <p attrs="{'invisible': [('catalog_price_tax_included', '=', False)]}">
                                       This option should respect the same
                                       configuration than Magento.  Pay

=== modified file 'magentoerpconnect/product.py'
--- magentoerpconnect/product.py	2014-01-14 10:41:05 +0000
+++ magentoerpconnect/product.py	2014-04-03 16:28:05 +0000
@@ -282,7 +282,14 @@
     def _get_binary_image(self, image_data):
         url = image_data['url']
         try:
-            binary = urllib2.urlopen(url)
+            request = urllib2.Request(url)
+            if self.backend_record.auth_basic_username \
+                    and self.backend_record.auth_basic_password:
+                base64string = base64.encodestring(
+                    '%s:%s' % (self.backend_record.auth_basic_username,
+                               self.backend_record.auth_basic_password))
+                request.add_header("Authorization", "Basic %s" % base64string)
+            binary = urllib2.urlopen(request)
         except urllib2.HTTPError as err:
             if err.code == 404:
                 # the image is just missing, we skip it

=== modified file 'magentoerpconnect/unit/backend_adapter.py'
--- magentoerpconnect/unit/backend_adapter.py	2013-06-26 06:36:58 +0000
+++ magentoerpconnect/unit/backend_adapter.py	2014-04-03 16:28:05 +0000
@@ -121,11 +121,23 @@
         """ Delete a record on the external system """
         raise NotImplementedError
 
+    def _complete_url(self):
+        location = self.magento.location
+        if self.backend_record.auth_basic_username \
+                and self.backend_record.auth_basic_password:
+            replacement = self.backend_record.auth_basic_username + ':'
+            replacement += self.backend_record.auth_basic_password + '@'
+            location = location.replace('://', '://' + replacement)
+        return location
+
     def _call(self, method, arguments):
         try:
-            with magentolib.API(self.magento.location,
+            location = self._complete_url()
+            full_url = self.backend_record.use_custom_api_path
+            with magentolib.API(location,
                                 self.magento.username,
-                                self.magento.password) as api:
+                                self.magento.password,
+                                full_url=full_url) as api:
                 result = api.call(method, arguments)
                 # Uncomment to record requests/responses in ``recorder``
                 # record(method, arguments, result)