Support restricting user search #6960
Conversation
ThiefMaster
force-pushed
the
restricted-user-search
branch
3 times, most recently
from
78aab00 to
aa79c73
Compare
|
Looks great, I saw that all the TODO points were completed successfully. The new implementation is definitely cleaner and more maintainable. If I manage to free up some time in the next few days, I'll try to validate an extra scenario, but everything looks aligned so far. Thanks again for the support and for accepting the CVE. |
ThiefMaster
force-pushed
the
restricted-user-search
branch
from
2e019e4 to
0493589
Compare
This means you cannot register on behalf of someone else w/ an Indico account, but it's probably an uncommon case, even more so since it will only affect instances that restrict user search.
ThiefMaster
force-pushed
the
restricted-user-search
branch
from
0493589 to
c57d3ee
Compare
| # where no context is passed much harder. of course any of the access checks below will still | ||
| # be short-circuited for an admin, so calling this endpoint with `category_id=0` would always | ||
| # work for an admin | ||
| if self.category and self.category.can_create_events(session.user): |
There was a problem hiding this comment.
This is currently only used for categories, but I think there's no harm in leaving the other object types here as well since they're all valid reasons to get a search token.
| if user.id != sig_uid: | ||
| raise BadSignature | ||
| except BadSignature: | ||
| raise Forbidden('Invalid search token') |
There was a problem hiding this comment.
This could happen if someone opened a page/form with an embedded search token, and then waited for 24h. I guess it's uncommon enough to not really be a problem...
|
Thanks for the detailed updates. The check in controllers.py makes sense, and I agree that supporting other object types for token retrieval is a reasonable design choice. Also, the expired token scenario seems like a valid edge case, but rare enough not to cause practical issues. Looks good to me. |
2 participants
Some admins would prefer to lock down the user search more tightly. This gives them the ability to do so.
This is a re-implementation of #5024 in what I think is a cleaner and more maintainable way.
TODO:
(or remove from setting description)disallowunlisted events w/o an ACL on who can create them when search is restricted. Added a warning instead.