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

Check permission for the appropriate unit type #14261

Merged
merged 4 commits into from
Jan 6, 2021

Conversation

jpraet
Copy link
Member

@jpraet jpraet commented Jan 5, 2021

Fixes false positives for "User created Issues in Repository which they no longer have access to" errors

Or could lines 548-556 be removed entirely? Not sure whether this permission check is really useful. It doesn't enforce anything, it just logs an error?

Fixes false positives for "User created Issues in Repository which they no longer have access to" errors
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 5, 2021
@zeripath
Copy link
Contributor

zeripath commented Jan 5, 2021

the reason it exists is because of different bug - that was causing other issues - you'd need to look at the issues around the time it was merged.

@zeripath
Copy link
Contributor

zeripath commented Jan 5, 2021

OK make it log.Trace or log.Debug

@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 Jan 6, 2021
@lunny
Copy link
Member

lunny commented Jan 6, 2021

Should we still put the repoID in showReposMap if he lost the permissions ?

@codecov-io
Copy link

Codecov Report

Merging #14261 (44b49b5) into master (126c933) will decrease coverage by 0.10%.
The diff coverage is 26.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14261      +/-   ##
==========================================
- Coverage   41.97%   41.86%   -0.11%     
==========================================
  Files         735      742       +7     
  Lines       78933    79324     +391     
==========================================
+ Hits        33132    33211      +79     
- Misses      40346    40649     +303     
- Partials     5455     5464       +9     
Impacted Files Coverage Δ
modules/auth/admin.go 0.00% <ø> (ø)
modules/auth/auth.go 61.53% <ø> (-2.51%) ⬇️
modules/auth/sso/reverseproxy.go 9.52% <0.00%> (ø)
modules/middlewares/cookie.go 0.00% <0.00%> (ø)
modules/middlewares/locale.go 0.00% <0.00%> (ø)
routers/user/home.go 60.00% <0.00%> (ø)
modules/middlewares/redis.go 2.15% <2.15%> (ø)
routers/admin/users.go 27.77% <26.66%> (-0.08%) ⬇️
modules/templates/base.go 28.57% <28.57%> (ø)
routers/routes/recovery.go 29.41% <29.41%> (ø)
... and 25 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 91ceba0...44b49b5. Read the comment docs.

@lafriks lafriks merged commit 8224f03 into go-gitea:master Jan 6, 2021
a1012112796 added a commit to a1012112796/gitea that referenced this pull request Jan 14, 2021
* master: (252 commits)
  Issues overview should not show issues from archived repos (go-gitea#13220)
  Display SVG files as images instead of text (go-gitea#14101)
  [skip ci] Updated translations via Crowdin
  Update docs to clarify issues raised in go-gitea#14272 (go-gitea#14318)
  [skip ci] Updated translations via Crowdin
  [Refactor] Passwort Hash/Set (go-gitea#14282)
  Add option to change username to the admin panel (go-gitea#14229)
  fix mailIssueCommentBatch for pull request (go-gitea#14252)
  Remove self from MAINTAINERS (go-gitea#14286)
  Do not reload page after adding comments in Pull Request reviews (go-gitea#13877)
  Fix session bug when introduce chi (go-gitea#14287)
  [skip ci] Updated translations via Crowdin
  Add secure/httpOnly attributes to the lang cookie (go-gitea#9690) (go-gitea#14279)
  Some code improvements (go-gitea#14266)
  [skip ci] Updated translations via Crowdin
  Fix wrong type on hooktask to convert typ from char(16) to varchar(16) (go-gitea#14148)
  Upgrade XORM links in documentation. (go-gitea#14265)
  Check permission for the appropriate unit type (go-gitea#14261)
  Add compliance check for windows to ensure cross platform build (go-gitea#14260)
  [skip ci] Updated translations via Crowdin
  ...
@jpraet jpraet deleted the fix-incorrect-error branch January 22, 2021 18:38
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants