diff --git a/api/cache_test.go b/api/cache_test.go index fa49f5a6..161099e8 100644 --- a/api/cache_test.go +++ b/api/cache_test.go @@ -21,7 +21,7 @@ func TestUserTierCache(t *testing.T) { if ok || tier != database.TierAnonymous { t.Fatalf("Expected to get tier %d and %t, got %d and %t.", database.TierAnonymous, false, tier, ok) } - // Set the use in the cache. + // Set the user in the cache. cache.Set(u.Sub, u) // Check again. tier, qe, ok := cache.Get(u.Sub) diff --git a/api/handlers.go b/api/handlers.go index dfbad232..d121894a 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -731,6 +731,34 @@ func (api *API) userPUT(u *database.User, w http.ResponseWriter, req *http.Reque api.loginUser(w, u, true) } +// userPubKeyDELETE removes a given pubkey from the list of pubkeys associated +// with this user. It does not require a challenge-response because the used +// does not need to prove the key is theirs. +func (api *API) userPubKeyDELETE(u *database.User, w http.ResponseWriter, req *http.Request, ps httprouter.Params) { + ctx := req.Context() + var pk database.PubKey + err := pk.LoadString(ps.ByName("pubKey")) + if err != nil { + api.WriteError(w, err, http.StatusBadRequest) + return + } + if !u.HasKey(pk) { + // This pubkey does not belong to this user. + api.WriteError(w, errors.New("the given pubkey is not associated with this user"), http.StatusBadRequest) + return + } + err = api.staticDB.UserPubKeyRemove(ctx, *u, pk) + if errors.Contains(err, mongo.ErrNoDocuments) { + api.WriteError(w, err, http.StatusNotFound) + return + } + if err != nil { + api.WriteError(w, err, http.StatusInternalServerError) + return + } + api.WriteSuccess(w) +} + // userPubKeyRegisterGET generates an update challenge for the caller. func (api *API) userPubKeyRegisterGET(u *database.User, w http.ResponseWriter, req *http.Request, _ httprouter.Params) { ctx := req.Context() @@ -814,8 +842,12 @@ func (api *API) userPubKeyRegisterPOST(u *database.User, w http.ResponseWriter, api.WriteError(w, errors.New("user's sub doesn't match update sub"), http.StatusBadRequest) return } - u.PubKeys = append(u.PubKeys, pk) - err = api.staticDB.UserSave(ctx, u) + err = api.staticDB.UserPubKeyAdd(ctx, *u, pk) + if err != nil { + api.WriteError(w, err, http.StatusInternalServerError) + return + } + updatedUser, err := api.staticDB.UserByID(ctx, u.ID) if err != nil { api.WriteError(w, err, http.StatusInternalServerError) return @@ -825,7 +857,7 @@ func (api *API) userPubKeyRegisterPOST(u *database.User, w http.ResponseWriter, api.WriteError(w, err, http.StatusInternalServerError) return } - api.loginUser(w, u, true) + api.loginUser(w, updatedUser, true) } // userUploadsGET returns all uploads made by the current user. diff --git a/api/routes.go b/api/routes.go index 1a221197..7d9b48d3 100644 --- a/api/routes.go +++ b/api/routes.go @@ -53,6 +53,7 @@ func (api *API) buildHTTPRoutes() { api.staticRouter.GET("/user/limits", api.noAuth(api.userLimitsGET)) api.staticRouter.GET("/user/limits/:skylink", api.noAuth(api.userLimitsSkylinkGET)) api.staticRouter.GET("/user/stats", api.withAuth(api.userStatsGET)) + api.staticRouter.DELETE("/user/pubkey/:pubKey", api.WithDBSession(api.withAuth(api.userPubKeyDELETE))) api.staticRouter.GET("/user/pubkey/register", api.WithDBSession(api.withAuth(api.userPubKeyRegisterGET))) api.staticRouter.POST("/user/pubkey/register", api.WithDBSession(api.withAuth(api.userPubKeyRegisterPOST))) api.staticRouter.GET("/user/uploads", api.withAuth(api.userUploadsGET)) diff --git a/database/user.go b/database/user.go index 4d8343b0..704ba073 100644 --- a/database/user.go +++ b/database/user.go @@ -124,10 +124,7 @@ type ( SubscriptionCancelAtPeriodEnd bool `bson:"subscription_cancel_at_period_end" json:"subscriptionCancelAtPeriodEnd"` StripeID string `bson:"stripe_id" json:"stripeCustomerId"` QuotaExceeded bool `bson:"quota_exceeded" json:"quotaExceeded"` - // The currently active (or default) key is going to be the first one in - // the list. If we want to activate a new pubkey, we'll just move it to - // the first position in the list. - PubKeys []PubKey `bson:"pub_keys" json:"-"` + PubKeys []PubKey `bson:"pub_keys" json:"-"` } // UserStats contains statistical information about the user. UserStats struct { @@ -494,6 +491,52 @@ func (db *DB) UserSave(ctx context.Context, u *User) error { return nil } +// UserPubKeyAdd adds a new PubKey to the given user's set. +func (db *DB) UserPubKeyAdd(ctx context.Context, u User, pk PubKey) (err error) { + filter := bson.M{"_id": u.ID} + // This update is so complicated because we can't use mutation operations + // like $push, $addToSet and so on if the target field is null. That's why + // here we check if the field is an array and then merge the key in. If the + // field is not an array (i.e. it's null) we set it to an empty array before + // performing the merge. + update := bson.A{ + bson.M{ + "$set": bson.M{ + "pub_keys": bson.M{ + "$cond": bson.A{ + bson.M{"$eq": bson.A{bson.M{"$type": "$pub_keys"}, "array"}}, + bson.M{"$setUnion": bson.A{"$pub_keys", bson.A{pk}}}, + bson.A{pk}, + }}, + }, + }, + } + opts := options.UpdateOptions{ + Upsert: &False, + } + _, err = db.staticUsers.UpdateOne(ctx, filter, update, &opts) + return err +} + +// UserPubKeyRemove removes a PubKey from the given user's set. +func (db *DB) UserPubKeyRemove(ctx context.Context, u User, pk PubKey) error { + filter := bson.M{ + "_id": u.ID, + "pub_keys": bson.M{"$ne": nil}, + } + update := bson.M{ + "$pull": bson.M{"pub_keys": pk}, + } + opts := options.UpdateOptions{ + Upsert: &False, + } + ur, err := db.staticUsers.UpdateOne(ctx, filter, update, &opts) + if err == nil && ur.MatchedCount == 0 { + err = mongo.ErrNoDocuments + } + return err +} + // UserSetStripeID changes the user's stripe id in the DB. func (db *DB) UserSetStripeID(ctx context.Context, u *User, stripeID string) error { filter := bson.M{"_id": u.ID} diff --git a/test/api/apikeys_test.go b/test/api/apikeys_test.go index 023dbaf0..102691ca 100644 --- a/test/api/apikeys_test.go +++ b/test/api/apikeys_test.go @@ -228,6 +228,11 @@ func testPublicAPIKeysFlow(t *testing.T, at *test.AccountsTester) { if len(aks) != 0 { t.Fatalf("Expected no API keys, got %d.", len(aks)) } + // Delete the same key again. Expect a 404. + status, err = at.UserAPIKeysDELETE(akr.ID) + if status != http.StatusNotFound { + t.Fatal("Expected status 404, got", status) + } } // testPublicAPIKeysUsage makes sure that we can use public API keys to make @@ -261,12 +266,12 @@ func testPublicAPIKeysUsage(t *testing.T, at *test.AccountsTester) { Public: true, Skylinks: []string{sl.Skylink}, } - akWithKey, _, err := at.UserAPIKeysPOST(apiKeyPOST) + pakWithKey, _, err := at.UserAPIKeysPOST(apiKeyPOST) if err != nil { t.Fatal(err) } // Stop using the cookie, use the public API key instead. - at.SetAPIKey(akWithKey.Key.String()) + at.SetAPIKey(pakWithKey.Key.String()) // Try to fetch the user's stats with the new public API key. // Expect this to fail. _, _, err = at.Get("/user/stats", nil) @@ -275,7 +280,7 @@ func testPublicAPIKeysUsage(t *testing.T, at *test.AccountsTester) { } // Get the user's limits for downloading a skylink covered by the public // API key. Expect to get TierFree values. - ul, _, err := at.UserLimitsSkylink(sl.Skylink, "byte", nil) + ul, _, err := at.UserLimitsSkylink(sl.Skylink, "byte", "", nil) if err != nil { t.Fatal(err) } @@ -284,11 +289,22 @@ func testPublicAPIKeysUsage(t *testing.T, at *test.AccountsTester) { } // Get the user's limits for downloading a skylink that is not covered by // the public API key. Expect to get TierAnonymous values. - ul, _, err = at.UserLimitsSkylink(sl2.Skylink, "byte", nil) + ul, _, err = at.UserLimitsSkylink(sl2.Skylink, "byte", "", nil) if err != nil { t.Fatal(err) } if ul.DownloadBandwidth != database.UserLimits[database.TierAnonymous].DownloadBandwidth { t.Fatalf("Expected to get download bandwidth of %d, got %d", database.UserLimits[database.TierAnonymous].DownloadBandwidth, ul.DownloadBandwidth) } + // Stop using the header, pass the skylink as a query parameter. + at.ClearCredentials() + // Get the user's limits for downloading a skylink covered by the public + // API key. Expect to get TierFree values. + ul, _, err = at.UserLimitsSkylink(sl.Skylink, "byte", pakWithKey.Key.String(), nil) + if err != nil { + t.Fatal(err) + } + if ul.DownloadBandwidth != database.UserLimits[database.TierFree].DownloadBandwidth { + t.Fatalf("Expected to get download bandwidth of %d, got %d", database.UserLimits[database.TierFree].DownloadBandwidth, ul.DownloadBandwidth) + } } diff --git a/test/api/challenge_test.go b/test/api/challenge_test.go index c3a91ab3..b5e628a5 100644 --- a/test/api/challenge_test.go +++ b/test/api/challenge_test.go @@ -309,3 +309,93 @@ func testUserAddPubKey(t *testing.T, at *test.AccountsTester) { t.Fatalf("Expected pubKey '%s', got '%s',", hex.EncodeToString(pk[:]), hex.EncodeToString(u3.PubKeys[0])) } } + +// testUserDeletePubKey ensures that users can delete pubkeys from their +// accounts. +func testUserDeletePubKey(t *testing.T, at *test.AccountsTester) { + name := test.DBNameForTest(t.Name()) + u, c, err := test.CreateUserAndLogin(at, name) + if err != nil { + t.Fatal("Failed to create a user and log in:", err) + } + defer func() { + if err = u.Delete(at.Ctx); err != nil { + t.Error(errors.AddContext(err, "failed to delete user in defer")) + } + }() + at.SetCookie(c) + defer at.ClearCredentials() + + sk, pk := crypto.GenerateKeyPair() + var ch database.Challenge + + // Request a new challenge. + queryParams := url.Values{} + queryParams.Set("pubKey", hex.EncodeToString(pk[:])) + r, b, err := at.Get("/user/pubkey/register", queryParams) + err = json.Unmarshal(b, &ch) + if err != nil || r.StatusCode != http.StatusOK { + t.Fatal("Failed to get a challenge:", err, r.Status, string(b)) + } + chBytes, err := hex.DecodeString(ch.Challenge) + if err != nil { + t.Fatal("Invalid challenge:", err) + } + // Solve the challenge. + response := append(chBytes, append([]byte(database.ChallengeTypeUpdate), []byte(database.PortalName)...)...) + bodyParams := url.Values{} + bodyParams.Set("response", hex.EncodeToString(response)) + bodyParams.Set("signature", hex.EncodeToString(ed25519.Sign(sk[:], response))) + r, b, err = at.Post("/user/pubkey/register", nil, bodyParams) + if err != nil { + t.Fatalf("Failed to confirm the update. Status %d, body '%s', error '%s'", r.StatusCode, string(b), err) + } + // Make sure the user's pubKey is properly set. + u1, err := at.DB.UserBySub(at.Ctx, u.Sub) + if err != nil { + t.Fatal(err) + } + if len(u1.PubKeys) != 1 { + t.Fatal("Expected one pubkey assigned, got none.") + } + if subtle.ConstantTimeCompare(u1.PubKeys[0], pk[:]) != 1 { + t.Fatalf("Expected pubKey '%s', got '%s',", hex.EncodeToString(pk[:]), hex.EncodeToString(u1.PubKeys[0])) + } + + // Call DELETE without a cookie. + at.ClearCredentials() + r, b, err = at.Delete("/user/pubkey/"+hex.EncodeToString(pk[:]), nil) + if err == nil || r.StatusCode != http.StatusUnauthorized { + t.Fatalf("Expected to fail with 401. Status %d, body '%s', error '%s'", r.StatusCode, string(b), err) + } + at.SetCookie(c) + // Call DELETE with an invalid key. + r, b, err = at.Delete("/user/pubkey/INVALID_KEY", nil) + if err == nil || r.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected to fail with 400. Status %d, body '%s', error '%s'", r.StatusCode, string(b), err) + } + _, pk1 := crypto.GenerateKeyPair() + // Call DELETE with a key that doesn't belong to this user. + r, b, err = at.Delete("/user/pubkey/"+hex.EncodeToString(pk1[:]), nil) + if err == nil || r.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected to fail with 400. Status %d, body '%s', error '%s'", r.StatusCode, string(b), err) + } + // Call DELETE with correct parameters. + r, b, err = at.Delete("/user/pubkey/"+hex.EncodeToString(pk[:]), nil) + if err != nil || r.StatusCode != http.StatusNoContent { + t.Fatalf("Expected to succeed. Status %d, body '%s', error '%s'", r.StatusCode, string(b), err) + } + // Verify that the key was deleted. + u2, err := at.DB.UserBySub(at.Ctx, u.Sub) + if err != nil { + t.Fatal(err) + } + if len(u2.PubKeys) > 0 { + t.Fatal("Expected no public keys, got", len(u2.PubKeys)) + } + // Call DELETE with the already deleted key. + r, b, err = at.Delete("/user/pubkey/"+hex.EncodeToString(pk[:]), nil) + if err == nil || r.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected to fail with 400. Status %d, body '%s', error '%s'", r.StatusCode, string(b), err) + } +} diff --git a/test/api/handlers_test.go b/test/api/handlers_test.go index 5b05b809..c7dc04a2 100644 --- a/test/api/handlers_test.go +++ b/test/api/handlers_test.go @@ -59,6 +59,7 @@ func TestHandlers(t *testing.T) { {name: "LoginLogout", test: testHandlerLoginPOST}, {name: "UserEdit", test: testUserPUT}, {name: "UserAddPubKey", test: testUserAddPubKey}, + {name: "DeletePubKey", test: testUserDeletePubKey}, {name: "UserDelete", test: testUserDELETE}, {name: "UserLimits", test: testUserLimits}, {name: "UserDeleteUploads", test: testUserUploadsDELETE}, diff --git a/test/database/user_test.go b/test/database/user_test.go index 81d086cf..de7faf57 100644 --- a/test/database/user_test.go +++ b/test/database/user_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "crypto/subtle" "reflect" "testing" "time" @@ -466,6 +467,90 @@ func TestUserSetStripeID(t *testing.T) { } } +// TestUserPubKey tests UserPubKeyAdd and UserPubKeyRemove. +func TestUserPubKey(t *testing.T) { + ctx := context.Background() + dbName := test.DBNameForTest(t.Name()) + db, err := database.NewCustomDB(ctx, dbName, test.DBTestCredentials(), nil) + if err != nil { + t.Fatal(err) + } + // Create a test user. + u, err := db.UserCreate(ctx, t.Name()+"@siasky.net", t.Name()+"pass", t.Name()+"sub", database.TierFree) + if err != nil { + t.Fatal(err) + } + defer func(user *database.User) { + _ = db.UserDelete(ctx, user) + }(u) + pk := database.PubKey(make([]byte, database.PubKeySize)) + copy(pk[:], fastrand.Bytes(database.PubKeySize)) + pk1 := database.PubKey(make([]byte, database.PubKeySize)) + copy(pk1[:], fastrand.Bytes(database.PubKeySize)) + // Try to remove a pubkey. Expect this to fail. + err = db.UserPubKeyRemove(ctx, *u, pk) + if err == nil { + t.Fatal(err) + } + // Add a pubkey. + err = db.UserPubKeyAdd(ctx, *u, pk) + if err != nil { + t.Fatal(err) + } + u1, err := db.UserByID(ctx, u.ID) + if err != nil { + t.Fatal(err) + } + if len(u1.PubKeys) == 1 && subtle.ConstantTimeCompare(u1.PubKeys[0][:], pk[:]) != 1 { + t.Fatalf("Expected the user to have a single pubkey which matches ours. Got %+v, pubkey %+v", u1.PubKeys, pk) + } + // Add another. + err = db.UserPubKeyAdd(ctx, *u, pk) + if err != nil { + t.Fatal(err) + } + u2, err := db.UserByID(ctx, u.ID) + if err != nil { + t.Fatal(err) + } + if len(u2.PubKeys) == 2 && subtle.ConstantTimeCompare(u1.PubKeys[1][:], pk1[:]) == 1 { + t.Fatalf("Expected the user to have a single pubkey which matches ours. Got %+v, pubkey %+v", u2.PubKeys, pk1) + } + // Delete a pubkey. + err = db.UserPubKeyRemove(ctx, *u, pk) + if err != nil { + t.Fatal(err) + } + u3, err := db.UserByID(ctx, u.ID) + if err != nil { + t.Fatal(err) + } + if len(u3.PubKeys) == 1 && subtle.ConstantTimeCompare(u3.PubKeys[0][:], pk1[:]) == 1 { + t.Fatalf("Expected the user to have a single pubkey which matches ours. Got %+v, pubkey %+v", u3.PubKeys, pk1) + } + // Make sure UserPubKeyRemove removes all copies of the pubkey from the set. + // We don't expect there to be multiple but we still want to make sure. + u.PubKeys = make([]database.PubKey, 0) + u.PubKeys = append(u.PubKeys, pk) + u.PubKeys = append(u.PubKeys, pk) + u.PubKeys = append(u.PubKeys, pk) + err = db.UserSave(ctx, u) + if err != nil { + t.Fatal(err) + } + err = db.UserPubKeyRemove(ctx, *u, pk) + if err != nil { + t.Fatal(err) + } + u4, err := db.UserByID(ctx, u.ID) + if err != nil { + t.Fatal(err) + } + if len(u4.PubKeys) > 0 { + t.Fatal("Expected zero pubkeys.") + } +} + // TestUserSetTier ensures that UserSetTier works as expected. func TestUserSetTier(t *testing.T) { ctx := context.Background() diff --git a/test/tester.go b/test/tester.go index 0ed24e77..ab4ff637 100644 --- a/test/tester.go +++ b/test/tester.go @@ -475,11 +475,10 @@ func (at *AccountsTester) UserLimits(unit string, headers map[string]string) (ap } // UserLimitsSkylink performs a `GET /user/limits/:skylink` request. -func (at *AccountsTester) UserLimitsSkylink(sl string, unit string, headers map[string]string) (api.UserLimitsGET, int, error) { +func (at *AccountsTester) UserLimitsSkylink(sl string, unit, apikey string, headers map[string]string) (api.UserLimitsGET, int, error) { queryParams := url.Values{} - if unit != "" { - queryParams.Set("unit", unit) - } + queryParams.Set("unit", unit) + queryParams.Set("apiKey", apikey) if !database.ValidSkylinkHash(sl) { return api.UserLimitsGET{}, 0, database.ErrInvalidSkylink }