Do not leak information about tokens in errors

services/auth-guest/pkg/server/http/errors.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 
 	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/authguest"
-	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/storage"
 	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/token"
 )
 
@@ -39,10 +38,6 @@ func writeRedeemError(w http.ResponseWriter, err error) {
 		status, errorType = http.StatusUnauthorized, "tokenExpired"
 	case errors.Is(re.ErrorType, token.ErrInvalidToken):
 		status, errorType = http.StatusUnauthorized, "tokenInvalid"
-	case errors.Is(re.ErrorType, storage.ErrNotFound):
-		status, errorType = http.StatusNotFound, "tokenNotFound"
-	case errors.Is(re.ErrorType, storage.ErrInvalidHash):
-		status, errorType = http.StatusUnauthorized, "tokenInvalid"
 	case errors.Is(re.ErrorType, authguest.ErrAlreadyRedeemed):
 		status, errorType = http.StatusConflict, "tokenAlreadyRedeemed"
 	case errors.Is(re.ErrorType, authguest.ErrShareNotFound):

services/auth-guest/pkg/server/http/redeem_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config"
 	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/authguest"
 	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/authguest/mocks"
-	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/storage"
 	"github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/token"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -83,12 +82,6 @@ func TestRedeemHandlerErrorMapping(t *testing.T) {
 			wantStatus: http.StatusUnauthorized,
 			wantType:   "tokenInvalid",
 		},
-		{
-			name:       "token not found",
-			err:        &authguest.RedeemError{ErrorType: storage.ErrNotFound},
-			wantStatus: http.StatusNotFound,
-			wantType:   "tokenNotFound",
-		},
 		{
 			name:       "token already redeemed",
 			err:        &authguest.RedeemError{ErrorType: authguest.ErrAlreadyRedeemed},

services/auth-guest/pkg/service/authguest/service.go

@@ -134,19 +134,26 @@ func (s *AuthGuestService) CleanupShare(shareID string) error {
 }
 
 // VerifyToken validates a token and returns its stored record.
+//
+// Until the secret has been verified, all failures are reported as
+// token.ErrInvalidToken without a share id, so that a caller holding only part
+// of a token learns neither the share id nor whether a record exists.
 func (s *AuthGuestService) verifyToken(tokenString string) (*storage.Record, error) {
 	tok, err := s.tokenSvc.Parse(tokenString)
 	if err != nil {
-		return nil, &RedeemError{ErrorType: err}
+		return nil, &RedeemError{ErrorType: token.ErrInvalidToken}
 	}
 
 	rec, err := s.store.Get(tok.ShareIDHash)
-	if err != nil {
-		return nil, &RedeemError{ErrorType: err}
+	switch {
+	case errors.Is(err, storage.ErrNotFound), errors.Is(err, storage.ErrInvalidHash):
+		return nil, &RedeemError{ErrorType: token.ErrInvalidToken}
+	case err != nil:
+		return nil, err
 	}
 
 	if err := s.tokenSvc.Verify(*tok, rec.SecretHash); err != nil {
-		return nil, &RedeemError{ErrorType: err, ShareID: rec.ShareID}
+		return nil, &RedeemError{ErrorType: token.ErrInvalidToken}
 	}
 
 	if !rec.Expiry.IsZero() && rec.Expiry.Before(time.Now()) {

services/auth-guest/pkg/service/authguest/service_test.go

@@ -5,6 +5,8 @@ package authguest
 
 import (
 	"context"
+	"errors"
+	"strings"
 	"testing"
 	"time"
 
@@ -158,6 +160,86 @@ func TestVerifyToken(t *testing.T) {
 	}
 }
 
+func TestVerifyTokenDoesNotDiscloseShare(t *testing.T) {
+	tok, rec := newToken(t)
+	// flip the last character of the secret
+	last := "x"
+	if strings.HasSuffix(tok, last) {
+		last = "y"
+	}
+	tamperedSecret := tok[:len(tok)-1] + last
+
+	tests := []struct {
+		name  string
+		token string
+		setup func(store *storagemocks.Manager)
+	}{
+		{
+			name:  "malformed token",
+			token: "not-a-token",
+		},
+		{
+			name:  "unknown record",
+			token: tok,
+			setup: func(store *storagemocks.Manager) {
+				store.On("Get", rec.ShareIDHash).Return(storage.Record{}, storage.ErrNotFound)
+			},
+		},
+		{
+			name:  "invalid hash",
+			token: "v1.ab.secret",
+			setup: func(store *storagemocks.Manager) {
+				store.On("Get", "ab").Return(storage.Record{}, storage.ErrInvalidHash)
+			},
+		},
+		{
+			name:  "wrong secret",
+			token: tamperedSecret,
+			setup: func(store *storagemocks.Manager) {
+				store.On("Get", rec.ShareIDHash).Return(rec, nil)
+			},
+		},
+		{
+			name:  "wrong secret on expired and redeemed record",
+			token: tamperedSecret,
+			setup: func(store *storagemocks.Manager) {
+				r := rec
+				r.Expiry = time.Now().Add(-time.Hour)
+				r.Redeemed = true
+				store.On("Get", rec.ShareIDHash).Return(r, nil)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			store := storagemocks.NewManager(t)
+			if tt.setup != nil {
+				tt.setup(store)
+			}
+			s := NewAuthGuestService(token.NewTokenService(), store)
+
+			_, err := s.verifyToken(tt.token)
+			var re *RedeemError
+			require.ErrorAs(t, err, &re)
+			assert.ErrorIs(t, re.ErrorType, token.ErrInvalidToken)
+			assert.Empty(t, re.ShareID)
+		})
+	}
+}
+
+func TestVerifyTokenStorageFailure(t *testing.T) {
+	tok, rec := newToken(t)
+	store := storagemocks.NewManager(t)
+	store.On("Get", rec.ShareIDHash).Return(storage.Record{}, errors.New("disk on fire"))
+	s := NewAuthGuestService(token.NewTokenService(), store)
+
+	_, err := s.verifyToken(tok)
+	require.Error(t, err)
+	var re *RedeemError
+	assert.False(t, errors.As(err, &re), "storage failures must surface as internal errors")
+}
+
 func TestValidateShare(t *testing.T) {
 	share := &collaboration.Share{Id: &collaboration.ShareId{OpaqueId: testShareID}}
 	notExpiredShare := &collaboration.Share{