-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add function to archive repository #2595
base: master
Are you sure you want to change the base?
Conversation
/** | ||
* Allow action on only unarchived repository | ||
*/ | ||
trait UnarchivedAuthenticator { self: ControllerBase with RepositoryService => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will have to ensure applying this authenticator to all possible write actions even in plugins, but ensuring it in all plugins is actually difficult or impossible.
One idea I came up with is that checking whether a repository is archived in writableUsersOnly
too. This is redundant and don't cover all cases, but will automatically apply the minimum protection for the existing plugins. Or adding an option like allowUnarchivedRepository
to all authenticator so that all actions currently use authenticator will need to be updated to specify that option explicitly.
Anyway, we would need to consider more carefully about how to apply the protection for archived repositories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@takezoe Thanks for you review! I agree that applying UnarchivedAuthenticator
to all write action is difficult and impossible.
checking whether a repository is archived in writableUsersOnly
That might be simple and good solution, but actually checking in writableUserOnly
is not enough. Sometime we want to restrict even readableUserOnly
user action if repository is archived such as "create new issue".
adding an option like allowUnarchivedRepository to all authenticator so that all actions currently use authenticator will need to be updated to specify that option explicitly.
It is more flexible than checking in writableUsersOnly
, but as you said need update all actions, and even plugins...
hmm...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking whether a repository is archived in
writableUsersOnly
too
@takezoe I misunderstood what you meant about this. You mean check if repository is archived in writableUsersOnly
and use UnarchivedAuthenticator
for other cases such as "create new issue" (readableUserOnly
) if restriction required.
In that case this idea sound better because it "will automatically apply the minimum protection for the existing plugins" as you said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is what I meant. But that is also just one option. We would need to see more closely and consider possible options carefully to figure out what is the best way for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before submitting a pull-request to GitBucket I have first:
About this PR
This PR add feature to archive repository (read-only). This resolves #2152.
Intended Behavior
If repo is archived,
Technical changes
ARCHIVED
column toREPOSITORY
table. => use likerepositpry.repository.isArchived
UnarchivedAuthenticator
to reject request on server side if repo. is archived.Screenshot