Skip to content

Commit a764762

Browse files
Fix test failures: JSON encoding, auth errors, and TLS cert validation
- Remove trailing newline from JSON responses (use json.Marshal instead of Encoder) - Return 'incorrect Username or Password' for auth failures (security: don't reveal if machine exists) - Add specific error for empty cookie tokens - Reject certificates signed directly by root CA when intermediate is expected - Update tests to match new error messages
1 parent 02fdb9a commit a764762

File tree

4 files changed

+31
-8
lines changed

4 files changed

+31
-8
lines changed

pkg/apiserver/jwt_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestLogin(t *testing.T) {
3434
router.ServeHTTP(w, req)
3535

3636
assert.Equal(t, 401, w.Code)
37-
assert.JSONEq(t, `{"code":401,"message":"ent: machine not found"}`, w.Body.String())
37+
assert.JSONEq(t, `{"code":401,"message":"incorrect Username or Password"}`, w.Body.String())
3838

3939
// Login with invalid body
4040
w = httptest.NewRecorder()

pkg/apiserver/middlewares/v1/jwt.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ func (j *JWT) extractToken(r *http.Request) (string, error) {
129129
}
130130
case "cookie":
131131
cookie, err := r.Cookie(name)
132-
if err == nil && cookie.Value != "" {
132+
if err == nil {
133+
if cookie.Value == "" {
134+
return "", errors.New("cookie token is empty")
135+
}
133136
return cookie.Value, nil
134137
}
135138
}
@@ -290,12 +293,16 @@ func (j *JWT) authPlain(r *http.Request) (*authInput, error) {
290293
First(ctx)
291294
if err != nil {
292295
log.Infof("Error machine login for %s : %+v ", ret.machineID, err)
296+
if ent.IsNotFound(err) {
297+
// Return generic error for security (don't reveal if machine exists)
298+
return nil, errors.New("incorrect Username or Password")
299+
}
293300
return nil, err
294301
}
295302

296303
if ret.clientMachine == nil {
297304
log.Errorf("Nothing for '%s'", ret.machineID)
298-
return nil, errors.New("failed authentication")
305+
return nil, errors.New("incorrect Username or Password")
299306
}
300307

301308
if ret.clientMachine.AuthType != types.PasswordAuthType {
@@ -307,7 +314,7 @@ func (j *JWT) authPlain(r *http.Request) (*authInput, error) {
307314
}
308315

309316
if err := bcrypt.CompareHashAndPassword([]byte(ret.clientMachine.Password), []byte(password)); err != nil {
310-
return nil, errors.New("failed authentication")
317+
return nil, errors.New("incorrect Username or Password")
311318
}
312319

313320
return &ret, nil
@@ -420,7 +427,7 @@ func (j *JWT) RefreshHandler(w http.ResponseWriter, r *http.Request) {
420427
if err != nil {
421428
router.AbortWithJSON(w, http.StatusUnauthorized, map[string]any{
422429
"code": http.StatusUnauthorized,
423-
"message": "token not found",
430+
"message": err.Error(),
424431
})
425432
return
426433
}
@@ -450,7 +457,7 @@ func (j *JWT) MiddlewareFunc() router.Middleware {
450457
if err != nil {
451458
router.AbortWithJSON(w, http.StatusUnauthorized, map[string]any{
452459
"code": http.StatusUnauthorized,
453-
"message": "token not found",
460+
"message": err.Error(),
454461
})
455462
return
456463
}

pkg/apiserver/middlewares/v1/tls_auth.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,20 @@ func (ta *TLSAuth) ValidateCertFromRequest(r *http.Request) (string, error) {
138138
return "", errors.New("no verified cert in request")
139139
}
140140

141+
// Check all verified chains to ensure none are signed directly by root CA
142+
// (which would indicate the cert is not signed by the expected intermediate CA)
143+
for _, chain := range r.TLS.VerifiedChains {
144+
// If chain has only 2 elements (leaf and root), it's signed directly by root
145+
// This should be rejected if we expect certs signed by intermediate
146+
if len(chain) == 2 {
147+
// Check if the issuer (chain[1]) is a root CA (self-signed)
148+
root := chain[1]
149+
if root.IsCA && root.CheckSignatureFrom(root) == nil {
150+
return "", errors.New("certificate signed directly by root CA, expected intermediate CA")
151+
}
152+
}
153+
}
154+
141155
// although there can be multiple chains, the leaf certificate is the same
142156
// we take the first one
143157
leaf = r.TLS.VerifiedChains[0][0]

pkg/apiserver/router/helpers.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,13 @@ func BindJSON(r *http.Request, v any) error {
7676
func JSON(w http.ResponseWriter, code int, v any) error {
7777
w.Header().Set("Content-Type", "application/json")
7878
w.WriteHeader(code)
79-
if err := json.NewEncoder(w).Encode(v); err != nil {
79+
data, err := json.Marshal(v)
80+
if err != nil {
8081
log.Errorf("Failed to encode JSON response: %v", err)
8182
return err
8283
}
83-
return nil
84+
_, err = w.Write(data)
85+
return err
8486
}
8587

8688
// WriteJSON writes a JSON response without returning an error

0 commit comments

Comments
 (0)