go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
45.21k stars 5.5k forks source link

API returns 201 even though not all data from the request has been processed due to the sudo user not beeing an andministrator #11320

Open PascalMinder opened 4 years ago

PascalMinder commented 4 years ago

Description

I'am currently in the process of migrating Trac issues to a new Gitea instance. For this reason I am currently working on a php script which reads issues from the Trac database and creates them over the Gitea API on the new gitea instance.

To create the issue in the name of the orginal author I use the "sudo" header with the user which created the issue in Trac (with a mapping table).

When I try to create an Issue with a user which is not an administrator I get a 201 http back and the issue gets created. BUT linked labels or milestones are not createt for the issue. However if the user is an administrator no such error occures and the issue gets created with all linked labels and milestones.

I would have expected, that the api returns a 400 http header or something similar if not all data could be processed. Is this an intended behaviour?

6543 commented 4 years ago

this is not intended behaviour

6543 commented 4 years ago

and if you have a working script it would be nice to publish it somewhere - could go on the list of #8689

6543 commented 4 years ago

I'll have a look at this but it would be nice if you can describe what exactly do you mean with "linked" labels

PascalMinder commented 4 years ago

So on the page https://try.gitea.io/api/swagger#/issue/issueCreateIssue it says, the json below can be used to create a new issue. There is a field labels and a field milestone. Both of them accept a int value to like one (milestone) or multiple labels. If I use sudo (with an administrator) all works as expected and the new issue has the right labels linked. But if the sudo user is not an administrator the issue gets created but is not linked.

{
  "assignee": "string",
  "assignees": [
    "string"
  ],
  "body": "string",
  "closed": true,
  "due_date": "2020-05-07T07:40:49.100Z",
  "labels": [
    0
  ],
  "milestone": 0,
  "title": "string"
}

The script has a big limitation, that there is no api which support setting the created_at value. This is a bit of an issue for a nice import.

zeripath commented 4 years ago

I don't like the idea of arbitrary users creating things in the past, future or whenever. It seems like we would be better off creating a proper migration from trac instead.

PascalMinder commented 4 years ago

Yeah that would probably be the better way.

6543 commented 4 years ago

@PascalMinder the API is working fine, this is because of securety conzernes too since with sudo you act with the same permission as the user you put into sudo, and if this user is not alowed to add labels API should not allow this too

so this issue is infalid

@PascalMinder for you: your script has to create the issue first and the admin acount should add labels and milestones afterwards

PascalMinder commented 4 years ago

@6543 Okay, but if there is a security concern about a user which adds a label with sudo, the API should probably return another status code than 20X and not apply half of the requested changes?

6543 commented 4 years ago

the API function's itself do not know if the user is introduced by sudo or if he acts itself with the api

6543 commented 4 years ago

Instead of silent droping we could return a 422 ... but It will be a breaking change ...

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

6543 commented 4 years ago

ping