Some endpoints ignore specified account and use auth account instead

[lonix1]

Description

Reproduction:

• I created an access token at the /api/v1/users/bob/tokens endpoint, using basic auth with my admin account.
• I expected the access token to be created for bob, but it was actually created for admin.
• the same is true for other related endpoints

This was very surprising and took a while to debug. Is it a bug or by design?

If it's a bug (I think it is), then this issue will track it.

If it's by design:

• It's common to use an admin account to interact with an app, even when managing other users' accounts. So this behaviour is very surprising.
• I specified that I wanted to act on the bob account, not the admin account.
• If this cannot be fixed/changed, then at least those API endpoints should return errors. If it will ignore the account that I specified and use the authenticating account instead, it must return an error rather than perform an action I didn't request.
• The docs should reflect this weird behaviour (I looked but couldn't find anything).

Thanks!

Gitea Version

1.20.2

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

n/a

Screenshots

n/a

Git Version

n/a

Operating System

n/a

How are you running Gitea?

docker

Database

None

[CaiCandong]

I think the current api is ambiguous, it doesn't make sense for one user to manipulate another user's token (unless that user is an admin)

1. I think the current /api/v1/users/{username}/tokens should be changed to /api/v1/user/tokens.(Users manipulate their own tokens)
2. And the admin manipulate token should be placed in: /api/v1/admin/users/{username}/tokens.(Admin manipulate user's tokens)

[lonix1]

Agreed: it's ambiguous.

Agreed: an admin user should be able to do it. (Like in my example, but it must operate on the target account rather than the admin account.)

But...If you change the API then it's a breaking change, and in theory, you should change from /api/v1 to /api/v2. Which is a big headache for everyone.

Maybe there is a simpler solution? Maybe

• keep the old endpoints, for now, but mark them as deprecated
• or change the existing endpoints to work as documented/expected (it will break for some users, but then it's because they were not following the specification; it won't break for everyone else)
• or, ...? 🤔

[CaiCandong]

• keep the old endpoints, for now, but mark them as deprecated.

We give a permission error if the {username} in path does not match the currently logged in user. Prompting the user that they should use /api/v1/admin/users/{username}/tokens to complete the action.

[lonix1]

That is a good idea.

But what about the admin user? He should be able to operate on another user's account.

[CaiCandong]

But what about the admin user? He should be able to operate on another user's account.

The admin user should use /api/v1/admin/users/{username}/tokens to accomplish this. Manipulating other people's tokens should only be done by administrators.
I see that {username} is not considered in the implementation of this endpoints , so perhaps it was misplaced to begin with, causing the current misunderstanding. This endpoints is not meant to manipulate other user tokens. We should dispel this misunderstanding, not go along with it and modify the endpoint .

[lonix1]

I agree with your conclusion.

(Just keep in mind it's a breaking change, both for those who were using the API correctly and for those who weren't.)

[CaiCandong]

(Just keep in mind it's a breaking change, both for those who were using the API correctly and for those who weren't.)

People who want to use this endpoints to enable token modification operations by admin will encounter the same confusion as you. Those who use it correctly will not be affected by this modification.

[lonix1]

After the change, yes. The point is it will break existing integrations. It should be labelled as "breaking change" so people know not to upgrade until they fix their api code.

[CaiCandong]

The point is it will break existing integrations

The changes I've made are guaranteed compatibility #26323

[techknowlogick]

The creation of tokens via API is purposefully limited to non-app tokens because otherwise if an app token is leaked then it could be used to have a chain attack and have new tokens created to maintain access from a malicious user.

[CaiCandong]

The creation of tokens via API is purposefully limited to non-app tokens because otherwise if an app token is leaked then it could be used to have a chain attack and have new tokens created to maintain access from a malicious user.

Yes, I noticed that token manipulation can only be done using basic Auth authentication. I'm wondering if gitea's current design allows administrators to manipulate ordinary users' tokens via the API. It seems to me that there are security and privacy issues with this as well, but @lonix1's desire for such an interface seems a bit unreasonable to me right now.

[waTeim]

What's happening with this? I'm experiencing the problem explained in this bug. There was a PR to fix this apparently (#26323), but it was abandoned, why?

[CaiCandong]

but it was abandoned, why?

No one Reviewed it for a long time, so I closed it. I'm turning it back on now.

[waTeim]

kk, well in summary. I too depend on being able to use the admin account to take on the personality of another user for a specific operation (create a fork). Since it remained in review for a long time, maybe it will continue to do so. Do you have an expectation about how long? If long. Is there a workaround? Downgrading to a version prior to this problem is fine.

[lonix1]

I think I opened a can of worms when I opened that issue, and the new code and discussions were so complicated that I couldn't understand / follow any longer.

I'll install the next version when it's released and just hope my setup doesn't break.

[waTeim]

Well, I'm confused now. From the title of the merged PR, it appears that a fix is to remove the ability to specify the user in the path because it gets ignored; so the fix is to make the path consistent with that notion. My objective is to use the admin account to obtain a token to become a user for purposes of forking a repo. Embedded in the discussion, there's some sort of sudo functionality on (some of) the paths? Is that documented somewhere?

[CaiCandong]

@waTeim @lonix1
Now the endpoints mean that the doer operates on the contextUser's token.

• If the doer and the contextUser are the same user, the operation will succeed.
• If the doer is an administrator, the operation will work.
• But if the doer is an ordinary user, his operation on the other contextUser will be rejected.

[waTeim]

Got it, so in scenario 2, what is the mechanism for the admin (doer) to set the contextUser to some arbitrary other user?

[CaiCandong]

Got it, so in scenario 2, what is the mechanism for the admin (doer) to set the contextUser to some arbitrary other user?

I'm sorry, I don't quite understand your question.

[CaiCandong]

I think I opened a can of worms when I opened that issue, and the new code and discussions were so complicated that I couldn't understand / follow any longer.

The discussion clarifies whether it is necessary to split this endpoint into two parts: normal user operations and administrator operations.

[waTeim]

In other words, what forces ContetUser != Doer, what sort of difference in the REST endpoint call is needed? That kinda came up in one of the linked issues above.

[CaiCandong]

In other words, what forces ContetUser != Doer, what sort of difference in the REST endpoint call is needed?

Doer represents the user who provided the account and password, which is the user in basicAuth.
ContextUser represents the path in the URL path.
Doer applies an action to ContextUser.

[waTeim]

Ok so if this change is to remove user from the path, will ContextUser ever be different from Doer?

[CaiCandong]

But the contextUser is not removed from the path.

[waTeim]

oh, my bad, I misinterpreted the discussion, then. I thought you had opted to remove it.

[CaiCandong]

So now the behavior of this endpoint is what you expected?

[waTeim]

I think so? I'll modify my code, remove the workaround, re-test, and report back. Is there already a container image that has the change, or do I need to compile, etc, I've just been using 1.20.4 (feel free to point me to docs).

[lonix1]

Thanks for the explanation.

If the doer is an administrator, the operation will work.

If I understand correctly, that will resolve the original issue: the admin user can create a token for a different user.