[test] Add tests for oidc.extractJWTExpiry#3939
Conversation
Directly tests all branches of the private extractJWTExpiry function
in package oidc. Previously this function was only exercised indirectly
through the HTTP-server-level Provider tests, which used a fixed JWT
payload that always produces a length%4==0 base64 string.
New unit tests cover:
- All three base64url padding cases (mod4=0, mod4=2→"==", mod4=3→"=")
- Realistic Unix timestamp expiry
- Extra JWT claims ignored (iss, sub, aud, iat)
- exp=0 returns error ("JWT has no exp claim")
- Missing exp field returns same error
- Wrong part count (1, 2, 4, 5 parts via table-driven test)
- Invalid base64 in payload segment
- Invalid JSON in payload (valid base64, bad JSON)
- Empty JSON object payload ({})
- Empty string token
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds direct unit tests for internal/oidc.extractJWTExpiry to cover all branches and error paths, improving confidence in JWT exp parsing (including base64url padding normalization).
Changes:
- Introduces a new white-box test file (
package oidc) to directly exerciseextractJWTExpiry. - Adds coverage for base64url padding cases (mod4=0/2/3) plus realistic timestamps and ignored extra claims.
- Adds coverage for malformed tokens and payload decode/parse/claim-missing error paths.
Show a summary per file
| File | Description |
|---|---|
| internal/oidc/jwt_expiry_test.go | New unit tests and helpers covering success/error branches of extractJWTExpiry. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/oidc/jwt_expiry_test.go:171
- This test payload is encoded with
base64.URLEncoding(padded), while the helper/comment above describe JWT payloads as base64url without padding. Consider usingencodePayloadRaw("{}")(orbase64.RawURLEncoding) to keep the test token format aligned with JWTs and consistent with the rest of the file.
func TestExtractJWTExpiry_EmptyPayload(t *testing.T) {
emptyPayload := base64.URLEncoding.EncodeToString([]byte(`{}`))
token := fmt.Sprintf("header.%s.sig", emptyPayload)
- Files reviewed: 1/1 changed files
- Comments generated: 2
| func encodePayloadRaw(json string) string { | ||
| return base64.RawURLEncoding.EncodeToString([]byte(json)) |
There was a problem hiding this comment.
The parameter name json is easy to confuse with the conventional json package identifier and can become awkward if this file later imports encoding/json. Renaming it to something like payloadJSON (or claimsJSON) would make call sites clearer and avoid potential shadowing confusion.
| func encodePayloadRaw(json string) string { | |
| return base64.RawURLEncoding.EncodeToString([]byte(json)) | |
| func encodePayloadRaw(payloadJSON string) string { | |
| return base64.RawURLEncoding.EncodeToString([]byte(payloadJSON)) |
| invalidJSON := base64.URLEncoding.EncodeToString([]byte(`{not valid json`)) | ||
| token := fmt.Sprintf("header.%s.sig", invalidJSON) | ||
|
|
There was a problem hiding this comment.
encodePayloadRaw is intended to generate standard JWT payload segments (base64url without padding), but this test uses base64.URLEncoding.EncodeToString, which adds = padding. Using encodePayloadRaw(...) (or base64.RawURLEncoding) here would keep the token format consistent with real JWTs and with the other tests in this file.
This issue also appears on line 169 of the same file.
Test Coverage Improvement:
oidc.extractJWTExpiryFunction Analyzed
internal/oidcextractJWTExpiry(unexported)ProviderHTTP mock tests with a single fixed JWT tokenWhy This Function?
extractJWTExpiryis a private helper that parses theexpclaim from a JWT without verifying its signature. It implements manual base64url padding logic (a common source of subtle bugs) and has four distinct error paths:expclaimThe existing
provider_test.go(package oidc_test) tests these paths only indirectly through an HTTP mock server and uses a single fixed JWT payload whose raw base64 length happens to be divisible by 4 — leaving thecase 2("==") andcase 3("=") padding branches of theswitchstatement uncovered by any test.Tests Added
{"exp":1}→ 12-char raw payload, no padding needed{"exp":12}→ 14-char raw payload,"=="appended{"exp":123}→ 15-char raw payload,"="appendediss,sub,aud,iat) ignoredexp=0→"JWT has no exp claim"errorexpfield → same error (zero-valued struct field){}payload → exp=0 errorImplementation Notes
The new test file uses
package oidc(white-box test) to access the unexported function, following Go convention of using_test.gofiles in the same package for testing unexported code. The existingprovider_test.gousespackage oidc_test(black-box), which is why it cannot testextractJWTExpirydirectly.Two test helpers are introduced:
makeRawJWT(rawPayload)— assemblesheader.payload.sigwith a dummy header/signatureencodePayloadRaw(json)— base64url-encodes JSON without padding (matching real JWT format)Test File
internal/oidc/jwt_expiry_test.go— 12 test functions, 183 linesGenerated by Test Coverage Improver
Next run will target the next most complex under-tested function
Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.