Preserve PVE authentication endings while reading responses

A password login can end while its response is being read: the caller
cancels, its deadline passes, or the client's own login timeout expires.
Go's HTTP transport can still hand back the server's answer to that
abandoned request. The construction fixture answers a departed client
with an empty 200, which surfaced as "invalid authentication response"
and failed TestClientConstructionHonoursContext and its client-timeout
twin intermittently in the required backend check.

The shared password exchange now reports the ending instead of
diagnosing that answer, keeps transport errors that already carry the
ending, and starts no form attempt after an ended JSON refusal. This
mirrors the PBS client fix from 18 October. Renewal shares the exchange,
so the same rule applies when its own bounded timeout ends a login.
Complete sessions are still accepted, and construction still discards a
client whose caller has ended.

A new control ends the login during response reading, by caller
cancellation and by a simulated login deadline, for every unusable
answer; without this change all of its cases fail. It also checks that
both encoding attempts carry the one login budget the client timeout
sets. The TLS fixture now aborts a request whose client has left rather
than answering it with an empty 200.
This commit is contained in:
rcourtman
2026-10-23 13:46:51 +01:00
parent 751dfcce0a
commit ae19fcf5d6
4 changed files with 145 additions and 2 deletions
@@ -4260,6 +4260,26 @@ during response reading for JSON/form, complete and incomplete success,
encoding and credential refusal, and body-read failure. The real TLS
constructor cancellation/deadline and shared-renewal controls remain separate.
### PVE authentication keeps an ended login's outcome
When a PVE password exchange ends while its response is being read (the caller
cancels, the caller's deadline passes, or the client's own login timeout
expires), the login now reports that context error. The server's answer to the
abandoned exchange, such as an empty 200 or a refusal, is no longer reported as
a malformed session or a credential refusal, and an ended JSON refusal does not
start a form attempt. Transport errors that already report the ending keep
their wording. A complete session is still accepted, and construction still
discards a client whose caller has ended. Shared renewal gets the same rule
through the common password exchange, where only its own bounded timeout can
end the detached login. `TestClientEndedAuthResponseOwnsErrorAndSession` ends
the login during response reading, by caller cancellation and by a simulated
login deadline, for JSON/form, empty and invalid success, encoding and
credential refusal, and body-read failure. It also checks that both encoding
attempts carry the one login budget. The real TLS construction fixture now
aborts a request whose client has left instead of answering it with an empty
200; reading that answer as a malformed session is what made
`TestClientConstructionHonoursContext` fail intermittently in CI.
### AI Discovery removed (2026-10-11)
The [AI runtime retirement boundary](ai-runtime.md#ai-discovery-removed-2026-10-11)
+8 -1
View File
@@ -446,11 +446,18 @@ func (c *Client) authenticate(ctx context.Context) error {
return c.authenticatePassword(loginCtx, auth.user+"@"+auth.realm)
}
// A response can finish just as ctx ends. The server's answer to that
// abandoned exchange, such as an empty or refused response, is not a
// credential diagnosis: an ended exchange tries no other encoding and reports
// the ending, keeping a transport error that already carries it.
func (c *Client) authenticatePassword(ctx context.Context, username string) error {
err := c.authenticateJSON(ctx, username, c.config.Password)
if shouldFallbackToForm(err) {
if ctx.Err() == nil && shouldFallbackToForm(err) {
err = c.authenticateForm(ctx, username, c.config.Password)
}
if ended := ctx.Err(); err != nil && ended != nil && !errors.Is(err, ended) {
return ended
}
return err
}
+113 -1
View File
@@ -77,7 +77,8 @@ func testClientConstructionHonoursContext(t *testing.T, endings ...string) {
select {
case <-r.Context().Done():
close(requestEnded)
return
// Abort: returning would answer the departed client with an empty 200.
panic(http.ErrAbortHandler)
case <-release:
}
}
@@ -169,3 +170,114 @@ func testClientConstructionHonoursContext(t *testing.T, endings ...string) {
}
}
}
// End the login while its response is consumed, after HTTP has already
// returned an unusable 200 or a refusal. This reproduces the constructor race
// without scheduler timing.
type endingAuthBody struct {
io.Reader
end func()
}
func (r endingAuthBody) Read(p []byte) (int, error) {
r.end()
return r.Reader.Read(p)
}
// The caller's deadline and the client's own login timeout both reach the
// password exchange as a login context ending with DeadlineExceeded. The
// response read ends this one, so no wall-clock timer races the answer.
type endedLoginDeadline struct {
context.Context
done <-chan struct{}
}
func (c endedLoginDeadline) Done() <-chan struct{} { return c.done }
func (c endedLoginDeadline) Err() error {
select {
case <-c.done:
return context.DeadlineExceeded
default:
return nil
}
}
func testClientEndedAuthResponseOwnsErrorAndSession(t *testing.T) {
const good = `{"data":{"ticket":"recovered-ticket","CSRFPreventionToken":"recovered-csrf"}}`
for _, ending := range []string{"cancel", "deadline"} {
for _, encoding := range []string{"json", "form"} {
for _, outcome := range []struct {
name, body string
status int
readError bool
}{
{"empty-success", ``, http.StatusOK, false},
{"invalid-success", `{}`, http.StatusOK, false},
{"encoding-refusal", `{}`, http.StatusUnsupportedMediaType, false},
{"credential-refusal", `{}`, http.StatusUnauthorized, false},
{"body-read-failure", good, http.StatusOK, true},
} {
t.Run(ending+"/"+encoding+"/"+outcome.name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
deadline := make(chan struct{})
end, want := func() { cancel() }, context.Canceled
if ending == "deadline" {
end, want = sync.OnceFunc(func() { close(deadline) }), context.DeadlineExceeded
}
calls, recovering := 0, false
var budget time.Time
c := &Client{
baseURL: "https://pve.fixture.invalid/api2/json",
config: ClientConfig{Password: "fixture-password"},
auth: auth{user: "fixture", realm: "pam"},
}
c.httpClient = &http.Client{Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
calls++
if ending == "cancel" && !recovering {
// authenticate gives every encoding attempt one login budget.
deadline, ok := r.Context().Deadline()
if !ok || (!budget.IsZero() && !deadline.Equal(budget)) {
t.Error("login attempts lost their shared login budget")
}
budget = deadline
}
if encoding == "form" && r.Header.Get("Content-Type") == "application/json" {
return &http.Response{StatusCode: http.StatusUnsupportedMediaType, Body: io.NopCloser(strings.NewReader(`{}`)), ContentLength: -1}, nil
}
if recovering {
return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(good)), ContentLength: -1}, nil
}
var body io.Reader = strings.NewReader(outcome.body)
if outcome.readError {
body = sessionAdmissionReadError{body}
}
return &http.Response{StatusCode: outcome.status, Body: io.NopCloser(endingAuthBody{body, end}), ContentLength: -1}, nil
})}
before, generation := c.authSnapshot()
var err error
if ending == "deadline" {
err = c.authenticatePassword(endedLoginDeadline{context.Background(), deadline}, "fixture@pam")
} else {
err = c.authenticate(ctx)
}
if after, afterGeneration := c.authSnapshot(); !errors.Is(err, want) || after != before || afterGeneration != generation {
t.Errorf("ended login response lost its ending or changed the session: error=%v", err)
}
wantCalls := 1
if encoding == "form" {
wantCalls = 2
}
if calls != wantCalls {
t.Errorf("ended login tried another encoding: calls=%d", calls)
}
recovering = true
if err := c.authenticate(context.Background()); err != nil || c.auth.ticket != "recovered-ticket" || c.auth.csrfToken != "recovered-csrf" || calls != 2*wantCalls {
t.Errorf("distinct later login did not recover: error=%v calls=%d", err, calls)
}
})
}
}
}
}
@@ -167,6 +167,10 @@ func TestClientConstructionHonoursClientTimeout(t *testing.T) {
testClientConstructionHonoursContext(t, "client-timeout")
}
func TestClientEndedAuthResponseOwnsErrorAndSession(t *testing.T) {
testClientEndedAuthResponseOwnsErrorAndSession(t)
}
func TestClusterCancelledCallerStartsNoSession(t *testing.T) {
testClusterCancelledCallerStartsNoSession(t)
}