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 api uses in the web ui #5249

Closed
kolaente opened this issue Nov 1, 2018 · 9 comments
Closed

Fix api uses in the web ui #5249

kolaente opened this issue Nov 1, 2018 · 9 comments
Labels
issue/stale modifies/api This PR adds API routes or modifies them

Comments

@kolaente
Copy link
Member

kolaente commented Nov 1, 2018

#4840 introduced some significant changes to the way the authentication for the api works. It broke some things where the web ui called the api, namely:

https://github.com/go-gitea/gitea/blob/master/public/js/index.js#L1459
https://github.com/go-gitea/gitea/blob/master/public/js/index.js#L1486
https://github.com/go-gitea/gitea/blob/master/public/js/index.js#L2087
https://github.com/go-gitea/gitea/blob/master/public/js/index.js#L2507
https://github.com/go-gitea/gitea/blob/master/public/js/index.js#L2594
https://github.com/go-gitea/gitea/blob/master/public/js/index.js#L2630

We have two possibilities to solve this:

  • Either call the routes using an api token
  • Accept using a cookie, but if using a cookie, some kind of crf token to validate the request

IMHO the first solution would be cleaner because if an attacker has a cookie he/she could easily use that to get a new api request token via the web ui, rendering the whole double-check useless. So we could save the hassle to implement the double check with cookie/crf token and use the mechanism which is already there.

@techknowlogick
Copy link
Member

@kolaente a malicious user wouldn't need the cookie to be able to post to the API. They could create a form on a different domain that POSTs to the API, and because the user has the cookie the API would accept it, which is why the CSRF would be needed.

Option 2 would be better than 1 because the CSRF is shortlived and autogenerated, and an API token is long living, so I believe 2 is cleaner.

@zeripath
Copy link
Contributor

zeripath commented Nov 1, 2018

@zeripath
Copy link
Contributor

zeripath commented Nov 1, 2018

Looking at the code, and following some rudimentary testing I'm not convinced that the GET urls are affected.

@kolaente
Copy link
Member Author

kolaente commented Nov 1, 2018

@zeripath The issue search does not work when on a private repo, user search shouldn't work either, neither should topic search.

@techknowlogick techknowlogick added the modifies/api This PR adds API routes or modifies them label Nov 1, 2018
zeripath added a commit to zeripath/gitea that referenced this issue Nov 2, 2018
techknowlogick pushed a commit that referenced this issue Nov 4, 2018
… POST header for deadline (#5250)

* Add CSRF checking to reqToken and place CSRF in the post for deadline creation

Fixes #5226, #5249

* /api/v1/admin/users routes should have reqToken middleware
zeripath added a commit to zeripath/gitea that referenced this issue Nov 4, 2018
techknowlogick pushed a commit that referenced this issue Nov 4, 2018
…es with CSRF token (#5272)

* Add CSRF checking to reqToken and place CSRF in the post for deadline creation

Fixes #5226, #5249

* /api/v1/admin/users routes should have reqToken middleware
@lafriks
Copy link
Member

lafriks commented Nov 15, 2018

I would prefer to stick to current behaviour to not use API for UI

@zeripath
Copy link
Contributor

@lafriks I'm not sure I understand.

@lafriks
Copy link
Member

lafriks commented Nov 16, 2018

I meant that we should create routes that would be called for getting data for UI and not call API routes

@stale
Copy link

stale bot commented Jan 15, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 15, 2019
@stale
Copy link

stale bot commented Feb 19, 2019

This issue has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this as completed Feb 19, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale modifies/api This PR adds API routes or modifies them
Projects
None yet
Development

No branches or pull requests

4 participants