The client currently sets all options, even if values are not provided. For example, /pks/lookup?fingerprint=on&op=index&options=&search=blah&x-pagesize=256&x-pagetoken=. For query parameters that are not set, exclude them from the URL.

Update CI to verify this module is (and continues to be) compatible with Go v1.13. The cache_go_mod step should be unnecessary, with the default module proxy serving a similar purpose.

API calls do not work if key server base URL contains path element(s) (ie. https://server.com/keys)

hkp:// and hkps:// are commonly used protocol schemes for the HTTP Keyserver Protocol. They are functionally equivalent to http://<host>:11371, and https://<host>:443 respectively.

The SCS Key Client should support these protocol schemes out of the box. Currently, only http and https prefixes are supported.

https://godoc.org/ will eventually be replaced by https://pkg.go.dev/. We should update our documentation badge to the new location when appropriate.

Only minor blocker I can see at the moment is golang/go#36982. Suggest progressing once that is sorted.

(*Client).GetKey() was modified to take a []byte rather than a [20]byte, which seems like a reasonable change, but the documentation makes the new usage unclear. The parameter is still named fingerprint, but the point of the change is that a key ID or short key ID can also be used. We should improve that to make it more clear.

Calling this function results in an HTTP request to /pks/lookup, with the value prefixed by "0x", defined in section 3.1.1.1 of the HKP specification. This section also defines the valid values:

Key ID strings may be 8 digits (32-bit key ID), 16 digits (64-bit key ID), 32 digits (version 3 fingerprint), or 40 digits (version 4 fingerprint).

The current implementation allows any length of slice to be passed in (including zero). I believe it would be beneficial to add some client-side validation of the length, with a descriptive error message.

Supplying a hostname of localhost is supposed to disable the TLS check that was added in version 0.4.0. But in certain cases, particularly where the normalized hostname contains a port number, an error of TLS required when auth token provided is returned.

Currently, it is possible for a user of this package to create a client with an auth token, as well as a plaintext scheme in the BaseURL (http or hkp). An auth token is a sensitive user credential, and thus sending it in plaintext is not a good idea. Not sure how best to handle this. I'm tempted to say we should return an error from NewClient when these two conditions are met.

This might be annoying for developers, so perhaps we could relax the restriction when the BaseURL refers to localhost?

Leverage func ReadError from sylabs/json-resp#11 to provide error messages with higher fidelity from func PKSAdd, func PKSLookup and func GetKey.

As an example, the Sylabs Key Service currently verifies email identity. When this verification fails, the error currently returned by PKSAdd is 400 Bad Request (400 Bad Request). This is not incorrect, however with this change the error would be much more descriptive: entity ABCDEF does not contain identity with email '[email protected]' (400 Bad Request).

It doesn't look like #36 successfully addressed this, as the badge is now showing up like so:

Perhaps https://badgen.net/ will fix this issue in time, or GitHub will release native badge support.

As requested by @cclerget in #45 (comment), there is a use case to obtain the HTTP response body associated with a successful call to PKSAdd, to improve the user experience when using key servers like Hagrid (https://keys.openpgp.org/about/api).

When a Client is created with a path component (ex. https://example.com/path), the path component ends up being ignored when building up request URLs.

The problem originates in (*Client).newRequest, which calls (*url.URL).ResolveReference to construct a request URI to BaseURL/path. When the path parameter begins with a /, it is treated as an absolute path and thus any path component of the BaseURL is ignored.

Update CI to verify this module is (and continues to be) compatible with Go v1.13. The cache_go_mod step should be unnecessary, with the default module proxy serving a similar purpose.