Skip to content

Disable unnecessary SSH elements #18482

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

Closed
wants to merge 2 commits into from
Closed

Disable unnecessary SSH elements #18482

Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ appearance = Appearance
password = Password
security = Security
avatar = Avatar
ssh_gpg_keys = SSH / GPG Keys
keys = Keys
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO: I think Keys is too vague to be a navbar item.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect that we might need to have a GPG Keys menu option if SSH is absolutely disabled.

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.

Menu item should not contain technology name (SSH, GPG, etc.) because disabled items should be removed from page (clean UI should not contain stuff user don't use). It should also be short and clear. "Keys" are fine IHMO and not more vague than other existing items there. Propose better name if you can't accept simple "Keys".

Copy link
Contributor

Choose a reason for hiding this comment

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

If you check GitHub's UI and Gitlab's UI you'll find that the technology name is included in the tab name.

We have to include the technology type to disambiguate the type of keys.

There are authentication keys - eg. Yubikeys which are not configured on this tab. If you call it Keys then this will become confusing.

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.

If you check GitHub's UI and Gitlab's UI you'll find that the technology name is included in the tab name.

Gitea may have cleaner UI :)

Propose better name if you can't accept simple "Keys" (avoid leaving unnecessary items like SSH words when SSH is disabled in config).

There are authentication keys - eg. Yubikeys which are not configured on this tab.

SSH keys are also authentication keys. Consider merging all this auth/sign/token stuff in one page like "Security" in the future. Or maybe move all this stuff to existing Account page if space allows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry didn't noticed GPG Keys proposal - will extend mod with this. Still consider merging all this auth/sign/token stuff in one page in user settings in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

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"}}
{{.i18n.Tr "settings.keys"}}
</a>
<a class="{{if .PageIsSettingsOrganization}}active{{end}} item" href="{{AppSubUrl}}/user/settings/organization">
{{.i18n.Tr "settings.organization"}}
Expand Down