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

Use ghost user if package creator does not exist #23822

Merged
merged 3 commits into from Apr 4, 2023

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Mar 30, 2023

Fixes #23818

@KN4CK3R KN4CK3R added type/bug topic/packages outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 30, 2023
@KN4CK3R KN4CK3R added this to the 1.20.0 milestone Mar 30, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 30, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 30, 2023
@jolheiser jolheiser added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 30, 2023
@@ -99,7 +101,11 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc
}
creator, err := user_model.GetUserByID(ctx, pv.CreatorID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be GetPossibleUserByID?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, if the user gets deleted, the CreatorID is not -1 or -2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe we need to check if it's -2 here?

Copy link
Contributor

@yp05327 yp05327 Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does actions user have write permission to create packages?
If not, maybe we don't need to check -2 here.

Copy link
Member

@wolfogre wolfogre Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does actions user have write permission to create packages?

I think it doesn't. Gitea will use the trigger user instead of actions bot user as the package creator in #23729.

However, I think it will be more robust to support actions bot as package creator. (BTW, not the owner)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it is not merged. It seems that actions user can not create packages in current version, but I’m not sure about this, so I asked this question.

Copy link
Member

@wolfogre wolfogre Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it is not merged. It seems that actions user can not create packages in current version, but I’m not sure about this, so I asked this question.

OK, I misunderstand your question. So the answer to Does actions user have write permission to create packages is no now, and I 90% sure it should be no by default, but maybe we can provide an option to set it on UI for users in the future.

@codecov-commenter

This comment was marked as outdated.

@delvh delvh self-requested a review March 30, 2023 20:29
@lunny lunny merged commit d93f322 into go-gitea:main Apr 4, 2023
1 check passed
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 4, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Apr 4, 2023
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Apr 4, 2023
lunny pushed a commit that referenced this pull request Apr 4, 2023
Backport #23822 by @KN4CK3R

Fixes #23818

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 5, 2023
* giteaoffical/main:
  Fix image border-radius (go-gitea#23886)
  [skip ci] Updated translations via Crowdin
  Scroll collapsed file into view (go-gitea#23702)
  docs: make the required backticks in email password more explicit (go-gitea#23923)
  docs: fix typo (go-gitea#23924)
  Update docs markdown file weight to make it clear (go-gitea#23909)
  Add activity feeds API (go-gitea#23494)
  Fix code view (diff) broken layout (go-gitea#23096)
  Use ghost user if package creator does not exist (go-gitea#23822)
  Org pages style fixes (go-gitea#23901)
  User/Org Feed render description as per web (go-gitea#23887)
@KN4CK3R KN4CK3R deleted the fix-23818 branch April 17, 2023 22:06
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 topic/packages type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Packages cannot be displayed after a user is deleted
9 participants