DEV Community

Akanksha Trehun
Akanksha Trehun

Posted on

Week 13: A Kid Only the Platform Can Read

The access token work that jumped the queue last week went up as
#7764. It took a
security minded review to catch a bug that would have made every single token
request fail, and a second reviewer's confusion, arriving weeks later, to remind
me that a review thread can point at code that no longer exists.

What the tool needs before it can call anything

Every LTI Advantage service call, posting a grade, reading a roster, is
authenticated the same way. The tool signs a JWT assertion with its own private
key, POSTs it to the platform's token endpoint under the OAuth 2.0 client
credentials grant, and gets back a bearer token scoped to whatever it asked for.
None of that is CircuitVerse specific. It is 1EdTech's Security Framework
section 5.4.1 built on top of RFC 7523, the JWT profile for client assertions,
which itself sits on RFC 6749's client credentials grant.

Tokens are cached per deployment and per scope set, refreshed thirty seconds
before they expire rather than exactly at expiry, so a request landing in that
window does not race a token that is technically still valid when read and
expired by the time it reaches the platform. A blank or non string
access_token in the response is refused instead of cached, and a response with
no expires_in at all gets no token stored, rather than a guessed default
lifetime. Both of those came from reading the spec's failure modes rather than
just its happy path.

The bug a reviewer caught before merge

The first version signed the assertion like this:

JWT.encode(
  { iss: deployment.client_id, sub: deployment.client_id, ... },
  signing_key, "RS256", { kid: JWT::JWK.new(signing_key).kid }
)
Enter fullscreen mode Exit fullscreen mode

A reviewer flagged the kid within a day. Lti::KeyManager, the class that
publishes CircuitVerse's public key at /lti/jwks, derives its key id from a
JSON::JWK thumbprint. JWT::JWK.new(signing_key).kid computes a kid using a
different digest entirely. The two never agree, which means the assertion would
have named a key that does not match anything the platform can find at our own
published JWKS endpoint. Every token request would fail, not occasionally, every
time, because the mismatch is structural rather than a bug that only shows up
under some inputs.

The fix reuses the exact computation KeyManager already does:

def kid_for(signing_key)
  JSON::JWK.new(signing_key.public_key).thumbprint
end
Enter fullscreen mode Exit fullscreen mode

Same input, the public key, same digest, JSON::JWK's thumbprint. I checked
this by rereading KeyManager#public_jwk line by line rather than trusting that
two thumbprint calls with similar names must produce the same output, and they
do:

def public_jwk
  jwk = JSON::JWK.new(private_key.public_key)
  jwk[:kid] ||= jwk.thumbprint
  jwk.merge(use: "sig", alg: "RS256")
end
Enter fullscreen mode Exit fullscreen mode

A comment that arrived three weeks late, pointed at code that no longer existed

The fix shipped within a day of the review comment. Three weeks later, a second
reviewer reread the thread from the top and asked why I was "still" using
JWT::JWK.new(signing_key), pointing straight at the original code block.

GitHub pins a review comment to the diff position it was written against. Once
that hunk changes, the thread gets marked "Outdated," but the stale code stays
visible inline right where the comment is anchored, and the label is easy to
miss if you are reading a long thread quickly rather than checking the Files
Changed tab. The question was reasonable. The premise, that the flagged line
was still in the branch, was not.

The reply pointed at the fix commit by hash, quoted the current kid_for
method, and reread KeyManager's source alongside it to show the two
computations are the same, rather than just asserting "it's fixed, trust me."
That habit, rederiving a claim instead of restating it, is the same one from
week 9's JWT validator work: a test that passes for the wrong reason is worse
than no test, and an explanation that only restates the conclusion is worse than
one that shows the work.

A question with a better answer than "I just know"

The same reviewer asked, on a different thread, how I knew which fields
belonged in the token request body at all, and whether it was written down
anywhere or just something I had picked up.

It is written down, in three places that stack on each other. grant_type and
scope come from RFC 6749 section 4.4, the client credentials grant itself.
client_assertion and client_assertion_type come from RFC 7523 section 2.2,
the general mechanism for authenticating with a signed JWT instead of a shared
secret. The specific URI value platforms expect for
client_assertion_type, and the requirement that the JWT's iss and sub both
equal the tool's own client id, come from the IMS Security Framework, which is
the layer that actually ties LTI Advantage to that OAuth mechanism.

I answered with the three documents and the section numbers rather than a
summary of what they say, because a summary is something to remember and a
citation is something to reopen. Anyone auditing this code later, including me
in six months, needs the second thing.

Where the project stands

The access token pull request is open with both review threads addressed, the
kid bug fixed and explained, and the spec provenance question answered with
citations rather than recollection. Roster sync and grade passback can now, once
this merges, actually authenticate the calls they have been assuming all along.

Next week: going back to merge an older, already approved pull request, and
finding out it had stopped passing while nobody was looking at it.

Top comments (0)