From 23927a78c4a35cb04cbc3be99ea3c7d9f19efa70 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 5 Feb 2026 08:49:42 +0100 Subject: [PATCH 1/7] feat(appauth): Add some debug logging --- internal/grpc/services/applicationauth/applicationauth.go | 5 ++++- pkg/appauth/manager/jsoncs3/jsoncs3.go | 8 ++++++++ pkg/storage/utils/metadata/cs3.go | 6 ++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/applicationauth/applicationauth.go b/internal/grpc/services/applicationauth/applicationauth.go index a37cc4f4c1..8ab540a4a3 100644 --- a/internal/grpc/services/applicationauth/applicationauth.go +++ b/internal/grpc/services/applicationauth/applicationauth.go @@ -25,6 +25,7 @@ import ( "github.com/mitchellh/mapstructure" "github.com/opencloud-eu/reva/v2/pkg/appauth" "github.com/opencloud-eu/reva/v2/pkg/appauth/manager/registry" + "github.com/opencloud-eu/reva/v2/pkg/appctx" "github.com/opencloud-eu/reva/v2/pkg/errtypes" "github.com/opencloud-eu/reva/v2/pkg/rgrpc" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" @@ -104,8 +105,10 @@ func (s *service) UnprotectedEndpoints() []string { } func (s *service) GenerateAppPassword(ctx context.Context, req *appauthpb.GenerateAppPasswordRequest) (*appauthpb.GenerateAppPasswordResponse, error) { + logger := appctx.GetLogger(ctx) pwd, err := s.am.GenerateAppPassword(ctx, req.TokenScope, req.Label, req.Expiration) if err != nil { + logger.Debug().Err(err).Msg("error generating app password") return &appauthpb.GenerateAppPasswordResponse{ Status: status.NewInternal(ctx, "error generating app password"), }, nil @@ -148,7 +151,7 @@ func (s *service) GetAppPassword(ctx context.Context, req *appauthpb.GetAppPassw pwd, err := s.am.GetAppPassword(ctx, req.User, req.Password) if err != nil { return &appauthpb.GetAppPasswordResponse{ - Status: status.NewInternal(ctx, "error getting app password via username/password"), + Status: status.NewStatusFromErrType(ctx, "error getting app password via username/password", err), }, nil } diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3.go b/pkg/appauth/manager/jsoncs3/jsoncs3.go index ea32249782..0074f505d7 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3.go @@ -116,20 +116,24 @@ func NewWithOptions(mds metadata.Storage, generator PasswordGenerator) (*manager // GenerateAppPassword creates a password with specified scope to be used by // third-party applications. func (m *manager) GenerateAppPassword(ctx context.Context, scope map[string]*authpb.Scope, label string, expiration *typespb.Timestamp) (*apppb.AppPassword, error) { + logger := appctx.GetLogger(ctx) ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "GenerateAppPassword") defer span.End() if err := m.initialize(ctx); err != nil { + logger.Error().Err(err).Msg("initializing appauth manager failed") span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return nil, err } token, err := m.generator.GeneratePassword() if err != nil { + logger.Debug().Err(err).Msg("error generating new password") return nil, errors.Wrap(err, "error creating new token") } tokenHashed, err := argon2id.CreateHash(token, argon2id.DefaultParams) if err != nil { + logger.Debug().Err(err).Msg("error generating password hash") return nil, errors.Wrap(err, "error creating new token") } @@ -137,6 +141,7 @@ func (m *manager) GenerateAppPassword(ctx context.Context, scope map[string]*aut if user, ok := ctxpkg.ContextGetUser(ctx); ok { userID = user.GetId() } else { + logger.Debug().Err(err).Msg("no user in context") return nil, errtypes.BadRequest("no user in context") } @@ -162,6 +167,7 @@ func (m *manager) GenerateAppPassword(ctx context.Context, scope map[string]*aut }) if err != nil { + logger.Debug().Err(err).Msg("failed to store new app password") return nil, err } @@ -317,6 +323,7 @@ func (m *manager) GetAppPassword(ctx context.Context, user *userpb.UserId, secre func (m *manager) initialize(ctx context.Context) error { _, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "initialize") + logger := appctx.GetLogger(ctx) defer span.End() if m.initialized { span.SetStatus(codes.Ok, "already initialized") @@ -332,6 +339,7 @@ func (m *manager) initialize(ctx context.Context) error { } ctx = context.Background() + ctx = appctx.WithLogger(ctx, logger) err := m.mds.Init(ctx, "jsoncs3-appauth-data") if err != nil { span.RecordError(err) diff --git a/pkg/storage/utils/metadata/cs3.go b/pkg/storage/utils/metadata/cs3.go index 85ae238d52..20d678a4b5 100644 --- a/pkg/storage/utils/metadata/cs3.go +++ b/pkg/storage/utils/metadata/cs3.go @@ -39,6 +39,7 @@ import ( "google.golang.org/grpc/metadata" "github.com/opencloud-eu/reva/v2/internal/http/services/owncloud/ocdav/net" + "github.com/opencloud-eu/reva/v2/pkg/appctx" ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/errtypes" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" @@ -97,16 +98,19 @@ func (cs3 *CS3) Backend() string { // Init creates the metadata space func (cs3 *CS3) Init(ctx context.Context, spaceid string) (err error) { + logger := appctx.GetLogger(ctx) ctx, span := tracer.Start(ctx, "Init") defer span.End() client, err := cs3.spacesClient() if err != nil { + logger.Err(err).Msg("error getting spaces client") return err } ctx, err = cs3.getAuthContext(ctx) if err != nil { + logger.Err(err).Msg("error getting auth context") return err } @@ -146,12 +150,14 @@ func (cs3 *CS3) Init(ctx context.Context, spaceid string) (err error) { }) switch { case err != nil: + logger.Err(err).Msg("error creating storage space") return err case cssr.Status.Code == rpc.Code_CODE_OK: cs3.SpaceRoot = cssr.StorageSpace.Root case cssr.Status.Code == rpc.Code_CODE_ALREADY_EXISTS: return errtypes.AlreadyExists(fmt.Sprintf("user %s does not have access to metadata space %s, but it exists", cs3.serviceUser.Id.OpaqueId, spaceid)) default: + logger.Debug().Str("Status", cssr.Status.Message).Msg("error creating storage space") return errtypes.NewErrtypeFromStatus(cssr.Status) } return nil From 0ca701f7fb7784d30779c3fdc9b8ae6408cb13ea Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 5 Feb 2026 08:50:22 +0100 Subject: [PATCH 2/7] feat(appauth): integration test for applicationauth including test for https://github.com/opencloud-eu/opencloud/issues/2023 --- pkg/appauth/manager/jsoncs3/jsoncs3.go | 35 ++-- pkg/appauth/manager/jsoncs3/jsoncs3_test.go | 2 +- .../integration/grpc/applicationauth_test.go | 159 ++++++++++++++++++ .../fixtures/applicationauth-jsoncs3.toml | 18 ++ .../grpc/fixtures/storage-system.toml | 84 +++++++++ 5 files changed, 287 insertions(+), 11 deletions(-) create mode 100644 tests/integration/grpc/applicationauth_test.go create mode 100644 tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml create mode 100644 tests/integration/grpc/fixtures/storage-system.toml diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3.go b/pkg/appauth/manager/jsoncs3/jsoncs3.go index 0074f505d7..bb5a0817a6 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3.go @@ -37,10 +37,11 @@ func init() { } type manager struct { - sync.RWMutex // for lazy initialization - mds metadata.Storage - generator PasswordGenerator - initialized bool + sync.RWMutex // for lazy initialization + mds metadata.Storage + generator PasswordGenerator + uTimeUpdateInterval time.Duration + initialized bool } type config struct { @@ -50,6 +51,9 @@ type config struct { MachineAuthAPIKey string `mapstructure:"machine_auth_apikey"` Generator string `mapstructure:"password_generator"` GeneratorConfig map[string]any `mapstructure:"generator_config"` + // Time interval in seconds to update the UTime of a token when calling GetAppPassword. Default is 5 min. + // For testing set this -1 to disable automatic updates. + UTimeUpdateInterval int `mapstructure:"utime_update_interval_seconds"` } type updaterFunc func(map[string]*apppb.AppPassword) (map[string]*apppb.AppPassword, error) @@ -83,6 +87,16 @@ func New(m map[string]any) (appauth.Manager, error) { c.Generator = "diceware" } + var updateInterval time.Duration + switch c.UTimeUpdateInterval { + case -1: + updateInterval = 0 + case 0: + updateInterval = 5 * time.Minute + default: + updateInterval = time.Duration(c.UTimeUpdateInterval) * time.Second + } + var pwgen PasswordGenerator var err error switch c.Generator { @@ -103,13 +117,14 @@ func New(m map[string]any) (appauth.Manager, error) { return nil, err } - return NewWithOptions(cs3, pwgen) + return NewWithOptions(cs3, pwgen, updateInterval) } -func NewWithOptions(mds metadata.Storage, generator PasswordGenerator) (*manager, error) { +func NewWithOptions(mds metadata.Storage, generator PasswordGenerator, uTimeUpdateInterval time.Duration) (*manager, error) { return &manager{ - mds: mds, - generator: generator, + mds: mds, + generator: generator, + uTimeUpdateInterval: uTimeUpdateInterval, }, nil } @@ -297,8 +312,8 @@ func (m *manager) GetAppPassword(ctx context.Context, user *userpb.UserId, secre matchedID = id // password not expired // Updating the Utime will cause an Upload for every single GetAppPassword request. We are limiting this to one - // update per 5 minutes otherwise this backend will become unusable. - if time.Since(utils.TSToTime(pw.Utime)) > 5*time.Minute { + // update per 'uTimeUpdateInterval' (default 5 min) otherwise this backend will become unusable. + if time.Since(utils.TSToTime(pw.Utime)) > m.uTimeUpdateInterval { a[id].Utime = utils.TSNow() return a, nil } diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3_test.go b/pkg/appauth/manager/jsoncs3/jsoncs3_test.go index 0d0707235d..e8784f2b30 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3_test.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3_test.go @@ -69,7 +69,7 @@ var _ = Describe("Jsoncs3", func() { md = mdMock.NewStorage(GinkgoT()) md.EXPECT().Init(mock.Anything, "jsoncs3-appauth-data").Return(nil).Once() - manager, err = jsoncs3.NewWithOptions(md, gen) + manager, err = jsoncs3.NewWithOptions(md, gen, 5*time.Minute) Expect(err).ToNot(HaveOccurred()) Expect(manager).ToNot(BeNil()) diff --git a/tests/integration/grpc/applicationauth_test.go b/tests/integration/grpc/applicationauth_test.go new file mode 100644 index 0000000000..109af16d54 --- /dev/null +++ b/tests/integration/grpc/applicationauth_test.go @@ -0,0 +1,159 @@ +// Copyright 2018-2021 CERN +// Copyright 2026 OpenCloud GmbH +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package grpc_test + +import ( + "context" + "sync" + + appauthpb "github.com/cs3org/go-cs3apis/cs3/auth/applications/v1beta1" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + "github.com/opencloud-eu/reva/v2/pkg/auth/scope" + ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + jwt "github.com/opencloud-eu/reva/v2/pkg/token/manager/jwt" + "google.golang.org/grpc/metadata" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("applicationauth providers", func() { + var ( + dependencies []RevadConfig + revads map[string]*Revad + variables map[string]string + + serviceClient appauthpb.ApplicationsAPIClient + + ctx context.Context + ) + + JustBeforeEach(func() { + var err error + ctx = context.Background() + + // Add auth token + user := &userpb.User{ + Id: &userpb.UserId{ + Idp: "http://idp", + OpaqueId: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", + Type: userpb.UserType_USER_TYPE_PRIMARY, + }, + } + tokenManager, err := jwt.New(map[string]interface{}{"secret": "secret"}) + Expect(err).ToNot(HaveOccurred()) + scope, err := scope.AddOwnerScope(nil) + Expect(err).ToNot(HaveOccurred()) + t, err := tokenManager.MintToken(ctx, user, scope) + Expect(err).ToNot(HaveOccurred()) + ctx = ctxpkg.ContextSetToken(ctx, t) + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, t) + ctx = ctxpkg.ContextSetUser(ctx, user) + + revads, err = startRevads(dependencies, variables) + Expect(err).ToNot(HaveOccurred()) + serviceClient, err = pool.GetAppAuthProviderServiceClient(revads["applicationauth"].GrpcAddress) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + for _, r := range revads { + Expect(r.Cleanup(CurrentSpecReport().Failed())).To(Succeed()) + } + }) + + Describe("the jsoncs3 appauth manager", func() { + BeforeEach(func() { + variables = map[string]string{ + "storage_system_grpc_address": "localhost:19002", + "storage_system_http_address": "localhost:19003", + } + dependencies = []RevadConfig{ + { + Name: "applicationauth", + Config: "applicationauth-jsoncs3.toml", + }, + { + Name: "storage-system", + Config: "storage-system.toml", + }, + } + }) + It("creates an app password", func() { + servResp, err := serviceClient.GenerateAppPassword(ctx, &appauthpb.GenerateAppPasswordRequest{}) + Expect(err).ToNot(HaveOccurred()) + Expect(servResp).ToNot(BeNil()) + Expect(servResp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + }) + It("verifies an app password", func() { + servResp, err := serviceClient.GenerateAppPassword(ctx, &appauthpb.GenerateAppPasswordRequest{}) + Expect(err).ToNot(HaveOccurred()) + Expect(servResp).ToNot(BeNil()) + Expect(servResp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + + resp, err := serviceClient.GetAppPassword(ctx, &appauthpb.GetAppPasswordRequest{ + User: ctxpkg.ContextMustGetUser(ctx).Id, + Password: servResp.GetAppPassword().GetPassword(), + }) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).ToNot(BeNil()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + + // make sure fail with a wrong password + resp, err = serviceClient.GetAppPassword(ctx, &appauthpb.GetAppPasswordRequest{ + User: ctxpkg.ContextMustGetUser(ctx).Id, + Password: "", + }) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).ToNot(BeNil()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_NOT_FOUND)) + }) + It("handles conncurrent access", func() { + servResp, err := serviceClient.GenerateAppPassword(ctx, &appauthpb.GenerateAppPasswordRequest{}) + Expect(err).ToNot(HaveOccurred()) + Expect(servResp).ToNot(BeNil()) + Expect(servResp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + + var numIterations = 10 + var concurrency = 3 + wg := &sync.WaitGroup{} + wg.Add(concurrency) + for i := 0; i < concurrency; i++ { + go func(wg *sync.WaitGroup) { + defer GinkgoRecover() + defer wg.Done() + for j := 0; j < numIterations; j++ { + resp, err := serviceClient.GetAppPassword(ctx, &appauthpb.GetAppPasswordRequest{ + User: ctxpkg.ContextMustGetUser(ctx).Id, + Password: servResp.GetAppPassword().GetPassword(), + }) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).ToNot(BeNil()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + } + }(wg) + } + wg.Wait() + Expect(true).ToNot(BeFalse()) + }) + }) +}) diff --git a/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml b/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml new file mode 100644 index 0000000000..637961baf7 --- /dev/null +++ b/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml @@ -0,0 +1,18 @@ +[shared] +jwt_secret = "secret" + +[log] +level = "debug" + +[grpc] +address = "{{grpc_address}}" + +[grpc.services.applicationauth] +driver = "jsoncs3" + +[grpc.services.applicationauth.drivers.jsoncs3] +provider_addr = "{{storage_system_grpc_address}}" +service_user_id = "service-user-id" +service_user_idp = "internal" +machine_auth_apikey = "secret" +utime_update_interval_seconds = -1 diff --git a/tests/integration/grpc/fixtures/storage-system.toml b/tests/integration/grpc/fixtures/storage-system.toml new file mode 100644 index 0000000000..ff7ba97fda --- /dev/null +++ b/tests/integration/grpc/fixtures/storage-system.toml @@ -0,0 +1,84 @@ +[shared] +jwt_secret = "secret" +#gatewaysvc = "cfg.Reva.Address" + +[log] +level = "debug" + +[grpc] +address = "{{storage_system_grpc_address}}" + +[grpc.services.gateway] +authregistrysvc = "{{storage_system_grpc_address}}" +storageregistrysvc = "{{storage_system_grpc_address}}" +userprovidersvc = "{{storage_system_grpc_address}}" +groupprovidersvc = "{{storage_system_grpc_address}}" +permissionssvc = "{{storage_system_grpc_address}}" +cache_store = "noop" +cache_database = "system" +disable_home_creation_on_login = true + +[grpc.services.userprovider] +driver = "memory" +[grpc.services.userprovider.drivers.memory] +users = { "serviceuser"= { "id" = { "opaqueId"= "service-user-id", "idp" = "internal", "type" = 3 }, "username" = "serviceuser", "display_name"= "System User" } } + +[grpc.services.authregistry] +driver = "static" + +[grpc.services.authregistry.drivers.static.rules] +machine = "{{storage_system_grpc_address}}" + +[grpc.services.authprovider] +auth_manager = "machine" + +[grpc.services.authprovider.auth_managers.machine] +api_key = "secret" +gateway_addr = "{{storage_system_grpc_address}}" + +[grpc.services.permissions] +driver = "demo" + +[grpc.services.storageregistry] +driver = "static" + +[grpc.services.storageregistry.drivers.static.rules] +"/" = {address = "{{storage_system_grpc_address}}" } + + +[grpc.services.storageprovider] +driver = "decomposed" +data_server_url = "http://{{storage_system_http_address}}/data" + +[grpc.services.storageprovider.drivers.decomposed] +root = "{{root}}/storage" +permissionssvc = "{{storage_system_grpc_address}}" +disable_versioning = true + +[http] +address = "{{storage_system_http_address}}" + +[http.services.dataprovider] +prefix = "data" +driver = "decomposed" + +[http.services.dataprovider.drivers.decomposed] +root = "{{root}}/storage" +permissionssvc = "{{storage_system_grpc_address}}" +disable_versioning = true + +[http.services.dataprovider.data_txs.simple] +cache_store ="noop" +cache_database = "system" +cache_table = "stat" + +[http.services.dataprovider.data_txs.tus] +cache_store ="noop" +cache_database = "system" +cache_table = "stat" + +[http.services.dataprovider.data_txs.spaces] +cache_store ="noop" +cache_database = "system" +cache_table = "stat" + From c8e54e1ea0b155f081d82cf522c9705f67cb4338 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 11 Feb 2026 12:45:06 +0100 Subject: [PATCH 3/7] fix: consistently return the same status with the etag condition fails According to the CS3 specs (https://github.com/cs3org/cs3apis/blob/main/cs3/rpc/v1beta1/code.proto) the correct status code to return for a request that a client "should retry on a higher level" is 'Aborted' (we map that to the HTTP Status "Precondition Failed"). The 'FailedPrecondition' CS3 status is supposed to be used for cases where a client should not retry (usually mapped to the HTTP Status "Conflict" or "Bad Request") --- internal/grpc/services/storageprovider/storageprovider.go | 4 ++-- pkg/appauth/manager/jsoncs3/jsoncs3.go | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 41738cda0c..a39b9eab8c 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -365,7 +365,7 @@ func (s *Service) InitiateFileUpload(ctx context.Context, req *provider.Initiate if ifMatch != "" { if !validateIfMatch(ifMatch, sRes.GetInfo()) { return &provider.InitiateFileUploadResponse{ - Status: status.NewFailedPrecondition(ctx, errors.New("etag mismatch"), "etag mismatch"), + Status: status.NewAborted(ctx, errors.New("etag mismatch"), "etag mismatch"), }, nil } metadata["if-match"] = ifMatch @@ -375,7 +375,7 @@ func (s *Service) InitiateFileUpload(ctx context.Context, req *provider.Initiate metadata["if-unmodified-since"] = utils.TSToTime(ifUnmodifiedSince).Format(time.RFC3339Nano) if !validateIfUnmodifiedSince(ifUnmodifiedSince, sRes.GetInfo()) { return &provider.InitiateFileUploadResponse{ - Status: status.NewFailedPrecondition(ctx, errors.New("resource has been modified"), "resource has been modified"), + Status: status.NewAborted(ctx, errors.New("resource has been modified"), "resource has been modified"), }, nil } } diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3.go b/pkg/appauth/manager/jsoncs3/jsoncs3.go index bb5a0817a6..c122ce0ca6 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3.go @@ -405,17 +405,20 @@ func (m *manager) updateWithRetry(ctx context.Context, retries int, createIfNotF switch err.(type) { case nil: retry = false - case errtypes.PreconditionFailed: + case errtypes.Aborted: + log.Debug().Err(err).Int("attempt", i).Msg("updateUserAppPassword failed (retrying)") retry = true default: + log.Debug().Err(err).Int("attempt", i).Msg("updateUserAppPassword failed (not retrying)") span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return err } } if retry { + log.Debug().Err(err).Msg("updateUserAppPassword failed") span.RecordError(err) - span.SetStatus(codes.Error, "updating app tokens failed") + span.SetStatus(codes.Error, "updating app token failed") return err } return nil From 9f5fe9c57669ba7dc99684511aa03d67c044e3ea Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 11 Feb 2026 14:09:23 +0100 Subject: [PATCH 4/7] fix(appauth): fix retry loop - check for the 'Aborted' status code for whether the upload should be retries. Due to a bug previously the storageprovider could return either 'Aborted' or 'FailedPrecondition'. We were only checking for 'FailedPrecondtion' and missed a couple of case for retry due to that. Now the storeageprovider always returns 'Aborted'. - workaround for 425 Status: Even with async_uploads disable we sometimes get a 425 Status when trying to download a file to which an upload is currently in progress (bug report pending). This adds a workaround to the jsoncs3 apptoken manager to retry the download after a short timeout. Fixes: https://github.com/opencloud-eu/opencloud/issues/2023 --- pkg/appauth/manager/jsoncs3/jsoncs3.go | 17 ++++++++++++++++- pkg/appauth/manager/jsoncs3/jsoncs3_test.go | 4 ++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3.go b/pkg/appauth/manager/jsoncs3/jsoncs3.go index c122ce0ca6..cf8df8d7e7 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "math/rand" "strings" "sync" "time" @@ -366,6 +367,7 @@ func (m *manager) initialize(ctx context.Context) error { } func (m *manager) updateWithRetry(ctx context.Context, retries int, createIfNotFound bool, userid *userpb.UserId, updater updaterFunc) error { + log := appctx.GetLogger(ctx) _, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "initialize") defer span.End() @@ -378,6 +380,12 @@ func (m *manager) updateWithRetry(ctx context.Context, retries int, createIfNotF // retry for the specified number of times, then error out for i := 0; i < retries && retry; i++ { + if i > 0 { + // if we're retrying, wait a bit before the next try + jitter := time.Duration(rand.Int63n(int64(100 * time.Millisecond))) + time.Sleep(jitter + 100*time.Millisecond) + } + etag, userAppPasswords, err = m.getUserAppPasswords(ctx, userid) switch err.(type) { case nil: @@ -386,11 +394,18 @@ func (m *manager) updateWithRetry(ctx context.Context, retries int, createIfNotF if createIfNotFound { userAppPasswords = map[string]*apppb.AppPassword{} } else { + log.Debug().Err(err).Msg("getUserAppPasswords failed (not found)") span.RecordError(err) span.SetStatus(codes.Error, "downloading app tokens failed") return err } + case errtypes.TooEarly: + // Ideally this should never happen as we disable asynchronous uploads for the metadata storage. + log.Debug().Err(err).Int("try", i).Msg("getUserAppPasswords failed (too early, retrying)") + retry = true + continue default: + log.Debug().Err(err).Msg("getUserAppPasswords failed") span.RecordError(err) span.SetStatus(codes.Error, "downloading app tokens failed") return err @@ -450,7 +465,7 @@ func (m *manager) updateUserAppPassword(ctx context.Context, userid *userpb.User if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - log.Debug().Err(err).Msg("persisting provider cache failed") + log.Debug().Err(err).Msg("failed to upload AppPasswword") return err } return nil diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3_test.go b/pkg/appauth/manager/jsoncs3/jsoncs3_test.go index e8784f2b30..ad98165999 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3_test.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3_test.go @@ -173,7 +173,7 @@ var _ = Describe("Jsoncs3", func() { err := json.Unmarshal(req.Content, &uploadedPw) return err == nil }), - ).Return(nil, errtypes.PreconditionFailed("etag mismatch")).Once() + ).Return(nil, errtypes.Aborted("etag mismatch")).Once() md.EXPECT().Upload( mock.Anything, mock.MatchedBy(func(req metadata.UploadRequest) bool { @@ -213,7 +213,7 @@ var _ = Describe("Jsoncs3", func() { err := json.Unmarshal(req.Content, &uploadedPw) return err == nil }), - ).Return(nil, errtypes.PreconditionFailed("etag mismatch")).Times(5) + ).Return(nil, errtypes.Aborted("etag mismatch")).Times(5) apppw, err := manager.GenerateAppPassword(ctx, scopes, "testing", nil) Expect(err).To(HaveOccurred()) Expect(len(uploadedPw)).To(Equal(2)) From 4965ba52caf5ea149fd75e75cfd5720dba667473 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 11 Feb 2026 17:28:33 +0100 Subject: [PATCH 5/7] extend test-timeout for integration test With the additional test the test-suite not often needs more that 10 minutes --- .woodpecker/test-integration.yaml | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.woodpecker/test-integration.yaml b/.woodpecker/test-integration.yaml index 3e6907bd2d..09b2ba8f43 100644 --- a/.woodpecker/test-integration.yaml +++ b/.woodpecker/test-integration.yaml @@ -40,4 +40,4 @@ steps: from_secret: ci_http_proxy commands: # `make test-integration` rebuilds the binaries, which is unnecessary in the pipeline, so only using `go test` here - - cd tests/integration && go test -race ./... + - cd tests/integration && go test -race ./... -timeout 15m diff --git a/Makefile b/Makefile index ea981e80a5..8d282cf318 100644 --- a/Makefile +++ b/Makefile @@ -119,7 +119,7 @@ test: off .PHONY: test-integration test-integration: build-ci - cd tests/integration && go test -race ./... + cd tests/integration && go test -timeout 15m -race ./... .PHONY: test-benchmark test-benchmark: From fee5441ca1995f465c3881c09101dfa9649f4e6d Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 12 Feb 2026 12:07:01 +0100 Subject: [PATCH 6/7] fix: avoid deprecated Ginkgo calls --- tests/integration/grpc/ocm_invitation_test.go | 2 +- tests/integration/grpc/ocm_share_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/grpc/ocm_invitation_test.go b/tests/integration/grpc/ocm_invitation_test.go index 6eb39396a8..19ec9d26c9 100644 --- a/tests/integration/grpc/ocm_invitation_test.go +++ b/tests/integration/grpc/ocm_invitation_test.go @@ -188,7 +188,7 @@ var _ = Describe("ocm invitation workflow", func() { AfterEach(func() { for _, r := range revads { - Expect(r.Cleanup(CurrentGinkgoTestDescription().Failed)).To(Succeed()) + Expect(r.Cleanup(CurrentSpecReport().Failed())).To(Succeed()) } Expect(os.RemoveAll(inviteTokenFile)).To(Succeed()) }) diff --git a/tests/integration/grpc/ocm_share_test.go b/tests/integration/grpc/ocm_share_test.go index ec0d6335ca..c35e7b6d94 100644 --- a/tests/integration/grpc/ocm_share_test.go +++ b/tests/integration/grpc/ocm_share_test.go @@ -191,7 +191,7 @@ var _ = Describe("ocm share", func() { AfterEach(func() { for _, r := range revads { - Expect(r.Cleanup(CurrentGinkgoTestDescription().Failed)).To(Succeed()) + Expect(r.Cleanup(CurrentSpecReport().Failed())).To(Succeed()) } }) From 5ad198d84cf681e8a09cb87d4570b123decbcc7a Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 17 Feb 2026 09:17:25 +0100 Subject: [PATCH 7/7] tests(appauth): Make number of retries configurable Allow to increase the number of retries for the apptoken update for testing. --- pkg/appauth/manager/jsoncs3/jsoncs3.go | 16 +++++++++++----- pkg/appauth/manager/jsoncs3/jsoncs3_test.go | 2 +- .../grpc/fixtures/applicationauth-jsoncs3.toml | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3.go b/pkg/appauth/manager/jsoncs3/jsoncs3.go index cf8df8d7e7..64b4addf33 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3.go @@ -42,6 +42,7 @@ type manager struct { mds metadata.Storage generator PasswordGenerator uTimeUpdateInterval time.Duration + updateRetryCount int initialized bool } @@ -55,6 +56,7 @@ type config struct { // Time interval in seconds to update the UTime of a token when calling GetAppPassword. Default is 5 min. // For testing set this -1 to disable automatic updates. UTimeUpdateInterval int `mapstructure:"utime_update_interval_seconds"` + UpdateRetryCount int `mapstructure:"update_retry_count"` } type updaterFunc func(map[string]*apppb.AppPassword) (map[string]*apppb.AppPassword, error) @@ -87,6 +89,9 @@ func New(m map[string]any) (appauth.Manager, error) { if c.Generator == "" { c.Generator = "diceware" } + if c.UpdateRetryCount <= 0 { + c.UpdateRetryCount = 5 + } var updateInterval time.Duration switch c.UTimeUpdateInterval { @@ -118,14 +123,15 @@ func New(m map[string]any) (appauth.Manager, error) { return nil, err } - return NewWithOptions(cs3, pwgen, updateInterval) + return NewWithOptions(cs3, pwgen, updateInterval, c.UpdateRetryCount) } -func NewWithOptions(mds metadata.Storage, generator PasswordGenerator, uTimeUpdateInterval time.Duration) (*manager, error) { +func NewWithOptions(mds metadata.Storage, generator PasswordGenerator, uTimeUpdateInterval time.Duration, updateRetries int) (*manager, error) { return &manager{ mds: mds, generator: generator, uTimeUpdateInterval: uTimeUpdateInterval, + updateRetryCount: updateRetries, }, nil } @@ -177,7 +183,7 @@ func (m *manager) GenerateAppPassword(ctx context.Context, scope map[string]*aut id := uuid.New().String() - err = m.updateWithRetry(ctx, 5, true, userID, func(a map[string]*apppb.AppPassword) (map[string]*apppb.AppPassword, error) { + err = m.updateWithRetry(ctx, m.updateRetryCount, true, userID, func(a map[string]*apppb.AppPassword) (map[string]*apppb.AppPassword, error) { a[id] = appPass return a, nil }) @@ -270,7 +276,7 @@ func (m *manager) InvalidateAppPassword(ctx context.Context, secretOrId string) return a, errtypes.NotFound("password not found") } - err := m.updateWithRetry(ctx, 5, false, userID, updater) + err := m.updateWithRetry(ctx, m.updateRetryCount, false, userID, updater) if err != nil { log.Error().Err(err).Msg("getUserAppPasswords failed") return errtypes.NotFound("password not found") @@ -324,7 +330,7 @@ func (m *manager) GetAppPassword(ctx context.Context, user *userpb.UserId, secre return nil, errtypes.NotFound("password not found") } - err := m.updateWithRetry(ctx, 5, false, user, updater) + err := m.updateWithRetry(ctx, m.updateRetryCount, false, user, updater) switch { case err == nil: fallthrough diff --git a/pkg/appauth/manager/jsoncs3/jsoncs3_test.go b/pkg/appauth/manager/jsoncs3/jsoncs3_test.go index ad98165999..c26df340c4 100644 --- a/pkg/appauth/manager/jsoncs3/jsoncs3_test.go +++ b/pkg/appauth/manager/jsoncs3/jsoncs3_test.go @@ -69,7 +69,7 @@ var _ = Describe("Jsoncs3", func() { md = mdMock.NewStorage(GinkgoT()) md.EXPECT().Init(mock.Anything, "jsoncs3-appauth-data").Return(nil).Once() - manager, err = jsoncs3.NewWithOptions(md, gen, 5*time.Minute) + manager, err = jsoncs3.NewWithOptions(md, gen, 5*time.Minute, 5) Expect(err).ToNot(HaveOccurred()) Expect(manager).ToNot(BeNil()) diff --git a/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml b/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml index 637961baf7..8a264f6a6b 100644 --- a/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml +++ b/tests/integration/grpc/fixtures/applicationauth-jsoncs3.toml @@ -16,3 +16,4 @@ service_user_id = "service-user-id" service_user_idp = "internal" machine_auth_apikey = "secret" utime_update_interval_seconds = -1 +update_retry_count = 25