allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From brond...@apache.org
Subject [allura] 01/03: [#8330] use original username in /u/foo_bar situations when there is an underscore for example
Date Fri, 30 Aug 2019 17:34:06 GMT
This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch db/8330
in repository https://gitbox.apache.org/repos/asf/allura.git

commit aa615d451806429d890488a6331b261aad153241
Author: Dave Brondsema <dave@brondsema.net>
AuthorDate: Fri Aug 30 12:24:08 2019 -0400

    [#8330] use original username in /u/foo_bar situations when there is an underscore for
example
---
 Allura/allura/controllers/rest.py                  | 18 ++++++-------
 .../ext/user_profile/templates/user_index.html     |  2 +-
 Allura/allura/lib/plugin.py                        | 23 ++++++++++++++++-
 Allura/allura/model/auth.py                        |  2 +-
 Allura/allura/model/project.py                     |  8 ++++--
 Allura/allura/model/repository.py                  | 15 ++++++++---
 .../allura/tests/functional/test_user_profile.py   | 30 ++++++++++------------
 7 files changed, 64 insertions(+), 34 deletions(-)

diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py
index 291c133..b3ae80d 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -319,16 +319,6 @@ def nbhd_lookup_first_path(nbhd, name, current_user, remainder, api=False):
         user = M.User.query.get(username=pname, disabled=False, pending=False)
         if user:
             project = user.private_project()
-            if project.shortname != prefix + pname:
-                # might be different URL than the URL requested
-                # e.g. if username isn't valid project name and user_project_shortname()
converts the name
-                new_url = project.url()
-                if api:
-                    new_url = '/rest' + new_url
-                new_url += '/'.join(remainder)
-                if request.query_string:
-                    new_url += '?' + request.query_string
-                redirect(new_url)
     if project is None:
         # look for neighborhood tools matching the URL
         project = nbhd.neighborhood_project
@@ -338,6 +328,14 @@ def nbhd_lookup_first_path(nbhd, name, current_user, remainder, api=False):
         user = project.user_project_of
         if not user or user.disabled or user.pending:
             raise exc.HTTPNotFound
+        if not api and user.url() != '/{}{}/'.format(prefix, pname):
+            # might be different URL than the URL requested
+            # e.g. if username isn't valid project name and user_project_shortname() converts
the name
+            new_url = user.url()
+            new_url += '/'.join(remainder)
+            if request.query_string:
+                new_url += '?' + request.query_string
+            redirect(new_url)
     if project.database_configured is False:
         if remainder == ('user_icon',):
             redirect(g.forge_static('images/user.png'))
diff --git a/Allura/allura/ext/user_profile/templates/user_index.html b/Allura/allura/ext/user_profile/templates/user_index.html
index 5232a5a..1ac5231 100644
--- a/Allura/allura/ext/user_profile/templates/user_index.html
+++ b/Allura/allura/ext/user_profile/templates/user_index.html
@@ -37,7 +37,7 @@
     {{ g.icons['mail'].render(
         title='Send Message',
         show_title=True,
-        href='send_message',
+        href=c.app.url + 'send_message',
         extra_css='btn',
         id='user-message') }}
     {% endif %}
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 3f9769a..1fde18c 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -350,6 +350,14 @@ class AuthenticationProvider(object):
         '''
         raise NotImplementedError('user_project_shortname')
 
+    def user_project_url(self, user):
+        '''
+        :param user: a :class:`User <allura.model.auth.User>`
+        :rtype: str
+        '''
+        # default implementation for any providers that haven't implemented this newer method
yet
+        return '/{}/'.format(self.user_project_shortname(user))
+
     def user_by_project_shortname(self, shortname):
         '''
         :param str: shortname
@@ -557,11 +565,24 @@ class LocalAuthenticationProvider(AuthenticationProvider):
         return 'sha256' + salt + b64encode(hashpass)
 
     def user_project_shortname(self, user):
+        # "_" isn't valid for subdomains (which project names are used with)
+        # so if a username has a "_" we change it to "-"
+        # may need to handle other characters at some point
         return 'u/' + user.username.replace('_', '-')
 
+    def user_project_url(self, user):
+        # in contrast with above user_project_shortname()
+        # we allow the URL of a user-project to match the username exactly, even if user-project's
name is different
+        # (nbhd_lookup_first_path will figure it out)
+        return '/u/{}/'.format(user.username)
+
     def user_by_project_shortname(self, shortname):
         from allura import model as M
-        return M.User.query.get(username=shortname, disabled=False, pending=False)
+        user = M.User.query.get(username=shortname, disabled=False, pending=False)
+        if not user and '-' in shortname:
+            # try alternate version in case username & user-project shortname differ
- see user_project_shortname()
+            user = M.User.query.get(username=shortname.replace('-', '_'), disabled=False,
pending=False)
+        return user
 
     def update_notifications(self, user):
         return ''
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 7188e20..356355e 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -601,7 +601,7 @@ class User(MappedClass, ActivityNode, ActivityObject, SearchIndexable):
         This includes any special handling via the :class:`~allura.lib.plugin.AuthenticationProvider`
to determine
         the proper user-project name
         '''
-        return '/%s/' % plugin.AuthenticationProvider.get(request).user_project_shortname(self)
+        return plugin.AuthenticationProvider.get(request).user_project_url(self)
 
     @memoize
     def icon_url(self, gravatar_default_url=None, return_more=False):
diff --git a/Allura/allura/model/project.py b/Allura/allura/model/project.py
index d4edf27..04fe67f 100644
--- a/Allura/allura/model/project.py
+++ b/Allura/allura/model/project.py
@@ -338,10 +338,14 @@ class Project(SearchIndexable, MappedClass, ActivityNode, ActivityObject):
         else:
             return url
 
-    def url(self):
+    def url(self, use_userproject_shortname=False):
         if self.is_nbhd_project:
             return self.neighborhood.url()
         shortname = self.shortname[len(self.neighborhood.shortname_prefix):]
+        if self.neighborhood.url_prefix == '/u/' and not use_userproject_shortname:
+            user = self.user_project_of
+            if user:
+                return user.url()
         url = self.neighborhood.url_prefix + shortname + '/'
         if url.startswith('//'):
             try:
@@ -452,7 +456,7 @@ class Project(SearchIndexable, MappedClass, ActivityNode, ActivityObject):
     def is_user_project(self):
         return self.is_root and self.shortname.startswith('u/')
 
-    @LazyProperty
+    @property
     def user_project_of(self):
         '''
         If this is a user-project, return the User, else None
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index 7221a86..98200fa 100644
--- a/Allura/allura/model/repository.py
+++ b/Allura/allura/model/repository.py
@@ -380,11 +380,15 @@ class Repository(Artifact, ActivityObject):
     @classmethod
     def default_fs_path(cls, project, tool):
         repos_root = tg.config.get('scm.repos.root', '/')
-        return os.path.join(repos_root, tool, project.url()[1:])
+        # if a user-project, the repository path on disk needs to use the actual shortname
+        # the nice url might have invalid chars
+        return os.path.join(repos_root, tool, project.url(use_userproject_shortname=True)[1:])
 
     @classmethod
     def default_url_path(cls, project, tool):
-        return project.url()
+        # if a user-project, the repository checkout path needs to use the actual shortname
used on disk
+        # not a nicer username (e.g. with underscores) used on web browsing urls
+        return project.url(use_userproject_shortname=True)
 
     @property
     def tarball_path(self):
@@ -612,7 +616,12 @@ class Repository(Artifact, ActivityObject):
         return os.path.join(self.fs_path, self.name)
 
     def suggested_clone_dest_path(self):
-        return '%s-%s' % (c.project.shortname.replace('/', '-'), self.name)
+        owning_user = c.project.user_project_of
+        if owning_user:
+            projname = owning_user.username
+        else:
+            projname = c.project.shortname.replace('/', '-')
+        return '%s-%s' % (projname, self.name)
 
     def clone_url(self, category, username=''):
         '''Return a URL string suitable for copy/paste that describes _this_ repo,
diff --git a/Allura/allura/tests/functional/test_user_profile.py b/Allura/allura/tests/functional/test_user_profile.py
index 007ba3a..8f8f027 100644
--- a/Allura/allura/tests/functional/test_user_profile.py
+++ b/Allura/allura/tests/functional/test_user_profile.py
@@ -73,25 +73,23 @@ class TestUserProfile(TestController):
 
     def test_differing_profile_proj_shortname(self):
         User.upsert('foo_bar')
+        # default auth provider's user_project_shortname() converts _ to - (for subdomain
name validation reasons)
+        # but can access user URL with "_" still
+        self.app.get('/u/foo_bar/profile/')
 
-        # default auth provider's user_project_shortname() converts _ to - for the project
name
-        response = self.app.get('/u/foo_bar/', status=302)
-        assert_equal(response.location, 'http://localhost/u/foo-bar/')
-        response = self.app.get('/u/foo_bar/profile/xyz?a=b', status=302)
-        assert_equal(response.location, 'http://localhost/u/foo-bar/profile/xyz?a=b')
-
-        # unfortunately this doesn't work because the default auth provider's user_by_project_shortname()
-        # doesn't try converting back (and it probably shouldn't since you could get multiple
users with conflicting proj names)
-        # at least this works with other auth providers that have a more complete implementation
of both
-        # user_project_shortname() and user_by_project_shortname()
-        #self.app.get('/u/foo-bar/profile/')
+        # and accessing it by "-" which was the previous way, will redirect
+        response = self.app.get('/u/foo-bar/', status=302)
+        assert_equal(response.location, 'http://localhost/u/foo_bar/')
+        response = self.app.get('/u/foo-bar/profile/xyz?a=b', status=302)
+        assert_equal(response.location, 'http://localhost/u/foo_bar/profile/xyz?a=b')
 
     def test_differing_profile_proj_shortname_rest_api(self):
         User.upsert('foo_bar')
-
-        # default auth provider's user_project_shortname() converts _ to - for the project
name
-        response = self.app.get('/rest/u/foo_bar/', status=302)
-        assert_equal(response.location, 'http://localhost/rest/u/foo-bar/')
+        # default auth provider's user_project_shortname() converts _ to - (for subdomain
name validation reasons)
+        # but can access user URL with "_" still
+        self.app.get('/rest/u/foo_bar/')
+        # and with "-" too, no redirect here to avoid api clients having to deal with unexpected
redirects
+        self.app.get('/rest/u/foo-bar/')
 
     @td.with_user_project('test-admin')
     @td.with_wiki
@@ -173,7 +171,7 @@ class TestUserProfile(TestController):
                                                'user@example.com')
         r = self.app.get('/u/test-user/profile',
                          status=200)
-        assert r.html.find('a', dict(href='send_message'))
+        assert r.html.find('a', dict(href='/u/test-user/profile/send_message'))
 
     @td.with_user_project('test-user')
     def test_disable_user_messages(self):


Mime
View raw message