reverted original commit

AcquiredDatasetsControllerUI(base.BaseController)

.gitignore

@@ -20,7 +20,7 @@ var/
 *.egg-info/
 .installed.cfg
 *.egg
-
+.eggs
 # Installer logs
 pip-log.txt
 pip-delete-this-directory.txt
@@ -50,3 +50,6 @@ coverage.xml
 
 # Sphinx documentation
 docs/_build/
+
+.idea
+venv

.travis.yml

@@ -4,10 +4,12 @@ python:
     - "2.7"
 env:
     - CKANVERSION=2.7.3
-    - CKANVERSION=2.8.0
+    - CKANVERSION=2.8.1
+    - CKANVERSION=2.8.2
 services:
     - redis-server
     - postgresql
+    - xvfb
 addons:
     firefox: "60.1.0esr"
 before_install:
@@ -19,7 +21,6 @@ install:
     - bash bin/travis-build.bash
 before_script:
     - "export DISPLAY=:99.0"
-    - "sh -e /etc/init.d/xvfb start"
     - sleep 3 # give xvfb some time to start
 script:
     - sh bin/travis-run.sh

README.md

@@ -1,4 +1,4 @@
-CKAN Private Datasets [![Build Status](https://travis-ci.org/conwetlab/ckanext-privatedatasets.svg?branch=develop)](https://travis-ci.org/conwetlab/ckanext-privatedatasets) [![Coverage Status](https://coveralls.io/repos/github/conwetlab/ckanext-privatedatasets/badge.svg?branch=develop)](https://coveralls.io/github/conwetlab/ckanext-privatedatasets?branch=develop)
+D4Science CKAN Private Datasets [![Build Status](https://travis-ci.org/conwetlab/ckanext-privatedatasets.svg?branch=master)](https://travis-ci.org/conwetlab/ckanext-privatedatasets) [![Coverage Status](https://coveralls.io/repos/github/conwetlab/ckanext-privatedatasets/badge.svg?branch=master)](https://coveralls.io/github/conwetlab/ckanext-privatedatasets?branch=develop)
 =====================
 
 This CKAN extension allows a user to create private datasets that only certain users will be able to see. When a dataset is being created, it's possible to specify the list of users that can see this dataset. In addition, the extension provides an HTTP API that allows to add users programmatically.

bin/travis-build.bash

@@ -16,18 +16,21 @@ python setup.py develop
 
 sed -i "s|psycopg2==2.4.5|psycopg2==2.7.1|g" requirements.txt
 
-pip install -r requirements.txt --allow-all-external
-pip install -r dev-requirements.txt --allow-all-external
+pip install -r requirements.txt
+pip install -r dev-requirements.txt
 cd -
 
+echo "Checking solr"
+ls -la /etc/
+
 echo "Setting up Solr..."
 # solr is multicore for tests on ckan master now, but it's easier to run tests
 # on Travis single-core still.
 # see https://github.com/ckan/ckan/issues/2972
-sed -i -e 's/solr_url.*/solr_url = http:\/\/127.0.0.1:8983\/solr/' ckan/test-core.ini
-printf "NO_START=0\nJETTY_HOST=127.0.0.1\nJETTY_PORT=8983\nJAVA_HOME=$JAVA_HOME" | sudo tee /etc/default/jetty
+sed -i -e 's/solr_url.*/solr_url = http:\/\/127.0.0.1:8080\/solr/' ckan/test-core.ini
+printf "NO_START=0\nJETTY_HOST=127.0.0.1\nJETTY_PORT=8080\nJAVA_HOME=$JAVA_HOME" | sudo tee /etc/default/jetty
 sudo cp ckan/ckan/config/solr/schema.xml /etc/solr/conf/schema.xml
-sudo service jetty restart
+sudo service jetty8 restart
 
 echo "Creating the PostgreSQL user and database..."
 sudo -u postgres psql -c "CREATE USER ckan_default WITH PASSWORD 'pass';"

bin/travis-run.sh

@@ -1,3 +1,8 @@
 #!/usr/bin/env bash
 
+echo "Starting Jetty"
+sudo service jetty8 restart
+
+sudo netstat -ntlp
+
 python setup.py nosetests

ckanext/privatedatasets/auth.py

@@ -41,41 +41,37 @@ def package_show(context, data_dict):
     # Not active packages can only be seen by its owners
     if package.state == 'active':
         # anyone can see a public package
-        if not package.private:
-            return {'success': True}
+        if package.private:
 
-        # if the user has rights to read in the organization or in the group
-        if package.owner_org:
-            authorized = authz.has_user_permission_for_group_or_org(
-                package.owner_org, user, 'read')
-        else:
-            authorized = False
+            acquired = False
 
-        # if the user is not authorized yet, we should check if the
-        # user is in the allowed_users object
-        if not authorized:
-            # Init the model
-            db.init_db(context['model'])
+            if package.owner_org:
+                acquired = authz.has_user_permission_for_group_or_org(
+                    package.owner_org, user, 'read')
 
-            # Branch not executed if the database return an empty list
-            if db.AllowedUser.get(package_id=package.id, user_name=user):
-                authorized = True
+            if not acquired:
+                # Init the model
+                db.init_db(context['model'])
 
-        if not authorized:
-            # Show a flash message with the URL to acquire the dataset
-            # This message only can be shown when the user tries to access the dataset via its URL (/dataset/...)
-            # The message cannot be displayed in other pages that uses the package_show function such as
-            # the user profile page
+                # Branch not executed if the database return an empty list
+                if db.AllowedUser.get(package_id=package.id, user_name=user):
+                    acquired = True
 
-            if hasattr(package, 'extras') and 'acquire_url' in package.extras and request.path.startswith('/dataset/')\
-                    and package.extras['acquire_url'] != '':
-                helpers.flash_notice(_('This private dataset can be acquired. To do so, please click ' +
-                                       '<a target="_blank" href="%s">here</a>') % package.extras['acquire_url'],
-                                     allow_html=True)
+            if not acquired:
 
-            return {'success': False, 'msg': _('User %s not authorized to read package %s') % (user, package.id)}
-        else:
-            return {'success': True}
+                # Show a flash message with the URL to acquire the dataset
+                # This message only can be shown when the user tries to access the dataset via its URL (/dataset/...)
+                # The message cannot be displayed in other pages that uses the package_show function such as
+                # the user profile page
+
+                if hasattr(package, 'extras') and 'acquire_url' in package.extras and request.path.startswith(
+                        '/dataset/') \
+                        and package.extras['acquire_url'] != '':
+                    helpers.flash_notice(_('This private dataset can be acquired. To do so, please click ' +
+                                           '<a target="_blank" href="%s">here</a>') % package.extras['acquire_url'],
+                                         allow_html=True)
+
+        return {'success': True}
     else:
         return {'success': False, 'msg': _('User %s not authorized to read package %s') % (user, package.id)}
 
@@ -104,32 +100,49 @@ def package_update(context, data_dict):
 
 @tk.auth_allow_anonymous_access
 def resource_show(context, data_dict):
-    # This function is needed since CKAN resource_show function uses the default package_show
-    # function instead of the one defined in the plugin.
-    # A bug is openend in order to be able to remove this function
-    # https://github.com/ckan/ckan/issues/1818
-    # It's fixed now, so this function can be deleted when the new version is released.
-    _model = context['model']
-    user = context.get('user')
-    resource = logic_auth.get_resource_object(context, data_dict)
 
+    user = context.get('user')
+    user_obj = context.get('auth_user_obj')
+    resource = logic_auth.get_resource_object(context, data_dict)
     # check authentication against package
-    query = _model.Session.query(_model.Package)\
-        .join(_model.ResourceGroup)\
-        .join(_model.Resource)\
-        .filter(_model.ResourceGroup.id == resource.resource_group_id)
-    pkg = query.first()
-    if not pkg:
+    package_dict = {'id': resource.package_id}
+    package = logic_auth.get_package_object(context, package_dict)
+    if not package:
         raise tk.ObjectNotFound(_('No package found for this resource, cannot check auth.'))
 
-    pkg_dict = {'id': pkg.id}
-    authorized = package_show(context, pkg_dict).get('success')
-
-    if not authorized:
-        return {'success': False, 'msg': _('User %s not authorized to read resource %s') % (user, resource.id)}
-    else:
+    if package and user_obj and package.creator_user_id == user_obj.id:
         return {'success': True}
 
+    # active packages can only be seen by its owners
+    if package.state == 'active':
+
+        # anyone can see a public package
+        if not package.private:
+            return {'success': True}
+
+        # if the user has rights to read in the organization or in the group
+        if package.owner_org:
+            authorized = authz.has_user_permission_for_group_or_org(
+                package.owner_org, user, 'read')
+        else:
+            authorized = False
+
+        if not authorized:
+            # Init the model
+            db.init_db(context['model'])
+
+            # Branch not executed if the database return an empty list
+            if db.AllowedUser.get(package_id=package.id, user_name=user):
+                authorized = True
+
+        if not authorized:
+            return {'success': False, 'msg': _('User %s not authorized to read resource %s') % (user, resource.id)}
+
+        else:
+            return {'success': True}
+
+    else:
+        return {'success': False, 'msg': _('User %s not authorized to read resource %s') % (user, resource.id)}
 
 @tk.auth_allow_anonymous_access
 def package_acquired(context, data_dict):

ckanext/privatedatasets/converters_validators.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 # Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid
+# Copyright (c) 2019 Future Internet Consulting and Development Solutions S.L.
 
 # This file is part of CKAN Private Dataset Extension.
 
@@ -74,7 +75,6 @@ def allowed_users_convert(key, data, errors, context):
         else:
             for num, allowed_user in zip(count(current_index + 1), allowed_users):
                 allowed_user = allowed_user.strip()
-                toolkit.get_validator('name_validator')(allowed_user, context)      # User name should be validated
                 data[(key[0], num)] = allowed_user
 
 

ckanext/privatedatasets/parsers/fiware.py

@@ -52,12 +52,18 @@ class FiWareNotificationParser(object):
                 parsed_url = urlparse(resource['url'])
                 dataset_name = re.findall('^/dataset/([^/]+).*$', parsed_url.path)
 
+                resource_url = parsed_url.netloc
+                if ':' in my_host and ':' not in resource_url:
+                    # Add the default port depending on the protocol
+                    default_port = '80' if parsed_url.protocol == 'http' else '443'
+                    resource_url = resource_url + default_port
+
                 if len(dataset_name) == 1:
-                    if parsed_url.netloc == my_host:
+                    if resource_url == my_host:
                         datasets.append(dataset_name[0])
                     else:
-                        raise tk.ValidationError({'message': 'Dataset %s is associated with the CKAN instance located at %s'
-                                                 % (dataset_name[0], parsed_url.netloc)})
+                        raise tk.ValidationError({'message': 'Dataset %s is associated with the CKAN instance located at %s, expected %s'
+                                                 % (dataset_name[0], resource_url, my_host)})
             else:
                 raise tk.ValidationError({'message': 'Invalid resource format'})
 

ckanext/privatedatasets/plugin.py

@@ -29,6 +29,7 @@ from flask import Blueprint
 from ckanext.privatedatasets import auth, actions, constants, converters_validators as conv_val, db, helpers
 from ckanext.privatedatasets.views import acquired_datasets
 
+
 HIDDEN_FIELDS = [constants.ALLOWED_USERS, constants.SEARCHABLE]
 
 
@@ -43,6 +44,7 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio
     p.implements(p.IPackageController, inherit=True)
     p.implements(p.ITemplateHelpers)
     p.implements(p.IPermissionLabels)
+    p.implements(p.IResourceController)
 
     ######################################################################
     ############################ DATASET FORM ############################
@@ -112,16 +114,11 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio
     def get_auth_functions(self):
         auth_functions = {'package_show': auth.package_show,
                           'package_update': auth.package_update,
-                          # 'resource_show': auth.resource_show,
+                          'resource_show': auth.resource_show,
                           constants.PACKAGE_ACQUIRED: auth.package_acquired,
                           constants.ACQUISITIONS_LIST: auth.acquisitions_list,
                           constants.PACKAGE_DELETED: auth.revoke_access}
 
-        # resource_show is not required in CKAN 2.3 because it delegates to
-        # package_show
-        if not tk.check_ckan_version(min_version='2.3'):
-            auth_functions['resource_show'] = auth.resource_show
-
         return auth_functions
 
     ######################################################################
@@ -162,11 +159,11 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio
     ######################################################################
 
     def get_actions(self):
-        return {
-            constants.PACKAGE_ACQUIRED: actions.package_acquired,
-            constants.ACQUISITIONS_LIST: actions.acquisitions_list,
-            constants.PACKAGE_DELETED: actions.revoke_access
-        }
+        action_functions = {constants.PACKAGE_ACQUIRED: actions.package_acquired,
+                            constants.ACQUISITIONS_LIST: actions.acquisitions_list,
+                            constants.PACKAGE_DELETED: actions.revoke_access}
+
+        return action_functions
 
     ######################################################################
     ######################### IPACKAGECONTROLLER #########################
@@ -244,6 +241,16 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio
 
     def after_show(self, context, pkg_dict):
 
+        void = False;
+
+        for resource in pkg_dict['resources']:
+            if resource == {}:
+                void = True
+
+        if void:
+            del pkg_dict['resources']
+            del pkg_dict['num_resources']
+
         user_obj = context.get('auth_user_obj')
         updating_via_api = context.get(constants.CONTEXT_CALLBACK, False)
 
@@ -294,13 +301,29 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio
                 # NotAuthorized exception is risen when the user is not allowed
                 # to read the package.
                 attrs.append('resources')
-
             # Delete
             self._delete_pkg_atts(result, attrs)
 
         return search_results
 
     ####
+    def before_view(self, pkg_dict):
+
+        for resource in pkg_dict['resources']:
+
+            context = {
+                'model': model,
+                'session': model.Session,
+                'user': tk.c.user,
+                'user_obj': tk.c.userobj
+            }
+
+            try:
+                tk.check_access('resource_show', context, resource)
+            except tk.NotAuthorized:
+                pkg_dict['resources'].remove(resource)
+                pkg_dict = self.before_view(pkg_dict)
+        return pkg_dict
 
     def get_dataset_labels(self, dataset_obj):
         labels = super(PrivateDatasets, self).get_dataset_labels(
@@ -318,6 +341,34 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio
         labels.append('searchable')
         return labels
 
+    ######################################################################
+    ######################### IRESOURCECONTROLLER ########################
+    ######################################################################
+
+    def before_create(self, context, resource):
+        pass
+
+    def before_update(self, context, current, resource):
+        pass
+
+    def before_delete(self, context, resource, resources):
+        pass
+
+    def before_show(self, resource_dict):
+
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': tk.c.user,
+            'user_obj': tk.c.userobj
+        }
+
+        try:
+            tk.check_access('resource_show', context, resource_dict)
+        except tk.NotAuthorized:
+            resource_dict.clear()
+        return resource_dict
+
     ######################################################################
     ######################### ITEMPLATESHELPER ###########################
     ######################################################################

ckanext/privatedatasets/templates/snippets/package_item.html

@@ -26,7 +26,7 @@ Example:
   {% block package_item_content %}
     <div class="dataset-content">
       <h3 class="dataset-heading">
-        {% if package.private and not h.can_read(package) %}
+        {% if package.private and not owner and not acquired %}
           <span class="dataset-private label label-inverse">
             <i class="icon-lock fa fa-lock"></i>
             {{ _('Private') }}
@@ -46,8 +46,8 @@ Example:
         {% endif %}
         
         <!-- Customizations Acquire Button -->
-        {% if package.private and not h.can_read(package) %}
-            {{ _(h.truncate(title, truncate_title)) }}
+        {% if package.private and not owner and not acquired %}
+            {{ h.link_to(h.truncate(title, truncate_title), h.url_for(controller='package', action='read', id=package.name)) }}
              <div class="divider"/>
             {{ h.acquire_button(package) }}
         {% else %}

ckanext/privatedatasets/templates_2.8/snippets/package_item.html

@@ -26,7 +26,7 @@ Example:
   {% block package_item_content %}
     <div class="dataset-content">
       <h3 class="dataset-heading">
-        {% if package.private and not h.can_read(package) %}
+        {% if package.private and not owner and not acquired%}
           <span class="dataset-private label label-inverse">
             <i class="icon-lock fa fa-lock"></i>
             {{ _('Private') }}
@@ -46,8 +46,8 @@ Example:
         {% endif %}
         
         <!-- Customizations Acquire Button -->
-        {% if package.private and not h.can_read(package) %}
-            {{ _(h.truncate(title, truncate_title)) }}
+        {% if package.private and not owner and not acquired %}
+            {{ h.link_to(h.truncate(title, truncate_title), h.url_for(controller='package', action='read', id=package.name)) }}
              <div class="divider"/>
             {{ h.acquire_button(package) }}
         {% else %}

ckanext/privatedatasets/tests/test_auth.py

@@ -53,7 +53,6 @@ class AuthTest(unittest.TestCase):
         auth.authz = self._authz
         auth.tk = self._tk
         auth.db = self._db
-
         if hasattr(self, '_package_show'):
             auth.package_show = self._package_show
 
@@ -66,12 +65,12 @@ class AuthTest(unittest.TestCase):
         # Anonymous user (public)
         (None, None, None,   False, 'active', None,     None,  None,  None,        None,              True),
         # Anonymous user (private)
-        (None, None, None,   True,  'active', None,     None,  None,  None,        '/',               False),
-        (None, None, '',     True,  'active', None,     None,  '',    None,        '/',               False),
+        (None, None, None,   True,  'active', None,     None,  None,  None,        '/',               True),
+        (None, None, '',     True,  'active', None,     None,  '',    None,        '/',               True),
         # Anonymous user (private). Buy URL not shown
-        (None, None, None,   True,  'active', None,     None,  None,  'google.es', '/',               False),
+        (None, None, None,   True,  'active', None,     None,  None,  'google.es', '/',               True),
         # Anonymous user (private). Buy URL show
-        (None, None, None,   True,  'active', None,     None,  None,  'google.es', '/dataset/testds', False),
+        (None, None, None,   True,  'active', None,     None,  None,  'google.es', '/dataset/testds', True),
         # The creator can always see the dataset
         (1,    1,    None,   False, 'active', None,     None,  None,  None,        None,              True),
         (1,    1,    None,   True,  'active', 'conwet', None,  None,  None,        None,              True),
@@ -79,25 +78,26 @@ class AuthTest(unittest.TestCase):
         (1,    1,    None,   False, 'draft',  None,     None,  None,  None,        None,              True),
         # Other user (no organizations)
         (1,    2,    'test', False, 'active', None,     None,  None,  None,        None,              True),
-        (1,    2,    'test', True,  'active', None,     None,  None,  'google.es', '/',               False),  # Buy MSG not shown
-        (1,    2,    'test', True,  'active', None,     None,  None,  None,        '/dataset/testds', False),  # Buy MSG not shown
-        (1,    2,    'test', True,  'active', None,     None,  None,  'google.es', '/dataset/testds', False),  # Buy MSG shown
-        (1,    2,    'test', False, 'draft',  None,     None,  None,  None,        None, False),
+        (1,    2,    'test', True,  'active', None,     None,  None,  'google.es', '/',               True),  # Buy MSG not shown
+        (1,    2,    'test', True,  'active', None,     None,  None,  None,        '/dataset/testds', True),  # Buy MSG not shown
+        (1,    2,    'test', True,  'active', None,     None,  None,  'google.es', '/dataset/testds', True),  # Buy MSG shown
+        (1,    2,    'test', False, 'draft',  None,     None,  None,  None,        None,              False),
         # Other user but authorized in the list of authorized users
         (1,    2,    'test', True,  'active', None,     None,  True,  None,        None,              True),
         # Other user and not authorized in the list of authorized users
-        (1,    2,    'test', True,  'active', None,     None,  False, 'google.es', '/',               False),
-        (1,    2,    'test', True,  'active', None,     None,  False, 'google.es', '/dataset/testds', False),
+        (1,    2,    'test', True,  'active', None,     None,  False, 'google.es', '/',               True),
+        (1,    2,    'test', True,  'active', None,     None,  False, 'google.es', '/dataset/testds', True),
         # Other user with organizations
         (1,    2,    'test', False, 'active', 'conwet', False, None,  None,        None,              True),
-        (1,    2,    'test', True,  'active', 'conwet', False, None,  None,        None,              False),
+        (1,    2,    'test', True,  'active', 'conwet', False, None,  None,        None,              True),
         (1,    2,    'test', True,  'active', 'conwet', True,  None,  None,        None,              True),
         (1,    2,    'test', True,  'draft',  'conwet', True,  None,  None,        None,              False),
         # Other user with organizations (user is not in the organization)
         (1,    2,    'test', True,  'active', 'conwet', False, True,  None,        None,              True),
-        (1,    2,    'test', True,  'active', 'conwet', False, False, None,        None,              False),
-        (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/dataset/testds', False),
-        (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/',               False)    ])
+        (1,    2,    'test', True,  'active', 'conwet', False, False, None,        None,              True),
+        (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/dataset/testds', True),
+        (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/',               True)
+    ])
     def test_auth_package_show(self, creator_user_id, user_obj_id, user, private, state, owner_org,
                                owner_member, db_auth, acquire_url, request_path, authorized):
 
@@ -140,14 +140,14 @@ class AuthTest(unittest.TestCase):
         # Check the result
         self.assertEquals(authorized, result['success'])
 
-        # Premissions for organization are checked when the dataset is private, it belongs to an organization
+        # Permissions for organization are checked when the dataset is private, it belongs to an organization
         # and when the dataset has not been created by the user who is asking for it
         if private and owner_org and state == 'active' and creator_user_id != user_obj_id:
             auth.authz.has_user_permission_for_group_or_org.assert_called_once_with(owner_org, user, 'read')
         else:
             self.assertEquals(0, auth.authz.has_user_permission_for_group_or_org.call_count)
 
-        # The databse is only initialized when:
+        # The database is only initialized when:
         # * the dataset is private AND
         # * the dataset is active AND
         # * the dataset has no organization OR the user does not belong to that organization AND
@@ -159,7 +159,7 @@ class AuthTest(unittest.TestCase):
             self.assertEquals(0, auth.db.init_db.call_count)
 
         # Conditions to buy a dataset; It should be private, active and should not belong to any organization
-        if not authorized and state == 'active' and request_path and request_path.startswith('/dataset/') and acquire_url:
+        if authorized and state == 'active' and request_path and request_path.startswith('/dataset/') and acquire_url:
             auth.helpers.flash_notice.assert_called_once()
         else:
             self.assertEquals(0, auth.helpers.flash_notice.call_count)
@@ -203,56 +203,101 @@ class AuthTest(unittest.TestCase):
             self.assertEquals(0, auth.authz.has_user_permission_for_group_or_org.call_count)
 
     @parameterized.expand([
-        (True,  True),
-        (True,  False),
-        (False, False),
-        (False, False)
+        # if package dont exist
+        (False, None, None, None, False, 'active', None, None, None, False),
+        # Anonymous user only can view resources of a public and active package
+        (True, None, None, None, False, 'active', None, None, None, True),
+        (True, None, None, None, True, 'active', None, None, None, False),
+        (True, None, None, '', True, 'active', None, None, None, False),
+        # The creator can always see the resource
+        (True, 1, 1, None, False, 'active', None, None, None, True),
+        (True, 1, 1, None, True, 'active', None, None, None, True),
+        (True, 1, 1, None, True, 'draft', None, None, None, True),
+        (True, 1, 1, None, False, 'draft', None, None, None, True),
+        # Other user (no organizations)
+        (True, 1, 2, 'test', False, 'active', None, None, None, True),
+        (True, 1, 2, 'test', True, 'active', None, None, None, False),
+        (True, 1, 2, 'test', True, 'draft', None, None, None, False),
+        # Other user but authorized in the list of authorized users
+        (True, 1, 2, 'test', True, 'active', None, None, True, True),
+        # Other user and not authorized in the list of authorized users
+        (True, 1, 2, 'test', True, 'active', None, None, False, False),
+        # Other user with organizations
+        (True, 1, 2, 'test', True, 'active', 'conwet', False, None, False),
+        (True, 1, 2, 'test', True, 'active', 'conwet', True, None, True),
+
     ])
-    def test_auth_resource_show(self, exist_pkg=True, authorized_pkg=True):
+    def test_auth_resource_show(self, exist_pkg, creator_user_id, user_obj_id, user, private, state, owner_org,
+                                owner_member, db_auth, authorized):
         #Recover the exception
         auth.tk.ObjectNotFound = self._tk.ObjectNotFound
 
-        # Mock the calls
-        package = MagicMock()
-        package.id = '1'
+        # Configure the mocks
+        if exist_pkg:
+            returned_package = MagicMock()
+            returned_package.creator_user_id = creator_user_id
+            returned_package.private = private
+            returned_package.state = state
+            returned_package.owner_org = owner_org
+            returned_package.extras = {}
+        else:
+            returned_package = None
 
-        final_query = MagicMock()
-        final_query.first = MagicMock(return_value=package if exist_pkg else None)
+        returned_resource = MagicMock()
+        returned_resource.package_id = 1
 
-        second_join = MagicMock()
-        second_join.filter = MagicMock(return_value=final_query)
+        # Configure the database
+        db_response = []
+        if db_auth is True:
+            out = auth.db.AllowedUser()
+            out.package_id = 'package_id'
+            out.user_name = user
+            db_response.append(out)
 
-        first_join = MagicMock()
-        first_join.join = MagicMock(return_value=second_join)
+        # Prepare the context
+        context = {'model': MagicMock()}
+        if user is not None:
+            context['user'] = user
+        if user_obj_id is not None:
+            context['auth_user_obj'] = MagicMock()
+            context['auth_user_obj'].id = user_obj_id
 
-        query = MagicMock()
-        query.join = MagicMock(return_value=first_join)
+        auth.db.AllowedUser.get = MagicMock(return_value=db_response)
+        auth.logic_auth.get_resource_object = MagicMock(return_value=returned_resource)
+        auth.logic_auth.get_package_object = MagicMock(return_value=returned_package)
+        auth.authz.has_user_permission_for_group_or_org = MagicMock(return_value=owner_member)
 
-        model = MagicMock()
-        session = MagicMock()
-        session.query = MagicMock(return_value=query)
-        model.Session = session
-
-        # Create the context
-        context = {}
-        context['model'] = model
-
-        # Mock the package_show function
-        self._package_show = auth.package_show
-        success = True if authorized_pkg else False
-        auth.package_show = MagicMock(return_value={'success': success})
+        # Prepare the context
+        context = {'model': MagicMock()}
+        if user is not None:
+            context['user'] = user
+        if user_obj_id is not None:
+            context['auth_user_obj'] = MagicMock()
+            context['auth_user_obj'].id = user_obj_id
 
         if not exist_pkg:
             self.assertRaises(self._tk.ObjectNotFound, auth.resource_show, context, {})
         else:
             result = auth.resource_show(context, {})
-            self.assertEquals(authorized_pkg, result['success'])
+            self.assertEquals(authorized, result['success'])
+
+            if private and owner_org and state == 'active' and creator_user_id != user_obj_id:
+                auth.authz.has_user_permission_for_group_or_org.assert_called_once_with(owner_org, user, 'read')
+            else:
+                self.assertEquals(0, auth.authz.has_user_permission_for_group_or_org.call_count)
+
+            if private and state == 'active' and (not owner_org or not owner_member) and (creator_user_id != user_obj_id or user_obj_id is None):
+                # Check that the database has been initialized properly
+                auth.db.init_db.assert_called_once_with(context['model'])
+            else:
+                self.assertEquals(0, auth.db.init_db.call_count)
+
 
     def test_package_acquired(self):
         self.assertTrue(auth.package_acquired({}, {})['success'])
 
     def test_package_deleted(self):
-        self.assertTrue(auth.revoke_access({},{})['success'])
+        self.assertTrue(auth.revoke_access({}, {})['success'])
 
     @parameterized.expand([
         ({'user': 'user_1'}, {'user': 'user_1'}, True),

ckanext/privatedatasets/tests/test_converters_validators.py

@@ -127,10 +127,6 @@ class ConvertersValidatorsTest(unittest.TestCase):
     def test_allowed_user_convert(self, users, previous_users, expected_users):
         key_str = 'allowed_users_str'
         key = 'allowed_users'
-
-        # Configure mock
-        name_validator = MagicMock()
-        conv_val.toolkit.get_validator = MagicMock(return_value=name_validator)
         
         # Fullfill the data dictionary
         # * list should be included in the allowed_users filed
@@ -151,7 +147,6 @@ class ConvertersValidatorsTest(unittest.TestCase):
 
         # Check that the users are set properly
         for i in range(previous_users, previous_users + len(expected_users)):
-            name_validator.assert_any_call(expected_users[i - previous_users], context)
             self.assertEquals(expected_users[i - previous_users], data[(key, i)])
 
     @parameterized.expand([

ckanext/privatedatasets/tests/test_fiware_parser.py

@@ -39,26 +39,26 @@ TEST_CASES = {
     'error': {
         'host': 'localhost',
         'json': {"customer_name": "test", "resources": [{"url": "http://localhosta/dataset/ds1"}]},
-        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta',
+        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta, expected localhost',
     },
     'error_one_ds': {
         'host': 'localhost',
         'json': {"customer_name": "test", "resources": [{"url": "http://localhosta/dataset/ds1"},
                 {"url": "http://localhost/dataset/ds2"}]},
-        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta',
+        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta, expected localhost',
     },
     'two_errors': {
         'host': 'localhost',
         'json': {"customer_name": "test", "resources": [{"url": "http://localhosta/dataset/ds1"},
                 {"url": "http://localhostb/dataset/ds2"}]},
-        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta',
+        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta, expected localhost',
     },
     'two_errors_two_ds': {
         'host': 'example.com',
         'json': {"customer_name": "test", "resources": [{"url": "http://localhosta/dataset/ds1"},
                 {"url": "http://example.es/dataset/ds2"}, {"url": "http://example.com/dataset/ds3"},
                 {"url": "http://example.com/dataset/ds4"}]},
-        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta',
+        'error': 'Dataset ds1 is associated with the CKAN instance located at localhosta, expected example.com',
     },
     'no_customer_name': {
         'host': 'localhost',

ckanext/privatedatasets/tests/test_helpers.py

@@ -39,7 +39,6 @@ class HelpersTest(unittest.TestCase):
         self._db = helpers.db
         helpers.db = MagicMock()
 
-
         self._request = helpers.request
         helpers.request = MagicMock()
 

ckanext/privatedatasets/tests/test_plugin.py

@@ -65,19 +65,13 @@ class PluginTest(unittest.TestCase):
         ('package_show',      plugin.auth.package_show),
         ('package_update',    plugin.auth.package_update),
         ('resource_show',     plugin.auth.resource_show),
-        ('resource_show',     plugin.auth.resource_show,     True,  False),
         ('package_acquired',  plugin.auth.package_acquired),
         ('acquisitions_list', plugin.auth.acquisitions_list),
         ('revoke_access',   plugin.auth.revoke_access)
     ])
-    def test_auth_function(self, function_name, expected_function, is_ckan_23=False, expected=True):
-        plugin.tk.check_ckan_version = MagicMock(return_value=is_ckan_23)
+    def test_auth_function(self, function_name, expected_function):
         auth_functions = self.privateDatasets.get_auth_functions()
-
-        if expected:
-            self.assertEquals(auth_functions[function_name], expected_function)
-        else:
-            self.assertNotIn(function_name, auth_functions)
+        self.assertEquals(auth_functions[function_name], expected_function)
 
     def test_update_config(self):
         # Call the method
@@ -215,41 +209,73 @@ class PluginTest(unittest.TestCase):
             self.assertTrue(found)
 
     @parameterized.expand([
-        (True,  1, 1,    False, True,  True),
-        (True,  1, 2,    False, True,  True),
-        (True,  1, 1,    True,  True,  True),
-        (True,  1, 2,    True,  True,  True),
-        (True,  1, None, None,  True,  True),
-        (True,  1, 1,    None,  True,  True),
-        (True,  1, None, True,  True,  True),
-        (True,  1, None, False, True,  True),
-        (False, 1, 1,    False, True,  True),
-        (False, 1, 2,    False, True,  False),
-        (False, 1, 1,    True,  True,  True),
-        (False, 1, 2,    True,  True,  True),
-        (False, 1, None, None,  True,  False),
-        (False, 1, 1,    None,  True,  True),
-        (False, 1, None, True,  True,  True),
-        (False, 1, None, False, True,  False),
-        (True,  1, 1,    False, False, False),
-        (True,  1, 2,    False, False, False),
-        (True,  1, 1,    True,  False, False),
-        (True,  1, 2,    True,  False, False),
-        (True,  1, None, None,  False, False),
-        (True,  1, 1,    None,  False, False),
-        (True,  1, None, True,  False, False),
-        (True,  1, None, False, False, False),
-        (False, 1, 1,    False, False, False),
-        (False, 1, 2,    False, False, False),
-        (False, 1, 1,    True,  False, False),
-        (False, 1, 2,    True,  False, False),
-        (False, 1, None, None,  False, False),
-        (False, 1, 1,    None,  False, False),
-        (False, 1, None, True,  False, False),
-        (False, 1, None, False, False, False),
+        (True,  1, 1,    False, True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 2,    False, True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 1,    True,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 2,    True,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, None, None,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 1,    None,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, None, True,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (True,  1, None, False, True,  True, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 1,    False, True,  True, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 2,    False, True,  False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 1,    True,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 2,    True,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (False, 1, None, None,  True,  False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 1,    None,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (False, 1, None, True,  True,  True, [{'id': 1}, {'id': 2}], True),
+        (False, 1, None, False, True,  False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 1,    False, False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 2,    False, False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 1,    True,  False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 2,    True,  False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, None, None,  False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, 1,    None,  False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, None, True,  False, False, [{'id': 1}, {'id': 2}], True),
+        (True,  1, None, False, False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 1,    False, False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 2,    False, False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 1,    True,  False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 2,    True,  False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, None, None,  False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, 1,    None,  False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, None, True,  False, False, [{'id': 1}, {'id': 2}], True),
+        (False, 1, None, False, False, False, [{'id': 1}, {'id': 2}], True),
+        (True, 1, 1, False, True, True, [{}, {}], False),
+        (True, 1, 2, False, True, True, [{}, {}], False),
+        (True, 1, 1, True, True, True, [{}, {}], False),
+        (True, 1, 2, True, True, True, [{}, {}], False),
+        (True, 1, None, None, True, True, [{}, {}], False),
+        (True, 1, 1, None, True, True, [{}, {}], False),
+        (True, 1, None, True, True, True, [{}, {}], False),
+        (True, 1, None, False, True, True, [{}, {}], False),
+        (False, 1, 1, False, True, True, [{}, {}], False),
+        (False, 1, 2, False, True, False, [{}, {}], False),
+        (False, 1, 1, True, True, True, [{}, {}], False),
+        (False, 1, 2, True, True, True, [{}, {}], False),
+        (False, 1, None, None, True, False, [{}, {}], False),
+        (False, 1, 1, None, True, True, [{}, {}], False),
+        (False, 1, None, True, True, True, [{}, {}], False),
+        (False, 1, None, False, True, False, [{}, {}], False),
+        (True, 1, 1, False, False, False, [{}, {}], False),
+        (True, 1, 2, False, False, False, [{}, {}], False),
+        (True, 1, 1, True, False, False, [{}, {}], False),
+        (True, 1, 2, True, False, False, [{}, {}], False),
+        (True, 1, None, None, False, False, [{}, {}], False),
+        (True, 1, 1, None, False, False, [{}, {}], False),
+        (True, 1, None, True, False, False, [{}, {}], False),
+        (True, 1, None, False, False, False, [{}, {}], False),
+        (False, 1, 1, False, False, False, [{}, {}], False),
+        (False, 1, 2, False, False, False, [{}, {}], False),
+        (False, 1, 1, True, False, False, [{}, {}], False),
+        (False, 1, 2, True, False, False, [{}, {}], False),
+        (False, 1, None, None, False, False, [{}, {}], False),
+        (False, 1, 1, None, False, False, [{}, {}], False),
+        (False, 1, None, True, False, False, [{}, {}], False),
+        (False, 1, None, False, False, False, [{}, {}], False),
     ])
-    def test_packagecontroller_after_show(self, update_via_api, creator_id, user_id, sysadmin, private, fields_expected):
-        
+    def test_packagecontroller_after_show(self, update_via_api, creator_id, user_id, sysadmin, private, fields_expected, resources, resources_fields):
+
         context = {'updating_via_cb': update_via_api}
 
         if creator_id is not None or sysadmin is not None:
@@ -258,7 +284,7 @@ class PluginTest(unittest.TestCase):
             user.sysadmin = sysadmin
             context['auth_user_obj'] = user
 
-        pkg_dict = {'creator_user_id': creator_id, 'allowed_users': ['a', 'b', 'c'], 'searchable': True, 'acquire_url': 'http://google.es', 'private': private}
+        pkg_dict = {'creator_user_id': creator_id, 'allowed_users': ['a', 'b', 'c'], 'searchable': True, 'acquire_url': 'http://google.es', 'private': private, 'resources': resources, 'num_resources': 2}
 
         # Call the function
         result = self.privateDatasets.after_show(context, pkg_dict)    # Call the function
@@ -271,6 +297,14 @@ class PluginTest(unittest.TestCase):
             else:
                 self.assertFalse(field in result)
 
+        fields = ['resources', 'num_resources']
+        for field in fields:
+            if resources_fields:
+                self.assertTrue(field in result)
+            else:
+                self.assertFalse(field in result)
+
+
     @parameterized.expand([
         ('public',  None,    'public'),
         ('public',  'False', 'private'),
@@ -449,3 +483,40 @@ class PluginTest(unittest.TestCase):
 
         self.assertEquals(final_search_results['facets'], search_results['facets'])
         self.assertEquals(final_search_results['elements'], search_results['elements'])
+
+    @parameterized.expand([
+        (True,),
+        (False,)
+    ])
+    def test_package_controller_before_view(self, user_allowed):
+
+        pkg_dict = {'resources': [{'id': 1}, {'id': 2}, {'id': 3}]}
+        pkg_dict_not_allowed = {'resources': []}
+
+        plugin.tk.check_access.side_effect = None if user_allowed else plugin.tk.NotAuthorized
+
+        result = self.privateDatasets.before_view(pkg_dict)
+
+        if user_allowed:
+            self.assertEquals(result['resources'], pkg_dict['resources'])
+        else:
+            self.assertEquals(result['resources'], pkg_dict_not_allowed['resources'])
+
+    @parameterized.expand([
+        (True,),
+        (False,)
+    ])
+    def test_resource_controller_before_show(self, user_allowed):
+
+        resource_dict = {'id': 1, 'resource_name': 'resource_test'}
+
+        plugin.tk.check_access.side_effect = None if user_allowed else plugin.tk.NotAuthorized
+
+        result = self.privateDatasets.before_show(resource_dict)
+
+        if user_allowed:
+            self.assertEquals(result['id'], resource_dict['id'])
+            self.assertEquals(result['resource_name'], resource_dict['resource_name'])
+        else:
+            self.assertNotIn('id', result)
+            self.assertNotIn('resource_name', result)

ckanext/privatedatasets/tests/test_selenium.py

@@ -270,13 +270,13 @@ class TestSelenium(unittest.TestCase):
         driver.get(self.base_url + 'dataset/' + dataset_url)
 
         if not acquired and private and not in_org:
-            # If the user has not access to the dataset the 404 error page is displayed
-            xpath = '//*[@id="content"]/div[2]/article/div/h1'
-            msg = '404 Not Found'
+            # If the dataset is private and the user hasnt access to the resources, the field resources dont appear
 
-            self.assertEqual(driver.find_element_by_xpath(xpath).text, msg)
+            self.assertEquals('empty', driver.find_element_by_class_name('empty').get_attribute('class'))
+            self.assertEqual(self.base_url + 'dataset/%s' % dataset_url, driver.current_url)
 
         else:
+            self.assertEquals('resource-list', driver.find_element_by_class_name('resource-list').get_attribute('class'))
             self.assertEqual(self.base_url + 'dataset/%s' % dataset_url, driver.current_url)
 
     def check_acquired(self, dataset, dataset_url, acquired, private):
@@ -337,14 +337,6 @@ class TestSelenium(unittest.TestCase):
             self.check_acquired(pkg_name, url, acquired, private)
 
     @parameterized.expand([
-        (['upm', 'a'],     'http://upm.es',      'Allowed users: Must be at least 2 characters long'),
-        (['upm', 'a a a'], 'http://upm.es',      'Allowed users: Must be purely lowercase alphanumeric (ascii) characters and these symbols: -_'),
-        (['upm', 'a?-vz'], 'http://upm.es',      'Allowed users: Must be purely lowercase alphanumeric (ascii) characters and these symbols: -_'),
-        (
-            ['thisisaveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylongname'],
-            'http://upm.es',
-            'Allowed users: Name must be a maximum of 100 characters long'
-        ),
         (['conwet'],       'ftp://google.es',    'Acquire URL: The URL "ftp://google.es" is not valid.'),
         (['conwet'],       'google',             'Acquire URL: The URL "google" is not valid.'),
         (['conwet'],       'http://google',      'Acquire URL: The URL "http://google" is not valid.'),

setup.py

@@ -20,7 +20,7 @@
 
 from setuptools import setup, find_packages
 
-version = '0.4'
+version = '0.4.1'
 
 setup(
     name='ckanext-privatedatasets',
@@ -31,8 +31,8 @@ setup(
     ''',
     classifiers=[], # Get strings from http://pypi.python.org/pypi?%3Aaction=list_classifiers
     keywords='ckan, private, datasets',
-    author='Aitor Magan',
-    author_email='amagan@conwet.com',
+    author='Aitor Magan, Francisco de la Vega',
+    author_email='fdelavega@ficodes.com',
     url='https://conwet.fi.upm.es',
     download_url='https://github.com/conwetlab/ckanext-privatedatasets/tarball/v' + version,
     license='',

test.ini

@@ -4,6 +4,7 @@ host = 0.0.0.0
 port = 5000
 
 [app:main]
+# use = config:/usr/lib/ckan/default/src/ckan/test-core.ini
 use = config:./ckan/test-core.ini
 
 ckan.site_id = ckanext.privatedatasets.test