|
|
Mads Kiilerich
|
e27ff6a90076
|
3 years ago
|
|
auth: always consider the repo group owner an admin when computing it's permissions
When computing repo group permissions in repository_group_permissions(), always give admin permissions to the group owner.
That is similar to how repository_permissions() gives admin permissions to the repo owner.
The extra computation shouldn't cause any extra database hits or make the computation more complex or expensive, so that should be fine for stable.
Note: This will leave behind some (automaticly added) explicit permissions. I consider this a very minor glitch, not worth addressing.
|
|
|
Mads Kiilerich
|
98cbebff6afc
|
4 years ago
|
|
auth: tweak log statements when redirecting unauthenticated users to login
Warning level was too verbose.
|
|
|
Mads Kiilerich
|
87c2cd07166a
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
710512deb83d
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
67e5b90801aa
|
5 years ago
|
|
lib: move webhelpers2 and friends to webutils
Gives less of the unfortunate use of helpers - especially in low level libs.
|
|
|
Mads Kiilerich
|
5e46f73f0d1c
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
b095e2fbba44
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
0be48652ca48
|
5 years ago
|
|
routing: separate url handling from routing - move it to webutils
This is a helper method relying on the thread local tg.request. We didn't have a good place to put it. Now we do.
This (re)moves unfortunate dependencies to the routing module (which almost is a controller).
|
|
|
Mads Kiilerich
|
2864cff1f12a
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
f14fd4cbb488
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
c101cafe9a0f
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
4c79659e11e5
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
8596a0a86673
|
5 years ago
|
|
auth: extract private parts from get_user_permissions
Prepare for splitting it up.
|
|
|
Mads Kiilerich
|
ae3d9b4c043e
|
5 years ago
|
|
auth: compute AuthUser.permissions lazily Prepare for computing each kind of permissions separately so they can be skipped in the common case. This will undo a part of 1ecd6c0e2787 where we took the detour of assigning permissions in __init__ - now we move it back to a LazyProperty, but in a better way.
|
|
|
Mads Kiilerich
|
1ecd6c0e2787
|
5 years ago
|
|
auth: refactor permissions
Avoid using complex vague typing in dict-of-dicts.
|
|
|
Mads Kiilerich
|
2ce710e81e61
|
6 years ago
|
|
permissions: drop hg.create.write_on_repogroup "Repository creation with group write access" setting
Simplify permissions system and get rid of some confusing tech debt.
Before, the global 'write_on_repogroup' setting controlled what write permission on a repo group meant.
With this change, users can create repositories in a repo group if and only if they have write access. Write access to a repo group will now mean the permission to create repositories in it.
Write access to repo groups must be granted explicitly. There should not be any other reason to grant write access than to allow users to create repos. There is thus no upgrade concerns for this change.
An admin that doesn't want users to create repos in a repogroup should just not give them write access.
These global settings might still exist in the database, but is ignored and no longer used and do no harm.
|
|
|
Mads Kiilerich
|
0456028c4ffe
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
f6be760c786c
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
a67bcc6f9118
|
6 years ago
|
|
db: drop SA caching_query and FromCache, and thus sql_cache_short beaker cache
It is not a good idea to have dead ORM objects. If we want caching, we should do it explicit.
It is unknown how much this cache helps, but we can profile and introduce better caching of simple data where relevant.
|
|
|
Mads Kiilerich
|
f734d107296e
|
6 years ago
|
|
auth: for default permissions, use existing explicit query result values instead of following dot references in ORM result objects There has been reports of spurious crashes on resolving references like .repository from Permissions: File ".../kallithea/lib/auth.py", line 678, in __wrapper if self.check_permissions(user): File ".../kallithea/lib/auth.py", line 718, in check_permissions return user.has_repository_permission_level(repo_name, self.required_perm) File ".../kallithea/lib/auth.py", line 450, in has_repository_permission_level actual_perm = self.permissions['repositories'].get(repo_name) File ".../kallithea/lib/vcs/utils/lazy.py", line 41, in __get__ value = self._func(obj) File ".../kallithea/lib/auth.py", line 442, in permissions return self.__get_perms(user=self, cache=False) File ".../kallithea/lib/auth.py", line 498, in __get_perms return compute(user_id, user_is_admin) File ".../kallithea/lib/auth.py", line 190, in _cached_perms_data r_k = perm.UserRepoToPerm.repository.repo_name File ".../sqlalchemy/orm/attributes.py", line 285, in __get__ return self.impl.get(instance_state(instance), dict_) File ".../sqlalchemy/orm/attributes.py", line 721, in get value = self.callable_(state, passive) File ".../sqlalchemy/orm/strategies.py", line 710, in _load_for_state % (orm_util.state_str(state), self.key) sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <UserRepoToPerm at ...> is not bound to a Session; lazy load operation of attribute 'repository' cannot proceed (Background on this error at: http://sqlalche.me/e/bhk3) Permissions are cached between requests: SA result records are stored in in beaker.cache.sql_cache_short and resued in following requests after the initial session as been removed. References in Permission objects would usually give lazy lookup ... but not outside the original session, where we would get an error like this. Permissions are indeed implemented/used incorrectly. That might explain a part of the problem. Even if not fully explaining or fixing this problem, it is still worth fixing: Permissions are fetched from the database using Session().query with multiple class/table names (joined together in way that happens to match the references specified in the table definitions) - including Repository. The results are thus "structs" with selected objects. If repositories always were retrieved using this selected repository, everything would be fine. In some places, this was what we did. But in some places, the code happened to do what was more intuitive: just use .repository and rely on "lazy" resolving. SA was not aware that this one already was present in the result struct, and would try to fetch it again. Best case, that could be inefficient. Worst case, it would fail as we see here. Fix this by only querying from one table but use the "joinedload" option to also fetch other referenced tables in the same select. (This might inefficiently return the main record multiple times ... but that was already the case with the previous approach.) This change is thus doing multiple things with circular dependencies that can't be split up in minor parts without taking detours: The existing repository join like: .join((Repository, UserGroupRepoToPerm.repository_id == Repository.repo_id)) is thus replaced by: .options(joinedload(UserGroupRepoToPerm.repository)) Since we only are doing Session.query() on one table, the results will be of that type instead of "structs" with multiple objects. If only querying for UserRepoToPerm this means: - perm.UserRepoToPerm.repository becomes perm.repository - perm.Permission.permission_name looked at the explicitly queried Permission in the result struct - instead it should look in the the dereferenced repository as perm.permission.permission_name
|
|
|
Mads Kiilerich
|
5b1f43027662
|
6 years ago
|
|
scripts: drop isort --wrap-length 160 - it is broken with py3 and not really necessary Under Python 3, isort 4.3.21 failed with https://github.com/timothycrosley/isort/issues/741 on kallithea/controllers/api/api.py : Traceback (most recent call last): File "data/env/bin/isort", line 10, in <module> sys.exit(main()) File ".../env/lib64/python3.7/site-packages/isort/main.py", line 379, in main for sort_attempt in attempt_iterator: File ".../env/lib64/python3.7/site-packages/isort/main.py", line 377, in <genexpr> attempt_iterator = (sort_imports(file_name, **arguments) for file_name in file_names) File ".../env/lib64/python3.7/site-packages/isort/main.py", line 88, in sort_imports result = SortImports(file_name, **arguments) File ".../env/lib64/python3.7/site-packages/isort/isort.py", line 207, in __init__ self._add_formatted_imports() File ".../env/lib64/python3.7/site-packages/isort/isort.py", line 606, in _add_formatted_imports self._add_from_imports(from_modules, section, section_output, sort_ignore_case) File ".../env/lib64/python3.7/site-packages/isort/isort.py", line 526, in _add_from_imports import_statement = self._multi_line_reformat(import_start, from_import_section, comments) File ".../env/lib64/python3.7/site-packages/isort/isort.py", line 552, in _multi_line_reformat dynamic_indent, indent, line_length, comments) File ".../env/lib64/python3.7/site-packages/isort/isort.py", line 705, in _output_grid if len(next_statement.split(self.line_separator)[-1]) + 1 > line_length: TypeError: '>' not supported between instances of 'int' and 'str'
|
|
|
Mads Kiilerich
|
afe7599f1d8b
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
1e0632b6ec27
|
6 years ago
|
|
auth: also use safe password hashing on Windows using bcrypt
For unknown reasons, Kallithea used different hashing algorithms on Windows and Posix. Perhaps because problems with bcrypt on Windows in the past. That should no longer be an issue, and it doesn't make sense to have different security properties on the platforms.
While changing to bcrypt everywhere, also remain backwards compatible and accept existing passwords hashed with sha256 - both on Windows (where it used to be used), and elsewhere (in case a system has been migrated from Windows to Unix).
|
|
|
Mads Kiilerich
|
e35373106528
|
6 years ago
|
|
py3: remove safe_unicode in places where it no longer is needed because all strings (except bytes) already *are* unicode strings
(The remaining safe_unicode calls are still needed and can't just be removed, generally because we in these cases still have to convert from bytes to unicode strings.)
|
|
|
Mads Kiilerich
|
756e46bd926b
|
6 years ago
|
|
py3: trivial renaming of .iteritems() to .items()
A bit like "2to3 -f dict", but we don't want list().
|
|
|
Mads Kiilerich
|
d1f091d4b765
|
6 years ago
|
|
py3: support __bool__
From 2to3 -f nonzero.
|
|
|
Mads Kiilerich
|
8f468d08f463
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
4e0442f914b9
|
6 years ago
|
|
auth: accept sha256 passwords on all platforms - not only on Windows
Give less surprises when changing platform.
Still, bcrypt is only supported and used on Posix.
bcrypt "hashes" will have length 60 and start with '$' and will thus immediately skip the sha256 check.
The change should be safe: Users can't influence what kind of hashed key will be in the database and can thus not influence the auth method.
(We really should use bcrypt on Windows too ... or change to something more state of the art.)
|
|
|
Mads Kiilerich
|
5ddd6b930dd0
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
2837b66f68bb
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
e7dbe089e10d
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
effd091203ae
|
6 years ago
|
|
py3: drop __unicode__ ... and generally move towards just providing a good __repr__
We should never show str() (or similar) to end users.
|
|
|
Mads Kiilerich
|
394c7814e710
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
fe4086096758
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
0a277465fddf
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
cbb85dc11e3a
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
d9421a78a534
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
1f831a8a590c
|
6 years ago
|
|
auth: fix failure when editing inactive users
AuthUser._fill_data did not work on users that not were active, and we could thus not even *talk* about inactive users.
To make things more simple, inline the function. That also makes it clear that dbuser can't be None.
|
|
|
Mads Kiilerich
|
6104f9106a5a
|
7 years ago
|
|
auth: drop authenticating_api_key from AuthUser
It doesn't belong as a user property - it is more of a session property ... which is what we already use instead.
|
|
|
Mads Kiilerich
|
5c5f0eb45681
|
7 years ago
|
|
auth: move CSRF checks from the optional LoginRequired to the more basic BaseController._before
_before is not called for the CSRF-immune JSON-API controller and is thus a good place to check CSRF. This also apply CSRF protection to the login controller.
The flag for needing CSRF checking is stored in the thread global request object when passed from __call__ to _before for regular controllers. It is thus also set for requests to the JSON-RPC controller, but not used.
|
|
|
Mads Kiilerich
|
797883404f17
|
7 years ago
|
|
auth: simplify API key auth - move it out of _determine_auth_user
This gives less of the special handling of API key auth in LoginRequired ... but we still need to disable the LoginRequired CSRF protection for API key auth.
|
|
|
Mads Kiilerich
|
226893a56a81
|
7 years ago
|
|
|
|
|
Mads Kiilerich
|
077ba994ee03
|
7 years ago
|
|
|
|
|
Mads Kiilerich
|
31aa5b6c107d
|
7 years ago
|
|
auth: remove AuthUser __init__ magic for fallback to default user instead of the requested user
Be reliably explicit about what user we expect. If we want default user / anonymous user, say so explicitly.
|
|
|
Mads Kiilerich
|
1e83cda87899
|
7 years ago
|
|
auth: drop unused AuthUser.is_authenticated
It seems like other ways of tracking authentication state are better. AuthUser is a *potentially* authenticated user. We prefer to keep it as that, without modifying the AuthUser object if the user actually should be authenticated.
The primariy indicator that a user is authenticated is when the AuthUser is set as request.authuser .
(Alternatively, we could create an AuthenticatedUser sub-class and move things like access control checks there. That would help ensuring it is used correctly, without having to check an is_authenticated flag.)
|
|
|
Mads Kiilerich
|
05dc948c9788
|
7 years ago
|
|
auth: use other and better checks than is_authenticated
These are the two only uses of is_authenticated, and we are fine without it.
|
|
|
Mads Kiilerich
|
0e3e0864f210
|
7 years ago
|
|
auth: drop api_access_controllers_whitelist and give API key auth same access as other kinds of auth
All authentication methods are created equal. There is no point in discriminating api key authentication and limit it to few APIs.
|
|
|
Mads Kiilerich
|
69421c730569
|
7 years ago
|
|
auth: refactor auth computation, introducing bump_permission helper function
Less lines of code, and less "repeat yourself" reduces the risk of writing code that incorrectly add extra permissions.
|
|
|
Mads Kiilerich
|
6d0573ba0721
|
7 years ago
|
|
auth: drop "multiple_counter" from computing permissions
This seems to have been something about having some permissions override existing permissions. It is not clear to me why anybody should want that.
test_user_group_permissions_on_repo_groups.py seems to have been testing for something we don't want. The new behaviour seems more reasonable. The test user is inhering access from the default user, and thus in this case getting read access (except when private).
|
|
|
Mads Kiilerich
|
8eed16b2a99b
|
7 years ago
|
|
auth: minor refactoring of computation of admin access for repo owners
Make the flow slightly simpler ... and now when permissions are merged, we only have to set repo owner access once.
BUT: because multiple_counter, we actually don't merge permissions in all cases. This will thus introduce a regression that will be fixed in next changeset.
|
|
|
Mads Kiilerich
|
1ab83bed8115
|
7 years ago
|
|
auth: drop the internal "explicit" flag - the new default is all we want; explicit permissions should never blindly overrule indirect permissions
The "explicit" flag is already always false. Just hardcode it everywhere and remove dead code.
|
|
|
Mads Kiilerich
|
b2634df81a11
|
7 years ago
|
|
auth: explicit user permission should not blindly overrule permissions through user groups
Before, explicit permissions of a user could shadow higher permissions that would otherwise be obtained through a group the user is member of. That was confusing and fragile: *removing* a permission could then suddenly give a user *more* permissions.
Instead, change the flag for controlling internal permission computation to *not* use "explicit". Permissions will then add up, no matter if they are explicit or through groups.
The change in auth.py is small, but read the body of __get_perms to see the actual impact ... and also the clean-up changeset that will come next.
This might in some cases be a behaviour change and give users more access ... but it will probably only give the user that was intended. This change can thus be seen as a bugfix.
Some tests assumed the old behaviour. Not for good reasons, but just because that is how they were written. These tests are updated to expect the new behaviour, and it has been reviewed that it makes sense.
Note that this 'explicit' flag mostly is for repo permissions and independent of the 'user_inherit_default_permissions' that just was removed and is about global permissions.
|
|
|
Mads Kiilerich
|
93834966ae01
|
7 years ago
|
|
auth: global permissions given to the default user are the bare minimum and should apply to *all* other users too
Drop the "subtractive permission" config option "inherit_from_default" that when set to false would give users less global permissions than the default unauthenticated user.
Instead, think positive and merge all positive permissions.
At the end, filter the global permissions to make sure we for each kind of permissions only keep the one with most weight.
|
|
|
Mads Kiilerich
|
4473f1094d3d
|
7 years ago
|
|
|
|
|
Thomas De Schampheleire
|
9f41dc6f328a
|
7 years ago
|
|
|
|
|
Thomas De Schampheleire
|
c9159e6fda04
|
7 years ago
|
|
cleanup: remove unnecessary (and potentially problematic) use of 'literal'
webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when the passed string may contain HTML that needs to be interpreted literally. It is unnecessary for plain strings.
Incorrect usage of literal can lead to XSS issues, via a malicious user controlling data which will be rendered in other users' browsers. The data could either be stored previously in the system or be part of a forged URL the victim clicks on.
For example, when a user browses to a forged URL where a repository changeset or branch name contains a javascript snippet, the snippet was executed when printed on the page using 'literal'.
Remaining uses of 'literal' have been reviewed with no apparent problems found.
Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).
|
|
|
Thomas De Schampheleire
|
81db5704b285
|
7 years ago
|
|
cleanup: remove unnecessary (and potentially problematic) use of 'literal'
webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when the passed string may contain HTML that needs to be interpreted literally. It is unnecessary for plain strings.
Incorrect usage of literal can lead to XSS issues, via a malicious user controlling data which will be rendered in other users' browsers. The data could either be stored previously in the system or be part of a forged URL the victim clicks on.
For example, when a user browses to a forged URL where a repository changeset or branch name contains a javascript snippet, the snippet was executed when printed on the page using 'literal'.
Remaining uses of 'literal' have been reviewed with no apparent problems found.
Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).
|
|
|
Mads Kiilerich
|
3abfc5aa0dd7
|
7 years ago
|
|
auth: drop support for different "algorithms" for computing permission
Drop partially implemented support for different permission "algorithms". Only "higherwin" is used and makes sense - just hardcode that.
|
|
|
Mads Kiilerich
|
088155584e2e
|
7 years ago
|
|
|
|
|
Mads Kiilerich
|
c6ce891312ef
|
7 years ago
|
|
auth: consistently use request.authuser - drop request.user
This seems like old tech debt. Now, just get rid of it.
|
|
|
Mads Kiilerich
|
e4af9e2deb83
|
7 years ago
|
|
|
|
|
Mads Kiilerich
|
d22a7430999f
|
7 years ago
|
|
auth: change get_allowed_ips to be more resilient when operating on a cached default user Before, random changes to how things are fetched and cached across database sessions could cause get_allowed_ips to fail with: DetachedInstanceError: Instance <User> is not bound to a Session; attribute refresh operation cannot proceed (Background on this error at: http://sqlalche.me/e/bhk3) Instead, just check for user_id, using same pattern as a bit later in same function.
|
|
|
Mads Kiilerich
|
c57d926edd39
|
7 years ago
|
|
auth: strip RFC4007 zone identifiers from IPv6 addresses before doing access control If using IPv6, the request IP address might contain a '%' that the ipaddr module that is used for IP filtering can't handle. https://tools.ietf.org/html/rfc4007#section-11 specifies how IPv6 addresses can have zone identifiers like trailing '%13' or '%eth0'. The zone identifier is used to help distinguish *if* the same address should be available on multiple interfaces. It *could* potentially have security implications in the odd case where the same address is different on different interfaces. The IP whitelist functionality does however not support zone filters, so there is no way users can expect the zone to be relevant for IP filtering. We can thus safely strip the zone index and only check for match on the other parts of the address.
|
|
|
Thomas De Schampheleire
|
4c4e1ec26e95
|
8 years ago
|
|
auth: raise log level of 'permission denied' from DEBUG to INFO (issue #243)
Denying an access can be quite relevant to an administrator. A log level of INFO is more reasonable than DEBUG.
|
|
|
Mads Kiilerich
|
aa25ef34ebab
|
8 years ago
|
|
auth: refactor to introduce @LoginRequired(allow_default_user=True) and deprecate @NotAnonymous() It was error prone that @LoginRequired defaulted to allow anonymous users (if 'default' user is enabled). See also 245b4e3abf39. Refactor code to make it more explicit and safe by default: Deprecate @NotAnonymous by making it the default of @LoginRequired. That will make it safe by default. To preserve same functionality, set allow_default_user=True in all the cases where @LoginRequired was *not* followed by @NotAnonymous or other permission checks - that was done with some script hacks: sed -i 's/ @LoginRequired(\(..*\))/ @LoginRequired(\1, allow_default_user=True)/g' `hg mani` sed -i 's/ @LoginRequired()/ @LoginRequired(allow_default_user=True)/g' `hg mani` perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)\n\s*\ @NotAnonymous\(\)/\ @LoginRequired()/g' `hg mani` perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)(\n\s*\ @Has(Repo)?Permission)/\ @LoginRequired()\1/g' `hg mani` It has been reviewed that all uses of allow_default_user=True are in places where the there indeed wasn't any checking for default user before. These may or may not be correct, but now they are explicit and can be spotted and fixed. The few remaining uses of @NotAnonymous should probably be removed somehow.
|
|
|
Mads Kiilerich
|
fefd7279e798
|
8 years ago
|
|
login: fix crash when entering non-ASCII password for login (Issue #300)
Avoid errors like UnicodeEncodeError: 'ascii' codec can't encode characters in position X: ordinal not in range(128) when the user enters non-ASCII passwords for existing internal accounts in the login prompt.
The password forms have "always" rejected non-ASCII passwords with Invalid characters (non-ASCII) in password
|
|
|
Thomas De Schampheleire
|
dedfa09af3af
|
8 years ago
|
|
auth: remove debug print of user password Commit 30d61922f24eb144190052818c3fc6a24562f42b (auth: fix crash on invalid bcrypt password) left a debug print statement of the user's password in plaintext and its hashed equivalent.
|
|
|
Lars Kruse
|
7691290837d2
|
8 years ago
|
|
codingstyle: trivial whitespace fixes
Reported by flake8.
|
|
|
Mads Kiilerich
|
30d61922f24e
|
8 years ago
|
|
auth: fix crash on invalid bcrypt password
When an invalid password was specified, it would with an exception:
File "kallithea/lib/auth.py", in check_password return bcrypt.checkpw(safe_str(password), safe_str(hashed)) ValueError: Invalid hashed_password salt
We do apparently have to catch ValueError and treat it as "invalid password".
|
|
|
domruf
|
cfbc0d6860ca
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
a32ca3200ca7
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
e9ac5698281d
|
9 years ago
|
|
tg: minimize future diff by some mocking and replacing some pylons imports with tg
No actual tg dependency yet, just a temporary hack faking tg as an alias for pylons.
Based on work by Alessandro Molina.
|
|
|
Søren Løvborg
|
25623c3c63aa
|
9 years ago
|
|
|
|
|
Søren Løvborg
|
ca77c6da2d34
|
9 years ago
|
|
auth: simplify user group permission checks
In practice, Kallithea has the 'usergroup.admin' permission imply the 'usergroup.write' permission, which again implies 'usergroup.read'.
This codifies this practice by replacing the HasUserGroupPermissionAny "perm function" with the new HasUserGroupLevel function, reducing the risk of errors and saving quite a lot of typing.
|
|
|
Søren Løvborg
|
b4d1e85265c1
|
9 years ago
|
|
auth: simplify repository group permission checks
In practice, Kallithea has the 'group.admin' permission imply the 'group.write' permission, which again implies 'group.read'.
This codifies this practice by replacing HasRepoGroupPermissionAny "perm function" with the new HasRepoGroupLevel function, reducing the risk of errors and saving quite a lot of typing.
|
|
|
Søren Løvborg
|
a17c8e5f6712
|
9 years ago
|
|
auth: simplify repository permission checks
In practice, Kallithea has the 'repository.admin' permission imply the 'repository.write' permission, which again implies 'repository.read'.
This codifies/enforces this practice by replacing HasRepoPermissionAny "perm function" with the new HasRepositoryLevel function, reducing the risk of errors and saving quite a lot of typing.
|
|
|
Mads Kiilerich
|
e81a99f9e365
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
ebe7d95f698b
|
9 years ago
|
|
auth: simplify the double invoked auth classes used for permission checking
Also avoid storing request state in the Has Permission instances. The instances er temporary and only used once and there is thus not real problem, but it is simpler and cleaner this way.
|
|
|
Mads Kiilerich
|
3dcf1f82311a
|
9 years ago
|
|
controllers: avoid setting request state in controller instances - set it in the thread global request variable
In TurboGears, controllers are singletons and we should avoid using instance variables for any volatile data. Instead, use the "global thread local" request context.
With everything in request, some use of c is dropped.
Note: kallithea/controllers/api/__init__.py still use instance variables that will cause problems with TurboGears.
|
|
|
Søren Løvborg
|
1341be63734a
|
9 years ago
|
|
cleanup: use standard NotImplementedError for abstract methods
From the Python docs on NotImplementedError:
In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method.
|
|
|
Søren Løvborg
|
06398585de03
|
9 years ago
|
|
auth: track API key used for authentication in AuthUser
This allows us to define only once how an API key is passed to the app. We might e.g. allow API keys to be passed in an HTTP header; with this change, we only need to update the code in one place.
Also change the code to verify up front that the API key resolved to a valid and active user, so LoginRequired doesn't need to do that.
Also return plain 403 Forbidden for bad API keys instead of redirecting to the login form, which makes more sense for non-interactive clients (the typical users of API keys).
|
|
|
Søren Løvborg
|
245b4e3abf39
|
9 years ago
|
|
auth: add AuthUser.is_anonymous, along with some exposition
This reveals the name of the NotAnonymous decorator to be misleading, an unfortunate detail only documented here, but which must be properly resolved in a later changeset.
Note that NotAnonymous behaves as advertised as long as it is used together with LoginRequired, which is always the case in the current code, so there's no actual security issue here, the code is just weird, hard to read and fragile.
---
Some thoughts on cleaning this up in a future changeset: As it turns out, every controller (except the login page!) should be LoginRequired decorated (since it doesn't actually block anonymous users, as long as anonymous access is enabled in the Kallithea config). Thus the most obvious solution would be to move the LoginRequired functionality into BaseController (with an override for LoginController), and delete the decorator entirely. However, LoginRequired does one other thing: it carries information about whether API access is enabled for individual controller methods ("@LoginRequired(api_key=True)"), and also performs the check for this, something which is not easily moved into the base controller class, since the base controller doesn't know which method is about to be called. Possibly that can be determined by poking Pylons, but such code is likely to break with the upcoming TurboGears 2 move. Thus such cleanup is probably better revisited after the switch to TG2.
|
|
|
Søren Løvborg
|
ee6b7e9f34e6
|
9 years ago
|
|
auth: perform basic HTTP security checks already in BaseController
There's no reason to postpone these to a LoginRequired decorated controller function. This way, they run unconditionally for all subclasses of BaseController (so everything except JSON-RPC and VCS controllers).
|
|
|
Søren Løvborg
|
97619528c270
|
9 years ago
|
|
auth: remove KallitheaCrypto pseudo-class
Every caller except one already used the module-level wrapper functions.
|
|
|
Mads Kiilerich
|
226e0154da46
|
9 years ago
|
|
|
|
|
Søren Løvborg
|
cd6176c0634a
|
9 years ago
|
|
db: PullRequest/Repository/RepoGroup/UserGroup: change 'user' to 'owner'
Rename the 'user' and 'user_id' fields on the four classes to something more informative. The database column names remain unchanged for now; a later Alembic script can fix the name of these and other columns to match their Python name.
This might break rcextensions, though, and external scripts that use the HTML form interface.
|
|
|
Thomas De Schampheleire
|
af3539a458f6
|
9 years ago
|
|
Turbogears2 migration: replace pylons.url by kallithea.config.routing.url
In preparation for the migration to Turbogears2, introduce a kallithea.config.routing.url to replace pylons.url. The implementation is basically the same: wrap around routes.url().
This change involves: - a number of import statement changes - fixing some tests in test_libs.py; to avoid duplication, the different implementations of fake_url were grouped in one place.
This change was first proposed by Alessandro Molina in his initial port. Following changes were made afterwards: - move UrlGenerator from kallithea.lib.utils to kallithea.config.routing - add documentation to UrlGenerator - kallithea/lib/auth.py used url_for instead of url, for no apparent reason so this was changed. - fix libs tests - rebase onto Pylons-based Kallithea first
|
|
|
Thomas De Schampheleire
|
5eec79420ce3
|
9 years ago
|
|
Turbogears2 migration: remove some references to Pylons in comments
In order to minimize the diff of the actual Turbogears2 migration, this commit already removes certain unnecessary references to Pylons from the Kallithea source base. Places where the reference to Pylons is important are still kept for now, as well as references in kallithea/config where many changes are made for Turbogears2 anyway.
|
|
|
Mads Kiilerich
|
591effa1fc4d
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
c96e05599877
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
41e70d120a5e
|
9 years ago
|
|
api: set authuser in the thread global request instace - and temporarily verify that it matches what is passed explicitly to auth methods
This makes it more like what middleware / controllers do for "normal" HTTP requests.
|
|
|
Mads Kiilerich
|
2ac4a70134b6
|
9 years ago
|
|
|
|
|
Søren Løvborg
|
9a35244c35b6
|
10 years ago
|
|
auth: clean up PermsFunction
Now shows scope in HasUserGroupPermissionAny instead of '?'.
|
|
|
Søren Løvborg
|
09bcde0eee6d
|
9 years ago
|
|
auth: remove HasPermissionAll and variants
First, find all calls to HasPermissionAll with only a single permission given, and convert to equivalent calls to HasPermissionAny.
Next, observe that it's hard to envision situations requiring multiple permissions (of the same scope: global/repo/repo group) to be satisfied. Sufficiently hard that there are actually no such examples in the code.
Finally, considering that (should it ever be needed) HasPermissionAll can be trivially built as a conjunction of HasPermissionAny calls (the decorators, too) with only a small performance impact, simply remove HasPermissionAll and related classes and functions.
|
|
|
Andrew Shadura
|
023d9202481e
|
9 years ago
|
|
setup: use modern bcrypt implementation instead of unsupported old one
py-bcrypt has been deprecated by bcrypt, and is no longer developed or supported.
bcrypt requires bytestrings instead of strings, use safe_str to ensure they're encoded before they're passed to bcrypt. Also, use check_pw to minimise the number of manual conversions and comparisons.
Installation of bcrypt will probably compile a C extension and require libffi-dev.
|
|
|
Søren Løvborg
|
ea02c8b2b529
|
10 years ago
|
|
auth: prevent misuse of PermFunction in bool context
Evaluating a PermFunction as a boolean, rather than calling it, is almost certainly an error. If not, "pf is not None" can be used.
|
|
|
timeless@gmail.com
|
1048307eb1f5
|
10 years ago
|
|
|
|
|
Mads Kiilerich
|
6feed82b76a3
|
10 years ago
|
|
|
|
|
Søren Løvborg
|
9b74296e6af6
|
10 years ago
|
|
auth: further sanitize requests to prevent GET CSRF (CVE-2016-3691)
Routes allows GET requests to override the HTTP method, which breaks the Kallithea CSRF protection (which only applies to POST requests).
This commit blocks such GET request, preventing CSRF attacks.
|
|
|
Mads Kiilerich
|
edb24bc0f71a
|
10 years ago
|
|
|