-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Do not allow edit of due date for restricted users #35550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Do not allow edit of due date for restricted users #35550
Conversation
…ers in web interface
… users in swagger API
NOTE: I suggested the default should be restricted as it is called restricted user and as a admin i expect an restricted user to be restricted to by default ... |
TBH it's unclear why this feature is needed and how many real users need it. So I have objection for it at the moment. If it is not a general-purpose feature (not needed by a large group of users), you can still build your own binary with the changes. |
I think the "restricted" isn't the meaning that you guessed. Fortunately, we have comments from "Update User model comments about permissions (#17583): " If I understand correctly, the "restricted" was designed to make the users can only access resources that they have explicit rights. But I don't know whether the design has been abused. // true: the user is only allowed to see organizations/repositories that they has explicit rights to.
// (ex: in private Gitea instances user won't be allowed to see even organizations/repositories that are set as public)
IsRestricted bool `xorm:"NOT NULL DEFAULT false"` |
First I also think that we need a better and way more granullar user permission model. And yes you can make your own binnary, but the more drift in code you have the harder to maintain it is ... so having it upstream let gitea users benefit with more usecases to configure/choose from and also more enhancements features etc ... also the restricted user was added for a single specific usecase in the past and now is widend to allow more addoption and usecases. It should not matter what the original edgecase was if we can have broader coverage of possible usecases ... so that's why it should be added - my opinion |
Then what's the real use case in your mind? |
We have a large group of users which having accessed to specific issue trackers. Gitea is used so that these users can report their problems and wishes. By default these users are restricted, so that have only access to the repositories they are assigned to. While these users should report their problems, they should not assign a due date to their issue. A due date should be assigned by some support manager to report back to the user when their issue is likely going to be resolved. |
Then the problem is more complex than a simple config option. IIRC the behavior you described belongs to the "issue reader" permission. "Issue reader" permissions means a user can create new issues and comments, and can edit their own issues/comments. If you don't want to allow them change the "due date", would you like to allow them to edit the issue title and content? I also recalled more users are requesting to add a new permission control to make "issue reader" be a real read-only permission (should not create new issues). So, if we only patch your requirement by some new config options without a global design, Gitea code base will become more messy and look like a colleague student homework. In my mind, if we'd like to resolve the "permission" problem, we should have a complete design first (also address the requests of "more users" mentioned above) |
For your case, I think you can just use existing permissions: you can make these users to be "issue reader", then they shouldn't be able to change the due date, if I understand correctly. Why do you need to make them as "issue writer"? "issue writer" also means your "a large group of users" can edit other's issues and comments, I think it isn't what you want. |
@t-h-i-s @6543 @confusedsushi Any new thoughts? Or this PR is not really needed? |
They should be able to set labels to the issues. I agree that the current permission system needs a redesign. We tried here to present a minimal invasive change which does not break other use cases. A larger rework of the permission system is appreciated, but at the same time we want to evolve our platform now. We try to bring our patches upstream, so that the community can benefit from them. If we cannot bring our patches upstream, one day our instance would not be able to merge upstream updates anymore. At this time we would have an incompatible fork... I fear that a rework of the permission system needs a lot of time. The concept of a restricted user already exists. So our idea was to make it possible to configure in a finer granularity what a restricted user is allowed to do. This granularity might can be adapted later to the permission system. For us the easiest option to go would be to simply disallow restricted users to set a due date. But because we want to bring our changes upstream, we made this option configurable. Maybe you can suggest another option which is comparable fast to implement. |
Well, that's the problem, if you assign "repo issue writer" permission, they can do far more than "setting labels". If you really need these users to set labels, you'd better make them "issue readers" and patch the "reader" permission.
I don't think upstream should take an immature design just because "we don't have time". It will cause more problems in the future.
I don't have a better solution "which is comparable fast to implement". That's just my opinion. |
As stated before, we want to make patches we can bring upstream. Would such patch be possible to bring upstream?
That's totally true. However, at the same time you need to make your users (and contributors) happy, so that they don't go to competitors.
The solution here does not require any database change. So it is easy to go. Also it is configurable per instance, so no new API is needed. Probably the best way forward to go would be to split the "Issues" permission from radio boxes "No access"/"Read"/"Write" to check boxes
But this is a more challenging work, including some UI design and database migrations. At the same time these finer granules can be configured with our approach to restricted users. |
At the moment, it doesn't look good to me. But you can also ask other maintainers and TOC
TBH I don't know, it's up to users. If you mean some other forks, feel free to try it, I only see more regression bugs (also caused by introducing badly designed code) and fewer features.
As I stated above, I don't think an immature design should be taken just because "we don't have time (which implies just do the easy work)". And this PR's design seems not proper. Also as stated above, if you need to "limit" something, I think it's better to do it from "reader" permission, but not patch the "writer" permission. There can be a feasible approach in my mind at the moment:
IMO there is nothing to do with "restricted" flag (you should also assign "reader" permission to your untrusted users). It should be a complete solution which satisfies all users. |
Summary
This PR introduces a new configuration section for restricted users, starting with a flag that controls whether restricted users are allowed to edit issue due dates.
Motivation
To improve permission granularity, this PR introduces a dedicated
[restricted_user]
configuration section that can be extended with additional restrictions in the future. As a first step, restricted users are prevented from editing issue due dates unless explicitly enabled by the admin.Changes
modules/setting/restricted_user.go
ALLOW_EDIT_DUE_DATE
(default:false
)PATCH /repos/{owner}/{repo}/issues/{index}
) and web routes.Configuration Example
Behavior
403 Forbidden
.Impact / Compatibility
ALLOW_EDIT_DUE_DATE = true
is explicitly set.