Skip to content

Commit 441f45a

Browse files
paskalumputun
authored andcommitted
docs: package-wide comment sweep (non-security)
Pre-existing comment drift in files unrelated to the recent security fix — unrelated docstrings that misled, said the wrong thing, or were copy-paste leftovers. Mirrored across v1 and v2. The security-fix docstring fixes (avatar/avatar.go security comments and auth.go withSecurityHeaders) live in PR #291 and are intentionally NOT included here so the two PRs stay independent. Findings: * avatar/gridfs.go — ID doc claimed MD5 sourced from gridfs; metadata.hash is the sha1 written at Put time. * avatar/bolt.go, avatar/gridfs.go — Put docs and BoltDB type doc claimed these layers "resize" the image; they only copy bytes. Resize happens in Proxy.resize upstream. * avatar/store.go — Migrate doc didn't mention that per-avatar Get/Put/Close errors are logged and skipped, and that the returned count is "ids attempted", not "ids stored". * provider/oauth1.go — initOauth1Handler doc and DEBUG log both said "oauth2". * provider/service.go — Service.Handler doc said "returns auth routes"; it dispatches login/callback/logout. * provider/telegram.go — processUpdates claimed to return an offset (no return value); checkToken doc described an "address or empty string" return shape but the signature is (*token.User, error). * provider/verify.go — Sender interface doc locked the contract to "send emails", contradicting the broader "email, IM, or anything else" on VerifyHandler. AuthHandler comment was copy-pasted from direct. * provider/dev_provider.go — NewDev doc said "for admin user"; just makes the dev oauth2 provider. * middleware/user_updater.go — UserUpdFunc adapter doc said result "is a Handler"; implements UserUpdater. * logger/interface.go — Func adapter doc said "Logf calls f(id)"; actually calls f(format, args...). * token/jwt.go — SendJWTHeader option doc said "instead of cookie"; Set sends header AND cookie. Set doc referenced a "permanent flag" that doesn't exist in the signature (controlled by claims.SessionOnly/Handshake). Get doc oversimplified the XSRF gate. Update/Validate adapter docs said "calls f(id)"; actually f(claims) and f(token, claims). * token/user.go — HashID inline "or empty" was wrong; an empty val never matches reValidSha. * auth.go — BasicAuthChecker doc grammar broken and understated behavior (AdminPasswd is bypassed entirely, not "ignored"); "peak dev provider" typo; AvatarProxy doc was a sentence fragment. All changes mirrored across v1 and v2. Build green, lint clean, go fix clean.
1 parent ba35ab3 commit 441f45a

26 files changed

Lines changed: 84 additions & 58 deletions

auth.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type Opts struct {
5858
XSRFHeaderKey string // default "X-XSRF-TOKEN"
5959
XSRFIgnoreMethods []string // disable XSRF protection for the specified request methods (ex. []string{"GET", "POST")}, default empty
6060
JWTQuery string // default "token"
61-
SendJWTHeader bool // if enabled send JWT as a header instead of cookie
61+
SendJWTHeader bool // if enabled, also send JWT as a response header (in addition to the cookie)
6262
SameSiteCookie http.SameSite // limit cross-origin requests with SameSite cookie attribute
6363

6464
Issuer string // optional value for iss claim, usually the application name, default "go-pkgz/auth"
@@ -81,7 +81,7 @@ type Opts struct {
8181
UseGravatar bool // for email based auth (verified provider) use gravatar service
8282

8383
AdminPasswd string // if presented, allows basic auth with user admin and given password
84-
BasicAuthChecker middleware.BasicAuthFunc // user custom checker for basic auth, if one defined then "AdminPasswd" will ignored
84+
BasicAuthChecker middleware.BasicAuthFunc // custom checker for basic auth; when set, AdminPasswd is bypassed entirely
8585
AudienceReader token.Audience // list of allowed aud values, default (empty) allows any
8686
AudSecrets bool // allow multiple secrets (secret per aud)
8787
Logger logger.L // logger interface, default is no logging at all
@@ -503,7 +503,7 @@ func (s *Service) AddCustomHandler(p provider.Provider) {
503503

504504
// DevAuth makes dev oauth2 server, for testing and development only!
505505
func (s *Service) DevAuth() (*provider.DevAuthServer, error) {
506-
p, err := s.Provider("dev") // peak dev provider
506+
p, err := s.Provider("dev") // peek dev provider
507507
if err != nil {
508508
return nil, fmt.Errorf("dev provider not registered: %w", err)
509509
}
@@ -531,7 +531,7 @@ func (s *Service) TokenService() *token.Service {
531531
return s.jwtService
532532
}
533533

534-
// AvatarProxy returns stored in service
534+
// AvatarProxy returns the avatar.Proxy configured on the service, or nil if no AvatarStore was set.
535535
func (s *Service) AvatarProxy() *avatar.Proxy {
536536
return s.avatarProxy
537537
}

avatar/bolt.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
// BoltDB implements avatar store with bolt
1313
// using separate db (file) with "avatars" bucket to keep image bin and "metas" bucket
14-
// to keep sha1 of picture. avatarID (base file name) used as a key for both.
14+
// to keep a sha1 fingerprint of the stored bytes. avatarID (base file name) used as a key for both.
1515
type BoltDB struct {
1616
fileName string // full path to boltdb
1717
db *bolt.DB
@@ -41,7 +41,9 @@ func NewBoltDB(fileName string, options bolt.Options) (*BoltDB, error) {
4141
return &BoltDB{db: db, fileName: fileName}, nil
4242
}
4343

44-
// Put avatar to bolt, key by avatarID. Trying to resize image and lso calculates sha1 of the file for ID func
44+
// Put stores avatar bytes read from reader in the "avatars" bucket and a sha1 of the
45+
// same bytes in the "metas" bucket, both keyed by the encoded userID with .image suffix.
46+
// Resizing happens upstream in Proxy.resize; this layer only writes what it is given.
4547
func (b *BoltDB) Put(userID string, reader io.Reader) (avatar string, err error) {
4648
id := encodeID(userID)
4749

avatar/gridfs.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ type GridFS struct {
2727
timeout time.Duration
2828
}
2929

30-
// Put avatar to gridfs object, try to resize
30+
// Put stores avatar bytes read from reader in GridFS, keyed by the encoded userID
31+
// with .image suffix, and records the sha1 of those bytes in the file's metadata.
32+
// Resizing happens upstream in Proxy.resize; this layer only writes what it is given.
3133
func (gf *GridFS) Put(userID string, reader io.Reader) (avatar string, err error) {
3234
id := encodeID(userID)
3335
bucket, err := gridfs.NewBucket(gf.db, &options.BucketOptions{Name: &gf.bucketName})
@@ -59,7 +61,8 @@ func (gf *GridFS) Get(avatar string) (reader io.ReadCloser, size int, err error)
5961
return io.NopCloser(buf), int(sz), nil
6062
}
6163

62-
// ID returns a fingerprint of the avatar content. Uses MD5 because gridfs provides it directly
64+
// ID returns the sha1 fingerprint of the avatar content stored in the GridFS file's
65+
// metadata at Put time, or an encoded fallback id when the file lookup/decode fails.
6366
func (gf *GridFS) ID(avatar string) (id string) {
6467

6568
finfo := struct {

avatar/store.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ func NewStore(uri string) (Store, error) {
7070
return nil, fmt.Errorf("can't parse store url %s", uri)
7171
}
7272

73-
// Migrate avatars between stores
73+
// Migrate copies avatars from src to dst, returning the number of ids enumerated
74+
// from src and the first List error if any. Per-avatar Get/Put/Close failures are
75+
// logged with [WARN] and skipped — they do not abort the migration or surface in
76+
// the returned error, so the returned count is "ids attempted", not "ids stored".
7477
func Migrate(dst, src Store) (int, error) {
7578
ids, err := src.List()
7679
if err != nil {

logger/interface.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type L interface {
1212
// Func type is an adapter to allow the use of ordinary functions as Logger.
1313
type Func func(format string, args ...any)
1414

15-
// Logf calls f(id)
15+
// Logf calls f(format, args...).
1616
func (f Func) Logf(format string, args ...any) { f(format, args...) }
1717

1818
// NoOp logger

middleware/user_updater.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type UserUpdater interface {
1212
}
1313

1414
// UserUpdFunc type is an adapter to allow the use of ordinary functions as UserUpdater. If f is a function
15-
// with the appropriate signature, UserUpdFunc(f) is a Handler that calls f.
15+
// with the appropriate signature, UserUpdFunc(f) is a UserUpdater that calls f.
1616
type UserUpdFunc func(user token.User) token.User
1717

1818
// Update calls f(user)

provider/dev_provider.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (d *DevAuthServer) Shutdown() {
167167
d.lock.Unlock()
168168
}
169169

170-
// NewDev makes dev oauth2 provider for admin user
170+
// NewDev makes a dev oauth2 provider intended for local development only.
171171
func NewDev(p Params) Oauth2Handler {
172172
if p.Port == 0 {
173173
p.Port = defDevAuthPort

provider/oauth1.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func (h Oauth1Handler) makeRedirURL(path string) string {
190190
return strings.TrimSuffix(h.URL, "/") + strings.TrimSuffix(newPath, "/") + urlCallbackSuffix
191191
}
192192

193-
// initOauth2Handler makes oauth1 handler for given provider
193+
// initOauth1Handler makes oauth1 handler for given provider
194194
func initOauth1Handler(p Params, service Oauth1Handler) Oauth1Handler {
195195
if p.L == nil {
196196
p.L = logger.NoOp
@@ -200,7 +200,7 @@ func initOauth1Handler(p Params, service Oauth1Handler) Oauth1Handler {
200200
service.conf.ConsumerKey = p.Cid
201201
service.conf.ConsumerSecret = p.Csecret
202202

203-
p.Logf("[DEBUG] created %s oauth2, id=%s, redir=%s, endpoint=%s",
203+
p.Logf("[DEBUG] created %s oauth1, id=%s, redir=%s, endpoint=%s",
204204
service.name, service.Cid, service.makeRedirURL("/{route}/"+service.name+"/"), service.conf.Endpoint)
205205
return service
206206
}

provider/service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ type Provider interface {
6363
LogoutHandler(w http.ResponseWriter, r *http.Request)
6464
}
6565

66-
// Handler returns auth routes for given provider
66+
// Handler dispatches login, callback, and logout requests to the underlying provider.
6767
func (p Service) Handler(w http.ResponseWriter, r *http.Request) {
6868

6969
if r.Method != http.MethodGet && r.Method != http.MethodPost {

provider/telegram.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ func (th *TelegramHandler) ProcessUpdate(ctx context.Context, textUpdate string)
156156
return nil
157157
}
158158

159-
// processUpdates processes a batch of updates from telegram servers
160-
// Returns offset for subsequent calls
159+
// processUpdates processes a batch of updates from telegram servers. Offset
160+
// tracking for subsequent polls is handled inside TelegramAPI.GetUpdates.
161161
func (th *TelegramHandler) processUpdates(ctx context.Context, updates *telegramUpdate) {
162162
for _, update := range updates.Result {
163163
if update.Message.Chat.Type != "private" {
@@ -229,7 +229,8 @@ func (th *TelegramHandler) addToken(token string, expires time.Time) error {
229229
return nil
230230
}
231231

232-
// checkToken verifies incoming token, returns the user address if it's confirmed and empty string otherwise
232+
// checkToken returns the confirmed user for a known token, or an error if the
233+
// request is missing, expired, or has not been verified yet.
233234
func (th *TelegramHandler) checkToken(token string) (*authtoken.User, error) {
234235
th.requests.RLock()
235236
authRequest, ok := th.requests.data[token]

0 commit comments

Comments
 (0)