mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
Exclude SSH certificate principals from output when viewing user's SSH keys (#12079)
Fixes #11590 When viewing a user's SSH keys, SSH principals are now excluded from the output. This would previously either result in a panic in [OmitEmail](cfd4d53e32/models/asymkey/ssh_key.go (L67)), if the principal name didn't contain any spaces, or truncate the principal name, if it did contain spaces. The TestExportUserSSHKeys test was also updated and fails if the fix(commitcfcbc33af0) is reverted. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing - [x] `make test` - [x] `make test-sqlite#TestExportUserSSHKeys` I have also manually tested the change. The full integration tests(`make test-sqlite`) report some errors, but I get the same errors without this PR(tested on commit [6a5dda7116](6a5dda7116)). I have not tested with the other database backends. ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12079 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org> Co-authored-by: Alec Walsh <code@alecwalsh.name> Co-committed-by: Alec Walsh <code@alecwalsh.name>
This commit is contained in:
parent
766c9c64f5
commit
83459905d1
4 changed files with 52 additions and 2 deletions
|
|
@ -9,3 +9,36 @@
|
||||||
created_unix: 1559593109
|
created_unix: 1559593109
|
||||||
updated_unix: 1565224552
|
updated_unix: 1565224552
|
||||||
login_source_id: 0
|
login_source_id: 0
|
||||||
|
-
|
||||||
|
id: 2
|
||||||
|
owner_id: 5
|
||||||
|
name: user5
|
||||||
|
fingerprint: ""
|
||||||
|
content: "user5"
|
||||||
|
mode: 2
|
||||||
|
type: 3
|
||||||
|
created_unix: 1775805112
|
||||||
|
updated_unix: 1775805112
|
||||||
|
login_source_id: 0
|
||||||
|
-
|
||||||
|
id: 3
|
||||||
|
owner_id: 9
|
||||||
|
name: user9@localhost
|
||||||
|
fingerprint: "SHA256:K3RfDvtQ/aYVzh6RfXGFxlffLLTRgksf9UQwTlwSM8M"
|
||||||
|
content: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDN7KuFUnlztx/UM6PUTyiBAq5SeIqr+qSVFC6JzLQAh user9@localhost"
|
||||||
|
mode: 2
|
||||||
|
type: 1
|
||||||
|
created_unix: 1775805112
|
||||||
|
updated_unix: 1775805112
|
||||||
|
login_source_id: 0
|
||||||
|
-
|
||||||
|
id: 4
|
||||||
|
owner_id: 9
|
||||||
|
name: user9
|
||||||
|
fingerprint: ""
|
||||||
|
content: "user9"
|
||||||
|
mode: 2
|
||||||
|
type: 3
|
||||||
|
created_unix: 1775805112
|
||||||
|
updated_unix: 1775805112
|
||||||
|
login_source_id: 0
|
||||||
|
|
|
||||||
|
|
@ -705,6 +705,9 @@ func ShowSSHKeys(ctx *context.Context) {
|
||||||
|
|
||||||
var buf bytes.Buffer
|
var buf bytes.Buffer
|
||||||
for i := range keys {
|
for i := range keys {
|
||||||
|
if keys[i].Type == asymkey_model.KeyTypePrincipal {
|
||||||
|
continue // Don't display SSH principals, only public keys
|
||||||
|
}
|
||||||
buf.WriteString(keys[i].OmitEmail())
|
buf.WriteString(keys[i].OmitEmail())
|
||||||
buf.WriteString("\n")
|
buf.WriteString("\n")
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -114,12 +114,12 @@ func TestAPIPrivateServ(t *testing.T) {
|
||||||
assert.Empty(t, results)
|
assert.Empty(t, results)
|
||||||
|
|
||||||
// Cannot pull from a private repo we're not associated with
|
// Cannot pull from a private repo we're not associated with
|
||||||
results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
|
results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
|
||||||
require.Error(t, extra.Error)
|
require.Error(t, extra.Error)
|
||||||
assert.Empty(t, results)
|
assert.Empty(t, results)
|
||||||
|
|
||||||
// Cannot pull from a public repo we're not associated with
|
// Cannot pull from a public repo we're not associated with
|
||||||
results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
|
results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
|
||||||
require.Error(t, extra.Error)
|
require.Error(t, extra.Error)
|
||||||
assert.Empty(t, results)
|
assert.Empty(t, results)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1286,4 +1286,18 @@ func TestExportUserSSHKeys(t *testing.T) {
|
||||||
|
|
||||||
assert.Equal(t, "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM=\n", resp.Body.String())
|
assert.Equal(t, "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM=\n", resp.Body.String())
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("No exported keys and SSH principal", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
resp := MakeRequest(t, NewRequest(t, "GET", "/user5.keys"), http.StatusOK)
|
||||||
|
|
||||||
|
assert.Equal(t, "# Note: This user hasn't uploaded any SSH keys.\n", resp.Body.String())
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Exported key and SSH principal", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
resp := MakeRequest(t, NewRequest(t, "GET", "/user9.keys"), http.StatusOK)
|
||||||
|
|
||||||
|
assert.Equal(t, "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDN7KuFUnlztx/UM6PUTyiBAq5SeIqr+qSVFC6JzLQAh\n", resp.Body.String())
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue