Skip to content

docs: clarify SecretEncrypter internals; add Claims struct and AdminC…#169

Merged
veverkap merged 1 commit into
mainfrom
docs/fix-secretencrypter-and-claims-5a415fedeffd39f5
May 2, 2026
Merged

docs: clarify SecretEncrypter internals; add Claims struct and AdminC…#169
veverkap merged 1 commit into
mainfrom
docs/fix-secretencrypter-and-claims-5a415fedeffd39f5

Conversation

@veverkap
Copy link
Copy Markdown
Contributor

@veverkap veverkap commented Apr 29, 2026

…hecker interface

  • docs/auth/crypto.md, README.md: Clarify that SecretEncrypter stores only the cipher.AEAD (GCM) as its single field. The previous text said 'the AES block cipher and the cipher.AEAD are both created once', which implied two separate stored objects. The block cipher is embedded within GCM; only the AEAD is retained. Matches auth/crypto.go: SecretEncrypter{gcm: gcm}.

  • docs/auth/jwt.md: Add the Claims struct definition so developers can see all available fields (UserID/sub + jwt.RegisteredClaims incl. ID/jti, ExpiresAt, IssuedAt, Issuer, Audience) without reading the source.

  • docs/auth/middleware.md: Add explicit AdminChecker interface definition and note that auth.UserStore satisfies it directly, with a pointer to auth.NewAdminCheckerFromRoleChecker for RBAC-based setups.

Greptile Summary

This is a documentation-only PR that corrects and expands three areas: (1) clarifies that SecretEncrypter stores only the cipher.AEAD as its single field (not both the block cipher and the AEAD), (2) adds the Claims struct definition to jwt.md, and (3) exposes the AdminChecker interface definition in middleware.md. All added snippets were verified against the source and are accurate.

Confidence Score: 4/5

Safe to merge; documentation-only changes with no runtime impact and one minor in-source doc inconsistency.

All changes are prose/doc clarifications with no logic modifications. The only finding is a P2 inconsistency between the updated external docs and the still-old Go doc comment in crypto.go.

auth/crypto.go — in-source doc comment was not updated to match the external docs change.

Important Files Changed

Filename Overview
README.md Accurate prose update: clarifies that SecretEncrypter stores only the cipher.AEAD as its single field, matching crypto.go.
docs/auth/crypto.md Same single-sentence clarification as README.md; wording matches the struct definition in crypto.go.
docs/auth/jwt.md Adds Claims struct snippet that exactly mirrors auth/jwt.go lines 17–21; fields, JSON tags, and embedded type are all correct.
docs/auth/middleware.md Adds AdminChecker interface definition matching middleware.go lines 175–178; UserStore satisfaction claim and NewAdminCheckerFromRoleChecker reference are both accurate.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[AdminMiddleware] --> B{AdminChecker}
    B -->|UserStore directly| C[UserStore.IsAdmin]
    B -->|RBAC adapter| D[NewAdminCheckerFromRoleChecker]
    D --> E[RoleChecker.HasRole with RoleAdmin]
    C --> F[cachingAdminChecker\n4096-entry FIFO, 5s TTL]
    D --> F
Loading

Comments Outside Diff (1)

  1. auth/crypto.go, line 57-58 (link)

    P2 In-source Go doc comment still uses the old phrasing

    The external docs (README.md, docs/auth/crypto.md) were updated to say only the cipher.AEAD is stored, but the Go doc comment on SecretEncrypter still reads "The AES block cipher and the cipher.AEAD returned by cipher.NewGCM are created once at construction time" — the same wording the PR is explicitly correcting. Keeping the old phrasing in-source creates an inconsistency for anyone reading the godoc output.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: auth/crypto.go
    Line: 57-58
    
    Comment:
    **In-source Go doc comment still uses the old phrasing**
    
    The external docs (`README.md`, `docs/auth/crypto.md`) were updated to say only the `cipher.AEAD` is stored, but the Go doc comment on `SecretEncrypter` still reads "The AES block cipher and the `cipher.AEAD` returned by `cipher.NewGCM` are created once at construction time" — the same wording the PR is explicitly correcting. Keeping the old phrasing in-source creates an inconsistency for anyone reading the godoc output.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Codex

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: auth/crypto.go
Line: 57-58

Comment:
**In-source Go doc comment still uses the old phrasing**

The external docs (`README.md`, `docs/auth/crypto.md`) were updated to say only the `cipher.AEAD` is stored, but the Go doc comment on `SecretEncrypter` still reads "The AES block cipher and the `cipher.AEAD` returned by `cipher.NewGCM` are created once at construction time" — the same wording the PR is explicitly correcting. Keeping the old phrasing in-source creates an inconsistency for anyone reading the godoc output.

```suggestion
// SecretEncrypter encrypts and decrypts sensitive values using AES-256-GCM.
// The cipher.AEAD (AES-256-GCM, which wraps the AES block cipher internally)
// is created once at construction time and stored as the only field; it is
// reused across Encrypt/Decrypt calls.
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "docs: clarify SecretEncrypter internals;..." | Re-trigger Greptile

…hecker interface

- docs/auth/crypto.md, README.md: Clarify that SecretEncrypter stores only
  the cipher.AEAD (GCM) as its single field. The previous text said 'the AES
  block cipher and the cipher.AEAD are both created once', which implied two
  separate stored objects. The block cipher is embedded within GCM; only the
  AEAD is retained. Matches auth/crypto.go: SecretEncrypter{gcm: gcm}.

- docs/auth/jwt.md: Add the Claims struct definition so developers can see
  all available fields (UserID/sub + jwt.RegisteredClaims incl. ID/jti,
  ExpiresAt, IssuedAt, Issuer, Audience) without reading the source.

- docs/auth/middleware.md: Add explicit AdminChecker interface definition
  and note that auth.UserStore satisfies it directly, with a pointer to
  auth.NewAdminCheckerFromRoleChecker for RBAC-based setups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
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

Updates authentication documentation to more accurately reflect implementation details in the auth package and to make key types/interfaces visible without requiring readers to jump into source.

Changes:

  • Clarifies SecretEncrypter internals to note it stores only a cipher.AEAD (GCM), not separate cached cipher objects.
  • Documents the Claims struct layout (including embedded jwt.RegisteredClaims) in the JWT docs.
  • Documents the AdminChecker interface explicitly and points to auth.NewAdminCheckerFromRoleChecker for RBAC setups.
Show a summary per file
File Description
docs/auth/middleware.md Adds AdminChecker interface definition and usage guidance (incl. RBAC adapter).
docs/auth/jwt.md Adds Claims struct definition and clarifies sub/jti mapping in examples.
docs/auth/crypto.md Clarifies SecretEncrypter retains only the cipher.AEAD instance.
README.md Mirrors the SecretEncrypter clarification in top-level docs.

Copilot's findings

Tip

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

@veverkap veverkap merged commit a6e2007 into main May 2, 2026
11 checks passed
@veverkap veverkap deleted the docs/fix-secretencrypter-and-claims-5a415fedeffd39f5 branch May 2, 2026 21:13
@github-actions github-actions Bot mentioned this pull request May 2, 2026
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.

2 participants