Keys API changes

[zeripath]

The results of the API for both the repo ssh keys and the user ssh keys lacks several important features.

This pull request adjusts the API to allow reporting of the key id for the keys reported by /repos/{owner}/{repo}/keys and the owner, keytype and read_only type for the keys reported by /user/keys.

(Of note /user/keys does not only just return user keys but also deploy keys.)

NB: This pull request requires: go-gitea/go-sdk#121

[zeripath]

I've made a few more changes/improvements to this pull-request.

• in /user/keys or /users/:username/keys User keys get information about their owner if the requester is the owner or an admin. All keys get told which type of key they are.
• in /repos/:owner/:repo/keys the fingerprint and the keyID are returned. Information about the real repository the key is attached to is given if the current user is the owner of the repository or an admin.
• /user/keys?fingerprint= will search for a key by fingerprint
• /repos/:owner/:repo/keys?fingerprint=&key_id= will search the provided repo keys for the key with the given key_id and/or fingerprint.

The main benefit of allowing searching by fingerprint is that it makes it possible to implement an SSH AuthorizedKeysCommand à la #1870. I would have implemented searching by key content but the simplest naïve implementation would allow listing of all keys due to the way Gitea stores the content of keys - it may not be needed in any case.

[zeripath]

Heya, any thoughts about this request? It should be possible to implement searching by key content if it was required.

[lafriks]

Need to wait for one more maintainer to approve sdk PR

[zeripath]

Ugh. It seems that I got go-gitea/go-sdk#121 out of sync with this request so another change to the SDK is needed. I've put another request in for that go-gitea/go-sdk#125 - if that's pulled I can adjust the first changeset and this might finally build!

[lafriks]

SDK PR is merged, please update

[codecov-io]

Codecov Report

Merging #4960 into master will increase coverage by 0.13%.
The diff coverage is 48.09%.

@@            Coverage Diff             @@
##           master    #4960      +/-   ##
==========================================
+ Coverage   37.48%   37.62%   +0.13%     
==========================================
  Files         310      310              
  Lines       45923    46039     +116     
==========================================
+ Hits        17213    17320     +107     
- Misses      26236    26240       +4     
- Partials     2474     2479       +5

Impacted Files	Coverage Δ
routers/api/v1/repo/key.go	16.66% <0%> (-3.58%)	⬇️
routers/api/v1/convert/convert.go	70.66% <100%> (+0.39%)	⬆️
models/ssh_key.go	39.14% <43.47%> (+0.17%)	⬆️
routers/api/v1/user/key.go	51.98% <71.42%> (+43.76%)	⬆️
models/repo_list.go	64.55% <0%> (+1.26%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 584844e...ce53323. Read the comment docs.

[zeripath]

@sapk I just had to remove my first commit as the dependencies had already been updated.

Is there anything else to be done for this request? I can remove the content searching if the query is thought to be too complex.

[lafriks]

To be honest I don't quite like that key searching by content. What is use case for that?

[zeripath]

Hi @lafriks ! The main use case for searching by key-content is creating a SSH AuthorizedKeysCommand [1][2] but you can do that using the fingerprint without the using the content. I'm happy to drop that bit or stick it behind an admin lock. (This is why the commits are separated rather than squashed in to one.)

I can create an AuthorizedKeysCommand command for the main gitea if you'd like.

[1] https://man.openbsd.org/sshd_config#AuthorizedKeysCommand
[2] https://man.openbsd.org/sshd_config#TOKENS

[techknowlogick]

I think it's fine to leave search by key in because keys are already available publicly (especially because they are "public keys"). I don't see this as a problem leaving in, and am supportive of the AuthorizedKeysCommand use case (I believe there is a ticket already for that).

[zeripath]

@techknowlogick

I think it's fine to leave search by key in because keys are already available publicly (especially because they are "public keys"). I don't see this as a problem leaving in, and am supportive of the AuthorizedKeysCommand use case (I believe there is a ticket already for that).

@lafriks was referring to the search by key content+/-type code in zeripath@e822116 I don't know if you saw that. It's a little awkward due to the way Gitea stores key content at present, so in order to make it not possible to just list all the keys and their comments I put some more logic in.

The main reason for me putting in this PR is so that I can take a key-id and map it back to a user without having to iterate across all the users. I want to use that functionality to provide svn+ssh support for git-as-svn. That doesn't need the searching by key fingerprint or content, but putting that support in would allow me to write a separate AuthorizedKeysCommand rather than for me to have to parse and strip the authorized_keys files. Once you can create a AuthorizedKeysCommand you can provide other ssh based services on your own Gitea server.

For plain old gitea you can create an AuthorizedKeysCommand without this PR and I've just put a pull request in for an AuthorizedKeysCommand #5236 that doesn't need this (although if you pull this request we can add the fingerprint stuff in.)

[lafriks]

Does searching by content really requires searching by part of it or searching by equal value wouldn't be enough?

[zeripath]

@lafriks Gitea is storing key type, content and comment as a single content string.

You don't get given the comment when you login from ssh so you can't just use equality. The function SearchPublicKeyByContent(content string) overloads the meaning of content in slightly different way by combining key type and content as a single string so it then uses a simplistic like, but if you just use that method you can retrieve all keys by passing in an empty content or 'ssh'. It also does no escaping of '%' or '_' in the like statement.

(It's safe to use that function in the AuthorizedKeysCommand PR #5236 because SSHD will still match the keys individually.)

[lafriks]

I would prefer to content searching be done in other PR right by spliting content out to different fields with migration to convert existing data

[zeripath]

@lafriks yeah that's fair enough. It's no longer in this PR.

[techknowlogick]

@zeripath seems my email reply to the comment above didn't make it through, but it was just thanking your for clarifying.