Skip to content

Conversation

@masv3971
Copy link
Collaborator

No description provided.

@leifj leifj requested a review from Copilot December 16, 2025 09:32
Copy link
Contributor

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

This PR implements a Token Status List (TSL) database and service per draft-ietf-oauth-status-list specification, replacing the previous Sparse Merkle Tree registry implementation with a MongoDB-based status list system.

Key Changes:

  • Replaces Merkle tree registry with Token Status List implementation supporting JWT/CWT formats
  • Adds MongoDB collections for status lists, metadata, and credential subjects
  • Implements mTLS support for gRPC communications with certificate fingerprint validation
  • Adds admin GUI for searching and updating credential status entries

Reviewed changes

Copilot reviewed 89 out of 227 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
proto/v1-registry.proto Replaces registry RPCs with TSL operations (add/update status, save credential subjects)
proto/v1-issuer.proto Adds TSL section/index fields to credential replies
pkg/tsl/*.go New package implementing TSL types, JWT/CWT token generation, compression/encoding
pkg/grpchelpers/*.go New helpers for gRPC mTLS server/client configuration with fingerprint validation
internal/registry/db/*.go MongoDB collections for TSL entries, metadata, and credential subjects
internal/registry/tslissuer/*.go Service for TSL token generation with caching and refresh
internal/registry/httpserver/*.go HTTP endpoints for status lists and admin GUI
internal/registry/grpcserver/*.go gRPC endpoints for TSL operations
internal/registry/apiv1/*.go API handlers for TSL operations, search, and status updates

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

}

// JWTConfig holds JWT-specific configuration for generating a Status List Token.
// Deprecated: Use StatusList.GenerateJWT with JWTSigningConfig instead.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The deprecation notice says 'Use StatusList.GenerateJWT with JWTSigningConfig instead' but doesn't explain the migration path. Consider adding an example or more detail about how to migrate existing code.

Suggested change
// Deprecated: Use StatusList.GenerateJWT with JWTSigningConfig instead.
// Deprecated: Use StatusList.GenerateJWT with JWTSigningConfig instead.
//
// Migration example:
// // Old usage:
// cfg := JWTConfig{
// SigningKey: myKey,
// SigningMethod: jwt.SigningMethodES256,
// // ... other fields ...
// }
// token, err := statusList.GenerateJWT(cfg)
//
// // New usage:
// signingCfg := JWTSigningConfig{
// SigningKey: cfg.SigningKey,
// SigningMethod: cfg.SigningMethod,
// }
// token, err := statusList.GenerateJWT(signingCfg)

Copilot uses AI. Check for mistakes.
for i := int64(0); i < sectionSize; i++ {
docs = append(docs, &TSLDoc{
Index: i,
Status: uint8(rand.IntN(3)),
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

Magic number 3 for random status generation. Consider using a constant like const MaxStatusValue = 3 or referencing tsl.StatusSuspended + 1 to make the upper bound explicit and maintainable.

Copilot uses AI. Check for mistakes.
}

filter := bson.M{"index": decoy.Index, "section": section}
updateDoc := bson.M{"$set": bson.M{"status": rand.Int64N(3)}}
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

Type mismatch: using rand.Int64N(3) which returns int64, but status field is uint8. This could cause data corruption. Use rand.IntN(3) and cast to uint8 like line 142.

Suggested change
updateDoc := bson.M{"$set": bson.M{"status": rand.Int64N(3)}}
updateDoc := bson.M{"$set": bson.M{"status": uint8(rand.IntN(3))}}

Copilot uses AI. Check for mistakes.
Comment on lines 296 to 297
s = strings.ReplaceAll(s, `"`, "&quot;")
s = strings.ReplaceAll(s, "'", "&#39;")
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The escapeHTML function implements custom HTML escaping instead of using html.EscapeString from the standard library. This is error-prone and may miss edge cases. Use the standard library function instead.

Copilot uses AI. Check for mistakes.
Comment on lines 89 to 93
if c.tslIssuer != nil {
if invalidator, ok := c.tslIssuer.(interface{ InvalidateSection(int64) }); ok {
invalidator.InvalidateSection(req.Section)
}
}
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The InvalidateSection method is called via type assertion but is not defined in the TSLIssuer interface. Either add this method to the interface or document why the optional behavior is needed.

Copilot uses AI. Check for mistakes.
Options: &options.IndexOptions{
Unique: &[]bool{true}[0],
},
Options: options.Index().SetUnique(true),
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'concent' to 'consent' in filename.

Copilot uses AI. Check for mistakes.
@leifj leifj self-requested a review December 16, 2025 12:32
@leifj
Copy link
Contributor

leifj commented Dec 16, 2025

Looks reasonable. I think most integrators will use the API to revoke but the GUI is a nice touch.

Copy link
Contributor

@leifj leifj left a comment

Choose a reason for hiding this comment

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

I think the TSL/CSL thing is pretty risky


## Decision

We want to return a variable, if applicable named reply
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested new language:

When returning a single variable use the name "reply" unless the context makes this inappropriate.

"strconv"

"vc/internal/gen/registry/apiv1_registry"
"vc/pkg/tsl"
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit uneasy about the term "tsl" here. I think there is a risk to confuse ourselves with "TSL" as in trust status list (ie etsi ts 119 612 trust status lists). If the intent is to model credential status lists then maybe CSL is the appropriate abreviation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just took the name from https://drafts.oauth.net/draft-ietf-oauth-status-list/draft-ietf-oauth-status-list.html#name-historical-resolution-2. How about to rename tsl -> token status list, than it will be pretty clear what's indented.

masv3971 and others added 7 commits December 16, 2025 17:11
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
masv3971 and others added 2 commits December 16, 2025 19:52
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@masv3971 masv3971 merged commit e3a9e04 into main Dec 17, 2025
2 checks passed
@masv3971 masv3971 deleted the masv_status-list-db branch December 17, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants