Files
coder/coderd/webpush_test.go
T
Cian Johnston d787b3cada fix(coderd): fix error handling in deleteUserWebpushSubscription (#22500)
## Summary

`deleteUserWebpushSubscription` in `coderd/webpush.go` had incorrect
error handling that masked database errors as 404 responses.

## Bug

`GetWebpushSubscriptionsByUserID` is a `:many` query — it returns `([],
nil)` when no rows match, never `sql.ErrNoRows`. The previous `if/else
if` chain:

```go
if existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID); err != nil && errors.Is(err, sql.ErrNoRows) {
    // dead code — :many queries never return sql.ErrNoRows
} else if idx := slices.IndexFunc(existing, ...); idx == -1 {
    // real DB errors fall through here, existing is nil, idx is -1 → 404
}
```

Any real database error (connection failure, timeout, authorization
error) fell through to the `else if` branch where `slices.IndexFunc(nil,
...)` returns `-1`, returning 404 "subscription not found" instead of
500.

## Fix

Split into two separate checks so database errors properly return 500:

```go
existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID)
if err != nil {
    // 500
}
if idx := slices.IndexFunc(existing, ...); idx == -1 {
    // 404
}
```

## Testing

Added `TestDeleteWebpushSubscription/database_error_returns_500` which
wraps the DB store to inject an error into
`GetWebpushSubscriptionsByUserID` and asserts the handler returns 500
(not 404).
2026-03-02 21:11:20 +00:00

140 lines
4.8 KiB
Go

package coderd_test
import (
"context"
"net/http"
"net/http/httptest"
"sync/atomic"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)
const (
// These are valid keys for a web push subscription.
// DO NOT REUSE THESE IN ANY REAL CODE.
validEndpointAuthKey = "zqbxT6JKstKSY9JKibZLSQ=="
validEndpointP256dhKey = "BNNL5ZaTfK81qhXOx23+wewhigUeFb632jN6LvRWCFH1ubQr77FE/9qV1FuojuRmHP42zmf34rXgW80OvUVDgTk="
)
func TestWebpushSubscribeUnsubscribe(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
dv := coderdtest.DeploymentValues(t)
dv.Experiments = []string{string(codersdk.ExperimentWebPush)}
client := coderdtest.New(t, &coderdtest.Options{
DeploymentValues: dv,
})
owner := coderdtest.CreateFirstUser(t, client)
memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
_, anotherMember := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
handlerCalled := make(chan bool, 1)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusCreated)
handlerCalled <- true
}))
defer server.Close()
err := memberClient.PostWebpushSubscription(ctx, "me", codersdk.WebpushSubscription{
Endpoint: server.URL,
AuthKey: validEndpointAuthKey,
P256DHKey: validEndpointP256dhKey,
})
require.NoError(t, err, "create webpush subscription")
require.True(t, <-handlerCalled, "handler should have been called")
err = memberClient.PostTestWebpushMessage(ctx)
require.NoError(t, err, "test webpush message")
require.True(t, <-handlerCalled, "handler should have been called again")
err = memberClient.DeleteWebpushSubscription(ctx, "me", codersdk.DeleteWebpushSubscription{
Endpoint: server.URL,
})
require.NoError(t, err, "delete webpush subscription")
// Deleting the subscription for a non-existent endpoint should return a 404
err = memberClient.DeleteWebpushSubscription(ctx, "me", codersdk.DeleteWebpushSubscription{
Endpoint: server.URL,
})
var sdkError *codersdk.Error
require.Error(t, err)
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
require.Equal(t, http.StatusNotFound, sdkError.StatusCode())
// Creating a subscription for another user should not be allowed.
err = memberClient.PostWebpushSubscription(ctx, anotherMember.ID.String(), codersdk.WebpushSubscription{
Endpoint: server.URL,
AuthKey: validEndpointAuthKey,
P256DHKey: validEndpointP256dhKey,
})
require.Error(t, err, "create webpush subscription for another user")
// Deleting a subscription for another user should not be allowed.
err = memberClient.DeleteWebpushSubscription(ctx, anotherMember.ID.String(), codersdk.DeleteWebpushSubscription{
Endpoint: server.URL,
})
require.Error(t, err, "delete webpush subscription for another user")
}
// testWebpushErrorStore wraps a real database.Store and allows injecting
// errors into GetWebpushSubscriptionsByUserID.
type testWebpushErrorStore struct {
database.Store
getWebpushSubscriptionsErr atomic.Pointer[error]
}
func (s *testWebpushErrorStore) GetWebpushSubscriptionsByUserID(ctx context.Context, userID uuid.UUID) ([]database.WebpushSubscription, error) {
if err := s.getWebpushSubscriptionsErr.Load(); err != nil {
return nil, *err
}
return s.Store.GetWebpushSubscriptionsByUserID(ctx, userID)
}
func TestDeleteWebpushSubscription(t *testing.T) {
t.Parallel()
t.Run("database error returns 500", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitMedium)
store, ps := dbtestutil.NewDB(t)
wrappedStore := &testWebpushErrorStore{Store: store}
dv := coderdtest.DeploymentValues(t)
dv.Experiments = []string{string(codersdk.ExperimentWebPush)}
client := coderdtest.New(t, &coderdtest.Options{
DeploymentValues: dv,
Database: wrappedStore,
Pubsub: ps,
})
owner := coderdtest.CreateFirstUser(t, client)
memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
// Inject a database error into
// GetWebpushSubscriptionsByUserID. The handler should
// return 500, not mask the error as 404.
dbErr := xerrors.New("database is unavailable")
wrappedStore.getWebpushSubscriptionsErr.Store(&dbErr)
err := memberClient.DeleteWebpushSubscription(ctx, "me", codersdk.DeleteWebpushSubscription{
Endpoint: "https://push.example.com/test",
})
var sdkError *codersdk.Error
require.Error(t, err)
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
require.Equal(t, http.StatusInternalServerError, sdkError.StatusCode(), "database errors should return 500, not be masked as 404")
})
}