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

Pushing a new cargo release returns 500 if user does not have write access to cargo index repo #26844

Open
merlleu opened this issue Aug 31, 2023 · 1 comment

Comments

@merlleu
Copy link
Contributor

merlleu commented Aug 31, 2023

Description

When you have an user with Packages: Read+Write access, he can't push to cargo registry if he does not have permission to the _cargo-index repo, returning a 500 error code.

I think at least we should change the error code for this, not sure if we should allow push for users without the repo write permissions still.

Gitea Version

1.21

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

https://gist.github.com/merlleu/50d26b2fe4dd1a1fb1ba9aa57d29c030

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

It is run from docker on the a587d25 commit's nightly build

Database

PostgreSQL

@lng2020
Copy link
Member

lng2020 commented Sep 1, 2023

I reproduced this issue as well.
Here is the detailed procedure to reproduce.

  1. Assume there are test_org and test_team
  2. Set write access in test_team->settings->packages
  3. remove write access of test_team in _cargo-index.git->settings->collaborators
  4. cargo publish then the error occurs

The credential passed the access control of the package, so cargo starts to use git to upload files. However, the repo denied the files because the credential can't write the repo. So here I come up with two possible solutions.

  1. Check the error type then use a different HTTP error in routers/api/packages/cargo/cargo.go#250
  2. A more elegant way is to sync the package permission with the package repo as mentioned in Permissions for package repositories #20596. It requires more effort but solves all issues of this kind.

I prefer the second solution but it's a little beyond my ability.
What do you think? @KN4CK3R

lunny pushed a commit that referenced this issue Oct 24, 2023
Hello there,
Cargo Index over HTTP is now prefered over git for package updates: we
should not force users who do not need the GIT repo to have the repo
created/updated on each publish (it can still be created in the packages
settings).

The current behavior when publishing is to check if the repo exist and
create it on the fly if not, then update it's content.
Cargo HTTP Index does not rely on the repo itself so this will be
useless for everyone not using the git protocol for cargo registry.

This PR only disable the creation on the fly of the repo when publishing
a crate.

This is linked to #26844 (error 500 when trying to publish a crate if
user is missing write access to the repo) because it's now optional.

---------

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Oct 24, 2023
Hello there,
Cargo Index over HTTP is now prefered over git for package updates: we
should not force users who do not need the GIT repo to have the repo
created/updated on each publish (it can still be created in the packages
settings).

The current behavior when publishing is to check if the repo exist and
create it on the fly if not, then update it's content.
Cargo HTTP Index does not rely on the repo itself so this will be
useless for everyone not using the git protocol for cargo registry.

This PR only disable the creation on the fly of the repo when publishing
a crate.

This is linked to go-gitea#26844 (error 500 when trying to publish a crate if
user is missing write access to the repo) because it's now optional.

---------

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
lunny pushed a commit that referenced this issue Oct 24, 2023
Backport #27266 by @merlleu

Hello there,
Cargo Index over HTTP is now prefered over git for package updates: we
should not force users who do not need the GIT repo to have the repo
created/updated on each publish (it can still be created in the packages
settings).

The current behavior when publishing is to check if the repo exist and
create it on the fly if not, then update it's content.
Cargo HTTP Index does not rely on the repo itself so this will be
useless for everyone not using the git protocol for cargo registry.

This PR only disable the creation on the fly of the repo when publishing
a crate.

This is linked to #26844 (error 500 when trying to publish a crate if
user is missing write access to the repo) because it's now optional.

Co-authored-by: merlleu <r.langdorph@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this issue Jan 17, 2024
Hello there,
Cargo Index over HTTP is now prefered over git for package updates: we
should not force users who do not need the GIT repo to have the repo
created/updated on each publish (it can still be created in the packages
settings).

The current behavior when publishing is to check if the repo exist and
create it on the fly if not, then update it's content.
Cargo HTTP Index does not rely on the repo itself so this will be
useless for everyone not using the git protocol for cargo registry.

This PR only disable the creation on the fly of the repo when publishing
a crate.

This is linked to go-gitea#26844 (error 500 when trying to publish a crate if
user is missing write access to the repo) because it's now optional.

---------

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
silverwind pushed a commit to silverwind/gitea that referenced this issue Feb 20, 2024
Hello there,
Cargo Index over HTTP is now prefered over git for package updates: we
should not force users who do not need the GIT repo to have the repo
created/updated on each publish (it can still be created in the packages
settings).

The current behavior when publishing is to check if the repo exist and
create it on the fly if not, then update it's content.
Cargo HTTP Index does not rely on the repo itself so this will be
useless for everyone not using the git protocol for cargo registry.

This PR only disable the creation on the fly of the repo when publishing
a crate.

This is linked to go-gitea#26844 (error 500 when trying to publish a crate if
user is missing write access to the repo) because it's now optional.

---------

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants