Skip to content

Commit 59b725b

Browse files
MrEthical07Copilot
andauthored
Fix/ci (#14)
* fix: resolve CI perf and scanner regressions * test: stabilize strict perf benchmark config * fix: use errors.As for legacy tenant type error detection in isLegacyTenantTypeError Agent-Logs-Url: https://github.com/MrEthical07/goAuth/sessions/3cf7977c-151d-484e-8808-4ca348f0d31c Co-authored-by: MrEthical07 <152254209+MrEthical07@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 9d3059c commit 59b725b

4 files changed

Lines changed: 108 additions & 46 deletions

File tree

auth_bench_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ func newBenchmarkEngine(tb testing.TB, mode ValidationMode) (*Engine, func()) {
9898
cfg.Password.Memory = 8 * 1024
9999
cfg.Password.Time = 1
100100
cfg.Password.Parallelism = 1
101+
cfg.Session.SlidingExpiration = false
102+
cfg.Session.JitterEnabled = false
101103
cfg.Metrics.Enabled = false
102104
cfg.Audit.Enabled = false
103105
cfg.SessionHardening.MaxSessionsPerUser = 0
@@ -183,6 +185,8 @@ func newBenchmarkEngineRealRedis(tb testing.TB, mode ValidationMode) (*Engine, f
183185
cfg.Password.Memory = 8 * 1024
184186
cfg.Password.Time = 1
185187
cfg.Password.Parallelism = 1
188+
cfg.Session.SlidingExpiration = false
189+
cfg.Session.JitterEnabled = false
186190
cfg.Metrics.Enabled = false
187191
cfg.Audit.Enabled = false
188192
cfg.SessionHardening.MaxSessionsPerUser = 0

examples/http-minimal/main.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,15 @@ func main() {
114114
mux.Handle("GET /protected", protected)
115115

116116
fmt.Println("listening on :8080")
117-
log.Fatal(http.ListenAndServe(":8080", mux))
117+
server := &http.Server{
118+
Addr: ":8080",
119+
Handler: mux,
120+
ReadHeaderTimeout: 5 * time.Second,
121+
ReadTimeout: 10 * time.Second,
122+
WriteTimeout: 15 * time.Second,
123+
IdleTimeout: 60 * time.Second,
124+
}
125+
log.Fatal(server.ListenAndServe())
118126
}
119127

120128
// ---------------------------------------------------------------------------
@@ -226,15 +234,18 @@ func bearerToken(h string) string {
226234
// Cookie helpers
227235
// ---------------------------------------------------------------------------
228236

237+
const refreshCookieMaxAgeSeconds = 7 * 24 * 60 * 60
238+
229239
func setRefreshCookie(w http.ResponseWriter, r *http.Request, token string) {
230240
// For localhost demo on plain HTTP, Secure cookies won't be sent.
231241
secure := r.TLS != nil
232242

243+
// #nosec G124 -- localhost demo intentionally allows non-TLS cookies.
233244
http.SetCookie(w, &http.Cookie{
234245
Name: "refresh_token",
235246
Value: token,
236247
Path: "/",
237-
MaxAge: int((7 * 24 * time.Hour).Seconds()),
248+
MaxAge: refreshCookieMaxAgeSeconds,
238249
HttpOnly: true,
239250
Secure: secure,
240251
SameSite: http.SameSiteLaxMode,
@@ -243,6 +254,7 @@ func setRefreshCookie(w http.ResponseWriter, r *http.Request, token string) {
243254

244255
func clearRefreshCookie(w http.ResponseWriter, r *http.Request) {
245256
secure := r.TLS != nil
257+
// #nosec G124 -- localhost demo intentionally allows non-TLS cookies.
246258
http.SetCookie(w, &http.Cookie{
247259
Name: "refresh_token",
248260
Value: "",

jwt/manager.go

Lines changed: 89 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ type Manager struct {
5656
config Config
5757
parsedSignKey interface{} // cached at init: ed25519.PrivateKey or []byte (HS256)
5858
fast *fastJWTState
59+
methodAlg string
60+
accessParser *jwt.Parser
61+
accessKeyFunc jwt.Keyfunc
5962
}
6063

6164
// fastJWTState holds pre-computed state for zero-overhead JWT creation.
@@ -95,6 +98,10 @@ type AccessClaims struct {
9598
jwt.RegisteredClaims
9699
}
97100

101+
// plainAccessClaims uses default JSON decoding (string tid only) for the fast path.
102+
// Legacy numeric tid tokens are handled by a fallback parse path.
103+
type plainAccessClaims AccessClaims
104+
98105
type accessClaimsJSON struct {
99106
UID string `json:"uid"`
100107
TID json.RawMessage `json:"tid,omitempty"`
@@ -186,6 +193,9 @@ func NewManager(cfg Config) (*Manager, error) {
186193
}
187194

188195
m := &Manager{config: cfg}
196+
m.methodAlg = m.getMethod().Alg()
197+
m.accessParser = jwt.NewParser(buildAccessParserOptions(cfg, m.methodAlg)...)
198+
m.accessKeyFunc = m.accessTokenKeyFunc
189199

190200
// Pre-parse and cache the signing key to avoid per-call parsing.
191201
switch cfg.SigningMethod {
@@ -418,71 +428,106 @@ func (j *Manager) createAccessLegacy(
418428
// Performance: single signature verify + claim decode.
419429
// Docs: docs/jwt.md
420430
func (j *Manager) ParseAccess(tokenStr string) (*AccessClaims, error) {
421-
options := []jwt.ParserOption{
422-
jwt.WithValidMethods([]string{j.getMethod().Alg()}),
431+
plain := &plainAccessClaims{}
432+
token, err := j.accessParser.ParseWithClaims(tokenStr, plain, j.accessKeyFunc)
433+
if err == nil {
434+
claims := (*AccessClaims)(plain)
435+
if !token.Valid {
436+
return nil, jwt.ErrTokenInvalidClaims
437+
}
438+
if err := j.validateParsedAccessClaims(claims); err != nil {
439+
return nil, err
440+
}
441+
return claims, nil
423442
}
424-
if j.config.Leeway > 0 {
425-
options = append(options, jwt.WithLeeway(j.config.Leeway))
443+
444+
if !isLegacyTenantTypeError(err) {
445+
return nil, err
426446
}
427-
if j.config.RequireIAT {
447+
448+
legacyToken, legacyErr := j.accessParser.ParseWithClaims(tokenStr, &AccessClaims{}, j.accessKeyFunc)
449+
if legacyErr != nil {
450+
return nil, legacyErr
451+
}
452+
453+
claims, ok := legacyToken.Claims.(*AccessClaims)
454+
if !ok || !legacyToken.Valid {
455+
return nil, jwt.ErrTokenInvalidClaims
456+
}
457+
if err := j.validateParsedAccessClaims(claims); err != nil {
458+
return nil, err
459+
}
460+
461+
return claims, nil
462+
}
463+
464+
func buildAccessParserOptions(cfg Config, methodAlg string) []jwt.ParserOption {
465+
options := make([]jwt.ParserOption, 0, 5)
466+
options = append(options, jwt.WithValidMethods([]string{methodAlg}))
467+
if cfg.Leeway > 0 {
468+
options = append(options, jwt.WithLeeway(cfg.Leeway))
469+
}
470+
if cfg.RequireIAT {
428471
options = append(options, jwt.WithIssuedAt())
429472
}
430-
if j.config.Issuer != "" {
431-
options = append(options, jwt.WithIssuer(j.config.Issuer))
473+
if cfg.Issuer != "" {
474+
options = append(options, jwt.WithIssuer(cfg.Issuer))
432475
}
433-
if j.config.Audience != "" {
434-
options = append(options, jwt.WithAudience(j.config.Audience))
476+
if cfg.Audience != "" {
477+
options = append(options, jwt.WithAudience(cfg.Audience))
435478
}
479+
return options
480+
}
436481

437-
parser := jwt.NewParser(options...)
438-
token, err := parser.ParseWithClaims(tokenStr, &AccessClaims{}, func(t *jwt.Token) (interface{}, error) {
439-
if t.Method.Alg() != j.getMethod().Alg() {
440-
return nil, fmt.Errorf("unexpected signing algorithm: %s", t.Method.Alg())
441-
}
482+
func (j *Manager) accessTokenKeyFunc(t *jwt.Token) (interface{}, error) {
483+
if t.Method.Alg() != j.methodAlg {
484+
return nil, fmt.Errorf("unexpected signing algorithm: %s", t.Method.Alg())
485+
}
442486

443-
if len(j.config.VerifyKeys) > 0 {
444-
kid, _ := t.Header["kid"].(string)
445-
if kid == "" {
446-
return nil, errors.New("missing kid")
447-
}
448-
key, ok := j.config.VerifyKeys[kid]
449-
if !ok {
450-
return nil, errors.New("unknown kid")
451-
}
452-
return j.keyBytesToVerifyKey(key)
487+
if len(j.config.VerifyKeys) > 0 {
488+
kid, _ := t.Header["kid"].(string)
489+
if kid == "" {
490+
return nil, errors.New("missing kid")
453491
}
454-
455-
if j.config.KeyID != "" {
456-
kid, _ := t.Header["kid"].(string)
457-
if kid == "" {
458-
return nil, errors.New("missing kid")
459-
}
460-
if kid != j.config.KeyID {
461-
return nil, errors.New("unknown kid")
462-
}
492+
key, ok := j.config.VerifyKeys[kid]
493+
if !ok {
494+
return nil, errors.New("unknown kid")
463495
}
464-
465-
return j.getVerifyKey()
466-
})
467-
if err != nil {
468-
return nil, err
496+
return j.keyBytesToVerifyKey(key)
469497
}
470498

471-
claims, ok := token.Claims.(*AccessClaims)
472-
if !ok || !token.Valid {
473-
return nil, jwt.ErrTokenInvalidClaims
499+
if j.config.KeyID != "" {
500+
kid, _ := t.Header["kid"].(string)
501+
if kid == "" {
502+
return nil, errors.New("missing kid")
503+
}
504+
if kid != j.config.KeyID {
505+
return nil, errors.New("unknown kid")
506+
}
474507
}
508+
509+
return j.getVerifyKey()
510+
}
511+
512+
func (j *Manager) validateParsedAccessClaims(claims *AccessClaims) error {
475513
if j.config.RequireIAT && claims.IssuedAt == nil {
476-
return nil, errors.New("token missing required iat claim")
514+
return errors.New("token missing required iat claim")
477515
}
478516
if claims.IssuedAt != nil && j.config.MaxFutureIAT > 0 {
479517
maxAllowed := time.Now().Add(j.config.MaxFutureIAT)
480518
if claims.IssuedAt.Time.After(maxAllowed) {
481-
return nil, errors.New("token iat too far in the future")
519+
return errors.New("token iat too far in the future")
482520
}
483521
}
522+
return nil
523+
}
484524

485-
return claims, nil
525+
func isLegacyTenantTypeError(err error) bool {
526+
var typeErr *json.UnmarshalTypeError
527+
if !errors.As(err, &typeErr) {
528+
return false
529+
}
530+
return typeErr.Value == "number" && (typeErr.Field == "tid" || strings.HasSuffix(typeErr.Field, ".tid"))
486531
}
487532

488533
func (j *Manager) getMethod() jwt.SigningMethod {

security/cmd/perf-regression/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ func main() {
124124
}
125125

126126
func parseBenchmarkFile(path string) (sampleSet, error) {
127+
// #nosec G304 -- benchmark file paths are controlled by local tooling/CI scripts.
127128
file, err := os.Open(path)
128129
if err != nil {
129130
return nil, err

0 commit comments

Comments
 (0)