-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Add post-installation redirect based on admin account status #34493
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't speak on frontend design, but otherwise this looks good to me.
Was iffy about the HasUsers()
calls ignoring DB errors, but after further reading it seems the worst that can happen is:
- User accesses sign up page (or completes the install config if they're on the install page)
- A DB error occurs
- Server sends sign up page + the
sign_up_tip
message to the user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
options/locale/locale_en-US.ini
Outdated
@@ -421,6 +421,7 @@ remember_me.compromised = The login token is not valid anymore which may indicat | |||
forgot_password_title= Forgot Password | |||
forgot_password = Forgot password? | |||
need_account = Need an account? | |||
sign_up_tip = You are registering the first account in the system. Please carefully remember your username and password, as losing this information could cause significant inconvenience later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an unclear message. For example: why the "first account" is special? What "significant inconvenience" would happen? If it is not clean, it only confuses or frightens end users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see if the new tip is appropriate.
And also please review admin related bug fix: Fix last admin check when syncing users #34649 |
@@ -421,6 +421,7 @@ remember_me.compromised = The login token is not valid anymore which may indicat | |||
forgot_password_title= Forgot Password | |||
forgot_password = Forgot password? | |||
need_account = Need an account? | |||
sign_up_tip = You are registering the first account in the system, which has administrator privileges. Please carefully remember your username and password, as forgetting these credentials may require system reset and reinitialization. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as forgetting these credentials may require system reset and reinitialization.
That's not true, site admin can use gitea
CLI to reset user password
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are registering the first account in the system. Please remember your username and password, as this account typically has administrative privileges.
How about this? Is it okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo the wording is fine..? If the sysadmin and the first registered user are not the same person (e.g. as a SaaS platform without terminal access), the first registered user may not be able to access the gitea
cli.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the first registered user may not be able to access the
gitea
cli.
- Why they can't contact their site admin with CLI access?
- If the "first" user doesn't have CLI access, then how could they do "system reset and reinitialization" as the message says if they forget password?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. as a SaaS platform without terminal access
As a SaaS platform, isn't there a separate instance management console to help to reset the password? For example, gitea cloud?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can require the account creation in the web installation UI? So that this PR could be closed. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a complete and feasible plan? For example: some users use the "install" page, while some others don't, they just prepare the app.ini manually and set INSTALL_LOCK=true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. There are only two ways to install Gitea.
If we changed the rule that the first user is admin. Then for web installation, an admin account creation should be required. For command line installation, they have to create the first user from command line gitea admin
instead of registering the first account from web UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does helmchart also work this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does helmchart also work this way?
You can make the helmchart skip admin user creation from CLI
Yes by default yes it follows the cli setup and admin creation from gitea admin
, but it seems to be currently possible to change your values.yml
to set the admin username/password to an empty string instead of using the preset admin username & password.
In that case you could indeed end up in a without admin user state in the helm chart and have to attach via kubectl to the gitea container for the admin account, but you can just update the admin password / create the admin later as well by redeploying/updating the helmchart.
Ref: https://gitea.com/gitea/helm-gitea/src/commit/0d532363ebef69e2baedbb8b9370519b373b5394/templates/gitea/init.yaml#L82, https://gitea.com/gitea/helm-gitea/src/commit/0d532363ebef69e2baedbb8b9370519b373b5394/values.yaml#L353
This PR adds a feature to direct users to appropriate pages after system installation: