Proposal: limit api routes to access by token/basic auth only #2252

New Issue

GiteaMirror · 2025-11-02T04:29:33-06:00

GiteaMirror commented

2025-11-02 04:29:33 -06:00

Originally created by @beeonthego on GitHub (Aug 26, 2018).

As pointed out in issue #4703 (content deleted now), it is a csrf vulnerability to allow users to access api routes using session cookie without second check.

Currently the CSRF middleware only checks POST requests, and skips API routes. In addition, the reqToken function in routers/api/v1/api.go accepts all signed in users, including authenticated with session cookie. This means a script can make a POST/DELETE request to api routes while user is logged in (cookie present), without user's knowledge or consent. This is not desirable in multi tenant environment.

We have implemented some extra checks internally to limit api routes to access by token/basic auth only, and prohibit access by cookie when the route requires token. This does not affect public routes.

We have searched the code base and haven't found any client side Javascript code accessing protected api routes with cookie. But maybe there are cases we are not aware.
Is this the right approach? Will a PR like this break someone's code in the pipeline (if merged)?

Originally created by @beeonthego on GitHub (Aug 26, 2018). As pointed out in issue #4703 (content deleted now), it is a csrf vulnerability to allow users to access api routes using session cookie without second check. Currently the CSRF middleware only checks POST requests, and skips API routes. In addition, the reqToken function in routers/api/v1/api.go accepts all signed in users, including authenticated with session cookie. This means a script can make a POST/DELETE request to api routes while user is logged in (cookie present), without user's knowledge or consent. This is not desirable in multi tenant environment. We have implemented some extra checks internally to limit api routes to access by token/basic auth only, and prohibit access by cookie when the route requires token. This does not affect public routes. We have searched the code base and haven't found any client side Javascript code accessing protected api routes with cookie. But maybe there are cases we are not aware. Is this the right approach? Will a PR like this break someone's code in the pipeline (if merged)?

GiteaMirror added the modifies/api label 2025-11-02 04:29:33 -06:00

GiteaMirror closed this issue

2025-11-02 04:29:33 -06:00

GiteaMirror commented

2025-11-02 04:29:35 -06:00

@lafriks commented on GitHub (Aug 26, 2018):

There is repo/org search in user dashboard that uses it. If that would be changed to not use API, I agree

@lafriks commented on GitHub (Aug 26, 2018): There is repo/org search in user dashboard that uses it. If that would be changed to not use API, I agree

GiteaMirror commented

2025-11-02 04:29:35 -06:00

@beeonthego commented on GitHub (Aug 26, 2018):

Thanks for the insight. I thought I may have missed something. I did.

Here are the JS functions and api routes I have found in compiled JS file in user dashboard. The reqToken middleware was not present when registering routes. That is why I thought these were public routes. However I have just looked at the handler function and the access check is actually in the function.

searchUsers: /api/v1/users/search (public ?)
searchRepositories: /api/v1/repos/search (access check in function)
searchURL: /api/v1/repos/search (access check in function)
apiSettings: /api/v1/topics/search (access check in function)

Is it better to inject an access token to the dashboard page, and send the token back when making API requests? this will be similar to how csrf token is handled now. As long as we don't allow dashboard to load in cross site iFrame, it should be safe, as there is no content generated by other users in dashboard.

your thoughts?

@beeonthego commented on GitHub (Aug 26, 2018): Thanks for the insight. I thought I may have missed something. I did. Here are the JS functions and api routes I have found in compiled JS file in user dashboard. The reqToken middleware was not present when registering routes. That is why I thought these were public routes. However I have just looked at the handler function and the access check is actually in the function. searchUsers: /api/v1/users/search (public ?) searchRepositories: /api/v1/repos/search (access check in function) searchURL: /api/v1/repos/search (access check in function) apiSettings: /api/v1/topics/search (access check in function) Is it better to inject an access token to the dashboard page, and send the token back when making API requests? this will be similar to how csrf token is handled now. As long as we don't allow dashboard to load in cross site iFrame, it should be safe, as there is no content generated by other users in dashboard. your thoughts?

GiteaMirror commented

2025-11-02 04:29:35 -06:00

@beeonthego commented on GitHub (Sep 1, 2018):

I have just submitted PR #4840 to address a critical security issue reported in issue #4357 . It has passed all tests except two db tests. When token is enforced on API routes, reqToken will do its job as its name suggests, ask for a token. It will reject requests if token is not provided.

The current tests look for records and the records are not supposed to be there when token is enforced on API routes. Could someone review and rewrite the tests?

IMHO, API routes are designed for consumption by apps or remote servers, and private routes are protected with a key or token. Accepting authentication by cookie on API routes opens a can of worms.

POST requests on UI routes check the second factor of CSRF token. The current API routes are not covered by csrf handler. If someone has discovered a creative way to bypass markdown parser/sanitizer and run their code, they can make requests to the API routes using someone else's cookie. This is not an issue in a single user/team environment. things are different when there are multiple users.

@beeonthego commented on GitHub (Sep 1, 2018): I have just submitted PR #4840 to address a critical security issue reported in issue #4357 . It has passed all tests except two db tests. When token is enforced on API routes, reqToken will do its job as its name suggests, ask for a token. It will reject requests if token is not provided. The current tests look for records and the records are not supposed to be there when token is enforced on API routes. Could someone review and rewrite the tests? IMHO, API routes are designed for consumption by apps or remote servers, and private routes are protected with a key or token. Accepting authentication by cookie on API routes opens a can of worms. POST requests on UI routes check the second factor of CSRF token. The current API routes are not covered by csrf handler. If someone has discovered a creative way to bypass markdown parser/sanitizer and run their code, they can make requests to the API routes using someone else's cookie. This is not an issue in a single user/team environment. things are different when there are multiple users.

GiteaMirror commented

2025-11-02 04:29:36 -06:00

@lunny commented on GitHub (Sep 11, 2018):

closed by #4840

@lunny commented on GitHub (Sep 11, 2018): closed by #4840

GiteaMirror referenced this issue

2025-11-02 12:07:16 -06:00

[PR #2252] [MERGED] Add changelog of release v1.1.3 #16307

Sign in to join this conversation.