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

Add new CLI flags to set name and scopes when creating a user with access token #34080

Merged
merged 15 commits into from
Apr 2, 2025

Conversation

kemzeb
Copy link
Contributor

@kemzeb kemzeb commented Mar 31, 2025

Resolves #33474.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 31, 2025
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 31, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin labels Mar 31, 2025
@kemzeb kemzeb changed the title Add --with-scopes CLI flag when creating a user with access token Add --access-token to accept scopes when creating a user via CLI Mar 31, 2025
@kemzeb kemzeb changed the title Add --access-token to accept scopes when creating a user via CLI Reimplement --access-token to accept scopes when creating a user via CLI Mar 31, 2025
@wxiaoguang wxiaoguang marked this pull request as draft March 31, 2025 06:12
@wxiaoguang wxiaoguang self-assigned this Mar 31, 2025
@wxiaoguang
Copy link
Contributor

After more testing, I think your old approach (using separate flags) is better and clearer.

I prefixed these new flags with --access-token to make them in the same "flag group" (a little wordy but IMO it's clearer to users):

   --access-token                  Generate access token for the user (default: false)
   --access-token-name value       Name of the generated access token (default: "gitea-admin")
   --access-token-scopes value     Scopes of the generated access token, comma separated. Examples: "all", "public-only,read:issue", "write:repository,write:user" (default: "all")

Does it look to you?

@wxiaoguang wxiaoguang removed their assignment Mar 31, 2025
@wxiaoguang wxiaoguang marked this pull request as ready for review March 31, 2025 06:34
@wxiaoguang wxiaoguang changed the title Reimplement --access-token to accept scopes when creating a user via CLI Add new CLI flags to set name and scopes when creating a user with access token Mar 31, 2025
@kemzeb
Copy link
Contributor Author

kemzeb commented Mar 31, 2025

I'll be able to check this out later today

@kemzeb
Copy link
Contributor Author

kemzeb commented Apr 1, 2025

Decided to add two more unit tests to cover using of --access-token-name or --access-token-scopes without --access-token

@kemzeb
Copy link
Contributor Author

kemzeb commented Apr 1, 2025

Decided to add two more unit tests to cover using of --access-token-name or --access-token-scopes without --access-token

One more thing I thought about.

We currently create a user even if the access token-related flags are misused, so we do create a user but end up not creating an access token if they are misused. Should we catch this misuse earlier to avoid this?

@wxiaoguang
Copy link
Contributor

The "scope" problem has some technical debts, in short:

  1. For legacy reasons, empty scope falls back to "all"
  2. Ideally we should not accept empty scope any more (will add more fixes and tests)

image

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 1, 2025
@wxiaoguang
Copy link
Contributor

One more thing I thought about.

We currently create a user even if the access token-related flags are misused, so we do create a user but end up not creating an access token if they are misused. Should we catch this misuse earlier to avoid this?

Maybe like this ac86171?

@kemzeb
Copy link
Contributor Author

kemzeb commented Apr 2, 2025

One more thing I thought about.
We currently create a user even if the access token-related flags are misused, so we do create a user but end up not creating an access token if they are misused. Should we catch this misuse earlier to avoid this?

Maybe like this ac86171?

LGTM!

Not able to approve since I'm the PR author

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 2, 2025
@wxiaoguang wxiaoguang added backport/v1.23 This PR should be backported to Gitea 1.23 type/bug labels Apr 2, 2025
@wxiaoguang wxiaoguang added this to the 1.24.0 milestone Apr 2, 2025
@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 Apr 2, 2025
@wxiaoguang wxiaoguang merged commit 55a69ae into go-gitea:main Apr 2, 2025
26 checks passed
wxiaoguang pushed a commit to wxiaoguang/gitea that referenced this pull request Apr 2, 2025
…cess token (go-gitea#34080)

Resolves go-gitea#33474.

---------

Co-authored-by: wxiaoguang <[email protected]>
# Conflicts:
#	cmd/admin_user_create_test.go
#	routers/web/user/setting/applications.go
wxiaoguang pushed a commit to wxiaoguang/gitea that referenced this pull request Apr 2, 2025
…cess token (go-gitea#34080)

Resolves go-gitea#33474.

---------

Co-authored-by: wxiaoguang <[email protected]>
# Conflicts:
#	cmd/admin_user_create_test.go
#	routers/web/user/setting/applications.go
wxiaoguang pushed a commit to wxiaoguang/gitea that referenced this pull request Apr 2, 2025
…cess token (go-gitea#34080)

Resolves go-gitea#33474.

---------

Co-authored-by: wxiaoguang <[email protected]>
# Conflicts:
#	cmd/admin_user_create_test.go
#	routers/web/user/setting/applications.go
@wxiaoguang wxiaoguang added backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! labels Apr 2, 2025
@kemzeb kemzeb deleted the feat/admin-cli branch April 2, 2025 17:00
wxiaoguang added a commit that referenced this pull request Apr 2, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2025
* giteaofficial/main:
  Refactor Branch struct in package modules/git (go-gitea#33980)
  Support creating relative link to raw path in markdown (go-gitea#34105)
  Add new CLI flags to set name and scopes when creating a user with access token (go-gitea#34080)
  Do not show 500 error when default branch doesn't exist (go-gitea#34096)
  Return default avatar url when user id is zero rather than updating database (go-gitea#34094)
  [skip ci] Updated translations via Crowdin
  Fix return bug (go-gitea#34093)
  Move ParseCommitWithSSHSignature to service layer (go-gitea#34087)
  fix(i18n): clarify ownership in password change error messages (go-gitea#34092)
  Enable addtional linters (go-gitea#34085)
  Add flat-square action badge style (go-gitea#34062)
  Fix some UI bugs and clean up unused tests (go-gitea#34088)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.23 This PR should be backported to Gitea 1.23 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The token generated by gitea admin user create --access-token has no scope
4 participants