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.

Sign up for GitHub

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

Disable unnecessary SSH elements #18482

Closed
wants to merge 2 commits into from
Closed

Disable unnecessary SSH elements #18482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a82f4ee
Disable unnecessary SSH elements
pboguslawski Jan 31, 2022
b234641
GPG word restored in menu
pboguslawski Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions modules/templates/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ func NewFuncMap() []template.FuncMap {
"DisableImportLocal": func() bool {
return !setting.ImportLocalPaths
},
"DisableSSH": func() bool {
return setting.SSH.Disabled
},
"Dict": func(values ...interface{}) (map[string]interface{}, error) {
if len(values)%2 != 0 {
return nil, errors.New("invalid dict call")
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ password = Password
security = Security
avatar = Avatar
ssh_gpg_keys = SSH / GPG Keys
gpg_keys = GPG Keys
social = Social Accounts
applications = Applications
orgs = Manage Organizations
Expand Down
8 changes: 6 additions & 2 deletions services/cron/tasks_extended.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,12 @@ func initExtendedTasks() {
registerDeleteInactiveUsers()
registerDeleteRepositoryArchives()
registerGarbageCollectRepositories()
registerRewriteAllPublicKeys()
registerRewriteAllPrincipalKeys()
if !setting.SSH.Disabled && !setting.SSH.StartBuiltinServer && setting.SSH.CreateAuthorizedKeysFile {
registerRewriteAllPublicKeys()
}
if !setting.SSH.Disabled && !setting.SSH.StartBuiltinServer && setting.SSH.CreateAuthorizedPrincipalsFile {
registerRewriteAllPrincipalKeys()
}
registerRepositoryUpdateHook()
registerReinitMissingRepositories()
registerDeleteMissingRepositories()
Expand Down
4 changes: 3 additions & 1 deletion templates/admin/dashboard.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@
<td>{{.i18n.Tr "admin.dashboard.git_gc_repos"}}</td>
<td><button type="submit" class="ui green button" name="op" value="git_gc_repos">{{svg "octicon-play"}} {{.i18n.Tr "admin.dashboard.operation_run"}}</button></td>
</tr>
{{if and (not .SSH.Disabled) (not .SSH.StartBuiltinServer)}}
{{if and (not .SSH.Disabled) (not .SSH.StartBuiltinServer) (.SSH.CreateAuthorizedKeysFile) }}
<tr>
<td>{{.i18n.Tr "admin.dashboard.resync_all_sshkeys"}}<br/>
{{.i18n.Tr "admin.dashboard.resync_all_sshkeys.desc"}}</td>
<td><button type="submit" class="ui green button" name="op" value="resync_all_sshkeys">{{svg "octicon-play"}} {{.i18n.Tr "admin.dashboard.operation_run"}}</button></td>
</tr>
{{end}}
{{if and (not .SSH.Disabled) (not .SSH.StartBuiltinServer) (.SSH.CreateAuthorizedPrincipalsFile) }}
<tr>
<td>{{.i18n.Tr "admin.dashboard.resync_all_sshprincipals"}}<br/>
{{.i18n.Tr "admin.dashboard.resync_all_sshprincipals.desc"}}</td>
Expand Down
4 changes: 0 additions & 4 deletions templates/repo/settings/deploy_keys.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@
<h4 class="ui top attached header">
{{.i18n.Tr "repo.settings.deploy_keys"}}
<div class="ui right">
{{if not .DisableSSH}}
<div class="ui blue tiny show-panel button" data-panel="#add-deploy-key-panel">{{.i18n.Tr "repo.settings.add_deploy_key"}}</div>
{{else}}
<div class="ui blue tiny button disabled">{{.i18n.Tr "settings.ssh_disabled"}}</div>
{{end}}
</div>
</h4>
<div class="ui attached segment">
Expand Down
2 changes: 2 additions & 0 deletions templates/repo/settings/nav.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
{{if or .SignedUser.AllowGitHook .SignedUser.IsAdmin}}
<li {{if .PageIsSettingsGitHooks}}class="current"{{end}}><a href="{{.RepoLink}}/settings/hooks/git">{{.i18n.Tr "repo.settings.githooks"}}</a></li>
{{end}}
{{if not DisableSSH}}
<li {{if .PageIsSettingsKeys}}class="current"{{end}}><a href="{{.RepoLink}}/settings/keys">{{.i18n.Tr "repo.settings.deploy_keys"}}</a></li>
{{end}}
</ul>
</div>
</div>
2 changes: 2 additions & 0 deletions templates/repo/settings/navbar.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
{{.i18n.Tr "repo.settings.githooks"}}
</a>
{{end}}
{{if not DisableSSH}}
<a class="{{if .PageIsSettingsKeys}}active{{end}} item" href="{{.RepoLink}}/settings/keys">
{{.i18n.Tr "repo.settings.deploy_keys"}}
</a>
{{end}}
{{if .LFSStartServer}}
<a class="{{if .PageIsSettingsLFS}}active{{end}} item" href="{{.RepoLink}}/settings/lfs">
{{.i18n.Tr "repo.settings.lfs"}}
Expand Down
6 changes: 1 addition & 5 deletions templates/user/settings/keys_principal.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
{{if .AllowPrincipals}}
{{if and .AllowPrincipals (not DisableSSH)}}
<h4 class="ui top attached header">
{{.i18n.Tr "settings.manage_ssh_principals"}}
<div class="ui right">
{{if not .DisableSSH}}
<div class="ui blue tiny show-panel button" data-panel="#add-ssh-principal-panel">{{.i18n.Tr "settings.add_new_principal"}}</div>
{{else}}
<div class="ui blue tiny button disabled">{{.i18n.Tr "settings.ssh_disabled"}}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for the user's information these should be kept.

Copy link
Contributor Author

@pboguslawski pboguslawski Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There should be no unnecessary items in UI present to make it as clean as possible. If owner disables SSH in the application there is no point in leaving SSH items in UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

From how I see it, if I happens to be on some gitea instance and I didn't saw such menu(and didn't had the knowledge it could be disabled) I would open a bug report that this menu is not present, I think it's safe to inform the users that it isn't enabled on the instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

SSH is de-facto standard for Git operations nowadays and is expected to be supported universally everywhere. User should most definitely be informed that SSH is not available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would open a bug report that this menu is not present,

Reading system manual should be enough.

There should be no unnecessary items in UI present to make it as clean as possible. We prefer to give users clean UI without unnecessary information noise and will hide all SSH stuff. Feel free to close this PR if you don't like it.

and is expected to be supported universally everywhere

We don't expect using git over SSH so the above is false :).

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't expect using git over SSH so the above is false :).

Your singular needs do not represent the bigger picture. Please understand that Gitea is a project aimed at far bigger audience - for a feature to be added it should take into consideration that fact. I see no bigger issues with this PR other than the mentioned notification about disabled SSH being removed.

Copy link
Contributor Author

@pboguslawski pboguslawski Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Your singular needs[...]

We only push PRs that may be valuable for bigger audience. Clean UI is IHMO. As mentioned above - fell free to close this PR if you don't like it.

{{end}}
</div>
</h4>
<div class="ui attached segment">
Expand Down
12 changes: 5 additions & 7 deletions templates/user/settings/keys_ssh.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
{{if not DisableSSH}}
<h4 class="ui top attached header">
{{.i18n.Tr "settings.manage_ssh_keys"}}
<div class="ui right">
{{if not .DisableSSH }}
<div id="add-ssh-button" class="ui blue tiny show-panel button" data-panel="#add-ssh-key-panel">
{{.i18n.Tr "settings.add_key"}}
</div>
{{else}}
<div class="ui blue tiny button disabled">{{.i18n.Tr "settings.ssh_disabled"}}</div>
{{end}}
<div id="add-ssh-button" class="ui blue tiny show-panel button" data-panel="#add-ssh-key-panel">
{{.i18n.Tr "settings.add_key"}}
</div>
</div>
</h4>
<div class="ui attached segment">
Expand Down Expand Up @@ -109,3 +106,4 @@
</div>
{{template "base/delete_modal_actions" .}}
</div>
{{end}}
2 changes: 1 addition & 1 deletion templates/user/settings/navbar.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
{{.i18n.Tr "settings.applications"}}
</a>
<a class="{{if .PageIsSettingsKeys}}active{{end}} item" href="{{AppSubUrl}}/user/settings/keys">
{{.i18n.Tr "settings.ssh_gpg_keys"}}
{{if DisableSSH}}{{.i18n.Tr "settings.gpg_keys"}}{{else}}{{.i18n.Tr "settings.ssh_gpg_keys"}}{{end}}
</a>
<a class="{{if .PageIsSettingsOrganization}}active{{end}} item" href="{{AppSubUrl}}/user/settings/organization">
{{.i18n.Tr "settings.organization"}}
Expand Down