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

Prevent git operations for inactive users #13527

Merged
merged 5 commits into from
Nov 12, 2020

Conversation

lunny
Copy link
Member

@lunny lunny commented Nov 12, 2020

As title.

@lunny lunny added this to the 1.14.0 milestone Nov 12, 2020
@lafriks lafriks added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Nov 12, 2020
routers/private/serv.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 12, 2020
routers/repo/http.go Outdated Show resolved Hide resolved
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is going to break some people's misuse of inactive accounts.

@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 Nov 12, 2020
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Servcommand also needs this check

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At line 235 of serv should check if deploy key owner is inactive and at 259 user of user key

@lunny
Copy link
Member Author

lunny commented Nov 12, 2020

At line 235 of serv should check if deploy key owner is inactive and at 259 user of user key

Good catch. All repositories of an inactive user's should not be displayed, that's a main difference between isactive and prohibit_login of user.

routers/private/serv.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member Author

lunny commented Nov 12, 2020

@zeripath done.

@codecov-io
Copy link

Codecov Report

Merging #13527 (ed512bf) into master (4117a44) will increase coverage by 0.06%.
The diff coverage is 8.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13527      +/-   ##
==========================================
+ Coverage   42.09%   42.15%   +0.06%     
==========================================
  Files         695      695              
  Lines       76370    76395      +25     
==========================================
+ Hits        32145    32208      +63     
+ Misses      38966    38903      -63     
- Partials     5259     5284      +25     
Impacted Files Coverage Δ
routers/repo/http.go 42.45% <0.00%> (-0.67%) ⬇️
routers/private/serv.go 28.08% <10.71%> (-1.23%) ⬇️
modules/util/timer.go 42.85% <0.00%> (-42.86%) ⬇️
models/repo_mirror.go 2.38% <0.00%> (-11.91%) ⬇️
modules/cron/tasks_basic.go 87.35% <0.00%> (-3.45%) ⬇️
modules/git/utils.go 73.77% <0.00%> (-3.28%) ⬇️
models/unit.go 46.57% <0.00%> (-2.74%) ⬇️
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.54%) ⬇️
services/pull/check.go 49.63% <0.00%> (-1.46%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4117a44...ed512bf. Read the comment docs.

@zeripath
Copy link
Contributor

do we need to handle prohibit login keys too?

@lunny
Copy link
Member Author

lunny commented Nov 12, 2020

do we need to handle prohibit login keys too?

done.

@lafriks
Copy link
Member

lafriks commented Nov 12, 2020

Didn't prohibid login was actually meant so that user can't login in UI but can pull/push to repo?

@lunny
Copy link
Member Author

lunny commented Nov 12, 2020

Didn't prohibid login was actually meant so that user can't login in UI but can pull/push to repo?

pull or push may also prompt username/password, it is still another login. prohibid user cannot login, but all his repositories could be accessed by others. All inactive user's repositories cannot be visited from others.

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be a considered a breaking change for by some users.

@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 Nov 12, 2020
@lafriks lafriks merged commit ff7341b into go-gitea:master Nov 12, 2020
@lafriks
Copy link
Member

lafriks commented Nov 12, 2020

@lunny i think there was discussion quite some time ago about option to prevent user form using UI while still be allowed to pull/push and prohibit login was an option for that. But I don't need that so I don't care 🤣

@lafriks
Copy link
Member

lafriks commented Nov 12, 2020

Please send backports

@lunny lunny deleted the lunny/inactive_user branch November 13, 2020 00:28
lunny added a commit to lunny/gitea that referenced this pull request Nov 13, 2020
* prevent git operations for inactive users

* Some fixes

* Deny push to the repositories which's owner is inactive

* deny operations also when user is ProhibitLogin

Co-authored-by: zeripath <art27@cantab.net>
lunny added a commit to lunny/gitea that referenced this pull request Nov 13, 2020
* prevent git operations for inactive users

* Some fixes

* Deny push to the repositories which's owner is inactive

* deny operations also when user is ProhibitLogin

Co-authored-by: zeripath <art27@cantab.net>
@lunny lunny added the backport/done All backports for this PR have been created label Nov 13, 2020
@lunny
Copy link
Member Author

lunny commented Nov 13, 2020

@lafriks done.

lunny added a commit that referenced this pull request Nov 13, 2020
* prevent git operations for inactive users

* Some fixes

* Deny push to the repositories which's owner is inactive

* deny operations also when user is ProhibitLogin

Co-authored-by: zeripath <art27@cantab.net>

Co-authored-by: zeripath <art27@cantab.net>
lunny added a commit that referenced this pull request Nov 13, 2020
* prevent git operations for inactive users

* Some fixes

* Deny push to the repositories which's owner is inactive

* deny operations also when user is ProhibitLogin

Co-authored-by: zeripath <art27@cantab.net>

Co-authored-by: zeripath <art27@cantab.net>
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request Nov 22, 2020
    SECURITY
        Prevent git operations for inactive users (#13527) (#13537)
        Disallow urlencoded new lines in git protocol paths if there is a port (#13521) (#13525)
    BUGFIXES
        API should only return Json (#13511) (#13564)
        Fix before and since query arguments at API (#13559) (#13560)
        Prevent panic on git blame by limiting lines to 4096 bytes at most (#13470) (#13492)
        Fix link detection in repository description with tailing ‘_’ (#13407) (#13408)
        Remove obsolete change of email on profile page (#13341) (#13348)
        Fix permission check on get Reactions API endpoints (#13344) (#13346)
        Add migrated pulls to pull request task queue (#13331) (#13335)
        API deny wrong pull creation options (#13308) (#13327)
        Fix initial commit page & binary munching problem (#13249) (#13259)
        Fix diff parsing (#13157) (#13136) (#13139)
        Return error 404 not 500 from API if team does not exist (#13118) (#13119)
        Prohibit automatic downgrades (#13108) (#13111)
        Fix GitLab Migration Option AuthToken (#13101)
        GitLab Label Color Normalizer (#12793) (#13100)
        Log the underlying panic in runMigrateTask (#13096) (#13098)
        Fix attachments list in edit comment (#13036) (#13097)
        Fix deadlock when deleting team user (#13093)
        Fix error create comment on outdated file (#13041) (#13042)
        Fix repository create/delete event webhooks (#13008) (#13027)
        Fix internal server error on README in submodule (#13006) (#13016)

PR:		251296
Submitted by:	maintainer
MFH:		2020Q4
Security:	go-gitea/gitea#13527
		go-gitea/gitea#13521


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@556058 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request Nov 22, 2020
Approved by:	portmgr (with hat)

www/gitea: Update to 1.12.5

Changes: https://github.com/go-gitea/gitea/releases/tag/v1.12.5

PR:		250372
Approved by:	maintainer

www/gitea: Update to 1.12.6

    SECURITY
        Prevent git operations for inactive users (#13527) (#13537)
        Disallow urlencoded new lines in git protocol paths if there is a port (#13521) (#13525)
    BUGFIXES
        API should only return Json (#13511) (#13564)
        Fix before and since query arguments at API (#13559) (#13560)
        Prevent panic on git blame by limiting lines to 4096 bytes at most (#13470) (#13492)
        Fix link detection in repository description with tailing ‘_’ (#13407) (#13408)
        Remove obsolete change of email on profile page (#13341) (#13348)
        Fix permission check on get Reactions API endpoints (#13344) (#13346)
        Add migrated pulls to pull request task queue (#13331) (#13335)
        API deny wrong pull creation options (#13308) (#13327)
        Fix initial commit page & binary munching problem (#13249) (#13259)
        Fix diff parsing (#13157) (#13136) (#13139)
        Return error 404 not 500 from API if team does not exist (#13118) (#13119)
        Prohibit automatic downgrades (#13108) (#13111)
        Fix GitLab Migration Option AuthToken (#13101)
        GitLab Label Color Normalizer (#12793) (#13100)
        Log the underlying panic in runMigrateTask (#13096) (#13098)
        Fix attachments list in edit comment (#13036) (#13097)
        Fix deadlock when deleting team user (#13093)
        Fix error create comment on outdated file (#13041) (#13042)
        Fix repository create/delete event webhooks (#13008) (#13027)
        Fix internal server error on README in submodule (#13006) (#13016)

PR:		251296
Submitted by:	maintainer
Security:	go-gitea/gitea#13527
		go-gitea/gitea#13521
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request Nov 22, 2020
    SECURITY
        Prevent git operations for inactive users (#13527) (#13537)
        Disallow urlencoded new lines in git protocol paths if there is a port (#13521) (#13525)
    BUGFIXES
        API should only return Json (#13511) (#13564)
        Fix before and since query arguments at API (#13559) (#13560)
        Prevent panic on git blame by limiting lines to 4096 bytes at most (#13470) (#13492)
        Fix link detection in repository description with tailing ‘_’ (#13407) (#13408)
        Remove obsolete change of email on profile page (#13341) (#13348)
        Fix permission check on get Reactions API endpoints (#13344) (#13346)
        Add migrated pulls to pull request task queue (#13331) (#13335)
        API deny wrong pull creation options (#13308) (#13327)
        Fix initial commit page & binary munching problem (#13249) (#13259)
        Fix diff parsing (#13157) (#13136) (#13139)
        Return error 404 not 500 from API if team does not exist (#13118) (#13119)
        Prohibit automatic downgrades (#13108) (#13111)
        Fix GitLab Migration Option AuthToken (#13101)
        GitLab Label Color Normalizer (#12793) (#13100)
        Log the underlying panic in runMigrateTask (#13096) (#13098)
        Fix attachments list in edit comment (#13036) (#13097)
        Fix deadlock when deleting team user (#13093)
        Fix error create comment on outdated file (#13041) (#13042)
        Fix repository create/delete event webhooks (#13008) (#13027)
        Fix internal server error on README in submodule (#13006) (#13016)

PR:		251296
Submitted by:	maintainer
MFH:		2020Q4
Security:	go-gitea/gitea#13527
		go-gitea/gitea#13521
Jehops pushed a commit to Jehops/freebsd-ports-legacy that referenced this pull request Nov 22, 2020
    SECURITY
        Prevent git operations for inactive users (#13527) (#13537)
        Disallow urlencoded new lines in git protocol paths if there is a port (#13521) (#13525)
    BUGFIXES
        API should only return Json (#13511) (#13564)
        Fix before and since query arguments at API (#13559) (#13560)
        Prevent panic on git blame by limiting lines to 4096 bytes at most (#13470) (#13492)
        Fix link detection in repository description with tailing ‘_’ (#13407) (#13408)
        Remove obsolete change of email on profile page (#13341) (#13348)
        Fix permission check on get Reactions API endpoints (#13344) (#13346)
        Add migrated pulls to pull request task queue (#13331) (#13335)
        API deny wrong pull creation options (#13308) (#13327)
        Fix initial commit page & binary munching problem (#13249) (#13259)
        Fix diff parsing (#13157) (#13136) (#13139)
        Return error 404 not 500 from API if team does not exist (#13118) (#13119)
        Prohibit automatic downgrades (#13108) (#13111)
        Fix GitLab Migration Option AuthToken (#13101)
        GitLab Label Color Normalizer (#12793) (#13100)
        Log the underlying panic in runMigrateTask (#13096) (#13098)
        Fix attachments list in edit comment (#13036) (#13097)
        Fix deadlock when deleting team user (#13093)
        Fix error create comment on outdated file (#13041) (#13042)
        Fix repository create/delete event webhooks (#13008) (#13027)
        Fix internal server error on README in submodule (#13006) (#13016)

PR:		251296
Submitted by:	maintainer
MFH:		2020Q4
Security:	go-gitea/gitea#13527
		go-gitea/gitea#13521


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@556058 35697150-7ecd-e111-bb59-0022644237b5
@go-gitea go-gitea locked and limited conversation to collaborators Dec 14, 2020
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. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants