Skip to content

Add decode-token command#285

Open
duanemay wants to merge 1 commit intomasterfrom
decode
Open

Add decode-token command#285
duanemay wants to merge 1 commit intomasterfrom
decode

Conversation

@duanemay
Copy link
Copy Markdown
Member

decode JWTs, display claims, verify signatures, and show human-readable timestamps

decode JWTs, display claims, verify signatures, and show human-readable timestamps
Copilot AI review requested due to automatic review settings April 24, 2026 19:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new uaa decode-token command to decode JWTs, optionally verify their signature using a provided PEM key, and optionally print human-readable timestamps for common time claims.

Changes:

  • Introduces decode-token Cobra command implementation with --key (signature verification) and --decode-times (timestamp decoding).
  • Adds Ginkgo/Gomega integration tests covering context token fallback, positional token arg, signature verification, and timestamp output.
  • Adds documentation for the new command and links it from the command reference.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
docs/commands/decode-token.md New user-facing documentation for decode-token, including usage, flags, examples, and troubleshooting.
docs/commands.md Adds decode-token to the command reference (also adds a targets entry).
cmd/decode_token_test.go New integration tests for decoding, signature verification, and timestamp decoding.
cmd/decode_token.go New decode-token command implementation, JWT decoding, signature verification helpers, and timestamp formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +56 to +57
iat (Issued At): 2017-09-10 18:03:43 UTC (7 years ago)
exp (Expires At): 2017-10-11 06:03:43 UTC (7 years ago)
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example output hard-codes relative times like “(7 years ago)”, which will become inaccurate as time passes. Consider removing the relative portion from the docs example or replacing it with a placeholder so the documentation doesn’t go stale.

Suggested change
iat (Issued At): 2017-09-10 18:03:43 UTC (7 years ago)
exp (Expires At): 2017-10-11 06:03:43 UTC (7 years ago)
iat (Issued At): 2017-09-10 18:03:43 UTC (relative time varies)
exp (Expires At): 2017-10-11 06:03:43 UTC (relative time varies)

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +93
# Fetch the UAA's public signing key
uaa get-token-key

# Verify and decode
uaa decode-token --key "$(cat signing-key.pem)"
uaa decode-token eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9... --key "$(cat signing-key.pem)" --decode-times
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the signature verification example, uaa get-token-key is shown but there’s no step that writes the PEM value into signing-key.pem before cat signing-key.pem. Please add an explicit command to extract/save the key (e.g., redirect or jq extraction), or adjust the example to pass the key directly.

Copilot uses AI. Check for mistakes.
Comment thread cmd/decode_token_test.go
It("exits 1 with a signature error when key does not match", func() {
session := runCommand("decode-token", testRS256JWT, "--key", testRS256WrongPublicKey)

Eventually(session).Should(Exit(1))
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case is named as if it validates the “invalid token signature” path, but it only asserts exit code and doesn’t assert the error text. Adding an assertion for the expected stderr message (e.g., containing "invalid token signature") will ensure the command is actually failing for signature mismatch (and not due to PEM/key parsing errors).

Suggested change
Eventually(session).Should(Exit(1))
Eventually(session).Should(Exit(1))
Expect(string(session.Err.Contents())).To(ContainSubstring("invalid token signature"))

Copilot uses AI. Check for mistakes.
Comment thread cmd/decode_token.go
if err != nil {
return nil, nil, nil, fmt.Errorf("invalid JWT signature encoding: %v", err)
}
_ = sigBytes
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ = sigBytes is redundant here because sigBytes is already used in the return values. Removing this avoids dead code and keeps the helper minimal.

Suggested change
_ = sigBytes

Copilot uses AI. Check for mistakes.
Comment thread cmd/decode_token.go
Comment on lines +85 to +112
alg, _ := header["alg"].(string)
signingInput := parts[0] + "." + parts[1]
sig, err := base64.RawURLEncoding.DecodeString(parts[2])
if err != nil {
return fmt.Errorf("invalid JWT signature: %v", err)
}

block, _ := pem.Decode([]byte(keyPEM))
if block == nil {
return errors.New("failed to decode PEM block from key")
}

switch alg {
case "RS256", "RS384", "RS512":
pub, err := parseRSAPublicKey(block)
if err != nil {
return err
}
return verifyRSA(alg, signingInput, sig, pub)
case "ES256", "ES384", "ES512":
pub, err := parseECPublicKey(block)
if err != nil {
return err
}
return verifyECDSA(alg, signingInput, sig, pub)
default:
return fmt.Errorf("unsupported algorithm: %s", alg)
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the JWT header is missing alg or it’s not a string, alg becomes "" and the user gets unsupported algorithm: (empty) which is hard to diagnose. Validate alg presence/type and return a clearer error like “JWT header missing/invalid alg”.

Copilot uses AI. Check for mistakes.
Comment thread cmd/decode_token.go
Comment on lines +116 to +139
switch block.Type {
case "PUBLIC KEY":
key, err := x509.ParsePKIXPublicKey(block.Bytes)
if err != nil {
return nil, fmt.Errorf("failed to parse public key: %v", err)
}
rsaKey, ok := key.(*rsa.PublicKey)
if !ok {
return nil, errors.New("key is not an RSA public key")
}
return rsaKey, nil
case "CERTIFICATE":
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return nil, fmt.Errorf("failed to parse certificate: %v", err)
}
rsaKey, ok := cert.PublicKey.(*rsa.PublicKey)
if !ok {
return nil, errors.New("certificate does not contain an RSA public key")
}
return rsaKey, nil
default:
return nil, fmt.Errorf("unsupported PEM block type for RSA: %s", block.Type)
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--key is documented as accepting a PEM-encoded public key, but parseRSAPublicKey only supports PEM block types PUBLIC KEY and CERTIFICATE. Common RSA public keys are also provided as RSA PUBLIC KEY (PKCS#1). Consider supporting that block type (via x509.ParsePKCS1PublicKey) to avoid rejecting valid user-provided keys.

Copilot uses AI. Check for mistakes.
Comment thread docs/commands.md
| Command | Description |
|---------|-------------|
| [`target`](commands/target.md) | Set the URL of the UAA you'd like to target |
| [`targets`](commands/targets.md) | List all registered targets |
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command reference adds a targets entry linking to commands/targets.md, but this repo doesn’t include that doc page (and there’s no corresponding targets command). This is a broken link in the command reference; either add the missing command/docs or remove this row.

Suggested change
| [`targets`](commands/targets.md) | List all registered targets |

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants