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.

Sign up for GitHub

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

Jump to bottom

Remove dependent on session auth for api/v1 routers #19321

Merged
merged 5 commits into from
Apr 8, 2022
Merged

Remove dependent on session auth for api/v1 routers #19321

merged 5 commits into from
Apr 8, 2022

Conversation

lunny
Copy link
Member

@lunny lunny commented Apr 4, 2022 •
edited by 6543
Loading

partially replace #16052

@lunny lunny added type/enhancement An improvement of existing functionality pr/wip This PR is not ready for review labels Apr 4, 2022
@lunny lunny added this to the 1.17.0 milestone Apr 4, 2022
@lunny lunny mentioned this pull request Apr 4, 2022
Closed
@codecov-commenter
Copy link

Codecov Report

Merging #19321 (797524c) into main (d242511) will decrease coverage by 0.17%.
The diff coverage is 10.09%.

@@            Coverage Diff             @@
##             main   #19321      +/-   ##
==========================================
- Coverage   47.51%   47.33%   -0.18%     
==========================================
  Files         944      948       +4     
  Lines      131549   132028     +479     
==========================================
- Hits        62500    62499       -1     
- Misses      61541    62018     +477     
- Partials     7508     7511       +3
Impacted Files Coverage Δ
routers/api/v1/repo/issue_tracked_time.go 38.58% <0.00%> (ø)
routers/api/v1/repo/repo.go 66.55% <0.00%> (ø)
routers/web/explore/topic.go 0.00% <0.00%> (ø)
routers/web/org/teams.go 0.00% <0.00%> (ø)
routers/web/repo/issue.go 33.15% <0.00%> (-4.30%) ⬇️
routers/web/repo/repo.go 24.42% <0.00%> (-6.05%) ⬇️
routers/web/user/notification.go 36.17% <0.00%> (-0.53%) ⬇️
routers/web/user/search.go 0.00% <0.00%> (ø)
routers/web/user/stop_watch.go 0.00% <0.00%> (ø)
services/auth/auth.go 28.57% <ø> (ø)
... and 19 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 256204b...797524c. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 4, 2022
@lunny lunny removed the pr/wip This PR is not ready for review label Apr 4, 2022
@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 Apr 7, 2022
@6543
Copy link
Member

6543 commented Apr 7, 2022

@lunny TestSearchIssuesWithLabels -> do not work

@lunny lunny force-pushed the lunny/remove_api_session branch 2 times, most recently from 33dc558 to fca9bcd Compare April 8, 2022 01:32
@wxiaoguang
Copy link
Contributor

There are still calls to /api/v1 from UI after

Powered by Gitea Version: 1.17.0+dev-409-g783a02188 Page: 85ms Template : 15ms 
POST https://try.gitea.io/api/v1/repos/wxiaoguang/test/markdown

@lunny
Copy link
Member Author

lunny commented Apr 8, 2022

markdown

Good catch, I will fix them in this PR.

@lunny lunny force-pushed the lunny/remove_api_session branch from fca9bcd to c4ebfbc Compare April 8, 2022 02:43
@lunny
Copy link
Member Author

lunny commented Apr 8, 2022

There are still calls to /api/v1 from UI after

Powered by Gitea Version: 1.17.0+dev-409-g783a02188 Page: 85ms Template : 15ms 
POST https://try.gitea.io/api/v1/repos/wxiaoguang/test/markdown

Done.

6543
6543 approved these changes Apr 8, 2022
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

ah my grep missed the "APIURL" string in templates ... thanks for catch them too

@6543 6543 requested a review from wxiaoguang April 8, 2022 04:02
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 8, 2022 •
edited
Loading

More information: using HTMLURL(absolute URL) in templates is the root case that Gitea can only serve web under ROOT_URL. That's why we have to force users to set ROOT_URL correctly, and that's that PR comes:

If all URLs in template can be refactored to use Relative URL correctly, then we do not need to force users set ROOT_URL correctly again and again.

@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 Apr 8, 2022
@6543 6543 merged commit 3c3d498 into go-gitea:main Apr 8, 2022
@6543
Copy link
Member

6543 commented Apr 8, 2022

@lunny we might add some linter to ensure that templates and web_src dont contain any api/v1 or APIURL ?

@lunny lunny deleted the lunny/remove_api_session branch April 8, 2022 05:54
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 11, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
6764005 
* giteaofficial/main: (22 commits)
  Add logic to switch between source/rendered on Markdown (go-gitea#19356)
  Fixed registry host value. (go-gitea#19363)
  [skip ci] Updated translations via Crowdin
  Allow package linking to private repository (go-gitea#19348)
  Use "main" as default branch name (go-gitea#19354)
  Move milestone to models/issues/ (go-gitea#19278)
  Refactor CSRF protection modules, make sure CSRF tokens can be up-to-date. (go-gitea#19337)
  Remove dependent on session auth for api/v1 routers (go-gitea#19321)
  API: Search Issues, dont show 500 if filter result in empty list (go-gitea#19244)
  [skip ci] Updated translations via Crowdin
  Never use /api/v1 from Gitea UI Pages (go-gitea#19318)
  [skip ci] Updated translations via Crowdin
  Show ssh command directly in template instead of i18n translation (go-gitea#19335)
  Package registry changes (go-gitea#19305)
  [skip ci] Updated translations via Crowdin
  Add `ENABLE_SSH_LOG` to debugging problems (go-gitea#19316)
  Warn on SSH connection for incorrect configuration (go-gitea#19317)
  escape fake link
  Allow custom redirect for landing page (go-gitea#19324)
  [skip ci] Updated translations via Crowdin
  ...
@wxiaoguang wxiaoguang mentioned this pull request Apr 12, 2022
Closed
This was referenced Jul 28, 2022
Closed
Open
Closed
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
@lunny @AbdulrhmnGhanem
Remove dependent on session auth for api/v1 routers (go-gitea#19321)
9a6790b 
* Remove dependent on session auth for api/v1 routers

* Remove unnecessary session on API context

* remove missed header

* fix test

* fix missed api/v1
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.
5 participants
@lunny @codecov-commenter @6543 @wxiaoguang @GiteaBot