api: fix overly strict permissions on edit PR

[noerw]

We can drop this overly strict permission requirement, as the more granular check (IsIssuePoster) is done in repo.EditPullRequest()

fixes #14025

[codecov-commenter]

Codecov Report

Merging #15900 (9978a9e) into main (370cfde) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main   #15900      +/-   ##
==========================================
- Coverage   44.03%   44.02%   -0.01%     
==========================================
  Files         681      681              
  Lines       82340    82340              
==========================================
- Hits        36256    36254       -2     
+ Misses      40182    40179       -3     
- Partials     5902     5907       +5     

Impacted Files	Coverage Δ
routers/api/v1/api.go	78.15% <100.00%> (ø)
modules/util/timer.go	42.85% <0.00%> (-42.86%)	⬇️
modules/avatar/avatar.go	47.72% <0.00%> (-4.55%)	⬇️
modules/charset/charset.go	71.71% <0.00%> (-4.05%)	⬇️
modules/log/event.go	59.90% <0.00%> (-1.89%)	⬇️
routers/api/v1/repo/pull.go	28.74% <0.00%> (+0.59%)	⬆️
modules/queue/manager.go	68.36% <0.00%> (+2.82%)	⬆️
modules/notification/mail/mail.go	41.83% <0.00%> (+3.06%)	⬆️

---

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 370cfde...9978a9e. Read the comment docs.

[lunny]

We can drop this overly strict permission requirement, as the more granular check (IsIssuePoster) is done in repo.EditPullRequest()

fixes #14025

I think we just missed Poster, but the original permission check should be also there.

[noerw]

@lunny I can't follow, how would you do it instead? :)

Adding a new middleware reqIssueEditor() in api.go implementing the same checks as EditPullRequest() would be loads of duplicated code, as well as duplicated work during each request 🤔

[lunny]

Just change reqIssueEditor since Poster should have permission to edit the issue or pull request.

[6543]

The permission is checked in the func itselve again ...

[noerw]

Here is an alternative implementation, that does the additional permission check via middleware: main...noerw:fix-14025-variant2
Running some local benchmarks, there is no significant cost induced by the duplicate checks (before 105ms, after 110ms avg request time). Though this request time seems quite high in general, but that's another story ;)

(I don't think the (potentially) better readability in api.go on this 2nd variant is worth the additional complexity)

[6543]

-> #16081