Bug #4110
open[ce, ee] repos can be named _admin, _static
0%
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.
Updated by Daniel D about 8 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.
Updated by Marcin Kuzminski [CTO] about 8 years ago
Good find, maybe it's good time to introduce a common validation schema via COlander that API/Form/Create repo would use ?
Updated by Daniel D about 8 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
Updated by Daniel D about 8 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.
Updated by Daniel D about 8 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.