Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/pkg/httpserver/create_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ func (s *Server) CreateSubscription(
Message: "user not authorized to create this subscription using the specified channel",
},
), nil
} else if errors.Is(err, backendtypes.ErrUserMaxSubscriptions) {
return backend.CreateSubscription403JSONResponse(
backend.BasicErrorModel{
Code: http.StatusForbidden,
Message: "user has reached the maximum number of allowed subscriptions",
},
), nil
}
slog.ErrorContext(ctx, "failed to create subscription", "error", err)

Expand Down
30 changes: 30 additions & 0 deletions backend/pkg/httpserver/create_subscription_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,36 @@ func TestCreateSubscription(t *testing.T) {
"message":"user not authorized to create this subscription using the specified channel"
}`),
},
{
name: "forbidden - user max subscriptions reached",
cfg: &MockCreateSavedSearchSubscriptionConfig{
expectedUserID: "test-user",
expectedSubscription: backend.Subscription{
ChannelId: "channel-id",
SavedSearchId: "search-id",
Triggers: []backend.SubscriptionTriggerWritable{
backend.SubscriptionTriggerFeatureBrowserImplementationAnyComplete},
Frequency: "immediate",
},
output: nil,
err: backendtypes.ErrUserMaxSubscriptions,
},
expectedCallCount: 1,
authMiddlewareOption: withAuthMiddleware(mockAuthMiddleware(testUser)),
request: httptest.NewRequest(
http.MethodPost,
"/v1/users/me/subscriptions",
strings.NewReader(`{
"channel_id": "channel-id",
"saved_search_id": "search-id",
"triggers": ["feature_browser_implementation_any_complete"],
"frequency": "immediate"
}`)),
expectedResponse: testJSONResponse(http.StatusForbidden, `{
"code":403,
"message":"user has reached the maximum number of allowed subscriptions"
}`),
},
{
name: "internal server error",
cfg: &MockCreateSavedSearchSubscriptionConfig{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {expect, fixture, html, waitUntil} from '@open-wc/testing';
import sinon from 'sinon';
import {APIClient} from '../../api/client.js';
import {ForbiddenError} from '../../api/errors.js';
import {UserContext} from '../../contexts/firebase-user-context.js';
import '../webstatus-manage-subscriptions-dialog.js';
import {ManageSubscriptionsDialog} from '../webstatus-manage-subscriptions-dialog.js';
Expand Down Expand Up @@ -375,6 +376,34 @@ describe('webstatus-manage-subscriptions-dialog', () => {
}
});

it('sets actionState correctly on subscription limit exceeded error', async () => {
const limitError = new ForbiddenError(
'user has reached the maximum number of allowed subscriptions',
);
(apiClient.createSubscription as sinon.SinonStub).returns(
Promise.reject(limitError),
);

const eventSpy = sandbox.spy();
element.addEventListener('subscription-save-error', eventSpy);

element.savedSearchId = 'test-search-id';
element['_activeChannelId'] = mockNotificationChannels[0].id;
element['_selectedFrequency'] = 'monthly'; // Make it dirty
element['_initialSelectedFrequency'] = 'immediate';

await element['_handleSave']();
await element.updateComplete;

expect(eventSpy).to.have.been.calledOnce;
expect(element['_actionState'].phase).to.equal('error');
if (element['_actionState'].phase === 'error') {
expect(element['_actionState'].message).to.equal(
'user has reached the maximum number of allowed subscriptions',
);
}
});

it('sets actionState correctly on successful delete', async () => {
// Create a new element configured exactly for this test's needs.
const deleteElement = await fixture<ManageSubscriptionsDialog>(html`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
} from '../contexts/firebase-user-context.js';
import {SlCheckbox, SlDialog, SlRadioGroup} from '@shoelace-style/shoelace';
import {FREQUENCY_DISPLAY_NAMES} from '../utils/format.js';
import {ApiError} from '../api/errors.js';

const FREQUENCY_CONFIG: ReadonlyArray<
components['schemas']['SubscriptionFrequency']
Expand Down Expand Up @@ -637,11 +638,19 @@ export class ManageSubscriptionsDialog extends LitElement {
message: 'Subscription saved.',
};
} catch (e) {
const error = e instanceof Error ? e : new Error('Unknown error saving');
let message: string;
let error: Error;
if (e instanceof ApiError) {
message = e.message;
error = e;
} else {
error = e instanceof Error ? e : new Error('Unknown error saving');
message = `Error saving subscription: ${error.message}`;
}
this.dispatchEvent(new SubscriptionSaveErrorEvent(error));
this._actionState = {
phase: 'error',
message: `Error saving subscription: ${error.message}`,
message: message,
};
}
}
Expand Down
4 changes: 4 additions & 0 deletions lib/backendtypes/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ var (
// number of allowed bookmarks.
ErrUserMaxBookmarks = errors.New("user has reached the maximum number of allowed bookmarks")

// ErrUserMaxSubscriptions indicates the user has reached the maximum
// number of allowed subscriptions.
ErrUserMaxSubscriptions = errors.New("user has reached the maximum number of allowed subscriptions")

// ErrUserNotAuthorizedForAction indicates the user is not authorized to execute the requested action.
ErrUserNotAuthorizedForAction = errors.New("user not authorized to execute action")

Expand Down
4 changes: 4 additions & 0 deletions lib/gcpspanner/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ type searchConfig struct {
maxOwnedSearchesPerUser uint32
// Max number of bookmarks per user (excluding the saved searches they own)
maxBookmarksPerUser uint32
// Max number of subscriptions per user
maxSubscriptionsPerUser uint32
}

// notificationConfig holds the application configuation for notifications.
Expand All @@ -149,6 +151,7 @@ type notificationConfig struct {

const defaultMaxOwnedSearchesPerUser = 25
const defaultMaxBookmarksPerUser = 25
const defaultMaxSubscriptionsPerUser = 25
const defaultBatchSize = 5000
const defaultBatchWriters = 8
const defaultMaxConsecutiveFailuresPerChannel = 5
Expand Down Expand Up @@ -226,6 +229,7 @@ func NewSpannerClient(projectID string, instanceID string, name string) (*Client
searchConfig{
maxOwnedSearchesPerUser: defaultMaxOwnedSearchesPerUser,
maxBookmarksPerUser: defaultMaxBookmarksPerUser,
maxSubscriptionsPerUser: defaultMaxSubscriptionsPerUser,
},
notificationConfig{
maxConsecutiveFailuresPerChannel: defaultMaxConsecutiveFailuresPerChannel,
Expand Down
31 changes: 30 additions & 1 deletion lib/gcpspanner/saved_search_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ const (
SubscriptionTriggerUnknown SubscriptionTrigger = "unknown"
)

var (
// ErrSubscriptionLimitExceeded indicates that the user already has
// reached the limit of subscriptions that a given user can own.
ErrSubscriptionLimitExceeded = errors.New("subscription limit reached")
)

// CreateSavedSearchSubscriptionRequest is the request to create a subscription.
type CreateSavedSearchSubscriptionRequest struct {
UserID string
Expand Down Expand Up @@ -239,7 +245,30 @@ func (c *Client) createSavedSearchSubscription(
) (*string, error) {
var id *string
_, err := c.ReadWriteTransaction(ctx, func(ctx context.Context, txn *spanner.ReadWriteTransaction) error {
err := c.checkNotificationChannelOwnership(ctx, req.ChannelID, req.UserID, txn)
// 1. Check limit
var count int64
stmt := spanner.Statement{
SQL: `SELECT COUNT(*)
FROM SavedSearchSubscriptions sc
JOIN NotificationChannels nc ON sc.ChannelID = nc.ID
WHERE nc.UserID = @userID`,
Params: map[string]interface{}{
"userID": req.UserID,
},
}
row, err := txn.Query(ctx, stmt).Next()
if err != nil {
return err
}
if err := row.Columns(&count); err != nil {
return err
}

if count >= int64(c.searchCfg.maxSubscriptionsPerUser) {
return ErrSubscriptionLimitExceeded
}

err = c.checkNotificationChannelOwnership(ctx, req.ChannelID, req.UserID, txn)
if err != nil {
return err
}
Expand Down
59 changes: 59 additions & 0 deletions lib/gcpspanner/saved_search_subscription_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package gcpspanner

import (
"context"
"errors"
"slices"
"testing"
"time"
Expand Down Expand Up @@ -525,3 +526,61 @@ func TestFindAllActivePushSubscriptions(t *testing.T) {
// t.Error("did not find the expected WEBHOOK subscriber")
// }
}

func TestCreateSavedSearchSubscriptionLimitExceeded(t *testing.T) {
ctx := context.Background()
restartDatabaseContainer(t)

userID := uuid.NewString()

// Pre-populate dependencies
channelReq := CreateNotificationChannelRequest{
UserID: userID,
Name: "Test",
Type: NotificationChannelTypeEmail,
EmailConfig: &EmailConfig{Address: "test@example.com", IsVerified: true, VerificationToken: nil},
}
channelIDPtr, err := spannerClient.CreateNotificationChannel(ctx, channelReq)
if err != nil {
t.Fatalf("failed to create notification channel: %v", err)
}
channelID := *channelIDPtr

savedSearchIDPtr, err := spannerClient.CreateNewUserSavedSearch(ctx, CreateUserSavedSearchRequest{
Name: "Test Search",
Query: "is:widely",
OwnerUserID: userID,
Description: nil,
})
if err != nil {
t.Fatalf("failed to create saved search: %v", err)
}
savedSearchID := *savedSearchIDPtr

// Create subscriptions up to the limit
limit := defaultMaxSubscriptionsPerUser
for i := 0; i < limit; i++ {
_, err := spannerClient.CreateSavedSearchSubscription(ctx, CreateSavedSearchSubscriptionRequest{
UserID: userID,
ChannelID: channelID,
SavedSearchID: savedSearchID,
Triggers: []SubscriptionTrigger{SubscriptionTriggerFeatureBaselineRegressionToLimited},
Frequency: SavedSearchSnapshotTypeImmediate,
})
if err != nil {
t.Fatalf("failed to create subscription %d: %v", i, err)
}
}

// Try to create one more
_, err = spannerClient.CreateSavedSearchSubscription(ctx, CreateSavedSearchSubscriptionRequest{
UserID: userID,
ChannelID: channelID,
SavedSearchID: savedSearchID,
Triggers: []SubscriptionTrigger{SubscriptionTriggerFeatureBaselineRegressionToLimited},
Frequency: SavedSearchSnapshotTypeImmediate,
})
if !errors.Is(err, ErrSubscriptionLimitExceeded) {
t.Errorf("expected ErrSubscriptionLimitExceeded, got %v", err)
}
}
2 changes: 2 additions & 0 deletions lib/gcpspanner/spanneradapters/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,8 @@ func (s *Backend) CreateSavedSearchSubscription(ctx context.Context,
if err != nil {
if errors.Is(err, gcpspanner.ErrMissingRequiredRole) {
return nil, errors.Join(err, backendtypes.ErrUserNotAuthorizedForAction)
} else if errors.Is(err, gcpspanner.ErrSubscriptionLimitExceeded) {
return nil, errors.Join(err, backendtypes.ErrUserMaxSubscriptions)
}

return nil, err
Expand Down
27 changes: 27 additions & 0 deletions lib/gcpspanner/spanneradapters/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,33 @@ func (c mockBackendSpannerClient) UpdateSavedSearchSubscription(
return c.mockUpdateSavedSearchSubscriptionCfg.returnedError
}

func TestCreateSavedSearchSubscriptionMapsLimitError(t *testing.T) {
mock := new(mockBackendSpannerClient)
mock.t = t
mock.mockCreateSavedSearchSubscriptionCfg = &mockCreateSavedSearchSubscriptionConfig{
expectedRequest: gcpspanner.CreateSavedSearchSubscriptionRequest{
UserID: "user",
ChannelID: "channel",
SavedSearchID: "search",
Triggers: []gcpspanner.SubscriptionTrigger{},
Frequency: gcpspanner.SavedSearchSnapshotTypeImmediate,
},
result: nil,
returnedError: gcpspanner.ErrSubscriptionLimitExceeded,
}

bk := NewBackend(mock)
_, err := bk.CreateSavedSearchSubscription(context.Background(), "user", backend.Subscription{
ChannelId: "channel",
SavedSearchId: "search",
Triggers: []backend.SubscriptionTriggerWritable{},
Frequency: backend.SubscriptionFrequencyImmediate,
})
if !errors.Is(err, backendtypes.ErrUserMaxSubscriptions) {
t.Errorf("expected ErrUserMaxSubscriptions, got %v", err)
}
}

func TestListMetricsForFeatureIDBrowserAndChannel(t *testing.T) {
testCases := []struct {
name string
Expand Down
Loading