diff --git a/services/auth-guest/pkg/server/http/errors.go b/services/auth-guest/pkg/server/http/errors.go index 164d38a0df..a0c0c75c84 100644 --- a/services/auth-guest/pkg/server/http/errors.go +++ b/services/auth-guest/pkg/server/http/errors.go @@ -26,6 +26,11 @@ func writeError(w http.ResponseWriter, status int, body errorResponse) { } func writeGuestError(w http.ResponseWriter, err error) { + if errors.Is(err, authguest.ErrEventsNotConfigured) { + writeError(w, http.StatusServiceUnavailable, errorResponse{ErrorType: "serviceUnavailable", Message: err.Error()}) + return + } + var ge *authguest.GuestError if !errors.As(err, &ge) { writeError(w, http.StatusInternalServerError, errorResponse{ErrorType: "internalError", Message: "An internal error occurred."}) diff --git a/services/auth-guest/pkg/server/http/renew_test.go b/services/auth-guest/pkg/server/http/renew_test.go index 11cf981df7..2bcf35910a 100644 --- a/services/auth-guest/pkg/server/http/renew_test.go +++ b/services/auth-guest/pkg/server/http/renew_test.go @@ -66,6 +66,12 @@ func TestRenewHandlerErrorMapping(t *testing.T) { wantStatus: http.StatusInternalServerError, wantType: "internalError", }, + { + name: "events not configured", + err: authguest.ErrEventsNotConfigured, + wantStatus: http.StatusServiceUnavailable, + wantType: "serviceUnavailable", + }, } for _, tt := range tests { diff --git a/services/auth-guest/pkg/service/authguest/service.go b/services/auth-guest/pkg/service/authguest/service.go index 461627a05b..d217f38de7 100644 --- a/services/auth-guest/pkg/service/authguest/service.go +++ b/services/auth-guest/pkg/service/authguest/service.go @@ -119,11 +119,14 @@ func (s *AuthGuestService) Redeem(ctx context.Context, tokenString string) (*Ses return nil, err } - if err := s.store.Redeem(rec.ShareIDHash); err != nil { - if errors.Is(err, storage.ErrAlreadyRedeemed) { - return nil, &GuestError{ErrorType: ErrAlreadyRedeemed, ShareID: rec.ShareID} + if err := s.store.Update(rec.ShareIDHash, func(r *storage.Record) error { + if r.Redeemed { + return ErrAlreadyRedeemed } - return nil, err + r.Redeemed = true + return nil + }); err != nil { + return nil, &GuestError{ErrorType: err, ShareID: rec.ShareID} } sessionToken, err := s.jwtService.Sign(rec.ShareID) diff --git a/services/auth-guest/pkg/service/authguest/service_test.go b/services/auth-guest/pkg/service/authguest/service_test.go index 379274c0a8..049741673e 100644 --- a/services/auth-guest/pkg/service/authguest/service_test.go +++ b/services/auth-guest/pkg/service/authguest/service_test.go @@ -253,10 +253,9 @@ func TestValidateShare(t *testing.T) { } func TestRedeem(t *testing.T) { - store := storagemocks.NewManager(t) + store := storage.NewFileManager(t.TempDir()) tok, rec := newToken(t) - store.On("Get", rec.ShareIDHash).Return(rec, nil) - store.On("Redeem", rec.ShareIDHash).Return(nil) + require.NoError(t, store.Add(rec)) share := &collaboration.Share{Id: &collaboration.ShareId{OpaqueId: testShareID}} s := newRedeemService(t, store, newGatewayMock(&collaboration.GetShareResponse{ @@ -269,14 +268,16 @@ func TestRedeem(t *testing.T) { require.NotEmpty(t, result.SessionToken) assert.Equal(t, testShareID, result.ShareID) - store.AssertCalled(t, "Redeem", rec.ShareIDHash) + got, err := store.Get(rec.ShareIDHash) + require.NoError(t, err) + assert.True(t, got.Redeemed) } func TestRedeemAlreadyRedeemed(t *testing.T) { - store := storagemocks.NewManager(t) + store := storage.NewFileManager(t.TempDir()) tok, rec := newToken(t) - store.On("Get", rec.ShareIDHash).Return(rec, nil) - store.On("Redeem", rec.ShareIDHash).Return(storage.ErrAlreadyRedeemed) + rec.Redeemed = true + require.NoError(t, store.Add(rec)) share := &collaboration.Share{Id: &collaboration.ShareId{OpaqueId: testShareID}} s := newRedeemService(t, store, newGatewayMock(&collaboration.GetShareResponse{ diff --git a/services/auth-guest/pkg/service/storage/file_manager.go b/services/auth-guest/pkg/service/storage/file_manager.go index 6b30c0f9f2..0222993662 100644 --- a/services/auth-guest/pkg/service/storage/file_manager.go +++ b/services/auth-guest/pkg/service/storage/file_manager.go @@ -83,26 +83,6 @@ func (s *FileManager) Remove(shareIDHash string) error { return nil } -func (s *FileManager) Redeem(shareIDHash string) error { - lock, err := s.lockRecord(shareIDHash) - if err != nil { - return err - } - defer func() { _ = lock.Unlock() }() - - rec, err := s.get(shareIDHash) - if err != nil { - return err - } - - if rec.Redeemed { - return ErrAlreadyRedeemed - } - - rec.Redeemed = true - return s.upsert(rec) -} - // Update applies fn to the record under the record lock and writes it back. // If fn returns an error the record is left unchanged. func (s *FileManager) Update(shareIDHash string, fn func(*Record) error) error { diff --git a/services/auth-guest/pkg/service/storage/file_manager_test.go b/services/auth-guest/pkg/service/storage/file_manager_test.go index 4eadba1006..ef7c25438f 100644 --- a/services/auth-guest/pkg/service/storage/file_manager_test.go +++ b/services/auth-guest/pkg/service/storage/file_manager_test.go @@ -95,31 +95,6 @@ func TestFileManagerRemoveMissing(t *testing.T) { assert.ErrorIs(t, err, ErrNotFound) } -func TestFileManagerRedeem(t *testing.T) { - dir := t.TempDir() - s := NewFileManager(dir) - - rec := newRecord("e0123456-7890-abcd-ef01-234567890abc") - require.NoError(t, s.Add(rec)) - - require.NoError(t, s.Redeem(rec.ShareIDHash)) - - got, err := s.Get(rec.ShareIDHash) - require.NoError(t, err) - assert.True(t, got.Redeemed) - - err = s.Redeem(rec.ShareIDHash) - assert.ErrorIs(t, err, ErrAlreadyRedeemed) -} - -func TestFileManagerRedeemMissing(t *testing.T) { - dir := t.TempDir() - s := NewFileManager(dir) - - err := s.Redeem("doesnotexist") - assert.ErrorIs(t, err, ErrNotFound) -} - func TestFileManagerUpdate(t *testing.T) { dir := t.TempDir() s := NewFileManager(dir) @@ -191,7 +166,7 @@ func TestFileManagerAddConcurrent(t *testing.T) { assert.Equal(t, int32(1), success.Load()) } -func TestFileManagerRedeemConcurrent(t *testing.T) { +func TestFileManagerUpdateConcurrent(t *testing.T) { dir := t.TempDir() s := NewFileManager(dir) @@ -207,7 +182,14 @@ func TestFileManagerRedeemConcurrent(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - if err := s.Redeem(rec.ShareIDHash); err == nil { + err := s.Update(rec.ShareIDHash, func(r *Record) error { + if r.Redeemed { + return errors.New("already set") + } + r.Redeemed = true + return nil + }) + if err == nil { success.Add(1) } }() diff --git a/services/auth-guest/pkg/service/storage/mocks/manager.go b/services/auth-guest/pkg/service/storage/mocks/manager.go index aa0c066f69..8336d3f9e4 100644 --- a/services/auth-guest/pkg/service/storage/mocks/manager.go +++ b/services/auth-guest/pkg/service/storage/mocks/manager.go @@ -147,57 +147,6 @@ func (_c *Manager_Get_Call) RunAndReturn(run func(shareIDHash string) (storage.R return _c } -// Redeem provides a mock function for the type Manager -func (_mock *Manager) Redeem(shareIDHash string) error { - ret := _mock.Called(shareIDHash) - - if len(ret) == 0 { - panic("no return value specified for Redeem") - } - - var r0 error - if returnFunc, ok := ret.Get(0).(func(string) error); ok { - r0 = returnFunc(shareIDHash) - } else { - r0 = ret.Error(0) - } - return r0 -} - -// Manager_Redeem_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Redeem' -type Manager_Redeem_Call struct { - *mock.Call -} - -// Redeem is a helper method to define mock.On call -// - shareIDHash string -func (_e *Manager_Expecter) Redeem(shareIDHash any) *Manager_Redeem_Call { - return &Manager_Redeem_Call{Call: _e.mock.On("Redeem", shareIDHash)} -} - -func (_c *Manager_Redeem_Call) Run(run func(shareIDHash string)) *Manager_Redeem_Call { - _c.Call.Run(func(args mock.Arguments) { - var arg0 string - if args[0] != nil { - arg0 = args[0].(string) - } - run( - arg0, - ) - }) - return _c -} - -func (_c *Manager_Redeem_Call) Return(err error) *Manager_Redeem_Call { - _c.Call.Return(err) - return _c -} - -func (_c *Manager_Redeem_Call) RunAndReturn(run func(shareIDHash string) error) *Manager_Redeem_Call { - _c.Call.Return(run) - return _c -} - // Remove provides a mock function for the type Manager func (_mock *Manager) Remove(shareIDHash string) error { ret := _mock.Called(shareIDHash) diff --git a/services/auth-guest/pkg/service/storage/storage.go b/services/auth-guest/pkg/service/storage/storage.go index 58883c7cfd..00bb56ce1b 100644 --- a/services/auth-guest/pkg/service/storage/storage.go +++ b/services/auth-guest/pkg/service/storage/storage.go @@ -9,7 +9,6 @@ import ( ) var ErrNotFound = errors.New("record not found") -var ErrAlreadyRedeemed = errors.New("token already redeemed") var ErrInvalidHash = errors.New("invalid share id hash") // Record holds the data persisted for a guest share token. @@ -27,6 +26,5 @@ type Manager interface { Add(rec Record) error Get(shareIDHash string) (Record, error) Remove(shareIDHash string) error - Redeem(shareIDHash string) error Update(shareIDHash string, fn func(*Record) error) error }