groupware: upgrade Stalwart to 0.16.12 + fix modifying the VacationResponse

* fix jmap.SetVacationResponse: was using JMAP create instead of update
   with a patch; did so by using the modify template function as with
   most other JMAP APIs

 * better HTTP status code handling for the responses received from
   Stalwart

 * add support for not caching JMAP session retrieval failures in the
   Groupware backend, by setting the error TTL to 0
This commit is contained in:
Pascal Bleser
2026-07-09 14:12:00 +02:00
parent 7e3b722961
commit 279f8a2ab9
10 changed files with 111 additions and 88 deletions

View File

@@ -1,7 +1,6 @@
package jmap
import (
"fmt"
"time"
)
@@ -26,7 +25,7 @@ func (j *Client) GetVacationResponse(accountId AccountId, ctx Context) (Result[V
// Same as VacationResponse but without the id.
type VacationResponseChange struct {
// Should a vacation response be sent if a message arrives between the "fromDate" and "toDate"?
IsEnabled bool `json:"isEnabled"`
IsEnabled *bool `json:"isEnabled,omitempty"`
// If "isEnabled" is true, messages that arrive on or after this date-time (but before the "toDate" if defined) should receive the
// user's vacation response. If null, the vacation response is effective immediately.
FromDate *time.Time `json:"fromDate,omitzero"`
@@ -68,62 +67,18 @@ func (c VacationResponseChanges) GetCreated() []VacationResponse { return c.Crea
func (c VacationResponseChanges) GetUpdated() []VacationResponse { return c.Updated }
func (c VacationResponseChanges) GetDestroyed() []string { return c.Destroyed }
func (j *Client) SetVacationResponse(accountId AccountId, vacation VacationResponseChange,
func (j *Client) SetVacationResponse(accountId AccountId, change VacationResponseChange,
ctx Context) (Result[VacationResponse], error) {
logger := j.logger("SetVacationResponse", ctx)
ctx = ctx.WithLogger(logger)
set := VacationResponseSetCommand{
AccountId: accountId,
Create: map[string]VacationResponse{
vacationResponseId: {
IsEnabled: vacation.IsEnabled,
FromDate: vacation.FromDate,
ToDate: vacation.ToDate,
Subject: vacation.Subject,
TextBody: vacation.TextBody,
HtmlBody: vacation.HtmlBody,
},
return update(j, "SetVacationResponse", VacationResponseType,
func(update map[string]PatchObject) VacationResponseSetCommand {
return VacationResponseSetCommand{AccountId: accountId, Update: update}
},
}
get := VacationResponseGetCommand{AccountId: accountId}
cmd, err := j.request(ctx, NS_VACATION,
invocation(set, "0"),
// chain a second request to get the current complete VacationResponse object
// after performing the changes, as that makes for a better API
invocation(get, "1"),
func(_ string) VacationResponseGetCommand {
return VacationResponseGetCommand{AccountId: accountId}
},
func(resp VacationResponseSetResponse) map[string]SetError { return resp.NotUpdated },
func(resp VacationResponseGetResponse) VacationResponse { return resp.List[0] },
vacationResponseId, change,
ctx,
)
if err != nil {
return ZeroResultV[VacationResponse](), err
}
return command(j, Operation("SetVacationResponse"), ctx, cmd, func(body *Response) (VacationResponse, State, Error) {
var setResponse VacationResponseSetResponse
err = retrieveSet(ctx, body, set, "0", &setResponse)
if err != nil {
return VacationResponse{}, "", err
}
setErr, notok := setResponse.NotCreated[vacationResponseId]
if notok {
// this means that the VacationResponse was not updated
logger.Error().Msgf("%T.NotCreated contains an error: %v", setResponse, setErr)
return VacationResponse{}, "", setErrorError(setErr, VacationResponseType)
}
var getResponse VacationResponseGetResponse
err = retrieveGet(ctx, body, get, "1", &getResponse)
if err != nil {
return VacationResponse{}, "", err
}
if len(getResponse.List) != 1 {
berr := fmt.Errorf("failed to find %s in %s response", VacationResponseType, string(CommandVacationResponseGet))
logger.Error().Msg(berr.Error())
return VacationResponse{}, "", jmapError(berr, JmapErrorInvalidJmapResponsePayload)
}
return getResponse.List[0], setResponse.NewState, nil
})
}

View File

@@ -3,6 +3,7 @@ package jmap
import (
"errors"
"fmt"
"net/http"
"strings"
)
@@ -84,6 +85,15 @@ func (e JmapError) Description() string {
return e.description
}
func jmapErrorCode(statusCode int) int {
code := JmapErrorServerResponse
switch statusCode {
case http.StatusUnauthorized:
code = JmapErrorAuthenticationFailed
}
return code
}
func jmapError(err error, code int) Error {
if err != nil {
return JmapError{code: code, err: err}

View File

@@ -298,7 +298,7 @@ func (h *HttpJmapClient) GetSession(ctx context.Context, sessionUrl *url.URL, us
if res.StatusCode < 200 || res.StatusCode > 299 {
h.listener.OnFailedRequestWithStatus(endpoint, Operation("GetSession"), res.StatusCode)
logger.Error().Str(logHttpStatus, log.SafeString(res.Status)).Int(logHttpStatusCode, res.StatusCode).Msg("HTTP response status code is not 200")
return SessionResponse{}, jmapError(fmt.Errorf("JMAP API response status is %v", res.Status), JmapErrorServerResponse)
return SessionResponse{}, jmapError(fmt.Errorf("JMAP API response status is %v", res.Status), jmapErrorCode(res.StatusCode))
}
h.listener.OnSuccessfulRequest(endpoint, Operation("GetSession"), res.StatusCode)
@@ -383,12 +383,7 @@ func (h *HttpJmapClient) Command(operation Operation, request Request, ctx Conte
if res.StatusCode < 200 || res.StatusCode > 299 {
h.listener.OnFailedRequestWithStatus(endpoint, operation, res.StatusCode)
logger.Error().Str(logEndpoint, endpoint).Str(logHttpStatus, log.SafeString(res.Status)).Msg("HTTP response status code is not 2xx") //NOSONAR
switch res.StatusCode {
case http.StatusUnauthorized:
return nil, language, jmapError(fmt.Errorf("JMAP server responsed with '%s'", res.Status), JmapErrorAuthenticationFailed)
default:
return nil, language, jmapError(fmt.Errorf("JMAP server responsed with '%s'", res.Status), JmapErrorServerResponse)
}
return nil, language, jmapError(fmt.Errorf("JMAP server responsed with '%s'", res.Status), jmapErrorCode(res.StatusCode))
}
h.listener.OnSuccessfulRequest(endpoint, operation, res.StatusCode)
@@ -450,7 +445,7 @@ func (h *HttpJmapClient) UploadBinary(uploadUrl string, operation Operation, end
if res.StatusCode < 200 || res.StatusCode > 299 {
h.listener.OnFailedRequestWithStatus(endpoint, operation, res.StatusCode)
logger.Error().Str(logHttpStatus, log.SafeString(res.Status)).Int(logHttpStatusCode, res.StatusCode).Msg("HTTP response status code is not 2xx")
return UploadedBlob{}, language, jmapError(err, JmapErrorServerResponse)
return UploadedBlob{}, language, jmapError(err, jmapErrorCode(res.StatusCode))
}
h.listener.OnSuccessfulRequest(endpoint, operation, res.StatusCode)
@@ -518,7 +513,7 @@ func (h *HttpJmapClient) DownloadBinary(downloadUrl string, operation Operation,
if res.StatusCode < 200 || res.StatusCode > 299 {
h.listener.OnFailedRequestWithStatus(endpoint, operation, res.StatusCode)
logger.Error().Str(logHttpStatus, log.SafeString(res.Status)).Int(logHttpStatusCode, res.StatusCode).Msg("HTTP response status code is not 2xx")
return nil, language, jmapError(err, JmapErrorServerResponse)
return nil, language, jmapError(err, jmapErrorCode(res.StatusCode))
}
h.listener.OnSuccessfulRequest(endpoint, operation, res.StatusCode)
@@ -662,7 +657,7 @@ func (w *HttpWsClientFactory) connect(operation Operation, ctx context.Context,
if res.StatusCode != 101 {
w.eventListener.OnFailedRequestWithStatus(endpoint, operation, res.StatusCode)
logger.Error().Str(logHttpStatus, log.SafeString(res.Status)).Int(logHttpStatusCode, res.StatusCode).Msg("HTTP response status code is not 101")
return nil, "", endpoint, jmapError(fmt.Errorf("JMAP WS API response status is %v", res.Status), JmapErrorServerResponse)
return nil, "", endpoint, jmapError(fmt.Errorf("JMAP WS API response status is %v", res.Status), jmapErrorCode(res.StatusCode))
} else {
w.eventListener.OnSuccessfulWsRequest(endpoint, operation, res.StatusCode)
}

View File

@@ -4159,7 +4159,7 @@ type VacationResponse struct {
Id string `json:"id,omitempty"`
// Should a vacation response be sent if a message arrives between the "fromDate" and "toDate"?
IsEnabled bool `json:"isEnabled"`
IsEnabled *bool `json:"isEnabled,omitempty"`
// If "isEnabled" is true, messages that arrive on or after this date-time (but before the "toDate" if defined) should receive the
// user's vacation response. If null, the vacation response is effective immediately.

View File

@@ -882,7 +882,7 @@ func (e Exemplar) VacationResponse() VacationResponse {
to, _ := time.Parse(time.RFC3339, "20260114T23:59:59.999Z")
return VacationResponse{
Id: "aefee7ae",
IsEnabled: true,
IsEnabled: ptr(true),
FromDate: &from,
ToDate: &to,
Subject: "On PTO",

View File

@@ -114,7 +114,7 @@ var (
)
const (
StalwartVersion = "0.16.9"
StalwartVersion = "0.16.12"
StalwartCliVersion = "1.0.8"
stalwartImageTemplate = "ghcr.io/stalwartlabs/stalwart:v%s-alpine"

View File

@@ -12,9 +12,10 @@ func TestGroupwareIndex(t *testing.T) {
require := require.New(t)
g, err := newGroupwareTest(t)
require.NoError(err)
u := g.user()
index := IndexResponse{}
u := gget("get-index", g, "/", &index)
gget(g, "get-index", u, "/", &index)
require.Len(index.Accounts, 1)
require.Equal(u.Email, index.Accounts[0].Name)
require.Len(index.Accounts[0].Identities, 2) // email + alias

View File

@@ -2,6 +2,7 @@ package groupware
import (
"testing"
"time"
"github.com/opencloud-eu/opencloud/pkg/jmap"
"github.com/stretchr/testify/require"
@@ -11,12 +12,13 @@ func TestGroupwareVacation(t *testing.T) {
require := require.New(t)
g, err := newGroupwareTest(t)
require.NoError(err)
u := g.user()
{
resp := jmap.VacationResponse{}
gget("get-initial-vacation", g, "/accounts/_/vacation", &resp)
gget(g, "get-initial-vacation", u, "/accounts/_/vacation", &resp)
require.Equal("singleton", resp.Id)
require.False(resp.IsEnabled)
require.False(*resp.IsEnabled)
require.Empty(resp.Subject)
require.Empty(resp.TextBody)
require.Empty(resp.HtmlBody)
@@ -27,18 +29,51 @@ func TestGroupwareVacation(t *testing.T) {
{
resp := jmap.VacationResponse{}
req := jmap.VacationResponseChange{
IsEnabled: true,
IsEnabled: ptr(true),
Subject: "testing",
TextBody: "text",
HtmlBody: "html",
}
gput("set-vacation", g, "/accounts/_/vacation", req, &resp)
gput(g, "set-vacation", u, "/accounts/_/vacation", req, &resp)
require.Equal("singleton", resp.Id)
require.True(resp.IsEnabled)
require.True(*resp.IsEnabled)
require.Equal("testing", resp.Subject)
require.Equal("text", resp.TextBody)
require.Equal("html", resp.HtmlBody)
require.Nil(resp.FromDate)
require.Nil(resp.ToDate)
}
{
resp := jmap.VacationResponse{}
gget(g, "get-initial-vacation-after-change", u, "/accounts/_/vacation", &resp)
require.Equal("singleton", resp.Id)
require.True(*resp.IsEnabled)
require.Equal("testing", resp.Subject)
require.Equal("text", resp.TextBody)
require.Equal("html", resp.HtmlBody)
require.Nil(resp.FromDate)
require.Nil(resp.ToDate)
}
{
resp := jmap.VacationResponse{}
from, err := time.Parse(time.RFC3339, "2026-07-08T00:00:00Z")
require.NoError(err)
to, err := time.Parse(time.RFC3339, "2026-07-18T23:59:59.999Z")
require.NoError(err)
req := jmap.VacationResponseChange{
IsEnabled: nil,
FromDate: &from,
ToDate: &to,
}
gput(g, "set-vacation", u, "/accounts/_/vacation", req, &resp)
require.Equal("singleton", resp.Id)
require.False(*resp.IsEnabled) // TODO should be true: this is a bug in Stalwart up to 0.16.12, see https://support.stalw.art/t/vacationresponse-jmap-api-isenabled-reset-to-false-whenever-properties-are-changed/992
require.Equal("testing", resp.Subject)
require.Equal("text", resp.TextBody)
require.Equal("html", resp.HtmlBody)
require.Equal("2026-07-08T00:00:00Z", resp.FromDate.Format(time.RFC3339))
require.Equal("2026-07-18T23:59:59Z", resp.ToDate.Format(time.RFC3339)) // for some reason the .999 gets lost in Stalwart (?), but nothing critical
}
}

View File

@@ -11,19 +11,20 @@ import (
"net/url"
"strings"
"testing"
"time"
userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
"github.com/google/uuid"
"github.com/opencloud-eu/opencloud/pkg/cors"
ochttp "github.com/opencloud-eu/opencloud/pkg/http"
"github.com/opencloud-eu/opencloud/pkg/jmaptest"
opencloudmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware"
"github.com/opencloud-eu/opencloud/pkg/shared"
"github.com/opencloud-eu/opencloud/pkg/structs"
"github.com/opencloud-eu/opencloud/services/groupware/pkg/config"
"github.com/opencloud-eu/opencloud/services/groupware/pkg/logging"
groupwaremiddleware "github.com/opencloud-eu/opencloud/services/groupware/pkg/middleware"
revactx "github.com/opencloud-eu/reva/v2/pkg/ctx"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
@@ -39,29 +40,40 @@ type GroupwareTest struct {
Users []jmaptest.User
}
func gget[T any](id string, g GroupwareTest, path string, result *T) jmaptest.User {
// pick a user at random
func (g GroupwareTest) user() jmaptest.User {
return g.Users[rand.Intn(len(g.Users))]
}
func gget[T any](g GroupwareTest, id string, user jmaptest.User, path string, result *T) {
id = g.t.Name() + "/" + id
u, err := url.JoinPath(g.BaseURL, path)
require.NoError(g.t, err)
req, err := http.NewRequest(http.MethodGet, u, nil)
require.NoError(g.t, err)
user := g.Users[rand.Intn(len(g.Users))] // pick a user at random
req.SetBasicAuth(user.Name, user.Password)
rid := uuid.New().String()
req.Header.Add("X-Request-Id", rid)
req.Header.Add("Trace-Id", rid)
client := http.Client{}
resp, err := client.Do(req)
require.NoError(g.t, err)
var resp *http.Response = nil
require.Eventually(g.t, func() bool {
r, err := client.Do(req)
resp = r
if err == nil && r.StatusCode == 200 {
resp = r
return true
}
return false
}, time.Duration(5*time.Second), time.Duration(1*time.Second))
require.Equal(g.t, 200, resp.StatusCode)
defer resp.Body.Close()
err = json.NewDecoder(resp.Body).Decode(result)
require.NoError(g.t, err)
return user
}
func gput[B any, T any](id string, g GroupwareTest, path string, body B, result *T) jmaptest.User {
func gput[B any, T any](g GroupwareTest, id string, user jmaptest.User, path string, body B, result *T) {
id = g.t.Name() + "/" + id
u, err := url.JoinPath(g.BaseURL, path)
@@ -72,7 +84,6 @@ func gput[B any, T any](id string, g GroupwareTest, path string, body B, result
req, err := http.NewRequest(http.MethodPut, u, bytes.NewBuffer(jsonBody))
require.NoError(g.t, err)
user := g.Users[rand.Intn(len(g.Users))] // pick a user at random
req.SetBasicAuth(user.Name, user.Password)
rid := uuid.New().String()
req.Header.Add("X-Request-Id", id)
@@ -84,7 +95,6 @@ func gput[B any, T any](id string, g GroupwareTest, path string, body B, result
defer resp.Body.Close()
err = json.NewDecoder(resp.Body).Decode(result)
require.NoError(g.t, err)
return user
}
func newGroupwareTest(t *testing.T) (GroupwareTest, error) {
@@ -125,7 +135,9 @@ func newGroupwareTest(t *testing.T) (GroupwareTest, error) {
},
BaseUrl: s.JmapBaseUrl.String(),
SessionCache: config.MailSessionCache{
MaxCapacity: 1,
MaxCapacity: 10,
Ttl: time.Duration(30 * time.Minute),
FailureTtl: time.Duration(0),
},
},
HTTP: config.HTTP{
@@ -188,7 +200,18 @@ func newGroupwareTest(t *testing.T) (GroupwareTest, error) {
})
}
deeplog := func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ochttp.DumpHttpRequest(r, 4*1024, func(method string, uri string, content string, truncated bool) {
logger.Trace().Str("method", method).Str("uri", uri).Msg(content)
t.Logf("--> Groupware Request: %s %s:\n%s", method, uri, content)
})
next.ServeHTTP(w, r)
})
}
m.Use(
deeplog,
middleware.RequestID,
opencloudmiddleware.Cors(
cors.Logger(logger),
@@ -197,7 +220,7 @@ func newGroupwareTest(t *testing.T) (GroupwareTest, error) {
cors.AllowedHeaders(config.HTTP.CORS.AllowedHeaders),
cors.AllowCredentials(config.HTTP.CORS.AllowCredentials),
),
groupwaremiddleware.GroupwareLogger(logger),
// groupwaremiddleware.GroupwareLogger(logger),
auth,
)
m.Route(config.HTTP.Root, gw.Route)

View File

@@ -156,7 +156,11 @@ func (c *ttlcacheSessionCache) Get(ctx context.Context, username string) cachedS
// not great, but will do for now:
// - when the result is successful, the default TTL is used
// - when the result is a failure to retrieve the session, a (most probably shorter) TTL must be used instead
c.sessionCache.Set(key, value, c.errorTtl)
if c.errorTtl == 0 {
c.sessionCache.Delete(key)
} else {
c.sessionCache.Set(key, value, c.errorTtl)
}
}
}
return value