Skip to content
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

Fix the permission of team's Actions unit issue #24536

Merged
merged 14 commits into from
May 5, 2023

Conversation

sillyguodong
Copy link
Contributor

@sillyguodong sillyguodong commented May 5, 2023

close #24449

The unit of Actions should be contorlled not only by repository.DISABLED_REPO_UNITS but also by actions.ENABLED
in the app.ini.
Previously, the permission of the team's Actions unit was not controlled by actions.Enabled. So, even if the user sets actions.Enabled to false, he can still select the permission of the Actions unit for the team.

This PR makes the permissions of the team's Actions unit also controlled by actions.Enabled. Just appendTypeActions into DisabledRepoUnits slice when initializing if actions.Enabled is false.

Changes:

If Actions is set disbaled in app.ini, like below:

[actions]
ENABLED = false
  1. If user try to create/edit a team, will prompt user that Actions is disbaled.

image

  1. actions is not displayed in the sidebar on the team details page

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 5, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 5, 2023
@sillyguodong sillyguodong marked this pull request as draft May 5, 2023 03:47
@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 5, 2023
@sillyguodong sillyguodong marked this pull request as ready for review May 5, 2023 05:16
@lunny lunny added the type/bug label May 5, 2023
@lunny lunny added this to the 1.20.0 milestone May 5, 2023
@lunny lunny added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label May 5, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 5, 2023
@wolfogre
Copy link
Member

wolfogre commented May 5, 2023

The packages unit has the same logic, maybe we could follow what it does?

if !rootCfg.Section("packages").Key("ENABLED").MustBool(true) {
Repository.DisabledRepoUnits = append(Repository.DisabledRepoUnits, "repo.packages")
}

Actually, I don't like the hard code way of package, but I think we should keep be consistent, or it will be confusing.

@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 5, 2023
@sillyguodong
Copy link
Contributor Author

The packages unit has the same logic, maybe we could follow what it does?

if !rootCfg.Section("packages").Key("ENABLED").MustBool(true) {
Repository.DisabledRepoUnits = append(Repository.DisabledRepoUnits, "repo.packages")
}

Actually, I don't like the hard code way of package, but I think we should keep be consistent, or it will be confusing.

done and tested

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 5, 2023
@delvh delvh added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 5, 2023
@lunny lunny merged commit a866cb0 into go-gitea:main May 5, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request May 5, 2023
close go-gitea#24449

The unit of `Actions` should be contorlled not only by
`repository.DISABLED_REPO_UNITS` but also by `actions.ENABLED`
in the `app.ini`.
Previously, the permission of the team's `Actions` unit was not
controlled by `actions.Enabled`. So, even if the user sets
`actions.Enabled` to false, he can still select the permission of the
`Actions` unit for the team.

This PR makes the permissions of the team's `Actions` unit also
controlled by `actions.Enabled`. Just append`TypeActions` into
`DisabledRepoUnits` slice when initializing if `actions.Enabled` is
false.

### Changes:

If `Actions` is set disbaled in `app.ini`, like below:
```yaml
[actions]
ENABLED = false
```

1. If user try to create/edit a team, will prompt user that `Actions` is disabled.

![image](https://user-images.githubusercontent.com/33891828/236370415-961082b2-82d2-4d9e-8025-83872ad08cbb.png)

2. `actions` is not displayed in the sidebar on the team details page

![image](https://user-images.githubusercontent.com/33891828/236371817-f39f9bc9-5926-4b88-b5e6-d93617fcfb07.png)
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels May 5, 2023
silverwind pushed a commit that referenced this pull request May 5, 2023
Backport #24536 by @sillyguodong

close #24449

The unit of `Actions` should be contorlled not only by
`repository.DISABLED_REPO_UNITS` but also by `actions.ENABLED`
in the `app.ini`.
Previously, the permission of the team's `Actions` unit was not
controlled by `actions.Enabled`. So, even if the user sets
`actions.Enabled` to false, he can still select the permission of the
`Actions` unit for the team.

This PR makes the permissions of the team's `Actions` unit also
controlled by `actions.Enabled`. Just append`TypeActions` into
`DisabledRepoUnits` slice when initializing if `actions.Enabled` is
false.


### Changes:

If `Actions` is set disbaled in `app.ini`, like below:
```yaml
[actions]
ENABLED = false
```

1. If user try to create/edit a team, will prompt user that `Actions` is
disbaled.
 

![image](https://user-images.githubusercontent.com/33891828/236370415-961082b2-82d2-4d9e-8025-83872ad08cbb.png)

2. `actions` is not displayed in the sidebar on the team details page


![image](https://user-images.githubusercontent.com/33891828/236371817-f39f9bc9-5926-4b88-b5e6-d93617fcfb07.png)

Co-authored-by: sillyguodong <33891828+sillyguodong@users.noreply.github.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 6, 2023
* upstream/main:
  Add RPM registry (go-gitea#23380)
  Docs for Gitea Actions (go-gitea#24405)
  Update LDAP filters to include both username and email address (go-gitea#24547)
  Temporarily disable PATs until next release (go-gitea#24527)
  Replace placeholders in licenses (go-gitea#24354)
  Fix the permission of team's `Actions` unit issue (go-gitea#24536)
  Bump golang deps (go-gitea#24533)
  Fix mirrors repository disapeared on user dashboard (go-gitea#24520)
  Revert "Prevent a user with a different email from accepting the team invite" (go-gitea#24531)
  Fix form method/class (go-gitea#24535)
  Fix typo in rename branch dialog (go-gitea#24537)
  Check length of `LogIndexes` in case it is outdated (go-gitea#24516)
lunny pushed a commit that referenced this pull request May 19, 2023
Regression of #24536. If the user doesn't explicitly disable Actions, it
will be enabled.

1. Gitea will call `loadRepositoryFrom` before `loadActionsFrom`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/setting.go#L234-L237
2. In `loadRepositoryFrom`,
`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` will set
`actions.ENABLED` with `true`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/repository.go#L313-L315
3. In `loadActionsFrom`, `rootCfg.Section("actions")` will get a section
with Actions enabled.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/actions.go#L23-L26

Although the cause of the problem was using `true` by copy-paste
mistake, it also surprised me that
**`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` doesn't
only read, but also write.**
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request May 19, 2023
Regression of go-gitea#24536. If the user doesn't explicitly disable Actions, it
will be enabled.

1. Gitea will call `loadRepositoryFrom` before `loadActionsFrom`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/setting.go#L234-L237
2. In `loadRepositoryFrom`,
`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` will set
`actions.ENABLED` with `true`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/repository.go#L313-L315
3. In `loadActionsFrom`, `rootCfg.Section("actions")` will get a section
with Actions enabled.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/actions.go#L23-L26

Although the cause of the problem was using `true` by copy-paste
mistake, it also surprised me that
**`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` doesn't
only read, but also write.**
silverwind pushed a commit that referenced this pull request May 19, 2023
Backport #24802 by @wolfogre

Regression of #24536. If the user doesn't explicitly disable Actions, it
will be enabled.

1. Gitea will call `loadRepositoryFrom` before `loadActionsFrom`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/setting.go#L234-L237
2. In `loadRepositoryFrom`,
`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` will set
`actions.ENABLED` with `true`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/repository.go#L313-L315
3. In `loadActionsFrom`, `rootCfg.Section("actions")` will get a section
with Actions enabled.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/actions.go#L23-L26


Although the cause of the problem was using `true` by copy-paste
mistake, it also surprised me that
**`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` doesn't
only read, but also write.**

Co-authored-by: Jason Song <i@wolfogre.com>
Codeberg-org pushed a commit to Codeberg-org/gitea that referenced this pull request Jun 3, 2023
Backport go-gitea#24802 by @wolfogre

Regression of go-gitea#24536. If the user doesn't explicitly disable Actions, it
will be enabled.

1. Gitea will call `loadRepositoryFrom` before `loadActionsFrom`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/setting.go#L234-L237
2. In `loadRepositoryFrom`,
`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` will set
`actions.ENABLED` with `true`.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/repository.go#L313-L315
3. In `loadActionsFrom`, `rootCfg.Section("actions")` will get a section
with Actions enabled.

https://github.com/go-gitea/gitea/blob/25d4f95df25dae5226e96e813dde87b071d9155e/modules/setting/actions.go#L23-L26

Although the cause of the problem was using `true` by copy-paste
mistake, it also surprised me that
**`rootCfg.Section("actions").Key("ENABLED").MustBool(true)` doesn't
only read, but also write.**

Co-authored-by: Jason Song <i@wolfogre.com>
(cherry picked from commit b369ed5)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 3, 2023
@sillyguodong sillyguodong deleted the bugfix/issue_24449 branch February 29, 2024 03:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Actions , being not enabled, appears in Team settings
5 participants