Project

General

Profile

Bug #4110

[ce, ee] repos can be named _admin, _static

Added by Daniel D about 3 years ago. Updated about 3 years ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
20.07.2016
Due date:
% Done:

0%

Estimated time:
Sorting:
Commit Number:
Affected Version:

Description

There is a bug where repo name can have the name '_admin' this is because ADMIN_PREFIX which is used for checking the repo name had a slash prefixed to it making the check not work.

Must fix the bug and make this check more robust.

History

#1 Updated by Daniel D about 3 years ago

Further inspection shows that imported repos (eg. via remap/rescan) can also be named '_admin' - and those do not pass through form validation at all.

We must move validation to RepoModel().create_repo() - so that api/web/remap_rescan all use the same validation.

#2 Updated by Marcin Kuzminski [staff] about 3 years ago

Good find, maybe it's good time to introduce a common validation schema via COlander that API/Form/Create repo would use ?

#3 Updated by Daniel D about 3 years ago

That was my thought exactly - since we want to replace formencode anyway.

Things to note/check:

  • make sure each case (api/remap/web) all handle the failing cases gracefully
  • the form i18n works

#4 Updated by Daniel D about 3 years ago

The other thing to think about is that the form validator uses a function repo_name_slug(repo_name) to get a sanitized repo name by removing certain bad characters such as punctuation however this cannot be used in remap/rescan because for example two repos: myrepo and myrepo~ would conflict with each other during the scan, leading to unknown effects.

#5 Updated by Daniel D about 3 years ago

Current list of prefixes of routes that should not be used by a repo name:

set(['_users', '_repos', '_admin', '_goto_data', '_user_groups', '_profiles'])

This list is based on the current pylons routes, and doesn't take into account extra routes from pyramid/plugins

It might be a good idea to move all routes to a common prefix eg.

/_rc/
/_rc/users
/_rc/admin
etc..

That way we have a clear namespace instead of having to maintain a static list of allowed prefixes or dynamic routes created from (eg. plugins)

We should 301 /_admin to /_rc/admin to make sure old links work.

Also available in: Atom PDF