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

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
1 of 7 tasks
PascalMinder opened this issue May 7, 2020 · 12 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented modifies/api This PR adds API routes or modifies them

Comments

@PascalMinder
Copy link

  • Gitea version (or commit ref):
  • Git version: 1.12.0+dev-264-ga104864da
  • Operating system: Windows
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

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
Copy link
Member

6543 commented May 7, 2020

this is not intended behaviour

@6543
Copy link
Member

6543 commented May 7, 2020

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

@6543
Copy link
Member

6543 commented May 7, 2020

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
Copy link
Author

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
Copy link
Contributor

zeripath commented May 8, 2020

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
Copy link
Author

Yeah that would probably be the better way.

@6543
Copy link
Member

6543 commented May 9, 2020

@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
Copy link
Author

@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
Copy link
Member

6543 commented May 9, 2020

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

@6543
Copy link
Member

6543 commented May 9, 2020

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

@stale
Copy link

stale bot commented Jul 11, 2020

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.

@stale stale bot added the issue/stale label Jul 11, 2020
@6543
Copy link
Member

6543 commented Jul 11, 2020

ping

@stale stale bot removed the issue/stale label Jul 11, 2020
@lunny lunny added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented modifies/api This PR adds API routes or modifies them labels Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented modifies/api This PR adds API routes or modifies them
Projects
None yet
Development

No branches or pull requests

4 participants