Skip to content
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.

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

Support cache and conditional requests to fetch public keys from GET /meta/public_keys/copilot_api #9

Closed
gr2m opened this issue Aug 28, 2024 · 3 comments · Fixed by #81
Assignees
Labels
breaking change feature New feature or request pull request welcome Issue is actionable and anyone can claim it released

Comments

@gr2m
Copy link
Collaborator

gr2m commented Aug 28, 2024

Follow up to #8.

The GET /meta/public_keys/copilot_api route returns an Etag header, which it supports conditional requests.

$ gh api meta/public_keys/copilot_api -i

HTTP/2.0 200 OK
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset
Cache-Control: private, max-age=60, s-maxage=60
Content-Security-Policy: default-src 'none'
Content-Type: application/json; charset=utf-8
Date: Wed, 28 Aug 2024 19:13:39 GMT
Etag: W/"bdd20483b090130ff6ab72f309a50af6732f8f7cbb8909285f163311fede0fb3"
Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
Server: github.com
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
Vary: Accept, Authorization, Cookie, X-GitHub-OTP,Accept-Encoding, Accept, X-Requested-With
X-Accepted-Oauth-Scopes: 
X-Content-Type-Options: nosniff
X-Frame-Options: deny
X-Github-Api-Version-Selected: 2022-11-28
X-Github-Media-Type: github.v3; format=json
X-Github-Request-Id: C8CA:373C9D:CC38E0:CD9C04:66CF76E3
X-Oauth-Client-Id: 178c6fc778ccc68e1d6a
X-Oauth-Scopes: codespace, gist, read:org, repo, workflow
X-Ratelimit-Limit: 15000
X-Ratelimit-Remaining: 14954
X-Ratelimit-Reset: 1724872662
X-Ratelimit-Resource: core
X-Ratelimit-Used: 46
X-Xss-Protection: 0

{
  "public_keys": [
    {
      "key_identifier": "4fe6b016179b74078ade7581abf4e84fb398c6fae4fb973972235b84fcd70ca3",
      "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAELPuPiLVQbHY/clvpNnY+0BzYIXgo\nS0+XhEkTWUZEEznIVpS3rQseDTG6//gEWr4j9fY35+dGOxwOx3Z9mK3i7w==\n-----END PUBLIC KEY-----\n",
      "is_current": true
    },
    {
      "key_identifier": "df3454252d91570ae1bc597182d1183c7a8d42ff0ae96e0f2be4ba278d776546",
      "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEl5xbyr5bmETCJzqAvDnYl1ZKJrkf\n89Nyq5j06TTKrnHXXDw4FYNY1uF2S/w6EOaxbf9BxOidCLvjJ8ZgKzNpww==\n-----END PUBLIC KEY-----\n",
      "is_current": false
    }
  ]
}

In order to implement caching of the public keys, we need to return the etag hash as "cache ID" and the keys in the fetchVerificationKeys() method, and return the cache ID and the keys as part of verifyRequestByKeyId()

This will introduce a breaking change

const { keys, cacheId } = fetchVerificationKeys()

Once received, the result can be used as an optional cache argument, which will result in sending a conditional request

const result1 = fetchVerificationKeys()
const verificationKeysCache = { id: result1.cacheId, keys: result1.keys }
const result2 =fetchVerificationKeys({ cache: verificationKeysCache }) 

In this example, the 2nd request will get a 304 and return the keys that were passed as options.cache.keys

The change to verifyRequestByKeyId() would be similar

const { isValid, cacheId } = verifyRequestByKeyId(rawBody, signature, keyId, { cache })

This is an optimization and can be done later. Pull request welcome!

@gr2m gr2m added pull request welcome Issue is actionable and anyone can claim it breaking change feature New feature or request labels Aug 28, 2024
@gr2m gr2m changed the title support conditional requests to fetch public keys from GET /meta/public_keys/copilot_api Support cache and conditional requests to fetch public keys from GET /meta/public_keys/copilot_api Aug 29, 2024
@francisfuzz
Copy link
Collaborator

francisfuzz commented Sep 12, 2024

👋 It's like you may already have something in the works for this -- if you do, let me know and I can close out my draft PR: #81.

However, if you're open to me getting this over the finish line, I'm game! I have one more test to write (testing that we actually return what's in the cache); I'll read up on MockAgent and I can follow up on this thread or in the PR if I run into any blockers. 👍

@gr2m
Copy link
Collaborator Author

gr2m commented Sep 12, 2024

I didn't start working on this, it's all yours ✨

Copy link

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change feature New feature or request pull request welcome Issue is actionable and anyone can claim it released
Projects
None yet
2 participants