From aa1bda4c58d20e7d14267f9c87fab8efd7ae36ad Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Mon, 27 Mar 2023 11:26:26 +0200 Subject: [PATCH 1/9] Add AS invite test, fix issue with invitations being processed twice (#3020) The AS roomserver consumer would receive the events twice, one time as type `OutputTypeNewInviteEvent` and the other time as `OutputTypeNewRoomEvent`. [skip ci] --- appservice/appservice_test.go | 85 ++++++++++++++++++++++++++++++ appservice/consumers/roomserver.go | 6 --- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/appservice/appservice_test.go b/appservice/appservice_test.go index 6c8a07b5c..752901a9c 100644 --- a/appservice/appservice_test.go +++ b/appservice/appservice_test.go @@ -16,10 +16,12 @@ import ( "github.com/matrix-org/dendrite/internal/caching" "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/roomserver" + rsapi "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/setup/jetstream" "github.com/matrix-org/dendrite/test" "github.com/matrix-org/dendrite/userapi" + "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/dendrite/test/testrig" ) @@ -212,3 +214,86 @@ func testProtocol(t *testing.T, asAPI api.AppServiceInternalAPI, proto string, w t.Errorf("unexpected result for Protocols(%s): %+v, expected %+v", proto, protoResp.Protocols[proto], wantResult) } } + +// Tests that the roomserver consumer only receives one invite +func TestRoomserverConsumerOneInvite(t *testing.T) { + + alice := test.NewUser(t) + bob := test.NewUser(t) + room := test.NewRoom(t, alice) + + // Invite Bob + room.CreateAndInsert(t, alice, gomatrixserverlib.MRoomMember, map[string]interface{}{ + "membership": "invite", + }, test.WithStateKey(bob.ID)) + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, closeDB := testrig.CreateConfig(t, dbType) + defer closeDB() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + natsInstance := &jetstream.NATSInstance{} + + evChan := make(chan struct{}) + // create a dummy AS url, handling the events + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var txn gomatrixserverlib.ApplicationServiceTransaction + err := json.NewDecoder(r.Body).Decode(&txn) + if err != nil { + t.Fatal(err) + } + for _, ev := range txn.Events { + if ev.Type != gomatrixserverlib.MRoomMember { + continue + } + // Usually we would check the event content for the membership, but since + // we only invited bob, this should be fine for this test. + if ev.StateKey != nil && *ev.StateKey == bob.ID { + evChan <- struct{}{} + } + } + })) + defer srv.Close() + + // Create a dummy application service + cfg.AppServiceAPI.Derived.ApplicationServices = []config.ApplicationService{ + { + ID: "someID", + URL: srv.URL, + ASToken: "", + HSToken: "", + SenderLocalpart: "senderLocalPart", + NamespaceMap: map[string][]config.ApplicationServiceNamespace{ + "users": {{RegexpObject: regexp.MustCompile(bob.ID)}}, + "aliases": {{RegexpObject: regexp.MustCompile(room.ID)}}, + }, + }, + } + + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + // Create required internal APIs + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, natsInstance, caches, caching.DisableMetrics) + rsAPI.SetFederationAPI(nil, nil) + usrAPI := userapi.NewInternalAPI(processCtx, cfg, cm, natsInstance, rsAPI, nil) + // start the consumer + appservice.NewInternalAPI(processCtx, cfg, natsInstance, usrAPI, rsAPI) + + // Create the room + if err := rsapi.SendEvents(context.Background(), rsAPI, rsapi.KindNew, room.Events(), "test", "test", "test", nil, false); err != nil { + t.Fatalf("failed to send events: %v", err) + } + var seenInvitesForBob int + waitLoop: + for { + select { + case <-time.After(time.Millisecond * 50): // wait for the AS to process the events + break waitLoop + case <-evChan: + seenInvitesForBob++ + if seenInvitesForBob != 1 { + t.Fatalf("received unexpected invites: %d", seenInvitesForBob) + } + } + } + close(evChan) + }) +} diff --git a/appservice/consumers/roomserver.go b/appservice/consumers/roomserver.go index 528de63e8..16b3b8231 100644 --- a/appservice/consumers/roomserver.go +++ b/appservice/consumers/roomserver.go @@ -140,12 +140,6 @@ func (s *OutputRoomEventConsumer) onMessage( } } - case api.OutputTypeNewInviteEvent: - if output.NewInviteEvent == nil || !s.appserviceIsInterestedInEvent(ctx, output.NewInviteEvent.Event, state.ApplicationService) { - continue - } - events = append(events, output.NewInviteEvent.Event) - default: continue } From e8b2162a01bf0e735869d5a2b9be258cb380255e Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Mon, 27 Mar 2023 11:26:52 +0200 Subject: [PATCH 2/9] Add `/search` tests (#3025) --- internal/fulltext/bleve.go | 47 ++++++ internal/fulltext/bleve_test.go | 46 ++++-- internal/fulltext/bleve_wasm.go | 5 + syncapi/routing/search.go | 77 ++++++---- syncapi/routing/search_test.go | 264 ++++++++++++++++++++++++++++++++ 5 files changed, 389 insertions(+), 50 deletions(-) create mode 100644 syncapi/routing/search_test.go diff --git a/internal/fulltext/bleve.go b/internal/fulltext/bleve.go index dea7c504c..f7412470d 100644 --- a/internal/fulltext/bleve.go +++ b/internal/fulltext/bleve.go @@ -18,6 +18,7 @@ package fulltext import ( + "regexp" "strings" "github.com/blevesearch/bleve/v2" @@ -60,6 +61,7 @@ type Indexer interface { Index(elements ...IndexElement) error Delete(eventID string) error Search(term string, roomIDs, keys []string, limit, from int, orderByStreamPos bool) (*bleve.SearchResult, error) + GetHighlights(result *bleve.SearchResult) []string Close() error } @@ -124,6 +126,47 @@ func (f *Search) Delete(eventID string) error { return f.FulltextIndex.Delete(eventID) } +var highlightMatcher = regexp.MustCompile("(.*?)") + +// GetHighlights extracts the highlights from a SearchResult. +func (f *Search) GetHighlights(result *bleve.SearchResult) []string { + if result == nil { + return []string{} + } + + seenMatches := make(map[string]struct{}) + + for _, hit := range result.Hits { + if hit.Fragments == nil { + continue + } + fragments, ok := hit.Fragments["Content"] + if !ok { + continue + } + for _, x := range fragments { + substringMatches := highlightMatcher.FindAllStringSubmatch(x, -1) + for _, matches := range substringMatches { + for i := range matches { + if i == 0 { // skip first match, this is the complete substring match + continue + } + if _, ok := seenMatches[matches[i]]; ok { + continue + } + seenMatches[matches[i]] = struct{}{} + } + } + } + } + + res := make([]string, 0, len(seenMatches)) + for m := range seenMatches { + res = append(res, m) + } + return res +} + // Search searches the index given a search term, roomIDs and keys. func (f *Search) Search(term string, roomIDs, keys []string, limit, from int, orderByStreamPos bool) (*bleve.SearchResult, error) { qry := bleve.NewConjunctionQuery() @@ -163,6 +206,10 @@ func (f *Search) Search(term string, roomIDs, keys []string, limit, from int, or s.SortBy([]string{"-StreamPosition"}) } + // Highlight some words + s.Highlight = bleve.NewHighlight() + s.Highlight.Fields = []string{"Content"} + return f.FulltextIndex.Search(s) } diff --git a/internal/fulltext/bleve_test.go b/internal/fulltext/bleve_test.go index bd8289d58..a77c23937 100644 --- a/internal/fulltext/bleve_test.go +++ b/internal/fulltext/bleve_test.go @@ -160,14 +160,16 @@ func TestSearch(t *testing.T) { roomIndex []int } tests := []struct { - name string - args args - wantCount int - wantErr bool + name string + args args + wantCount int + wantErr bool + wantHighlights []string }{ { - name: "Can search for many results in one room", - wantCount: 16, + name: "Can search for many results in one room", + wantCount: 16, + wantHighlights: []string{"lorem"}, args: args{ term: "lorem", roomIndex: []int{0}, @@ -175,8 +177,9 @@ func TestSearch(t *testing.T) { }, }, { - name: "Can search for one result in one room", - wantCount: 1, + name: "Can search for one result in one room", + wantCount: 1, + wantHighlights: []string{"lorem"}, args: args{ term: "lorem", roomIndex: []int{16}, @@ -184,8 +187,9 @@ func TestSearch(t *testing.T) { }, }, { - name: "Can search for many results in multiple rooms", - wantCount: 17, + name: "Can search for many results in multiple rooms", + wantCount: 17, + wantHighlights: []string{"lorem"}, args: args{ term: "lorem", roomIndex: []int{0, 16}, @@ -193,8 +197,9 @@ func TestSearch(t *testing.T) { }, }, { - name: "Can search for many results in all rooms, reversed", - wantCount: 30, + name: "Can search for many results in all rooms, reversed", + wantCount: 30, + wantHighlights: []string{"lorem"}, args: args{ term: "lorem", limit: 30, @@ -202,8 +207,9 @@ func TestSearch(t *testing.T) { }, }, { - name: "Can search for specific search room name", - wantCount: 1, + name: "Can search for specific search room name", + wantCount: 1, + wantHighlights: []string{"testing"}, args: args{ term: "testing", roomIndex: []int{}, @@ -212,8 +218,9 @@ func TestSearch(t *testing.T) { }, }, { - name: "Can search for specific search room topic", - wantCount: 1, + name: "Can search for specific search room topic", + wantCount: 1, + wantHighlights: []string{"fulltext"}, args: args{ term: "fulltext", roomIndex: []int{}, @@ -222,6 +229,7 @@ func TestSearch(t *testing.T) { }, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { f, ctx := mustOpenIndex(t, "") @@ -238,6 +246,12 @@ func TestSearch(t *testing.T) { t.Errorf("Search() error = %v, wantErr %v", err, tt.wantErr) return } + + highlights := f.GetHighlights(got) + if !reflect.DeepEqual(highlights, tt.wantHighlights) { + t.Errorf("Search() got highligts = %v, want %v", highlights, tt.wantHighlights) + } + if !reflect.DeepEqual(len(got.Hits), tt.wantCount) { t.Errorf("Search() got = %v, want %v", len(got.Hits), tt.wantCount) } diff --git a/internal/fulltext/bleve_wasm.go b/internal/fulltext/bleve_wasm.go index 0053ed8c2..12709900b 100644 --- a/internal/fulltext/bleve_wasm.go +++ b/internal/fulltext/bleve_wasm.go @@ -33,6 +33,7 @@ type Indexer interface { Index(elements ...IndexElement) error Delete(eventID string) error Search(term string, roomIDs, keys []string, limit, from int, orderByStreamPos bool) (SearchResult, error) + GetHighlights(result SearchResult) []string Close() error } @@ -71,3 +72,7 @@ func (f *Search) Delete(eventID string) error { func (f *Search) Search(term string, roomIDs, keys []string, limit, from int, orderByStreamPos bool) (SearchResult, error) { return SearchResult{}, nil } + +func (f *Search) GetHighlights(result SearchResult) []string { + return []string{} +} diff --git a/syncapi/routing/search.go b/syncapi/routing/search.go index 13625b9cb..69fa52942 100644 --- a/syncapi/routing/search.go +++ b/syncapi/routing/search.go @@ -19,7 +19,6 @@ import ( "net/http" "sort" "strconv" - "strings" "time" "github.com/blevesearch/bleve/v2/search" @@ -123,8 +122,8 @@ func Search(req *http.Request, device *api.Device, syncDB storage.Database, fts return util.JSONResponse{ Code: http.StatusOK, JSON: SearchResponse{ - SearchCategories: SearchCategories{ - RoomEvents: RoomEvents{ + SearchCategories: SearchCategoriesResponse{ + RoomEvents: RoomEventsResponse{ Count: int(result.Total), NextBatch: nil, }, @@ -158,7 +157,7 @@ func Search(req *http.Request, device *api.Device, syncDB storage.Database, fts } groups := make(map[string]RoomResult) - knownUsersProfiles := make(map[string]ProfileInfo) + knownUsersProfiles := make(map[string]ProfileInfoResponse) // Sort the events by depth, as the returned values aren't ordered if orderByTime { @@ -180,7 +179,7 @@ func Search(req *http.Request, device *api.Device, syncDB storage.Database, fts return jsonerror.InternalServerError() } - profileInfos := make(map[string]ProfileInfo) + profileInfos := make(map[string]ProfileInfoResponse) for _, ev := range append(eventsBefore, eventsAfter...) { profile, ok := knownUsersProfiles[event.Sender()] if !ok { @@ -192,7 +191,7 @@ func Search(req *http.Request, device *api.Device, syncDB storage.Database, fts if stateEvent == nil { continue } - profile = ProfileInfo{ + profile = ProfileInfoResponse{ AvatarURL: gjson.GetBytes(stateEvent.Content(), "avatar_url").Str, DisplayName: gjson.GetBytes(stateEvent.Content(), "displayname").Str, } @@ -237,13 +236,13 @@ func Search(req *http.Request, device *api.Device, syncDB storage.Database, fts } res := SearchResponse{ - SearchCategories: SearchCategories{ - RoomEvents: RoomEvents{ + SearchCategories: SearchCategoriesResponse{ + RoomEvents: RoomEventsResponse{ Count: int(result.Total), Groups: Groups{RoomID: groups}, Results: results, NextBatch: nextBatchResult, - Highlights: strings.Split(searchReq.SearchCategories.RoomEvents.SearchTerm, " "), + Highlights: fts.GetHighlights(result), State: stateForRooms, }, }, @@ -286,30 +285,40 @@ func contextEvents( return eventsBefore, eventsAfter, err } +type EventContext struct { + AfterLimit int `json:"after_limit,omitempty"` + BeforeLimit int `json:"before_limit,omitempty"` + IncludeProfile bool `json:"include_profile,omitempty"` +} + +type GroupBy struct { + Key string `json:"key"` +} + +type Groupings struct { + GroupBy []GroupBy `json:"group_by"` +} + +type RoomEvents struct { + EventContext EventContext `json:"event_context"` + Filter gomatrixserverlib.RoomEventFilter `json:"filter"` + Groupings Groupings `json:"groupings"` + IncludeState bool `json:"include_state"` + Keys []string `json:"keys"` + OrderBy string `json:"order_by"` + SearchTerm string `json:"search_term"` +} + +type SearchCategories struct { + RoomEvents RoomEvents `json:"room_events"` +} + type SearchRequest struct { - SearchCategories struct { - RoomEvents struct { - EventContext struct { - AfterLimit int `json:"after_limit,omitempty"` - BeforeLimit int `json:"before_limit,omitempty"` - IncludeProfile bool `json:"include_profile,omitempty"` - } `json:"event_context"` - Filter gomatrixserverlib.RoomEventFilter `json:"filter"` - Groupings struct { - GroupBy []struct { - Key string `json:"key"` - } `json:"group_by"` - } `json:"groupings"` - IncludeState bool `json:"include_state"` - Keys []string `json:"keys"` - OrderBy string `json:"order_by"` - SearchTerm string `json:"search_term"` - } `json:"room_events"` - } `json:"search_categories"` + SearchCategories SearchCategories `json:"search_categories"` } type SearchResponse struct { - SearchCategories SearchCategories `json:"search_categories"` + SearchCategories SearchCategoriesResponse `json:"search_categories"` } type RoomResult struct { NextBatch *string `json:"next_batch,omitempty"` @@ -332,15 +341,15 @@ type SearchContextResponse struct { EventsAfter []gomatrixserverlib.ClientEvent `json:"events_after"` EventsBefore []gomatrixserverlib.ClientEvent `json:"events_before"` Start string `json:"start"` - ProfileInfo map[string]ProfileInfo `json:"profile_info"` + ProfileInfo map[string]ProfileInfoResponse `json:"profile_info"` } -type ProfileInfo struct { +type ProfileInfoResponse struct { AvatarURL string `json:"avatar_url"` DisplayName string `json:"display_name"` } -type RoomEvents struct { +type RoomEventsResponse struct { Count int `json:"count"` Groups Groups `json:"groups"` Highlights []string `json:"highlights"` @@ -348,6 +357,6 @@ type RoomEvents struct { Results []Result `json:"results"` State map[string][]gomatrixserverlib.ClientEvent `json:"state,omitempty"` } -type SearchCategories struct { - RoomEvents RoomEvents `json:"room_events"` +type SearchCategoriesResponse struct { + RoomEvents RoomEventsResponse `json:"room_events"` } diff --git a/syncapi/routing/search_test.go b/syncapi/routing/search_test.go new file mode 100644 index 000000000..05479300e --- /dev/null +++ b/syncapi/routing/search_test.go @@ -0,0 +1,264 @@ +package routing + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/matrix-org/dendrite/internal/fulltext" + "github.com/matrix-org/dendrite/internal/sqlutil" + "github.com/matrix-org/dendrite/syncapi/storage" + "github.com/matrix-org/dendrite/syncapi/types" + "github.com/matrix-org/dendrite/test" + "github.com/matrix-org/dendrite/test/testrig" + userapi "github.com/matrix-org/dendrite/userapi/api" + "github.com/matrix-org/gomatrixserverlib" + "github.com/stretchr/testify/assert" +) + +func TestSearch(t *testing.T) { + alice := test.NewUser(t) + aliceDevice := userapi.Device{UserID: alice.ID} + room := test.NewRoom(t, alice) + room.CreateAndInsert(t, alice, "m.room.message", map[string]interface{}{"body": "context before"}) + room.CreateAndInsert(t, alice, "m.room.message", map[string]interface{}{"body": "hello world3!"}) + room.CreateAndInsert(t, alice, "m.room.message", map[string]interface{}{"body": "context after"}) + + roomsFilter := []string{room.ID} + roomsFilterUnknown := []string{"!unknown"} + + emptyFromString := "" + fromStringValid := "1" + fromStringInvalid := "iCantBeParsed" + + testCases := []struct { + name string + wantOK bool + searchReq SearchRequest + device *userapi.Device + wantResponseCount int + from *string + }{ + { + name: "no user ID", + searchReq: SearchRequest{}, + device: &userapi.Device{}, + }, + { + name: "with alice ID", + wantOK: true, + searchReq: SearchRequest{}, + device: &aliceDevice, + }, + { + name: "searchTerm specified, found at the beginning", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{RoomEvents: RoomEvents{SearchTerm: "hello"}}, + }, + device: &aliceDevice, + wantResponseCount: 1, + }, + { + name: "searchTerm specified, found at the end", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{RoomEvents: RoomEvents{SearchTerm: "world3"}}, + }, + device: &aliceDevice, + wantResponseCount: 1, + }, + /* the following would need matchQuery.SetFuzziness(1) in bleve.go + { + name: "searchTerm fuzzy search", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{RoomEvents: RoomEvents{SearchTerm: "hell"}}, // this still should find hello world + }, + device: &aliceDevice, + wantResponseCount: 1, + }, + */ + { + name: "searchTerm specified but no result", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{RoomEvents: RoomEvents{SearchTerm: "i don't match"}}, + }, + device: &aliceDevice, + }, + { + name: "filter on room", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{ + RoomEvents: RoomEvents{ + SearchTerm: "hello", + Filter: gomatrixserverlib.RoomEventFilter{ + Rooms: &roomsFilter, + }, + }, + }, + }, + device: &aliceDevice, + wantResponseCount: 1, + }, + { + name: "filter on unknown room", + searchReq: SearchRequest{ + SearchCategories: SearchCategories{ + RoomEvents: RoomEvents{ + SearchTerm: "hello", + Filter: gomatrixserverlib.RoomEventFilter{ + Rooms: &roomsFilterUnknown, + }, + }, + }, + }, + device: &aliceDevice, + }, + { + name: "include state", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{ + RoomEvents: RoomEvents{ + SearchTerm: "hello", + Filter: gomatrixserverlib.RoomEventFilter{ + Rooms: &roomsFilter, + }, + IncludeState: true, + }, + }, + }, + device: &aliceDevice, + wantResponseCount: 1, + }, + { + name: "empty from does not error", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{ + RoomEvents: RoomEvents{ + SearchTerm: "hello", + Filter: gomatrixserverlib.RoomEventFilter{ + Rooms: &roomsFilter, + }, + }, + }, + }, + wantResponseCount: 1, + device: &aliceDevice, + from: &emptyFromString, + }, + { + name: "valid from does not error", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{ + RoomEvents: RoomEvents{ + SearchTerm: "hello", + Filter: gomatrixserverlib.RoomEventFilter{ + Rooms: &roomsFilter, + }, + }, + }, + }, + wantResponseCount: 1, + device: &aliceDevice, + from: &fromStringValid, + }, + { + name: "invalid from does error", + searchReq: SearchRequest{ + SearchCategories: SearchCategories{ + RoomEvents: RoomEvents{ + SearchTerm: "hello", + Filter: gomatrixserverlib.RoomEventFilter{ + Rooms: &roomsFilter, + }, + }, + }, + }, + device: &aliceDevice, + from: &fromStringInvalid, + }, + { + name: "order by stream position", + wantOK: true, + searchReq: SearchRequest{ + SearchCategories: SearchCategories{RoomEvents: RoomEvents{SearchTerm: "hello", OrderBy: "recent"}}, + }, + device: &aliceDevice, + wantResponseCount: 1, + }, + } + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, closeDB := testrig.CreateConfig(t, dbType) + defer closeDB() + + // create requisites + fts, err := fulltext.New(processCtx, cfg.SyncAPI.Fulltext) + assert.NoError(t, err) + assert.NotNil(t, fts) + + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + db, err := storage.NewSyncServerDatasource(processCtx.Context(), cm, &cfg.SyncAPI.Database) + assert.NoError(t, err) + + elements := []fulltext.IndexElement{} + // store the events in the database + var sp types.StreamPosition + for _, x := range room.Events() { + var stateEvents []*gomatrixserverlib.HeaderedEvent + var stateEventIDs []string + if x.Type() == gomatrixserverlib.MRoomMember { + stateEvents = append(stateEvents, x) + stateEventIDs = append(stateEventIDs, x.EventID()) + } + sp, err = db.WriteEvent(processCtx.Context(), x, stateEvents, stateEventIDs, nil, nil, false, gomatrixserverlib.HistoryVisibilityShared) + assert.NoError(t, err) + if x.Type() != "m.room.message" { + continue + } + elements = append(elements, fulltext.IndexElement{ + EventID: x.EventID(), + RoomID: x.RoomID(), + Content: string(x.Content()), + ContentType: x.Type(), + StreamPosition: int64(sp), + }) + } + // Index the events + err = fts.Index(elements...) + assert.NoError(t, err) + + // run the tests + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + reqBody := &bytes.Buffer{} + err = json.NewEncoder(reqBody).Encode(tc.searchReq) + assert.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, "/", reqBody) + + res := Search(req, tc.device, db, fts, tc.from) + if !tc.wantOK && !res.Is2xx() { + return + } + resp, ok := res.JSON.(SearchResponse) + if !ok && !tc.wantOK { + t.Fatalf("not a SearchResponse: %T: %s", res.JSON, res.JSON) + } + assert.Equal(t, tc.wantResponseCount, resp.SearchCategories.RoomEvents.Count) + + // if we requested state, it should not be empty + if tc.searchReq.SearchCategories.RoomEvents.IncludeState { + assert.NotEmpty(t, resp.SearchCategories.RoomEvents.State) + } + }) + } + }) +} From fa7710315a00f6e857bb9c315c0a7ba248288b79 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Mon, 27 Mar 2023 15:39:33 +0200 Subject: [PATCH 3/9] Add tests for the Dendrite admin APIs (#3028) Contains a breaking change, since the endpoints `/_dendrite/admin/evacuateRoom/{roomID}` and `/_dendrite/admin/evacuateUser/{userID}` are now using `POST` instead of `GET` --- clientapi/admin_test.go | 326 +++++++++++++++---- clientapi/routing/admin.go | 31 +- clientapi/routing/routing.go | 8 +- docs/administration/4_adminapi.md | 4 +- roomserver/internal/perform/perform_admin.go | 3 +- 5 files changed, 284 insertions(+), 88 deletions(-) diff --git a/clientapi/admin_test.go b/clientapi/admin_test.go index 4d2bf67b2..3e7cb875c 100644 --- a/clientapi/admin_test.go +++ b/clientapi/admin_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "reflect" "testing" "time" @@ -14,6 +15,7 @@ import ( "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/roomserver" "github.com/matrix-org/dendrite/roomserver/api" + basepkg "github.com/matrix-org/dendrite/setup/base" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/setup/jetstream" "github.com/matrix-org/dendrite/syncapi" @@ -57,34 +59,7 @@ func TestAdminResetPassword(t *testing.T) { bob: "", vhUser: "", } - for u := range accessTokens { - localpart, serverName, _ := gomatrixserverlib.SplitID('@', u.ID) - userRes := &uapi.PerformAccountCreationResponse{} - password := util.RandomString(8) - if err := userAPI.PerformAccountCreation(ctx, &uapi.PerformAccountCreationRequest{ - AccountType: u.AccountType, - Localpart: localpart, - ServerName: serverName, - Password: password, - }, userRes); err != nil { - t.Errorf("failed to create account: %s", err) - } - - req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{ - "type": authtypes.LoginTypePassword, - "identifier": map[string]interface{}{ - "type": "m.id.user", - "user": u.ID, - }, - "password": password, - })) - rec := httptest.NewRecorder() - routers.Client.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("failed to login: %s", rec.Body.String()) - } - accessTokens[u] = gjson.GetBytes(rec.Body.Bytes(), "access_token").String() - } + createAccessTokens(t, accessTokens, userAPI, ctx, routers) testCases := []struct { name string @@ -182,34 +157,7 @@ func TestPurgeRoom(t *testing.T) { accessTokens := map[*test.User]string{ aliceAdmin: "", } - for u := range accessTokens { - localpart, serverName, _ := gomatrixserverlib.SplitID('@', u.ID) - userRes := &uapi.PerformAccountCreationResponse{} - password := util.RandomString(8) - if err := userAPI.PerformAccountCreation(ctx, &uapi.PerformAccountCreationRequest{ - AccountType: u.AccountType, - Localpart: localpart, - ServerName: serverName, - Password: password, - }, userRes); err != nil { - t.Errorf("failed to create account: %s", err) - } - - req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{ - "type": authtypes.LoginTypePassword, - "identifier": map[string]interface{}{ - "type": "m.id.user", - "user": u.ID, - }, - "password": password, - })) - rec := httptest.NewRecorder() - routers.Client.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("failed to login: %s", rec.Body.String()) - } - accessTokens[u] = gjson.GetBytes(rec.Body.Bytes(), "access_token").String() - } + createAccessTokens(t, accessTokens, userAPI, ctx, routers) testCases := []struct { name string @@ -239,3 +187,269 @@ func TestPurgeRoom(t *testing.T) { }) } + +func TestAdminEvacuateRoom(t *testing.T) { + aliceAdmin := test.NewUser(t, test.WithAccountType(uapi.AccountTypeAdmin)) + bob := test.NewUser(t) + room := test.NewRoom(t, aliceAdmin) + + // Join Bob + room.CreateAndInsert(t, bob, gomatrixserverlib.MRoomMember, map[string]interface{}{ + "membership": "join", + }, test.WithStateKey(bob.ID)) + + ctx := context.Background() + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, close := testrig.CreateConfig(t, dbType) + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + natsInstance := jetstream.NATSInstance{} + defer close() + + routers := httputil.NewRouters() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics) + userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil) + + // this starts the JetStream consumers + fsAPI := federationapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, nil, rsAPI, caches, nil, true) + rsAPI.SetFederationAPI(fsAPI, nil) + + // Create the room + if err := api.SendEvents(ctx, rsAPI, api.KindNew, room.Events(), "test", "test", api.DoNotSendToOtherServers, nil, false); err != nil { + t.Fatalf("failed to send events: %v", err) + } + + // We mostly need the rsAPI for this test, so nil for other APIs/caches etc. + AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) + + // Create the users in the userapi and login + accessTokens := map[*test.User]string{ + aliceAdmin: "", + } + createAccessTokens(t, accessTokens, userAPI, ctx, routers) + + testCases := []struct { + name string + roomID string + wantOK bool + wantAffected []string + }{ + {name: "Can evacuate existing room", wantOK: true, roomID: room.ID, wantAffected: []string{aliceAdmin.ID, bob.ID}}, + {name: "Can not evacuate non-existent room", wantOK: false, roomID: "!doesnotexist:localhost", wantAffected: []string{}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/evacuateRoom/"+tc.roomID) + + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + + rec := httptest.NewRecorder() + routers.DendriteAdmin.ServeHTTP(rec, req) + t.Logf("%s", rec.Body.String()) + if tc.wantOK && rec.Code != http.StatusOK { + t.Fatalf("expected http status %d, got %d: %s", http.StatusOK, rec.Code, rec.Body.String()) + } + + affectedArr := gjson.GetBytes(rec.Body.Bytes(), "affected").Array() + affected := make([]string, 0, len(affectedArr)) + for _, x := range affectedArr { + affected = append(affected, x.Str) + } + if !reflect.DeepEqual(affected, tc.wantAffected) { + t.Fatalf("expected affected %#v, but got %#v", tc.wantAffected, affected) + } + }) + } + }) +} + +func TestAdminEvacuateUser(t *testing.T) { + aliceAdmin := test.NewUser(t, test.WithAccountType(uapi.AccountTypeAdmin)) + bob := test.NewUser(t) + room := test.NewRoom(t, aliceAdmin) + room2 := test.NewRoom(t, aliceAdmin) + + // Join Bob + room.CreateAndInsert(t, bob, gomatrixserverlib.MRoomMember, map[string]interface{}{ + "membership": "join", + }, test.WithStateKey(bob.ID)) + room2.CreateAndInsert(t, bob, gomatrixserverlib.MRoomMember, map[string]interface{}{ + "membership": "join", + }, test.WithStateKey(bob.ID)) + + ctx := context.Background() + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, close := testrig.CreateConfig(t, dbType) + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + natsInstance := jetstream.NATSInstance{} + defer close() + + routers := httputil.NewRouters() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics) + userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil) + + // this starts the JetStream consumers + fsAPI := federationapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, basepkg.CreateFederationClient(cfg, nil), rsAPI, caches, nil, true) + rsAPI.SetFederationAPI(fsAPI, nil) + + // Create the room + if err := api.SendEvents(ctx, rsAPI, api.KindNew, room.Events(), "test", "test", api.DoNotSendToOtherServers, nil, false); err != nil { + t.Fatalf("failed to send events: %v", err) + } + if err := api.SendEvents(ctx, rsAPI, api.KindNew, room2.Events(), "test", "test", api.DoNotSendToOtherServers, nil, false); err != nil { + t.Fatalf("failed to send events: %v", err) + } + + // We mostly need the rsAPI for this test, so nil for other APIs/caches etc. + AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) + + // Create the users in the userapi and login + accessTokens := map[*test.User]string{ + aliceAdmin: "", + } + createAccessTokens(t, accessTokens, userAPI, ctx, routers) + + testCases := []struct { + name string + userID string + wantOK bool + wantAffectedRooms []string + }{ + {name: "Can evacuate existing user", wantOK: true, userID: bob.ID, wantAffectedRooms: []string{room.ID, room2.ID}}, + {name: "invalid userID is rejected", wantOK: false, userID: "!notauserid:test", wantAffectedRooms: []string{}}, + {name: "Can not evacuate user from different server", wantOK: false, userID: "@doesnotexist:localhost", wantAffectedRooms: []string{}}, + {name: "Can not evacuate non-existent user", wantOK: false, userID: "@doesnotexist:test", wantAffectedRooms: []string{}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/evacuateUser/"+tc.userID) + + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + + rec := httptest.NewRecorder() + routers.DendriteAdmin.ServeHTTP(rec, req) + t.Logf("%s", rec.Body.String()) + if tc.wantOK && rec.Code != http.StatusOK { + t.Fatalf("expected http status %d, got %d: %s", http.StatusOK, rec.Code, rec.Body.String()) + } + + affectedArr := gjson.GetBytes(rec.Body.Bytes(), "affected").Array() + affected := make([]string, 0, len(affectedArr)) + for _, x := range affectedArr { + affected = append(affected, x.Str) + } + if !reflect.DeepEqual(affected, tc.wantAffectedRooms) { + t.Fatalf("expected affected %#v, but got %#v", tc.wantAffectedRooms, affected) + } + + }) + } + // Wait for the FS API to have consumed every message + js, _ := natsInstance.Prepare(processCtx, &cfg.Global.JetStream) + timeout := time.After(time.Second) + for { + select { + case <-timeout: + t.Fatalf("FS API didn't process all events in time") + default: + } + info, err := js.ConsumerInfo(cfg.Global.JetStream.Prefixed(jetstream.OutputRoomEvent), cfg.Global.JetStream.Durable("FederationAPIRoomServerConsumer")+"Pull") + if err != nil { + time.Sleep(time.Millisecond * 10) + continue + } + if info.NumPending == 0 && info.NumAckPending == 0 { + break + } + } + }) +} + +func TestAdminMarkAsStale(t *testing.T) { + aliceAdmin := test.NewUser(t, test.WithAccountType(uapi.AccountTypeAdmin)) + + ctx := context.Background() + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, close := testrig.CreateConfig(t, dbType) + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + natsInstance := jetstream.NATSInstance{} + defer close() + + routers := httputil.NewRouters() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics) + userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil) + + // We mostly need the rsAPI for this test, so nil for other APIs/caches etc. + AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) + + // Create the users in the userapi and login + accessTokens := map[*test.User]string{ + aliceAdmin: "", + } + createAccessTokens(t, accessTokens, userAPI, ctx, routers) + + testCases := []struct { + name string + userID string + wantOK bool + }{ + {name: "local user is not allowed", userID: aliceAdmin.ID}, + {name: "invalid userID", userID: "!notvalid:test"}, + {name: "remote user is allowed", userID: "@alice:localhost", wantOK: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/refreshDevices/"+tc.userID) + + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + + rec := httptest.NewRecorder() + routers.DendriteAdmin.ServeHTTP(rec, req) + t.Logf("%s", rec.Body.String()) + if tc.wantOK && rec.Code != http.StatusOK { + t.Fatalf("expected http status %d, got %d: %s", http.StatusOK, rec.Code, rec.Body.String()) + } + }) + } + }) +} + +func createAccessTokens(t *testing.T, accessTokens map[*test.User]string, userAPI uapi.UserInternalAPI, ctx context.Context, routers httputil.Routers) { + t.Helper() + for u := range accessTokens { + localpart, serverName, _ := gomatrixserverlib.SplitID('@', u.ID) + userRes := &uapi.PerformAccountCreationResponse{} + password := util.RandomString(8) + if err := userAPI.PerformAccountCreation(ctx, &uapi.PerformAccountCreationRequest{ + AccountType: u.AccountType, + Localpart: localpart, + ServerName: serverName, + Password: password, + }, userRes); err != nil { + t.Errorf("failed to create account: %s", err) + } + + req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{ + "type": authtypes.LoginTypePassword, + "identifier": map[string]interface{}{ + "type": "m.id.user", + "user": u.ID, + }, + "password": password, + })) + rec := httptest.NewRecorder() + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("failed to login: %s", rec.Body.String()) + } + accessTokens[u] = gjson.GetBytes(rec.Body.Bytes(), "access_token").String() + } +} diff --git a/clientapi/routing/admin.go b/clientapi/routing/admin.go index a01f6b944..76e18f2f8 100644 --- a/clientapi/routing/admin.go +++ b/clientapi/routing/admin.go @@ -22,23 +22,16 @@ import ( "github.com/matrix-org/dendrite/userapi/api" ) -func AdminEvacuateRoom(req *http.Request, cfg *config.ClientAPI, device *api.Device, rsAPI roomserverAPI.ClientRoomserverAPI) util.JSONResponse { +func AdminEvacuateRoom(req *http.Request, rsAPI roomserverAPI.ClientRoomserverAPI) util.JSONResponse { vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) if err != nil { return util.ErrorResponse(err) } - roomID, ok := vars["roomID"] - if !ok { - return util.JSONResponse{ - Code: http.StatusBadRequest, - JSON: jsonerror.MissingArgument("Expecting room ID."), - } - } res := &roomserverAPI.PerformAdminEvacuateRoomResponse{} if err := rsAPI.PerformAdminEvacuateRoom( req.Context(), &roomserverAPI.PerformAdminEvacuateRoomRequest{ - RoomID: roomID, + RoomID: vars["roomID"], }, res, ); err != nil { @@ -55,18 +48,13 @@ func AdminEvacuateRoom(req *http.Request, cfg *config.ClientAPI, device *api.Dev } } -func AdminEvacuateUser(req *http.Request, cfg *config.ClientAPI, device *api.Device, rsAPI roomserverAPI.ClientRoomserverAPI) util.JSONResponse { +func AdminEvacuateUser(req *http.Request, cfg *config.ClientAPI, rsAPI roomserverAPI.ClientRoomserverAPI) util.JSONResponse { vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) if err != nil { return util.ErrorResponse(err) } - userID, ok := vars["userID"] - if !ok { - return util.JSONResponse{ - Code: http.StatusBadRequest, - JSON: jsonerror.MissingArgument("Expecting user ID."), - } - } + userID := vars["userID"] + _, domain, err := gomatrixserverlib.SplitID('@', userID) if err != nil { return util.MessageResponse(http.StatusBadRequest, err.Error()) @@ -103,13 +91,8 @@ func AdminPurgeRoom(req *http.Request, cfg *config.ClientAPI, device *api.Device if err != nil { return util.ErrorResponse(err) } - roomID, ok := vars["roomID"] - if !ok { - return util.JSONResponse{ - Code: http.StatusBadRequest, - JSON: jsonerror.MissingArgument("Expecting room ID."), - } - } + roomID := vars["roomID"] + res := &roomserverAPI.PerformAdminPurgeRoomResponse{} if err := rsAPI.PerformAdminPurgeRoom( context.Background(), diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 6a86980da..e261cb3aa 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -155,15 +155,15 @@ func Setup( dendriteAdminRouter.Handle("/admin/evacuateRoom/{roomID}", httputil.MakeAdminAPI("admin_evacuate_room", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - return AdminEvacuateRoom(req, cfg, device, rsAPI) + return AdminEvacuateRoom(req, rsAPI) }), - ).Methods(http.MethodGet, http.MethodOptions) + ).Methods(http.MethodPost, http.MethodOptions) dendriteAdminRouter.Handle("/admin/evacuateUser/{userID}", httputil.MakeAdminAPI("admin_evacuate_user", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - return AdminEvacuateUser(req, cfg, device, rsAPI) + return AdminEvacuateUser(req, cfg, rsAPI) }), - ).Methods(http.MethodGet, http.MethodOptions) + ).Methods(http.MethodPost, http.MethodOptions) dendriteAdminRouter.Handle("/admin/purgeRoom/{roomID}", httputil.MakeAdminAPI("admin_purge_room", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { diff --git a/docs/administration/4_adminapi.md b/docs/administration/4_adminapi.md index 46cfac220..b11aeb1a6 100644 --- a/docs/administration/4_adminapi.md +++ b/docs/administration/4_adminapi.md @@ -32,7 +32,7 @@ UPDATE userapi_accounts SET account_type = 3 WHERE localpart = '$localpart'; Where `$localpart` is the username only (e.g. `alice`). -## GET `/_dendrite/admin/evacuateRoom/{roomID}` +## POST `/_dendrite/admin/evacuateRoom/{roomID}` This endpoint will instruct Dendrite to part all local users from the given `roomID` in the URL. It may take some time to complete. A JSON body will be returned containing @@ -41,7 +41,7 @@ the user IDs of all affected users. If the room has an alias set (e.g. is published), the room's ID will not be visible in the URL, but it can be found as the room's "internal ID" in Element Web (Settings -> Advanced) -## GET `/_dendrite/admin/evacuateUser/{userID}` +## POST `/_dendrite/admin/evacuateUser/{userID}` This endpoint will instruct Dendrite to part the given local `userID` in the URL from all rooms which they are currently joined. A JSON body will be returned containing diff --git a/roomserver/internal/perform/perform_admin.go b/roomserver/internal/perform/perform_admin.go index 45089bdd1..0f1249114 100644 --- a/roomserver/internal/perform/perform_admin.go +++ b/roomserver/internal/perform/perform_admin.go @@ -227,6 +227,7 @@ func (r *Admin) PerformAdminEvacuateUser( } return nil } + res.Affected = append(res.Affected, roomID) if len(outputEvents) == 0 { continue } @@ -237,8 +238,6 @@ func (r *Admin) PerformAdminEvacuateUser( } return nil } - - res.Affected = append(res.Affected, roomID) } return nil } From 69e3bd82a94470c4072637374dea82b8c2acfaec Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Mon, 27 Mar 2023 07:55:49 -0600 Subject: [PATCH 4/9] Add dendrite-demo-pinecone cypress tests --- .github/workflows/schedules.yaml | 41 +++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/.github/workflows/schedules.yaml b/.github/workflows/schedules.yaml index 254594912..e76cc82f3 100644 --- a/.github/workflows/schedules.yaml +++ b/.github/workflows/schedules.yaml @@ -219,7 +219,7 @@ jobs: flags: complement fail_ci_if_error: true - element_web: + element-web: timeout-minutes: 120 runs-on: ubuntu-latest steps: @@ -257,3 +257,42 @@ jobs: env: PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true TMPDIR: ${{ runner.temp }} + + element-web-pinecone: + timeout-minutes: 120 + runs-on: ubuntu-latest + steps: + - uses: tecolicom/actions-use-apt-tools@v1 + with: + # Our test suite includes some screenshot tests with unusual diacritics, which are + # supposed to be covered by STIXGeneral. + tools: fonts-stix + - uses: actions/checkout@v2 + with: + repository: matrix-org/matrix-react-sdk + - uses: actions/setup-node@v3 + with: + cache: 'yarn' + - name: Fetch layered build + run: scripts/ci/layered.sh + - name: Copy config + run: cp element.io/develop/config.json config.json + working-directory: ./element-web + - name: Build + env: + CI_PACKAGE: true + NODE_OPTIONS: "--openssl-legacy-provider" + run: yarn build + working-directory: ./element-web + - name: Edit Test Config + run: | + sed -i '/HOMESERVER/c\ HOMESERVER: "dendritePinecone",' cypress.config.ts + - name: "Run cypress tests" + uses: cypress-io/github-action@v4.1.1 + with: + browser: chrome + start: npx serve -p 8080 ./element-web/webapp + wait-on: 'http://localhost:8080' + env: + PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true + TMPDIR: ${{ runner.temp }} From f4104b4b5d21df9dfedd9873a5d8b6bc63e96e99 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Mon, 27 Mar 2023 17:19:53 -0600 Subject: [PATCH 5/9] Pinecone-demo: Wait on dendrite before shutting down --- cmd/dendrite-demo-pinecone/monolith/monolith.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/dendrite-demo-pinecone/monolith/monolith.go b/cmd/dendrite-demo-pinecone/monolith/monolith.go index 10a3493e1..d1a6e39e9 100644 --- a/cmd/dendrite-demo-pinecone/monolith/monolith.go +++ b/cmd/dendrite-demo-pinecone/monolith/monolith.go @@ -213,7 +213,7 @@ func (p *P2PMonolith) Stop() { } func (p *P2PMonolith) WaitForShutdown() { - p.ProcessCtx.WaitForShutdown() + base.WaitForShutdown(p.ProcessCtx) p.closeAllResources() } From 28d3e296a8adaeea34bca01c7e4e6a0e22918390 Mon Sep 17 00:00:00 2001 From: Rhea Danzey Date: Tue, 28 Mar 2023 01:30:19 -0500 Subject: [PATCH 6/9] Rdanzey/helm-fixes-existing-db-secrets (#3033) Fixes some Helm templating issues when setting up a deployment with an existing database / signing keys. - Allows for `.Values.postgresql.enabled: false` as long as `.Values.global.dendrite_config.database.connection_string` is defined - Allows for '.Values.signing_key.create: false' if `.Values.signing_key.existingSecret` is set Also fixes an error in the template resulting in profiling port not being set correctly: ``` Error: template: dendrite-meta/charts/dendrite/templates/deployment.yaml:60:35: executing "dendrite-meta/charts/dendrite/templates/deployment.yaml" at <$.Values.global.profiling.port>: nil pointer evaluating interface {}.port ``` ### Pull Request Checklist * [x] I have added Go unit tests or [Complement integration tests](https://github.com/matrix-org/complement) for this PR _or_ I have justified why this PR doesn't need tests - Helm template fixes, no golang changes * [x] Pull request includes a [sign off below using a legally identifiable name](https://matrix-org.github.io/dendrite/development/contributing#sign-off) _or_ I have already signed off privately Signed-off-by: Rhea Danzey --------- Signed-off-by: Rhea Danzey Co-authored-by: Till Faelligen <2353100+S7evinK@users.noreply.github.com> --- helm/dendrite/Chart.yaml | 2 +- helm/dendrite/templates/_helpers.tpl | 14 ++++---------- helm/dendrite/templates/deployment.yaml | 8 ++------ 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/helm/dendrite/Chart.yaml b/helm/dendrite/Chart.yaml index b352601e8..3ef45a6df 100644 --- a/helm/dendrite/Chart.yaml +++ b/helm/dendrite/Chart.yaml @@ -1,6 +1,6 @@ apiVersion: v2 name: dendrite -version: "0.12.0" +version: "0.12.1" appVersion: "0.12.0" description: Dendrite Matrix Homeserver type: application diff --git a/helm/dendrite/templates/_helpers.tpl b/helm/dendrite/templates/_helpers.tpl index 026706588..36bcefd8f 100644 --- a/helm/dendrite/templates/_helpers.tpl +++ b/helm/dendrite/templates/_helpers.tpl @@ -1,15 +1,9 @@ {{- define "validate.config" }} -{{- if not .Values.signing_key.create -}} -{{- fail "You must create a signing key for configuration.signing_key. (see https://github.com/matrix-org/dendrite/blob/master/docs/INSTALL.md#server-key-generation)" -}} +{{- if and (not .Values.signing_key.create) (eq .Values.signing_key.existingSecret "") -}} +{{- fail "You must create a signing key for configuration.signing_key OR specify an existing secret name in .Values.signing_key.existingSecret to mount it. (see https://github.com/matrix-org/dendrite/blob/master/docs/INSTALL.md#server-key-generation)" -}} {{- end -}} -{{- if not (or .Values.dendrite_config.global.database.host .Values.postgresql.enabled) -}} -{{- fail "Database server must be set." -}} -{{- end -}} -{{- if not (or .Values.dendrite_config.global.database.user .Values.postgresql.enabled) -}} -{{- fail "Database user must be set." -}} -{{- end -}} -{{- if not (or .Values.dendrite_config.global.database.password .Values.postgresql.enabled) -}} -{{- fail "Database password must be set." -}} +{{- if and (not .Values.postgresql.enabled) (eq .Values.dendrite_config.global.database.connection_string "") -}} +{{- fail "Database connection string must be set." -}} {{- end -}} {{- end -}} diff --git a/helm/dendrite/templates/deployment.yaml b/helm/dendrite/templates/deployment.yaml index b463c7d0b..2a0f3a9e9 100644 --- a/helm/dendrite/templates/deployment.yaml +++ b/helm/dendrite/templates/deployment.yaml @@ -17,11 +17,7 @@ spec: labels: {{- include "dendrite.selectorLabels" . | nindent 8 }} annotations: - confighash-global: secret-{{ .Values.global | toYaml | sha256sum | trunc 32 }} - confighash-clientapi: clientapi-{{ .Values.clientapi | toYaml | sha256sum | trunc 32 }} - confighash-federationapi: federationapi-{{ .Values.federationapi | toYaml | sha256sum | trunc 32 }} - confighash-mediaapi: mediaapi-{{ .Values.mediaapi | toYaml | sha256sum | trunc 32 }} - confighash-syncapi: syncapi-{{ .Values.syncapi | toYaml | sha256sum | trunc 32 }} + confighash: secret-{{ .Values.dendrite_config | toYaml | sha256sum | trunc 32 }} spec: volumes: - name: {{ include "dendrite.fullname" . }}-conf-vol @@ -57,7 +53,7 @@ spec: {{- if $.Values.dendrite_config.global.profiling.enabled }} env: - name: PPROFLISTEN - value: "localhost:{{- $.Values.global.profiling.port -}}" + value: "localhost:{{- $.Values.dendrite_config.global.profiling.port -}}" {{- end }} resources: {{- toYaml $.Values.resources | nindent 10 }} From 2854ffeb7d933f76cbdfe0eb6775a8f5699f4170 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 31 Mar 2023 10:15:01 +0200 Subject: [PATCH 7/9] Add CS API device tests (#3029) Adds tests for - `/devices` - `/delete_devices` (also adds UIA) --- clientapi/admin_test.go | 67 ++---- clientapi/clientapi_test.go | 373 ++++++++++++++++++++++++++++++++ clientapi/routing/deactivate.go | 5 +- clientapi/routing/device.go | 40 +++- clientapi/routing/routing.go | 2 +- userapi/api/api.go | 1 - userapi/internal/user_api.go | 5 - 7 files changed, 422 insertions(+), 71 deletions(-) create mode 100644 clientapi/clientapi_test.go diff --git a/clientapi/admin_test.go b/clientapi/admin_test.go index 3e7cb875c..79e88d137 100644 --- a/clientapi/admin_test.go +++ b/clientapi/admin_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/dendrite/federationapi" "github.com/matrix-org/dendrite/internal/caching" "github.com/matrix-org/dendrite/internal/httputil" @@ -54,10 +53,10 @@ func TestAdminResetPassword(t *testing.T) { AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) // Create the users in the userapi and login - accessTokens := map[*test.User]string{ - aliceAdmin: "", - bob: "", - vhUser: "", + accessTokens := map[*test.User]userDevice{ + aliceAdmin: {}, + bob: {}, + vhUser: {}, } createAccessTokens(t, accessTokens, userAPI, ctx, routers) @@ -103,7 +102,7 @@ func TestAdminResetPassword(t *testing.T) { } if tc.withHeader { - req.Header.Set("Authorization", "Bearer "+accessTokens[tc.requestingUser]) + req.Header.Set("Authorization", "Bearer "+accessTokens[tc.requestingUser].accessToken) } rec := httptest.NewRecorder() @@ -154,8 +153,8 @@ func TestPurgeRoom(t *testing.T) { AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) // Create the users in the userapi and login - accessTokens := map[*test.User]string{ - aliceAdmin: "", + accessTokens := map[*test.User]userDevice{ + aliceAdmin: {}, } createAccessTokens(t, accessTokens, userAPI, ctx, routers) @@ -174,7 +173,7 @@ func TestPurgeRoom(t *testing.T) { t.Run(tc.name, func(t *testing.T) { req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/purgeRoom/"+tc.roomID) - req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken) rec := httptest.NewRecorder() routers.DendriteAdmin.ServeHTTP(rec, req) @@ -224,8 +223,8 @@ func TestAdminEvacuateRoom(t *testing.T) { AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) // Create the users in the userapi and login - accessTokens := map[*test.User]string{ - aliceAdmin: "", + accessTokens := map[*test.User]userDevice{ + aliceAdmin: {}, } createAccessTokens(t, accessTokens, userAPI, ctx, routers) @@ -243,7 +242,7 @@ func TestAdminEvacuateRoom(t *testing.T) { t.Run(tc.name, func(t *testing.T) { req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/evacuateRoom/"+tc.roomID) - req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken) rec := httptest.NewRecorder() routers.DendriteAdmin.ServeHTTP(rec, req) @@ -308,8 +307,8 @@ func TestAdminEvacuateUser(t *testing.T) { AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) // Create the users in the userapi and login - accessTokens := map[*test.User]string{ - aliceAdmin: "", + accessTokens := map[*test.User]userDevice{ + aliceAdmin: {}, } createAccessTokens(t, accessTokens, userAPI, ctx, routers) @@ -329,7 +328,7 @@ func TestAdminEvacuateUser(t *testing.T) { t.Run(tc.name, func(t *testing.T) { req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/evacuateUser/"+tc.userID) - req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken) rec := httptest.NewRecorder() routers.DendriteAdmin.ServeHTTP(rec, req) @@ -390,8 +389,8 @@ func TestAdminMarkAsStale(t *testing.T) { AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) // Create the users in the userapi and login - accessTokens := map[*test.User]string{ - aliceAdmin: "", + accessTokens := map[*test.User]userDevice{ + aliceAdmin: {}, } createAccessTokens(t, accessTokens, userAPI, ctx, routers) @@ -409,7 +408,7 @@ func TestAdminMarkAsStale(t *testing.T) { t.Run(tc.name, func(t *testing.T) { req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/refreshDevices/"+tc.userID) - req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin]) + req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken) rec := httptest.NewRecorder() routers.DendriteAdmin.ServeHTTP(rec, req) @@ -421,35 +420,3 @@ func TestAdminMarkAsStale(t *testing.T) { } }) } - -func createAccessTokens(t *testing.T, accessTokens map[*test.User]string, userAPI uapi.UserInternalAPI, ctx context.Context, routers httputil.Routers) { - t.Helper() - for u := range accessTokens { - localpart, serverName, _ := gomatrixserverlib.SplitID('@', u.ID) - userRes := &uapi.PerformAccountCreationResponse{} - password := util.RandomString(8) - if err := userAPI.PerformAccountCreation(ctx, &uapi.PerformAccountCreationRequest{ - AccountType: u.AccountType, - Localpart: localpart, - ServerName: serverName, - Password: password, - }, userRes); err != nil { - t.Errorf("failed to create account: %s", err) - } - - req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{ - "type": authtypes.LoginTypePassword, - "identifier": map[string]interface{}{ - "type": "m.id.user", - "user": u.ID, - }, - "password": password, - })) - rec := httptest.NewRecorder() - routers.Client.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("failed to login: %s", rec.Body.String()) - } - accessTokens[u] = gjson.GetBytes(rec.Body.Bytes(), "access_token").String() - } -} diff --git a/clientapi/clientapi_test.go b/clientapi/clientapi_test.go new file mode 100644 index 000000000..d90915526 --- /dev/null +++ b/clientapi/clientapi_test.go @@ -0,0 +1,373 @@ +package clientapi + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/matrix-org/dendrite/clientapi/auth/authtypes" + "github.com/matrix-org/dendrite/internal/caching" + "github.com/matrix-org/dendrite/internal/httputil" + "github.com/matrix-org/dendrite/internal/sqlutil" + "github.com/matrix-org/dendrite/roomserver" + "github.com/matrix-org/dendrite/setup/jetstream" + "github.com/matrix-org/dendrite/test" + "github.com/matrix-org/dendrite/test/testrig" + "github.com/matrix-org/dendrite/userapi" + uapi "github.com/matrix-org/dendrite/userapi/api" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/tidwall/gjson" +) + +type userDevice struct { + accessToken string + deviceID string + password string +} + +func TestGetPutDevices(t *testing.T) { + alice := test.NewUser(t) + bob := test.NewUser(t) + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + testCases := []struct { + name string + requestUser *test.User + deviceUser *test.User + request *http.Request + wantStatusCode int + validateFunc func(t *testing.T, device userDevice, routers httputil.Routers) + }{ + { + name: "can get all devices", + requestUser: alice, + request: httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/devices", strings.NewReader("")), + wantStatusCode: http.StatusOK, + }, + { + name: "can get specific own device", + requestUser: alice, + deviceUser: alice, + request: httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/devices/", strings.NewReader("")), + wantStatusCode: http.StatusOK, + }, + { + name: "can not get device for different user", + requestUser: alice, + deviceUser: bob, + request: httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/devices/", strings.NewReader("")), + wantStatusCode: http.StatusNotFound, + }, + { + name: "can update own device", + requestUser: alice, + deviceUser: alice, + request: httptest.NewRequest(http.MethodPut, "/_matrix/client/v3/devices/", strings.NewReader(`{"display_name":"my new displayname"}`)), + wantStatusCode: http.StatusOK, + validateFunc: func(t *testing.T, device userDevice, routers httputil.Routers) { + req := httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/devices/"+device.deviceID, strings.NewReader("")) + req.Header.Set("Authorization", "Bearer "+device.accessToken) + rec := httptest.NewRecorder() + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String()) + } + gotDisplayName := gjson.GetBytes(rec.Body.Bytes(), "display_name").Str + if gotDisplayName != "my new displayname" { + t.Fatalf("expected displayname '%s', got '%s'", "my new displayname", gotDisplayName) + } + }, + }, + { + // this should return "device does not exist" + name: "can not update device for different user", + requestUser: alice, + deviceUser: bob, + request: httptest.NewRequest(http.MethodPut, "/_matrix/client/v3/devices/", strings.NewReader(`{"display_name":"my new displayname"}`)), + wantStatusCode: http.StatusNotFound, + }, + } + + cfg, processCtx, close := testrig.CreateConfig(t, dbType) + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + natsInstance := jetstream.NATSInstance{} + defer close() + + routers := httputil.NewRouters() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics) + userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil) + + // We mostly need the rsAPI for this test, so nil for other APIs/caches etc. + AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) + + accessTokens := map[*test.User]userDevice{ + alice: {}, + bob: {}, + } + createAccessTokens(t, accessTokens, userAPI, processCtx.Context(), routers) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dev := accessTokens[tc.requestUser] + if tc.deviceUser != nil { + tc.request = httptest.NewRequest(tc.request.Method, tc.request.RequestURI+accessTokens[tc.deviceUser].deviceID, tc.request.Body) + } + tc.request.Header.Set("Authorization", "Bearer "+dev.accessToken) + rec := httptest.NewRecorder() + routers.Client.ServeHTTP(rec, tc.request) + if rec.Code != tc.wantStatusCode { + t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String()) + } + if tc.wantStatusCode != http.StatusOK && rec.Code != http.StatusOK { + return + } + if tc.validateFunc != nil { + tc.validateFunc(t, dev, routers) + } + }) + } + }) +} + +// Deleting devices requires the UIA dance, so do this in a different test +func TestDeleteDevice(t *testing.T) { + alice := test.NewUser(t) + localpart, serverName, _ := gomatrixserverlib.SplitID('@', alice.ID) + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, closeDB := testrig.CreateConfig(t, dbType) + defer closeDB() + + natsInstance := jetstream.NATSInstance{} + routers := httputil.NewRouters() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics) + userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil) + + // We mostly need the rsAPI/ for this test, so nil for other APIs/caches etc. + AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) + + accessTokens := map[*test.User]userDevice{ + alice: {}, + } + + // create the account and an initial device + createAccessTokens(t, accessTokens, userAPI, processCtx.Context(), routers) + + // create some more devices + accessToken := util.RandomString(8) + devRes := &uapi.PerformDeviceCreationResponse{} + if err := userAPI.PerformDeviceCreation(processCtx.Context(), &uapi.PerformDeviceCreationRequest{ + Localpart: localpart, + ServerName: serverName, + AccessToken: accessToken, + NoDeviceListUpdate: true, + }, devRes); err != nil { + t.Fatal(err) + } + if !devRes.DeviceCreated { + t.Fatalf("failed to create device") + } + secondDeviceID := devRes.Device.ID + + // initiate UIA for the second device + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/_matrix/client/v3/devices/"+secondDeviceID, strings.NewReader("")) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected HTTP 401, got %d: %s", rec.Code, rec.Body.String()) + } + // get the session ID + sessionID := gjson.GetBytes(rec.Body.Bytes(), "session").Str + + // prepare UIA request body + reqBody := bytes.Buffer{} + if err := json.NewEncoder(&reqBody).Encode(map[string]interface{}{ + "auth": map[string]string{ + "session": sessionID, + "type": authtypes.LoginTypePassword, + "user": alice.ID, + "password": accessTokens[alice].password, + }, + }); err != nil { + t.Fatal(err) + } + + // copy the request body, so we can use it again for the successful delete + reqBody2 := reqBody + + // do the same request again, this time with our UIA, but for a different device ID, this should fail + rec = httptest.NewRecorder() + + req = httptest.NewRequest(http.MethodDelete, "/_matrix/client/v3/devices/"+accessTokens[alice].deviceID, &reqBody) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Fatalf("expected HTTP 403, got %d: %s", rec.Code, rec.Body.String()) + } + + // do the same request again, this time with our UIA, but for the correct device ID, this should be fine + rec = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodDelete, "/_matrix/client/v3/devices/"+secondDeviceID, &reqBody2) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String()) + } + + // verify devices are deleted + rec = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/devices", strings.NewReader("")) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String()) + } + for _, device := range gjson.GetBytes(rec.Body.Bytes(), "devices.#.device_id").Array() { + if device.Str == secondDeviceID { + t.Fatalf("expected device %s to be deleted, but wasn't", secondDeviceID) + } + } + }) +} + +// Deleting devices requires the UIA dance, so do this in a different test +func TestDeleteDevices(t *testing.T) { + alice := test.NewUser(t) + localpart, serverName, _ := gomatrixserverlib.SplitID('@', alice.ID) + + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + cfg, processCtx, closeDB := testrig.CreateConfig(t, dbType) + defer closeDB() + + natsInstance := jetstream.NATSInstance{} + routers := httputil.NewRouters() + cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions) + caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics) + rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics) + userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil) + + // We mostly need the rsAPI/ for this test, so nil for other APIs/caches etc. + AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics) + + accessTokens := map[*test.User]userDevice{ + alice: {}, + } + + // create the account and an initial device + createAccessTokens(t, accessTokens, userAPI, processCtx.Context(), routers) + + // create some more devices + var devices []string + for i := 0; i < 10; i++ { + accessToken := util.RandomString(8) + devRes := &uapi.PerformDeviceCreationResponse{} + if err := userAPI.PerformDeviceCreation(processCtx.Context(), &uapi.PerformDeviceCreationRequest{ + Localpart: localpart, + ServerName: serverName, + AccessToken: accessToken, + NoDeviceListUpdate: true, + }, devRes); err != nil { + t.Fatal(err) + } + if !devRes.DeviceCreated { + t.Fatalf("failed to create device") + } + devices = append(devices, devRes.Device.ID) + } + + // initiate UIA + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/delete_devices", strings.NewReader("")) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected HTTP 401, got %d: %s", rec.Code, rec.Body.String()) + } + // get the session ID + sessionID := gjson.GetBytes(rec.Body.Bytes(), "session").Str + + // prepare UIA request body + reqBody := bytes.Buffer{} + if err := json.NewEncoder(&reqBody).Encode(map[string]interface{}{ + "auth": map[string]string{ + "session": sessionID, + "type": authtypes.LoginTypePassword, + "user": alice.ID, + "password": accessTokens[alice].password, + }, + "devices": devices[5:], + }); err != nil { + t.Fatal(err) + } + + // do the same request again, this time with our UIA, + rec = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/delete_devices", &reqBody) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String()) + } + + // verify devices are deleted + rec = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/devices", strings.NewReader("")) + req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String()) + } + for _, device := range gjson.GetBytes(rec.Body.Bytes(), "devices.#.device_id").Array() { + for _, deletedDevice := range devices[5:] { + if device.Str == deletedDevice { + t.Fatalf("expected device %s to be deleted, but wasn't", deletedDevice) + } + } + } + }) +} + +func createAccessTokens(t *testing.T, accessTokens map[*test.User]userDevice, userAPI uapi.UserInternalAPI, ctx context.Context, routers httputil.Routers) { + t.Helper() + for u := range accessTokens { + localpart, serverName, _ := gomatrixserverlib.SplitID('@', u.ID) + userRes := &uapi.PerformAccountCreationResponse{} + password := util.RandomString(8) + if err := userAPI.PerformAccountCreation(ctx, &uapi.PerformAccountCreationRequest{ + AccountType: u.AccountType, + Localpart: localpart, + ServerName: serverName, + Password: password, + }, userRes); err != nil { + t.Errorf("failed to create account: %s", err) + } + req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{ + "type": authtypes.LoginTypePassword, + "identifier": map[string]interface{}{ + "type": "m.id.user", + "user": u.ID, + }, + "password": password, + })) + rec := httptest.NewRecorder() + routers.Client.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("failed to login: %s", rec.Body.String()) + } + accessTokens[u] = userDevice{ + accessToken: gjson.GetBytes(rec.Body.Bytes(), "access_token").String(), + deviceID: gjson.GetBytes(rec.Body.Bytes(), "device_id").String(), + password: password, + } + } +} diff --git a/clientapi/routing/deactivate.go b/clientapi/routing/deactivate.go index f213db7f3..3f4f539f6 100644 --- a/clientapi/routing/deactivate.go +++ b/clientapi/routing/deactivate.go @@ -33,7 +33,7 @@ func Deactivate( return *errRes } - localpart, _, err := gomatrixserverlib.SplitID('@', login.Username()) + localpart, serverName, err := gomatrixserverlib.SplitID('@', login.Username()) if err != nil { util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed") return jsonerror.InternalServerError() @@ -41,7 +41,8 @@ func Deactivate( var res api.PerformAccountDeactivationResponse err = accountAPI.PerformAccountDeactivation(ctx, &api.PerformAccountDeactivationRequest{ - Localpart: localpart, + Localpart: localpart, + ServerName: serverName, }, &res) if err != nil { util.GetLogger(ctx).WithError(err).Error("userAPI.PerformAccountDeactivation failed") diff --git a/clientapi/routing/device.go b/clientapi/routing/device.go index e3a02661c..331bacc3c 100644 --- a/clientapi/routing/device.go +++ b/clientapi/routing/device.go @@ -15,6 +15,7 @@ package routing import ( + "encoding/json" "io" "net" "net/http" @@ -146,12 +147,6 @@ func UpdateDeviceByID( JSON: jsonerror.Forbidden("device does not exist"), } } - if performRes.Forbidden { - return util.JSONResponse{ - Code: http.StatusForbidden, - JSON: jsonerror.Forbidden("device not owned by current user"), - } - } return util.JSONResponse{ Code: http.StatusOK, @@ -189,7 +184,7 @@ func DeleteDeviceById( if dev != deviceID { return util.JSONResponse{ Code: http.StatusForbidden, - JSON: jsonerror.Forbidden("session & device mismatch"), + JSON: jsonerror.Forbidden("session and device mismatch"), } } } @@ -242,16 +237,37 @@ func DeleteDeviceById( // DeleteDevices handles POST requests to /delete_devices func DeleteDevices( - req *http.Request, userAPI api.ClientUserAPI, device *api.Device, + req *http.Request, userInteractiveAuth *auth.UserInteractive, userAPI api.ClientUserAPI, device *api.Device, ) util.JSONResponse { ctx := req.Context() - payload := devicesDeleteJSON{} - if resErr := httputil.UnmarshalJSONRequest(req, &payload); resErr != nil { - return *resErr + bodyBytes, err := io.ReadAll(req.Body) + if err != nil { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON("The request body could not be read: " + err.Error()), + } + } + defer req.Body.Close() // nolint:errcheck + + // initiate UIA + login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, device) + if errRes != nil { + return *errRes } - defer req.Body.Close() // nolint: errcheck + if login.Username() != device.UserID { + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.Forbidden("unable to delete devices for other user"), + } + } + + payload := devicesDeleteJSON{} + if err = json.Unmarshal(bodyBytes, &payload); err != nil { + util.GetLogger(ctx).WithError(err).Error("unable to unmarshal device deletion request") + return jsonerror.InternalServerError() + } var res api.PerformDeviceDeletionResponse if err := userAPI.PerformDeviceDeletion(ctx, &api.PerformDeviceDeletionRequest{ diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index e261cb3aa..6c8035d40 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -1115,7 +1115,7 @@ func Setup( v3mux.Handle("/delete_devices", httputil.MakeAuthAPI("delete_devices", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - return DeleteDevices(req, userAPI, device) + return DeleteDevices(req, userInteractiveAuth, userAPI, device) }), ).Methods(http.MethodPost, http.MethodOptions) diff --git a/userapi/api/api.go b/userapi/api/api.go index fa297f773..19d486848 100644 --- a/userapi/api/api.go +++ b/userapi/api/api.go @@ -224,7 +224,6 @@ type PerformDeviceUpdateRequest struct { } type PerformDeviceUpdateResponse struct { DeviceExists bool - Forbidden bool } type PerformDeviceDeletionRequest struct { diff --git a/userapi/internal/user_api.go b/userapi/internal/user_api.go index 8977697b5..4049d13b6 100644 --- a/userapi/internal/user_api.go +++ b/userapi/internal/user_api.go @@ -386,11 +386,6 @@ func (a *UserInternalAPI) PerformDeviceUpdate(ctx context.Context, req *api.Perf } res.DeviceExists = true - if dev.UserID != req.RequestingUserID { - res.Forbidden = true - return nil - } - err = a.DB.UpdateDevice(ctx, localpart, domain, req.DeviceID, req.DisplayName) if err != nil { util.GetLogger(ctx).WithError(err).Error("deviceDB.UpdateDevice failed") From 44ed0a327948c0c8812fe2a895af4c3dfde0c33d Mon Sep 17 00:00:00 2001 From: George Antoniadis Date: Mon, 3 Apr 2023 07:13:06 +0100 Subject: [PATCH 8/9] add deployment strategy option to helm chart (#3021) @S7evinK minor update to the helm chart on top of you existing fixes to allow setting the update strategy as the default `RollingUpdate` one is a bit annoying if using `ReadWriteOnce` volumes for media. Hope this makes sense. ### Pull Request Checklist * [x] ~~I have added Go unit tests or [Complement integration tests](https://github.com/matrix-org/complement) for this PR _or_ I have justified why this PR doesn't need tests~~ Haven't touched any go files. * [x] Pull request includes a [sign off below using a legally identifiable name](https://matrix-org.github.io/dendrite/development/contributing#sign-off) _or_ I have already signed off privately Signed-off-by: `George Antoniadis ` [skip ci] --- helm/dendrite/templates/deployment.yaml | 7 +++++++ helm/dendrite/values.yaml | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/helm/dendrite/templates/deployment.yaml b/helm/dendrite/templates/deployment.yaml index 2a0f3a9e9..55ae053f2 100644 --- a/helm/dendrite/templates/deployment.yaml +++ b/helm/dendrite/templates/deployment.yaml @@ -19,6 +19,13 @@ spec: annotations: confighash: secret-{{ .Values.dendrite_config | toYaml | sha256sum | trunc 32 }} spec: + strategy: + type: {{ $.Values.strategy.type }} + {{- if eq $.Values.strategy.type "RollingUpdate" }} + rollingUpdate: + maxSurge: {{ $.Values.strategy.rollingUpdate.maxSurge }} + maxUnavailable: {{ $.Values.strategy.rollingUpdate.maxUnavailable }} + {{- end }} volumes: - name: {{ include "dendrite.fullname" . }}-conf-vol secret: diff --git a/helm/dendrite/values.yaml b/helm/dendrite/values.yaml index c219d27f8..d6be2cdbb 100644 --- a/helm/dendrite/values.yaml +++ b/helm/dendrite/values.yaml @@ -43,6 +43,16 @@ persistence: # -- PVC Storage Request for the search volume capacity: "1Gi" +strategy: + # -- Strategy to use for rolling updates (e.g. Recreate, RollingUpdate) + # If you are using ReadWriteOnce volumes, you should probably use Recreate + type: RollingUpdate + rollingUpdate: + # -- Maximum number of pods that can be unavailable during the update process + maxUnavailable: 25% + # -- Maximum number of pods that can be scheduled above the desired number of pods + maxSurge: 25% + dendrite_config: version: 2 global: From 01dd02dad2c76a8bb6d28e3456bf8df40e16d113 Mon Sep 17 00:00:00 2001 From: Rhea Danzey Date: Mon, 3 Apr 2023 02:00:32 -0500 Subject: [PATCH 9/9] chart - Add configuration for extra volumes / volume mounts (#3042) Adds configuration for additional volumes / volumeMounts to the Dendrite pod to inject configuration / secrets outside of the chart's templates ### Pull Request Checklist * [x] I have added Go unit tests or [Complement integration tests](https://github.com/matrix-org/complement) for this PR _or_ I have justified why this PR doesn't need tests - Helm chart changes * [x] Pull request includes a [sign off below using a legally identifiable name](https://matrix-org.github.io/dendrite/development/contributing#sign-off) _or_ I have already signed off privately Signed-off-by: Rhea Danzey --------- Signed-off-by: Rhea Danzey Co-authored-by: Till <2353100+S7evinK@users.noreply.github.com> [skip ci] --- helm/dendrite/Chart.yaml | 2 +- helm/dendrite/templates/deployment.yaml | 20 +++++++++++++------- helm/dendrite/values.yaml | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/helm/dendrite/Chart.yaml b/helm/dendrite/Chart.yaml index 3ef45a6df..6a428e00f 100644 --- a/helm/dendrite/Chart.yaml +++ b/helm/dendrite/Chart.yaml @@ -1,6 +1,6 @@ apiVersion: v2 name: dendrite -version: "0.12.1" +version: "0.12.2" appVersion: "0.12.0" description: Dendrite Matrix Homeserver type: application diff --git a/helm/dendrite/templates/deployment.yaml b/helm/dendrite/templates/deployment.yaml index 55ae053f2..df7dbbdc3 100644 --- a/helm/dendrite/templates/deployment.yaml +++ b/helm/dendrite/templates/deployment.yaml @@ -12,6 +12,13 @@ spec: matchLabels: {{- include "dendrite.selectorLabels" . | nindent 6 }} replicas: 1 + strategy: + type: {{ $.Values.strategy.type }} + {{- if eq $.Values.strategy.type "RollingUpdate" }} + rollingUpdate: + maxSurge: {{ $.Values.strategy.rollingUpdate.maxSurge }} + maxUnavailable: {{ $.Values.strategy.rollingUpdate.maxUnavailable }} + {{- end }} template: metadata: labels: @@ -19,13 +26,6 @@ spec: annotations: confighash: secret-{{ .Values.dendrite_config | toYaml | sha256sum | trunc 32 }} spec: - strategy: - type: {{ $.Values.strategy.type }} - {{- if eq $.Values.strategy.type "RollingUpdate" }} - rollingUpdate: - maxSurge: {{ $.Values.strategy.rollingUpdate.maxSurge }} - maxUnavailable: {{ $.Values.strategy.rollingUpdate.maxUnavailable }} - {{- end }} volumes: - name: {{ include "dendrite.fullname" . }}-conf-vol secret: @@ -47,6 +47,9 @@ spec: - name: {{ include "dendrite.fullname" . }}-search persistentVolumeClaim: claimName: {{ default (print ( include "dendrite.fullname" . ) "-search-pvc") $.Values.persistence.search.existingClaim | quote }} + {{- with .Values.extraVolumes }} + {{ . | toYaml | nindent 6 }} + {{- end }} containers: - name: {{ .Chart.Name }} {{- include "image.name" . | nindent 8 }} @@ -80,6 +83,9 @@ spec: name: {{ include "dendrite.fullname" . }}-jetstream - mountPath: {{ .Values.dendrite_config.sync_api.search.index_path }} name: {{ include "dendrite.fullname" . }}-search + {{- with .Values.extraVolumeMounts }} + {{ . | toYaml | nindent 8 }} + {{- end }} livenessProbe: initialDelaySeconds: 10 periodSeconds: 10 diff --git a/helm/dendrite/values.yaml b/helm/dendrite/values.yaml index d6be2cdbb..b0e8fc8a8 100644 --- a/helm/dendrite/values.yaml +++ b/helm/dendrite/values.yaml @@ -43,6 +43,20 @@ persistence: # -- PVC Storage Request for the search volume capacity: "1Gi" +# Add additional volumes to the Dendrite Pod +extraVolumes: [] +# ex. +# - name: extra-config +# secret: +# secretName: extra-config + + +# Configure additional mount points volumes in the Dendrite Pod +extraVolumeMounts: [] +# ex. +# - mountPath: /etc/dendrite/extra-config +# name: extra-config + strategy: # -- Strategy to use for rolling updates (e.g. Recreate, RollingUpdate) # If you are using ReadWriteOnce volumes, you should probably use Recreate