Hide add key button if SSH is disabled #2873
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2873 +/- ##
==========================================
- Coverage 27.26% 27.24% -0.02%
==========================================
Files 89 89
Lines 17640 17648 +8
==========================================
Hits 4809 4809
- Misses 12144 12152 +8
Partials 687 687
Continue to review full report at Codecov.
|
tboerger
added
the
lgtm/need 2
|
Can you please add also message that states that adding new keys is not allowed because SSH is disabled? |
| @@ -1,8 +1,10 @@ | |||
| <h4 class="ui top attached header"> | |||
| {{.i18n.Tr "settings.manage_ssh_keys"}} | |||
| {{if not .DisableSSH}} | |||
There was a problem hiding this comment.
I would also hide this entire tab TBH. Or at least should a warning here that SSH is disabled 🙂
|
Thanks for the feedback! I have updated the PR as follows:
|
routers/api/v1/repo/key.go
Outdated
| @@ -87,7 +87,7 @@ func HandleAddKeyError(ctx *context.APIContext, err error) { | |||
| // CreateDeployKey create deploy key for a repository | |||
| // see https://github.com/gogits/go-gogs-client/wiki/Repositories-Deploy-Keys#add-a-new-deploy-key | |||
| func CreateDeployKey(ctx *context.APIContext, form api.CreateKeyOption) { | |||
| content, err := models.CheckPublicKeyString(form.Key) | |||
| content, err := models.CheckPublicKeyString(form.Key, ctx.Locale.Language()) | |||
There was a problem hiding this comment.
It should be better to not pass context to CheckPublicKeyString and hanled the error inside HandleCheckKeyStringError.
routers/repo/setting.go
Outdated
| @@ -543,7 +544,7 @@ func DeployKeysPost(ctx *context.Context, form auth.AddKeyForm) { | |||
| return | |||
| } | |||
|
|
|||
| content, err := models.CheckPublicKeyString(form.Content) | |||
| content, err := models.CheckPublicKeyString(form.Content, ctx.Locale.Language()) | |||
There was a problem hiding this comment.
Echoing sapk's comment from above, it would be better to have CheckPublicKeyString return a special type of error (say ErrSSHDisabled) when SSH is disabled, which we can check after calling the function (like we currently do for ErrKeyUnableVerify on line 549).
|
I have updated the PR with the suggestions above:
|
|
LGTM as test failure does not seem related to this PR |
tboerger
added
lgtm/need 1
|
LGTM |
tboerger
added
lgtm/done
I have configured our Gitea instance with disabled SSH. However, the settings UI still shows an "Add Key" button but trying to add a key produces an error stating that SSH is disabled. A few users have already contacted me whether something is broken.
This PR simply hides the button if SSH is disabled. Alternatively, there could be some descriptive text stating that it is not possible to add SSH keys because SSH is disabled or the whole SSH key section could be hidden.