Merge branch 'no-urlid-in-username' into 'master'

No urlid in username

See merge request !120

README.md

@@ -39,7 +39,26 @@ INSTALLED_APPS = [
 
 IMPORTANT: DjangoLDP will register any models which haven't been registered, with the admin. As such it is important to add your own apps above DjangoLDP, so that you can use custom Admin classes if you wish
 
-4. Create your django model inside a file myldpserver/myldpserver/models.py
+### User model requirements
+
+When implementing authentication in your own application, you have two options:
+
+* Using or extending [DjangoLDP-Account](https://git.startinblox.com/djangoldp-packages/djangoldp-account), a DjangoLDP package modelling federated users
+* Using your own user model & defining the authentication behaviour yourself
+
+Please see the [Authentication guide](https://git.startinblox.com/djangoldp-packages/djangoldp/wikis/guides/authentication) for full information
+
+If you're going to use your own model then for federated login to work your user model must extend `DjangoLDP.Model`, or define a `urlid` field on the user model, for example:
+```python
+urlid = LDPUrlField(blank=True, null=True, unique=True)
+```
+If you don't include this field, then all users will be treated as users local to your instance
+
+The `urlid` field is used to uniquely identify the user and is part of the Linked Data Protocol standard. For local users it can be generated at runtime, but for some resources which are from distant servers this is required to be stored
+
+## Creating your first model
+
+1. Create your django model inside a file myldpserver/myldpserver/models.py
 Note that container_path will be use to resolve instance iri and container iri
 In the future it could also be used to auto configure django router (e.g. urls.py)
 
@@ -51,14 +70,14 @@ class Todo(Model):
     deadline = models.DateTimeField()
 ```
 
-4.1. Configure container path (optional)
+1.1. Configure container path (optional)
 By default it will be "todos/" with an S for model called Todo
 
 ```python
 <Model>._meta.container_path = "/my-path/"
 ```
 
-4.2. Configure field visibility (optional) 
+1.2. Configure field visibility (optional) 
 Note that at this stage you can limit access to certain fields of models using
 
 ```python
@@ -77,7 +96,7 @@ User._meta.serializer_fields  = ('username','first_name','last_name')
 
 Note that this will be overridden if you explicitly set the fields= parameter as an argument to LDPViewSet.urls(), and filtered if you set the excludes= parameter.
 
-5. Add a url in your urls.py:
+2. Add a url in your urls.py:
 
 ```python
 from django.conf.urls import url
@@ -99,7 +118,7 @@ You could also only use this line in settings.py instead:
 ROOT_URLCONF = 'djangoldp.urls'
 ```
 
-6. In the settings.py file, add your application name at the beginning of the application list, and add the following lines
+3. In the settings.py file, add your application name at the beginning of the application list, and add the following lines
 
 ```python
 STATIC_ROOT = os.path.join(os.path.dirname(BASE_DIR), 'static')
@@ -115,7 +134,7 @@ BASE_URL = SITE_URL
 * `BASE_URL` may be different from SITE_URL, e.g. `https://example.com/app/`
 
 
-7. You can also register your model for the django administration site
+4. You can also register your model for the django administration site
 
 ```python
 from django.contrib import admin
@@ -124,15 +143,15 @@ from .models import Todo
 admin.site.register(Todo)
 ```
 
-8. You then need to have your WSGI server pointing on myldpserver/myldpserver/wsgi.py
+5. You then need to have your WSGI server pointing on myldpserver/myldpserver/wsgi.py
 
-9. You will probably need to create a super user
+6. You will probably need to create a super user
 
 ```bash
 $ ./manage.py createsuperuser
 ```
 
-10. If you have no CSS on the admin screens :
+7. If you have no CSS on the admin screens :
 
 ```bash
 $ ./manage.py collectstatic

djangoldp/models.py

-import validators
+from urllib.parse import urlparse
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.db import models
@@ -188,13 +188,16 @@ def auto_urlid(sender, instance, **kwargs):
 
 if 'djangoldp_account' not in settings.DJANGOLDP_PACKAGES:
     def webid(self):
-        # hack : We user webid as username for external user (since it's an uniq identifier too)
-        if validators.url(self.username):
-            webid = self.username
-        # unable to use username, so use user-detail URL with primary key
+        # an external user should have urlid set
+        webid = getattr(self, 'urlid', None)
+        if webid is not None and urlparse(settings.BASE_URL).netloc != urlparse(webid).netloc:
+            webid = self.urlid
+        # local user use user-detail URL with primary key
         else:
             webid = '{0}{1}'.format(settings.BASE_URL, reverse_lazy('user-detail', kwargs={'pk': self.pk}))
         return webid
 
-    get_user_model()._meta.serializer_fields = ['@id']
+    if get_user_model()._meta.serializer_fields is None:
+        get_user_model()._meta.serializer_fields = []
+    get_user_model()._meta.serializer_fields.append('@id')
     get_user_model().webid = webid

djangoldp/serializers.py

+import uuid
 from collections import OrderedDict, Mapping, Iterable
 from typing import Any
 from urllib import parse
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
-from django.contrib.auth.models import AbstractUser
 from django.core.exceptions import ImproperlyConfigured
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.core.urlresolvers import get_resolver, resolve, get_script_prefix, Resolver404
@@ -491,13 +491,14 @@ class LDPSerializer(HyperlinkedModelSerializer):
         return serializer
 
     def to_internal_value(self, data):
-        user_case = self.Meta.model is get_user_model() and '@id' in data and not data['@id'].startswith(
+        is_user_and_external = self.Meta.model is get_user_model() and '@id' in data and not data['@id'].startswith(
             settings.BASE_URL)
-        if user_case:
+        if is_user_and_external:
             data['username'] = 'external'
         ret = super().to_internal_value(data)
-        if user_case:
-            ret['username'] = data['@id']
+        if is_user_and_external:
+            ret['urlid'] = data['@id']
+            ret.pop('username')
         return ret
 
     def get_value(self, dictionary):
@@ -562,8 +563,8 @@ class LDPSerializer(HyperlinkedModelSerializer):
                     field_name in validated_data) and not field_name is None:
                 many_to_many.append((field_name, validated_data.pop(field_name)))
         validated_data = self.remove_empty_value(validated_data)
-        if model is get_user_model() and 'urlid' in validated_data and not 'username' in validated_data:
-            validated_data['username'] = validated_data.pop('urlid')
+        if model is get_user_model() and not 'username' in validated_data:
+            validated_data['username'] = uuid.uuid4()
         instance = model.objects.create(**validated_data)
 
         for field_name, value in many_to_many:
@@ -625,9 +626,6 @@ class LDPSerializer(HyperlinkedModelSerializer):
                 elif hasattr(field_model, 'urlid'):
                     kwargs = {'urlid': field_dict['urlid']}
                     sub_inst = field_model.objects.get(**kwargs)
-                elif issubclass(field_model, AbstractUser):
-                    kwargs = {'username': field_dict['urlid']}
-                    sub_inst = field_model.objects.get(**kwargs)
             # try slug field, assuming that this is a local resource
             elif slug_field in field_dict:
                 kwargs = {slug_field: field_dict[slug_field]}
@@ -707,18 +705,18 @@ class LDPSerializer(HyperlinkedModelSerializer):
                 elif slug_field in item:
                     kwargs = {slug_field: item[slug_field]}
                     saved_item = self.get_or_create(field_model, item, kwargs)
-                elif 'urlid' in item and settings.BASE_URL in item['urlid']:
-                    model, old_obj = Model.resolve(item['urlid'])
-                    if old_obj is not None:
-                        saved_item = self.update(instance=old_obj, validated_data=item)
-                    else:
-                        saved_item = self.internal_create(validated_data=item, model=field_model)
-                elif 'urlid' in item and issubclass(field_model, AbstractUser):
-                    kwargs = {'username': item['urlid']}
-                    saved_item = self.get_or_create(field_model, item, kwargs)
                 elif 'urlid' in item:
-                    kwargs = {'urlid': item['urlid']}
-                    saved_item = self.get_or_create(field_model, item, kwargs)
+                    # has urlid and is a local resource
+                    if parse.urlparse(settings.BASE_URL).netloc == parse.urlparse(item['urlid']).netloc:
+                        model, old_obj = Model.resolve(item['urlid'])
+                        if old_obj is not None:
+                            saved_item = self.update(instance=old_obj, validated_data=item)
+                        else:
+                            saved_item = self.internal_create(validated_data=item, model=field_model)
+                    # has urlid and is external resource
+                    elif hasattr(field_model, 'urlid'):
+                        kwargs = {'urlid': item['urlid']}
+                        saved_item = self.get_or_create(field_model, item, kwargs)
                 else:
                     rel = getattr(instance._meta.model, field_name).rel
                     try:

djangoldp/tests/models.py

 from django.conf import settings
-from django.contrib.auth import get_user_model
+from django.contrib.auth.models import AbstractUser
 from django.db import models
 from django.utils.datetime_safe import date
 
 from djangoldp.models import Model
 
 
+class User(AbstractUser, Model):
+
+    class Meta(AbstractUser.Meta, Model.Meta):
+        serializer_fields = ['@id', 'username', 'first_name', 'last_name', 'email', 'userprofile',
+                             'conversation_set', 'circle_set']
+        anonymous_perms = ['view', 'add']
+        authenticated_perms = ['inherit', 'change']
+        owner_perms = ['inherit']
+
+
 class Skill(Model):
     title = models.CharField(max_length=255, blank=True, null=True)
     obligatoire = models.CharField(max_length=255)
@@ -183,7 +193,3 @@ class Circle(Model):
         authenticated_perms = ["inherit"]
         rdf_type = 'hd:circle'
         depth = 1
-
-get_user_model()._meta.serializer_fields = ['@id', 'username', 'first_name', 'last_name', 'email', 'userprofile',
-                                            'conversation_set', 'circle_set']
-get_user_model()._meta.anonymous_perms = ['view', 'add']

djangoldp/tests/runner.py

@@ -36,6 +36,8 @@ settings.configure(DEBUG=False,
                            "control": "acl:Control"
                        }
                    },
+                   AUTH_USER_MODEL='tests.User',
+                   ANONYMOUS_USER_NAME = None,
                    AUTHENTICATION_BACKENDS=(
                        'django.contrib.auth.backends.ModelBackend', 'guardian.backends.ObjectPermissionBackend'),
                    ROOT_URLCONF='djangoldp.urls',

djangoldp/tests/tests_update.py

+import uuid
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 from rest_framework.test import APIRequestFactory, APIClient
@@ -526,11 +527,11 @@ class Update(TestCase):
 
     def test_m2m_user_link_existing_external(self):
         circle = Circle.objects.create(description="cicle name")
-        ext_user = get_user_model().objects.create(username='http://external.user/user/1')
+        ext_user = get_user_model().objects.create(username=str(uuid.uuid4()), urlid='http://external.user/user/1')
         body = {
             'http://happy-dev.fr/owl/#description': 'circle name',
             'http://happy-dev.fr/owl/#team': {
-                'http://happy-dev.fr/owl/#@id': ext_user.username,
+                'http://happy-dev.fr/owl/#@id': ext_user.urlid,
             }
         }
 
@@ -538,8 +539,7 @@ class Update(TestCase):
                                    data=json.dumps(body),
                                    content_type='application/ld+json')
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data['team']['ldp:contains'][0]['@id'],
-                         ext_user.username)
+        self.assertEqual(response.data['team']['ldp:contains'][0]['@id'], ext_user.urlid)
 
         circle = Circle.objects.get(pk=circle.pk)
         self.assertEqual(circle.team.count(), 1)
@@ -574,7 +574,7 @@ class Update(TestCase):
         self.assertIn('userprofile', response.data)
 
     def test_m2m_user_link_remove_existing_link(self):
-        ext_user = get_user_model().objects.create(username='http://external.user/user/1')
+        ext_user = get_user_model().objects.create(username=str(uuid.uuid4()), urlid='http://external.user/user/1')
         circle = Circle.objects.create(description="cicle name")
         circle.team.add(ext_user)
         circle.save()