From f0139f12ca6f93b7626be369dc2a829c0326c6e0 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Mar 2021 14:01:34 +0000 Subject: [PATCH 01/10] Don't return error when account conflict is handled gracefully (#1782) --- userapi/internal/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/userapi/internal/api.go b/userapi/internal/api.go index 4abf908d..e104714b 100644 --- a/userapi/internal/api.go +++ b/userapi/internal/api.go @@ -87,7 +87,7 @@ func (a *UserInternalAPI) PerformAccountCreation(ctx context.Context, req *api.P ServerName: a.ServerName, UserID: fmt.Sprintf("@%s:%s", req.Localpart, a.ServerName), } - return err + return nil } if err = a.AccountDB.SetDisplayName(ctx, req.Localpart, req.Localpart); err != nil { From d15836e260130f85edd5d9a104e5304f001d2681 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Mar 2021 14:35:57 +0000 Subject: [PATCH 02/10] Increase gocyclo complexity to 25 (and remove all but 2 golint directives related to it) (#1783) --- .golangci.yml | 2 +- clientapi/routing/profile.go | 2 -- clientapi/routing/state.go | 1 - cmd/dendrite-demo-yggdrasil/main.go | 1 - cmd/dendrite-demo-yggdrasil/yggconn/node.go | 1 - cmd/dendrite-demo-yggdrasil/yggconn/session.go | 1 - cmd/furl/main.go | 1 - cmd/resolve-state/main.go | 1 - federationapi/routing/join.go | 2 -- federationapi/routing/leave.go | 2 -- federationapi/routing/publicrooms.go | 1 - federationapi/routing/send.go | 4 ---- federationsender/queue/destinationqueue.go | 3 --- roomserver/api/wrapper.go | 1 - roomserver/internal/helpers/helpers.go | 1 - roomserver/internal/perform/perform_backfill.go | 1 - roomserver/internal/perform/perform_invite.go | 1 - roomserver/internal/perform/perform_join.go | 1 - roomserver/internal/query/query.go | 2 -- roomserver/state/state.go | 1 - roomserver/storage/shared/storage.go | 4 ---- setup/base.go | 1 - setup/mscs/msc2836/msc2836.go | 1 - setup/mscs/msc2946/msc2946.go | 1 - syncapi/internal/keychange.go | 2 -- syncapi/routing/messages.go | 2 -- syncapi/storage/postgres/syncserver.go | 1 - syncapi/storage/shared/syncserver.go | 2 -- syncapi/storage/sqlite3/filtering.go | 1 - syncapi/storage/sqlite3/syncserver.go | 1 - syncapi/streams/stream_pdu.go | 2 -- 31 files changed, 1 insertion(+), 47 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 1499747b..a327370e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -102,7 +102,7 @@ linters-settings: #local-prefixes: github.com/org/project gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) - min-complexity: 13 + min-complexity: 25 maligned: # print struct with more effective memory layout or not, false by default suggest-new: true diff --git a/clientapi/routing/profile.go b/clientapi/routing/profile.go index 0d47c91e..7bea35e5 100644 --- a/clientapi/routing/profile.go +++ b/clientapi/routing/profile.go @@ -91,7 +91,6 @@ func GetAvatarURL( } // SetAvatarURL implements PUT /profile/{userID}/avatar_url -// nolint:gocyclo func SetAvatarURL( req *http.Request, accountDB accounts.Database, device *userapi.Device, userID string, cfg *config.ClientAPI, rsAPI api.RoomserverInternalAPI, @@ -209,7 +208,6 @@ func GetDisplayName( } // SetDisplayName implements PUT /profile/{userID}/displayname -// nolint:gocyclo func SetDisplayName( req *http.Request, accountDB accounts.Database, device *userapi.Device, userID string, cfg *config.ClientAPI, rsAPI api.RoomserverInternalAPI, diff --git a/clientapi/routing/state.go b/clientapi/routing/state.go index 57014bc3..088e412c 100644 --- a/clientapi/routing/state.go +++ b/clientapi/routing/state.go @@ -161,7 +161,6 @@ func OnIncomingStateRequest(ctx context.Context, device *userapi.Device, rsAPI a // state to see if there is an event with that type and state key, if there // is then (by default) we return the content, otherwise a 404. // If eventFormat=true, sends the whole event else just the content. -// nolint:gocyclo func OnIncomingStateTypeRequest( ctx context.Context, device *userapi.Device, rsAPI api.RoomserverInternalAPI, roomID, evType, stateKey string, eventFormat bool, diff --git a/cmd/dendrite-demo-yggdrasil/main.go b/cmd/dendrite-demo-yggdrasil/main.go index 8091298b..2a4a335a 100644 --- a/cmd/dendrite-demo-yggdrasil/main.go +++ b/cmd/dendrite-demo-yggdrasil/main.go @@ -52,7 +52,6 @@ var ( instancePeer = flag.String("peer", "", "an internet Yggdrasil peer to connect to") ) -// nolint:gocyclo func main() { flag.Parse() internal.SetupPprof() diff --git a/cmd/dendrite-demo-yggdrasil/yggconn/node.go b/cmd/dendrite-demo-yggdrasil/yggconn/node.go index c036c0d8..9c286dfb 100644 --- a/cmd/dendrite-demo-yggdrasil/yggconn/node.go +++ b/cmd/dendrite-demo-yggdrasil/yggconn/node.go @@ -73,7 +73,6 @@ func (n *Node) DialerContext(ctx context.Context, network, address string) (net. return n.Dialer(network, address) } -// nolint:gocyclo func Setup(instanceName, storageDirectory string) (*Node, error) { n := &Node{ core: &yggdrasil.Core{}, diff --git a/cmd/dendrite-demo-yggdrasil/yggconn/session.go b/cmd/dendrite-demo-yggdrasil/yggconn/session.go index 0cf524d9..7b56e736 100644 --- a/cmd/dendrite-demo-yggdrasil/yggconn/session.go +++ b/cmd/dendrite-demo-yggdrasil/yggconn/session.go @@ -128,7 +128,6 @@ func (n *Node) Dial(network, address string) (net.Conn, error) { } // Implements http.Transport.DialContext -// nolint:gocyclo func (n *Node) DialContext(ctx context.Context, network, address string) (net.Conn, error) { s, ok1 := n.sessions.Load(address) session, ok2 := s.(*session) diff --git a/cmd/furl/main.go b/cmd/furl/main.go index bec04f0a..75e22338 100644 --- a/cmd/furl/main.go +++ b/cmd/furl/main.go @@ -20,7 +20,6 @@ var requestFrom = flag.String("from", "", "the server name that the request shou var requestKey = flag.String("key", "matrix_key.pem", "the private key to use when signing the request") var requestPost = flag.Bool("post", false, "send a POST request instead of GET (pipe input into stdin or type followed by Ctrl-D)") -// nolint:gocyclo func main() { flag.Parse() diff --git a/cmd/resolve-state/main.go b/cmd/resolve-state/main.go index 69c3489d..30331fbb 100644 --- a/cmd/resolve-state/main.go +++ b/cmd/resolve-state/main.go @@ -24,7 +24,6 @@ import ( var roomVersion = flag.String("roomversion", "5", "the room version to parse events as") -// nolint:gocyclo func main() { ctx := context.Background() cfg := setup.ParseFlags(true) diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 3afc8d5e..e9414033 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -29,7 +29,6 @@ import ( ) // MakeJoin implements the /make_join API -// nolint:gocyclo func MakeJoin( httpReq *http.Request, request *gomatrixserverlib.FederationRequest, @@ -161,7 +160,6 @@ func MakeJoin( // SendJoin implements the /send_join API // The make-join send-join dance makes much more sense as a single // flow so the cyclomatic complexity is high: -// nolint:gocyclo func SendJoin( httpReq *http.Request, request *gomatrixserverlib.FederationRequest, diff --git a/federationapi/routing/leave.go b/federationapi/routing/leave.go index 1a854261..a5231004 100644 --- a/federationapi/routing/leave.go +++ b/federationapi/routing/leave.go @@ -25,7 +25,6 @@ import ( ) // MakeLeave implements the /make_leave API -// nolint:gocyclo func MakeLeave( httpReq *http.Request, request *gomatrixserverlib.FederationRequest, @@ -118,7 +117,6 @@ func MakeLeave( } // SendLeave implements the /send_leave API -// nolint:gocyclo func SendLeave( httpReq *http.Request, request *gomatrixserverlib.FederationRequest, diff --git a/federationapi/routing/publicrooms.go b/federationapi/routing/publicrooms.go index d923a236..ddd92c5c 100644 --- a/federationapi/routing/publicrooms.go +++ b/federationapi/routing/publicrooms.go @@ -111,7 +111,6 @@ func fillPublicRoomsReq(httpReq *http.Request, request *PublicRoomReq) *util.JSO } // due to lots of switches -// nolint:gocyclo func fillInRooms(ctx context.Context, roomIDs []string, rsAPI roomserverAPI.RoomserverInternalAPI) ([]gomatrixserverlib.PublicRoom, error) { avatarTuple := gomatrixserverlib.StateKeyTuple{EventType: "m.room.avatar", StateKey: ""} nameTuple := gomatrixserverlib.StateKeyTuple{EventType: "m.room.name", StateKey: ""} diff --git a/federationapi/routing/send.go b/federationapi/routing/send.go index 02683aea..ea0b54b6 100644 --- a/federationapi/routing/send.go +++ b/federationapi/routing/send.go @@ -279,7 +279,6 @@ func (t *txnReq) haveEventIDs() map[string]bool { return result } -// nolint:gocyclo func (t *txnReq) processEDUs(ctx context.Context) { for _, e := range t.EDUs { switch e.Type { @@ -540,7 +539,6 @@ func checkAllowedByState(e *gomatrixserverlib.Event, stateEvents []*gomatrixserv return gomatrixserverlib.Allowed(e, &authUsingState) } -// nolint:gocyclo func (t *txnReq) processEventWithMissingState(ctx context.Context, e *gomatrixserverlib.Event, roomVersion gomatrixserverlib.RoomVersion) error { // Do this with a fresh context, so that we keep working even if the // original request times out. With any luck, by the time the remote @@ -832,7 +830,6 @@ retryAllowedState: // begin from. Returns an error only if we should terminate the transaction which initiated /get_missing_events // This function recursively calls txnReq.processEvent with the missing events, which will be processed before this function returns. // This means that we may recursively call this function, as we spider back up prev_events. -// nolint:gocyclo func (t *txnReq) getMissingEvents(ctx context.Context, e *gomatrixserverlib.Event, roomVersion gomatrixserverlib.RoomVersion) (newEvents []*gomatrixserverlib.Event, err error) { logger := util.GetLogger(ctx).WithField("event_id", e.EventID()).WithField("room_id", e.RoomID()) needed := gomatrixserverlib.StateNeededForAuth([]*gomatrixserverlib.Event{e}) @@ -935,7 +932,6 @@ func (t *txnReq) lookupMissingStateViaState(ctx context.Context, roomID, eventID return &state, nil } -// nolint:gocyclo func (t *txnReq) lookupMissingStateViaStateIDs(ctx context.Context, roomID, eventID string, roomVersion gomatrixserverlib.RoomVersion) ( *gomatrixserverlib.RespState, error) { util.GetLogger(ctx).Infof("lookupMissingStateViaStateIDs %s", eventID) diff --git a/federationsender/queue/destinationqueue.go b/federationsender/queue/destinationqueue.go index be63290c..33f77a42 100644 --- a/federationsender/queue/destinationqueue.go +++ b/federationsender/queue/destinationqueue.go @@ -173,7 +173,6 @@ func (oq *destinationQueue) wakeQueueIfNeeded() { // getPendingFromDatabase will look at the database and see if // there are any persisted events that haven't been sent to this // destination yet. If so, they will be queued up. -// nolint:gocyclo func (oq *destinationQueue) getPendingFromDatabase() { // Check to see if there's anything to do for this server // in the database. @@ -238,7 +237,6 @@ func (oq *destinationQueue) getPendingFromDatabase() { } // backgroundSend is the worker goroutine for sending events. -// nolint:gocyclo func (oq *destinationQueue) backgroundSend() { // Check if a worker is already running, and if it isn't, then // mark it as started. @@ -353,7 +351,6 @@ func (oq *destinationQueue) backgroundSend() { // nextTransaction creates a new transaction from the pending event // queue and sends it. Returns true if a transaction was sent or // false otherwise. -// nolint:gocyclo func (oq *destinationQueue) nextTransaction( pdus []*queuedPDU, edus []*queuedEDU, diff --git a/roomserver/api/wrapper.go b/roomserver/api/wrapper.go index a6ef735c..2ebe2f64 100644 --- a/roomserver/api/wrapper.go +++ b/roomserver/api/wrapper.go @@ -172,7 +172,6 @@ func IsServerBannedFromRoom(ctx context.Context, rsAPI RoomserverInternalAPI, ro // PopulatePublicRooms extracts PublicRoom information for all the provided room IDs. The IDs are not checked to see if they are visible in the // published room directory. // due to lots of switches -// nolint:gocyclo func PopulatePublicRooms(ctx context.Context, roomIDs []string, rsAPI RoomserverInternalAPI) ([]gomatrixserverlib.PublicRoom, error) { avatarTuple := gomatrixserverlib.StateKeyTuple{EventType: "m.room.avatar", StateKey: ""} nameTuple := gomatrixserverlib.StateKeyTuple{EventType: "m.room.name", StateKey: ""} diff --git a/roomserver/internal/helpers/helpers.go b/roomserver/internal/helpers/helpers.go index 036c717a..a829bffc 100644 --- a/roomserver/internal/helpers/helpers.go +++ b/roomserver/internal/helpers/helpers.go @@ -270,7 +270,6 @@ func CheckServerAllowedToSeeEvent( } // TODO: Remove this when we have tests to assert correctness of this function -// nolint:gocyclo func ScanEventTree( ctx context.Context, db storage.Database, info types.RoomInfo, front []string, visited map[string]bool, limit int, serverName gomatrixserverlib.ServerName, diff --git a/roomserver/internal/perform/perform_backfill.go b/roomserver/internal/perform/perform_backfill.go index eb47ac21..d9d720f2 100644 --- a/roomserver/internal/perform/perform_backfill.go +++ b/roomserver/internal/perform/perform_backfill.go @@ -381,7 +381,6 @@ func (b *backfillRequester) StateBeforeEvent(ctx context.Context, roomVer gomatr // It returns a list of servers which can be queried for backfill requests. These servers // will be servers that are in the room already. The entries at the beginning are preferred servers // and will be tried first. An empty list will fail the request. -// nolint:gocyclo func (b *backfillRequester) ServersAtEvent(ctx context.Context, roomID, eventID string) []gomatrixserverlib.ServerName { // eventID will be a prev_event ID of a backwards extremity, meaning we will not have a database entry for it. Instead, use // its successor, so look it up. diff --git a/roomserver/internal/perform/perform_invite.go b/roomserver/internal/perform/perform_invite.go index 93a52350..fa65ce9b 100644 --- a/roomserver/internal/perform/perform_invite.go +++ b/roomserver/internal/perform/perform_invite.go @@ -37,7 +37,6 @@ type Inviter struct { Inputer *input.Inputer } -// nolint:gocyclo func (r *Inviter) PerformInvite( ctx context.Context, req *api.PerformInviteRequest, diff --git a/roomserver/internal/perform/perform_join.go b/roomserver/internal/perform/perform_join.go index 8eb6b648..453901fc 100644 --- a/roomserver/internal/perform/perform_join.go +++ b/roomserver/internal/perform/perform_join.go @@ -139,7 +139,6 @@ func (r *Joiner) performJoinRoomByAlias( } // TODO: Break this function up a bit -// nolint:gocyclo func (r *Joiner) performJoinRoomByID( ctx context.Context, req *api.PerformJoinRequest, diff --git a/roomserver/internal/query/query.go b/roomserver/internal/query/query.go index 3aa51726..29dc7840 100644 --- a/roomserver/internal/query/query.go +++ b/roomserver/internal/query/query.go @@ -49,7 +49,6 @@ func (r *Queryer) QueryLatestEventsAndState( } // QueryStateAfterEvents implements api.RoomserverInternalAPI -// nolint:gocyclo func (r *Queryer) QueryStateAfterEvents( ctx context.Context, request *api.QueryStateAfterEventsRequest, @@ -372,7 +371,6 @@ func (r *Queryer) QueryServerAllowedToSeeEvent( } // QueryMissingEvents implements api.RoomserverInternalAPI -// nolint:gocyclo func (r *Queryer) QueryMissingEvents( ctx context.Context, request *api.QueryMissingEventsRequest, diff --git a/roomserver/state/state.go b/roomserver/state/state.go index 2c01ca03..0d9511ac 100644 --- a/roomserver/state/state.go +++ b/roomserver/state/state.go @@ -770,7 +770,6 @@ func (v *StateResolution) resolveConflictsV1( // Returns a list that combines the entries without conflicts with the result of state resolution for the entries with conflicts. // The returned list is sorted by state key tuple. // Returns an error if there was a problem talking to the database. -// nolint:gocyclo func (v *StateResolution) resolveConflictsV2( ctx context.Context, notConflicted, conflicted []types.StateEntry, diff --git a/roomserver/storage/shared/storage.go b/roomserver/storage/shared/storage.go index b4d9d562..24b48772 100644 --- a/roomserver/storage/shared/storage.go +++ b/roomserver/storage/shared/storage.go @@ -412,7 +412,6 @@ func (d *Database) GetLatestEventsForUpdate( return updater, err } -// nolint:gocyclo func (d *Database) StoreEvent( ctx context.Context, event *gomatrixserverlib.Event, txnAndSessionID *api.TransactionID, authEventNIDs []types.EventNID, isRejected bool, @@ -672,7 +671,6 @@ func extractRoomVersionFromCreateEvent(event *gomatrixserverlib.Event) ( // to cross-reference with other tables when loading. // // Returns the redaction event and the event ID of the redacted event if this call resulted in a redaction. -// nolint:gocyclo func (d *Database) handleRedactions( ctx context.Context, txn *sql.Tx, eventNID types.EventNID, event *gomatrixserverlib.Event, ) (*gomatrixserverlib.Event, string, error) { @@ -802,7 +800,6 @@ func (d *Database) loadEvent(ctx context.Context, eventID string) *types.Event { // GetStateEvent returns the current state event of a given type for a given room with a given state key // If no event could be found, returns nil // If there was an issue during the retrieval, returns an error -// nolint:gocyclo func (d *Database) GetStateEvent(ctx context.Context, roomID, evType, stateKey string) (*gomatrixserverlib.HeaderedEvent, error) { roomInfo, err := d.RoomInfo(ctx, roomID) if err != nil { @@ -893,7 +890,6 @@ func (d *Database) GetRoomsByMembership(ctx context.Context, userID, membership // GetBulkStateContent returns all state events which match a given room ID and a given state key tuple. Both must be satisfied for a match. // If a tuple has the StateKey of '*' and allowWildcards=true then all state events with the EventType should be returned. -// nolint:gocyclo func (d *Database) GetBulkStateContent(ctx context.Context, roomIDs []string, tuples []gomatrixserverlib.StateKeyTuple, allowWildcards bool) ([]tables.StrippedEvent, error) { eventTypes := make([]string, 0, len(tuples)) for _, tuple := range tuples { diff --git a/setup/base.go b/setup/base.go index 6522426c..e9aa2a45 100644 --- a/setup/base.go +++ b/setup/base.go @@ -316,7 +316,6 @@ func (b *BaseDendrite) CreateFederationClient() *gomatrixserverlib.FederationCli // SetupAndServeHTTP sets up the HTTP server to serve endpoints registered on // ApiMux under /api/ and adds a prometheus handler under /metrics. -// nolint:gocyclo func (b *BaseDendrite) SetupAndServeHTTP( internalHTTPAddr, externalHTTPAddr config.HTTPAddress, certFile, keyFile *string, diff --git a/setup/mscs/msc2836/msc2836.go b/setup/mscs/msc2836/msc2836.go index 95473f97..f47a42d9 100644 --- a/setup/mscs/msc2836/msc2836.go +++ b/setup/mscs/msc2836/msc2836.go @@ -238,7 +238,6 @@ func federatedEventRelationship( } } -// nolint:gocyclo func (rc *reqCtx) process() (*gomatrixserverlib.MSC2836EventRelationshipsResponse, *util.JSONResponse) { var res gomatrixserverlib.MSC2836EventRelationshipsResponse var returnEvents []*gomatrixserverlib.HeaderedEvent diff --git a/setup/mscs/msc2946/msc2946.go b/setup/mscs/msc2946/msc2946.go index a125fd78..6fdfeed6 100644 --- a/setup/mscs/msc2946/msc2946.go +++ b/setup/mscs/msc2946/msc2946.go @@ -217,7 +217,6 @@ func (w *walker) markSent(id string) { w.inMemoryBatchCache[w.callerID()] = m } -// nolint:gocyclo func (w *walker) walk() *gomatrixserverlib.MSC2946SpacesResponse { var res gomatrixserverlib.MSC2946SpacesResponse // Begin walking the graph starting with the room ID in the request in a queue of unvisited rooms diff --git a/syncapi/internal/keychange.go b/syncapi/internal/keychange.go index e980437e..0bbaf31e 100644 --- a/syncapi/internal/keychange.go +++ b/syncapi/internal/keychange.go @@ -46,7 +46,6 @@ func DeviceOTKCounts(ctx context.Context, keyAPI keyapi.KeyInternalAPI, userID, // DeviceListCatchup fills in the given response for the given user ID to bring it up-to-date with device lists. hasNew=true if the response // was filled in, else false if there are no new device list changes because there is nothing to catch up on. The response MUST // be already filled in with join/leave information. -// nolint:gocyclo func DeviceListCatchup( ctx context.Context, keyAPI keyapi.KeyInternalAPI, rsAPI roomserverAPI.RoomserverInternalAPI, userID string, res *types.Response, from, to types.LogPosition, @@ -137,7 +136,6 @@ func DeviceListCatchup( } // TrackChangedUsers calculates the values of device_lists.changed|left in the /sync response. -// nolint:gocyclo func TrackChangedUsers( ctx context.Context, rsAPI roomserverAPI.RoomserverInternalAPI, userID string, newlyJoinedRooms, newlyLeftRooms []string, ) (changed, left []string, err error) { diff --git a/syncapi/routing/messages.go b/syncapi/routing/messages.go index ba739148..2ef25e03 100644 --- a/syncapi/routing/messages.go +++ b/syncapi/routing/messages.go @@ -61,7 +61,6 @@ const defaultMessagesLimit = 10 // OnIncomingMessagesRequest implements the /messages endpoint from the // client-server API. // See: https://matrix.org/docs/spec/client_server/latest.html#get-matrix-client-r0-rooms-roomid-messages -// nolint:gocyclo func OnIncomingMessagesRequest( req *http.Request, db storage.Database, roomID string, device *userapi.Device, federation *gomatrixserverlib.FederationClient, @@ -306,7 +305,6 @@ func (r *messagesReq) retrieveEvents() ( return clientEvents, start, end, err } -// nolint:gocyclo func (r *messagesReq) filterHistoryVisible(events []*gomatrixserverlib.HeaderedEvent) []*gomatrixserverlib.HeaderedEvent { // TODO FIXME: We don't fully implement history visibility yet. To avoid leaking events which the // user shouldn't see, we check the recent events and remove any prior to the join event of the user diff --git a/syncapi/storage/postgres/syncserver.go b/syncapi/storage/postgres/syncserver.go index a69fda4f..6f4e7749 100644 --- a/syncapi/storage/postgres/syncserver.go +++ b/syncapi/storage/postgres/syncserver.go @@ -36,7 +36,6 @@ type SyncServerDatasource struct { } // NewDatabase creates a new sync server database -// nolint:gocyclo func NewDatabase(dbProperties *config.DatabaseOptions) (*SyncServerDatasource, error) { var d SyncServerDatasource var err error diff --git a/syncapi/storage/shared/syncserver.go b/syncapi/storage/shared/syncserver.go index ee2c176c..b8271877 100644 --- a/syncapi/storage/shared/syncserver.go +++ b/syncapi/storage/shared/syncserver.go @@ -661,7 +661,6 @@ func (d *Database) fetchMissingStateEvents( // exclusive of oldPos, inclusive of newPos, for the rooms in which // the user has new membership events. // A list of joined room IDs is also returned in case the caller needs it. -// nolint:gocyclo func (d *Database) GetStateDeltas( ctx context.Context, device *userapi.Device, r types.Range, userID string, @@ -773,7 +772,6 @@ func (d *Database) GetStateDeltas( // requests with full_state=true. // Fetches full state for all joined rooms and uses selectStateInRange to get // updates for other rooms. -// nolint:gocyclo func (d *Database) GetStateDeltasForFullStateSync( ctx context.Context, device *userapi.Device, r types.Range, userID string, diff --git a/syncapi/storage/sqlite3/filtering.go b/syncapi/storage/sqlite3/filtering.go index 050f3a26..11f3e647 100644 --- a/syncapi/storage/sqlite3/filtering.go +++ b/syncapi/storage/sqlite3/filtering.go @@ -23,7 +23,6 @@ const ( // fields might come from either a StateFilter or an EventFilter, // and it's easier just to have the caller extract the relevant // parts. -// nolint:gocyclo func prepareWithFilters( db *sql.DB, txn *sql.Tx, query string, params []interface{}, senders, notsenders, types, nottypes []string, excludeEventIDs []string, diff --git a/syncapi/storage/sqlite3/syncserver.go b/syncapi/storage/sqlite3/syncserver.go index b0e43b68..ae0647fc 100644 --- a/syncapi/storage/sqlite3/syncserver.go +++ b/syncapi/storage/sqlite3/syncserver.go @@ -52,7 +52,6 @@ func NewDatabase(dbProperties *config.DatabaseOptions) (*SyncServerDatasource, e return &d, nil } -// nolint:gocyclo func (d *SyncServerDatasource) prepare(dbProperties *config.DatabaseOptions) (err error) { if err = d.PartitionOffsetStatements.Prepare(d.db, d.writer, "syncapi"); err != nil { return err diff --git a/syncapi/streams/stream_pdu.go b/syncapi/streams/stream_pdu.go index e11ac8dd..1486ad3c 100644 --- a/syncapi/streams/stream_pdu.go +++ b/syncapi/streams/stream_pdu.go @@ -137,7 +137,6 @@ func (p *PDUStreamProvider) CompleteSync( return to } -// nolint:gocyclo func (p *PDUStreamProvider) IncrementalSync( ctx context.Context, req *types.SyncRequest, @@ -254,7 +253,6 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse( return nil } -// nolint:gocyclo func (p *PDUStreamProvider) getJoinResponseForCompleteSync( ctx context.Context, roomID string, From a2773922d2fe40e6d95d73f532640702709ab526 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 3 Mar 2021 16:27:44 +0000 Subject: [PATCH 03/10] Send events to appservice based on room membership (#1680) * Check membership of room * Use QueryStateAfterEventsResponse * Fix complexity * Changes that I made a long time ago * Rename to appserviceJoinedAtEvent * Check membership in GetMemberships * Update QueryMembershipsForRoom * Tweaks in client API * Update appserviceJoinedAtEvent * Comments * Try QueryMembershipForUser instead * Undo some changes to client API that shouldn't be needed * More /event tweaks * Refactor /event bit * Go back to QueryMembershipsForRoom because appservices are hard * Fix bugs in onMessage * Add comments Co-authored-by: Neil Alexander --- appservice/consumers/roomserver.go | 42 +++++++++++++++++++-- appservice/workers/transaction_scheduler.go | 2 +- clientapi/routing/getevent.go | 16 +++++++- roomserver/api/query.go | 4 +- roomserver/internal/query/query.go | 21 +++++++++++ userapi/api/api.go | 3 ++ userapi/internal/api.go | 3 +- 7 files changed, 83 insertions(+), 8 deletions(-) diff --git a/appservice/consumers/roomserver.go b/appservice/consumers/roomserver.go index 5cbffa35..2ad7f68f 100644 --- a/appservice/consumers/roomserver.go +++ b/appservice/consumers/roomserver.go @@ -85,9 +85,6 @@ func (s *OutputRoomEventConsumer) onMessage(msg *sarama.ConsumerMessage) error { } if output.Type != api.OutputTypeNewRoomEvent { - log.WithField("type", output.Type).Debug( - "roomserver output log: ignoring unknown output type", - ) return nil } @@ -114,6 +111,7 @@ func (s *OutputRoomEventConsumer) filterRoomserverEvents( // Queue this event to be sent off to the application service if err := s.asDB.StoreEvent(ctx, ws.AppService.ID, event); err != nil { log.WithError(err).Warn("failed to insert incoming event into appservices database") + return err } else { // Tell our worker to send out new messages by updating remaining message // count and waking them up with a broadcast @@ -126,8 +124,43 @@ func (s *OutputRoomEventConsumer) filterRoomserverEvents( return nil } +// appserviceJoinedAtEvent returns a boolean depending on whether a given +// appservice has membership at the time a given event was created. +func (s *OutputRoomEventConsumer) appserviceJoinedAtEvent(ctx context.Context, event *gomatrixserverlib.HeaderedEvent, appservice config.ApplicationService) bool { + // TODO: This is only checking the current room state, not the state at + // the event in question. Pretty sure this is what Synapse does too, but + // until we have a lighter way of checking the state before the event that + // doesn't involve state res, then this is probably OK. + membershipReq := &api.QueryMembershipsForRoomRequest{ + RoomID: event.RoomID(), + JoinedOnly: true, + } + membershipRes := &api.QueryMembershipsForRoomResponse{} + + // XXX: This could potentially race if the state for the event is not known yet + // e.g. the event came over federation but we do not have the full state persisted. + if err := s.rsAPI.QueryMembershipsForRoom(ctx, membershipReq, membershipRes); err == nil { + for _, ev := range membershipRes.JoinEvents { + var membership gomatrixserverlib.MemberContent + if err = json.Unmarshal(ev.Content, &membership); err != nil || ev.StateKey == nil { + continue + } + if appservice.IsInterestedInUserID(*ev.StateKey) { + return true + } + } + } else { + log.WithFields(log.Fields{ + "room_id": event.RoomID(), + }).WithError(err).Errorf("Unable to get membership for room") + } + return false +} + // appserviceIsInterestedInEvent returns a boolean depending on whether a given // event falls within one of a given application service's namespaces. +// +// TODO: This should be cached, see https://github.com/matrix-org/dendrite/issues/1682 func (s *OutputRoomEventConsumer) appserviceIsInterestedInEvent(ctx context.Context, event *gomatrixserverlib.HeaderedEvent, appservice config.ApplicationService) bool { // No reason to queue events if they'll never be sent to the application // service @@ -162,5 +195,6 @@ func (s *OutputRoomEventConsumer) appserviceIsInterestedInEvent(ctx context.Cont }).WithError(err).Errorf("Unable to get aliases for room") } - return false + // Check if any of the members in the room match the appservice + return s.appserviceJoinedAtEvent(ctx, event, appservice) } diff --git a/appservice/workers/transaction_scheduler.go b/appservice/workers/transaction_scheduler.go index 6528fc1b..45748c21 100644 --- a/appservice/workers/transaction_scheduler.go +++ b/appservice/workers/transaction_scheduler.go @@ -62,7 +62,7 @@ func SetupTransactionWorkers( func worker(db storage.Database, ws types.ApplicationServiceWorkerState) { log.WithFields(log.Fields{ "appservice": ws.AppService.ID, - }).Info("starting application service") + }).Info("Starting application service") ctx := context.Background() // Create a HTTP client for sending requests to app services diff --git a/clientapi/routing/getevent.go b/clientapi/routing/getevent.go index 29340cc0..36f3ee9e 100644 --- a/clientapi/routing/getevent.go +++ b/clientapi/routing/getevent.go @@ -103,8 +103,22 @@ func GetEvent( } } + var appService *config.ApplicationService + if device.AppserviceID != "" { + for _, as := range cfg.Derived.ApplicationServices { + if as.ID == device.AppserviceID { + appService = &as + break + } + } + } + for _, stateEvent := range stateResp.StateEvents { - if !stateEvent.StateKeyEquals(device.UserID) { + if appService != nil { + if !appService.IsInterestedInUserID(*stateEvent.StateKey()) { + continue + } + } else if !stateEvent.StateKeyEquals(device.UserID) { continue } membership, err := stateEvent.Membership() diff --git a/roomserver/api/query.go b/roomserver/api/query.go index 43bbfd16..af35f7e7 100644 --- a/roomserver/api/query.go +++ b/roomserver/api/query.go @@ -151,7 +151,9 @@ type QueryMembershipsForRoomRequest struct { JoinedOnly bool `json:"joined_only"` // ID of the room to fetch memberships from RoomID string `json:"room_id"` - // ID of the user sending the request + // Optional - ID of the user sending the request, for checking if the + // user is allowed to see the memberships. If not specified then all + // room memberships will be returned. Sender string `json:"sender"` } diff --git a/roomserver/internal/query/query.go b/roomserver/internal/query/query.go index 29dc7840..f69f67f7 100644 --- a/roomserver/internal/query/query.go +++ b/roomserver/internal/query/query.go @@ -242,6 +242,27 @@ func (r *Queryer) QueryMembershipsForRoom( return err } + // If no sender is specified then we will just return the entire + // set of memberships for the room, regardless of whether a specific + // user is allowed to see them or not. + if request.Sender == "" { + var events []types.Event + var eventNIDs []types.EventNID + eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.JoinedOnly, false) + if err != nil { + return fmt.Errorf("r.DB.GetMembershipEventNIDsForRoom: %w", err) + } + events, err = r.DB.Events(ctx, eventNIDs) + if err != nil { + return fmt.Errorf("r.DB.Events: %w", err) + } + for _, event := range events { + clientEvent := gomatrixserverlib.ToClientEvent(event.Event, gomatrixserverlib.FormatAll) + response.JoinEvents = append(response.JoinEvents, clientEvent) + } + return nil + } + membershipEventNID, stillInRoom, isRoomforgotten, err := r.DB.GetMembership(ctx, info.RoomNID, request.Sender) if err != nil { return err diff --git a/userapi/api/api.go b/userapi/api/api.go index 809ba047..45e4e834 100644 --- a/userapi/api/api.go +++ b/userapi/api/api.go @@ -241,6 +241,9 @@ type Device struct { LastSeenTS int64 LastSeenIP string UserAgent string + // If the device is for an appservice user, + // this is the appservice ID. + AppserviceID string } // Account represents a Matrix account on this home server. diff --git a/userapi/internal/api.go b/userapi/internal/api.go index e104714b..0d01afa1 100644 --- a/userapi/internal/api.go +++ b/userapi/internal/api.go @@ -381,7 +381,8 @@ func (a *UserInternalAPI) queryAppServiceToken(ctx context.Context, token, appSe // Use AS dummy device ID ID: types.AppServiceDeviceID, // AS dummy device has AS's token. - AccessToken: token, + AccessToken: token, + AppserviceID: appService.ID, } localpart, err := userutil.ParseUsernameParam(appServiceUserID, &a.ServerName) From 9557ccada4efe50d0f370019ad0b9f017fc7ebcf Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 3 Mar 2021 17:00:31 +0000 Subject: [PATCH 04/10] Fix appsevice alias queries part 2 (#1684) * Check membership of room * Use QueryStateAfterEventsResponse * Fix complexity * Add field ShouldHitAppservice to GetRoomIDForAlias * Hit appservice when trying to join a non-existent alias * remove unused * Changes that I made a long time ago * Rename to appserviceJoinedAtEvent * Check membership in GetMemberships * Update QueryMembershipsForRoom * Tweaks in client API * Update appserviceJoinedAtEvent * Comments * Try QueryMembershipForUser instead * Undo some changes to client API that shouldn't be needed * More /event tweaks * Refactor /event bit * Go back to QueryMembershipsForRoom because appservices are hard * Fix bugs in onMessage * Add comments * More logical naming, clean up a bit Co-authored-by: Neil Alexander --- clientapi/routing/createroom.go | 3 +- clientapi/routing/directory.go | 9 +++-- federationapi/routing/query.go | 9 +++-- roomserver/api/alias.go | 3 ++ roomserver/internal/alias.go | 40 +++++++++++---------- roomserver/internal/api.go | 1 + roomserver/internal/perform/perform_join.go | 10 +++++- 7 files changed, 49 insertions(+), 26 deletions(-) diff --git a/clientapi/routing/createroom.go b/clientapi/routing/createroom.go index 9be0b512..2d886746 100644 --- a/clientapi/routing/createroom.go +++ b/clientapi/routing/createroom.go @@ -217,7 +217,8 @@ func createRoom( roomAlias = fmt.Sprintf("#%s:%s", r.RoomAliasName, cfg.Matrix.ServerName) // check it's free TODO: This races but is better than nothing hasAliasReq := roomserverAPI.GetRoomIDForAliasRequest{ - Alias: roomAlias, + Alias: roomAlias, + IncludeAppservices: false, } var aliasResp roomserverAPI.GetRoomIDForAliasResponse diff --git a/clientapi/routing/directory.go b/clientapi/routing/directory.go index 1b844c4e..0e994b64 100644 --- a/clientapi/routing/directory.go +++ b/clientapi/routing/directory.go @@ -61,9 +61,12 @@ func DirectoryRoom( var res roomDirectoryResponse // Query the roomserver API to check if the alias exists locally. - queryReq := roomserverAPI.GetRoomIDForAliasRequest{Alias: roomAlias} - var queryRes roomserverAPI.GetRoomIDForAliasResponse - if err = rsAPI.GetRoomIDForAlias(req.Context(), &queryReq, &queryRes); err != nil { + queryReq := &roomserverAPI.GetRoomIDForAliasRequest{ + Alias: roomAlias, + IncludeAppservices: true, + } + queryRes := &roomserverAPI.GetRoomIDForAliasResponse{} + if err = rsAPI.GetRoomIDForAlias(req.Context(), queryReq, queryRes); err != nil { util.GetLogger(req.Context()).WithError(err).Error("rsAPI.GetRoomIDForAlias failed") return jsonerror.InternalServerError() } diff --git a/federationapi/routing/query.go b/federationapi/routing/query.go index 6c25b4d3..b4158f0c 100644 --- a/federationapi/routing/query.go +++ b/federationapi/routing/query.go @@ -53,9 +53,12 @@ func RoomAliasToID( var resp gomatrixserverlib.RespDirectory if domain == cfg.Matrix.ServerName { - queryReq := roomserverAPI.GetRoomIDForAliasRequest{Alias: roomAlias} - var queryRes roomserverAPI.GetRoomIDForAliasResponse - if err = rsAPI.GetRoomIDForAlias(httpReq.Context(), &queryReq, &queryRes); err != nil { + queryReq := &roomserverAPI.GetRoomIDForAliasRequest{ + Alias: roomAlias, + IncludeAppservices: true, + } + queryRes := &roomserverAPI.GetRoomIDForAliasResponse{} + if err = rsAPI.GetRoomIDForAlias(httpReq.Context(), queryReq, queryRes); err != nil { util.GetLogger(httpReq.Context()).WithError(err).Error("aliasAPI.GetRoomIDForAlias failed") return jsonerror.InternalServerError() } diff --git a/roomserver/api/alias.go b/roomserver/api/alias.go index 61fdc611..2eb91129 100644 --- a/roomserver/api/alias.go +++ b/roomserver/api/alias.go @@ -34,6 +34,9 @@ type SetRoomAliasResponse struct { type GetRoomIDForAliasRequest struct { // Alias we want to lookup Alias string `json:"alias"` + // Should we ask appservices for their aliases as a part of + // the request? + IncludeAppservices bool `json:"include_appservices"` } // GetRoomIDForAliasResponse is a response to GetRoomIDForAlias diff --git a/roomserver/internal/alias.go b/roomserver/internal/alias.go index 843b0bcc..f15881d7 100644 --- a/roomserver/internal/alias.go +++ b/roomserver/internal/alias.go @@ -88,31 +88,35 @@ func (r *RoomserverInternalAPI) GetRoomIDForAlias( ) error { // Look up the room ID in the database roomID, err := r.DB.GetRoomIDForAlias(ctx, request.Alias) - if err != nil { - return err + if err == nil && roomID != "" { + response.RoomID = roomID + return nil } - if r.asAPI != nil { // appservice component is wired in - if roomID == "" { - // No room found locally, try our application services by making a call to - // the appservice component - aliasReq := asAPI.RoomAliasExistsRequest{Alias: request.Alias} - var aliasResp asAPI.RoomAliasExistsResponse - if err = r.asAPI.RoomAliasExists(ctx, &aliasReq, &aliasResp); err != nil { + // Check appservice on err, but only if the appservice API is + // wired in and no room ID was found. + if r.asAPI != nil && request.IncludeAppservices && roomID == "" { + // No room found locally, try our application services by making a call to + // the appservice component + aliasReq := &asAPI.RoomAliasExistsRequest{ + Alias: request.Alias, + } + aliasRes := &asAPI.RoomAliasExistsResponse{} + if err = r.asAPI.RoomAliasExists(ctx, aliasReq, aliasRes); err != nil { + return err + } + + if aliasRes.AliasExists { + roomID, err = r.DB.GetRoomIDForAlias(ctx, request.Alias) + if err != nil { return err } - - if aliasResp.AliasExists { - roomID, err = r.DB.GetRoomIDForAlias(ctx, request.Alias) - if err != nil { - return err - } - } + response.RoomID = roomID + return nil } } - response.RoomID = roomID - return nil + return err } // GetAliasesForRoomID implements alias.RoomserverInternalAPI diff --git a/roomserver/internal/api.go b/roomserver/internal/api.go index e10bdb46..5b8959b0 100644 --- a/roomserver/internal/api.go +++ b/roomserver/internal/api.go @@ -89,6 +89,7 @@ func (r *RoomserverInternalAPI) SetFederationSenderAPI(fsAPI fsAPI.FederationSen Cfg: r.Cfg, DB: r.DB, FSAPI: r.fsAPI, + RSAPI: r, Inputer: r.Inputer, } r.Peeker = &perform.Peeker{ diff --git a/roomserver/internal/perform/perform_join.go b/roomserver/internal/perform/perform_join.go index 453901fc..ada584a8 100644 --- a/roomserver/internal/perform/perform_join.go +++ b/roomserver/internal/perform/perform_join.go @@ -24,6 +24,7 @@ import ( fsAPI "github.com/matrix-org/dendrite/federationsender/api" "github.com/matrix-org/dendrite/internal/eventutil" "github.com/matrix-org/dendrite/roomserver/api" + rsAPI "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/roomserver/internal/helpers" "github.com/matrix-org/dendrite/roomserver/internal/input" "github.com/matrix-org/dendrite/roomserver/storage" @@ -36,6 +37,7 @@ type Joiner struct { ServerName gomatrixserverlib.ServerName Cfg *config.RoomServer FSAPI fsAPI.FederationSenderInternalAPI + RSAPI rsAPI.RoomserverInternalAPI DB storage.Database Inputer *input.Inputer @@ -121,11 +123,17 @@ func (r *Joiner) performJoinRoomByAlias( roomID = dirRes.RoomID req.ServerNames = append(req.ServerNames, dirRes.ServerNames...) } else { + var getRoomReq = rsAPI.GetRoomIDForAliasRequest{ + Alias: req.RoomIDOrAlias, + IncludeAppservices: true, + } + var getRoomRes = rsAPI.GetRoomIDForAliasResponse{} // Otherwise, look up if we know this room alias locally. - roomID, err = r.DB.GetRoomIDForAlias(ctx, req.RoomIDOrAlias) + err = r.RSAPI.GetRoomIDForAlias(ctx, &getRoomReq, &getRoomRes) if err != nil { return "", "", fmt.Errorf("Lookup room alias %q failed: %w", req.RoomIDOrAlias, err) } + roomID = getRoomRes.RoomID } // If the room ID is empty then we failed to look up the alias. From 1ad96e2e2df9dc1f5fa7d31522babd6a64ca517f Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 5 Mar 2021 10:40:27 +0000 Subject: [PATCH 05/10] Tweak AS registration check and AS component HTTP clients (#1785) * Tweak AS registration check * Check appservice usernames using correct function * Update sytest-whitelist * Use gomatrixserverlib.Client since that allows us to disable TLS validation using the config * Add appservice-specific client and ability to control TLS validation for appservices only * Set timeout on appservice client * Review comments * Remove dead code * Enforce LoginTypeApplicationService after all * Check correct auth type field --- appservice/appservice.go | 11 +++------ appservice/query/query.go | 25 +++---------------- appservice/workers/transaction_scheduler.go | 16 ++++-------- clientapi/routing/register.go | 27 +++++++++++++++------ cmd/generate-config/main.go | 1 + dendrite-config.yaml | 5 ++++ setup/base.go | 16 ++++++++++++ setup/config/config_appservice.go | 4 +++ sytest-whitelist | 6 +++++ 9 files changed, 64 insertions(+), 47 deletions(-) diff --git a/appservice/appservice.go b/appservice/appservice.go index d783c7eb..f608e8e7 100644 --- a/appservice/appservice.go +++ b/appservice/appservice.go @@ -16,9 +16,7 @@ package appservice import ( "context" - "net/http" "sync" - "time" "github.com/gorilla/mux" appserviceAPI "github.com/matrix-org/dendrite/appservice/api" @@ -48,6 +46,7 @@ func NewInternalAPI( userAPI userapi.UserInternalAPI, rsAPI roomserverAPI.RoomserverInternalAPI, ) appserviceAPI.AppServiceQueryAPI { + client := base.CreateAppserviceClient() consumer, _ := kafka.SetupConsumerProducer(&base.Cfg.Global.Kafka) // Create a connection to the appservice postgres DB @@ -79,10 +78,8 @@ func NewInternalAPI( // Create appserivce query API with an HTTP client that will be used for all // outbound and inbound requests (inbound only for the internal API) appserviceQueryAPI := &query.AppServiceQueryAPI{ - HTTPClient: &http.Client{ - Timeout: time.Second * 30, - }, - Cfg: base.Cfg, + HTTPClient: client, + Cfg: base.Cfg, } // Only consume if we actually have ASes to track, else we'll just chew cycles needlessly. @@ -98,7 +95,7 @@ func NewInternalAPI( } // Create application service transaction workers - if err := workers.SetupTransactionWorkers(appserviceDB, workerStates); err != nil { + if err := workers.SetupTransactionWorkers(client, appserviceDB, workerStates); err != nil { logrus.WithError(err).Panicf("failed to start app service transaction workers") } return appserviceQueryAPI diff --git a/appservice/query/query.go b/appservice/query/query.go index 7e5ac475..b4c33528 100644 --- a/appservice/query/query.go +++ b/appservice/query/query.go @@ -20,10 +20,10 @@ import ( "context" "net/http" "net/url" - "time" "github.com/matrix-org/dendrite/appservice/api" "github.com/matrix-org/dendrite/setup/config" + "github.com/matrix-org/gomatrixserverlib" opentracing "github.com/opentracing/opentracing-go" log "github.com/sirupsen/logrus" ) @@ -33,7 +33,7 @@ const userIDExistsPath = "/users/" // AppServiceQueryAPI is an implementation of api.AppServiceQueryAPI type AppServiceQueryAPI struct { - HTTPClient *http.Client + HTTPClient *gomatrixserverlib.Client Cfg *config.Dendrite } @@ -47,11 +47,6 @@ func (a *AppServiceQueryAPI) RoomAliasExists( span, ctx := opentracing.StartSpanFromContext(ctx, "ApplicationServiceRoomAlias") defer span.Finish() - // Create an HTTP client if one does not already exist - if a.HTTPClient == nil { - a.HTTPClient = makeHTTPClient() - } - // Determine which application service should handle this request for _, appservice := range a.Cfg.Derived.ApplicationServices { if appservice.URL != "" && appservice.IsInterestedInRoomAlias(request.Alias) { @@ -68,7 +63,7 @@ func (a *AppServiceQueryAPI) RoomAliasExists( } req = req.WithContext(ctx) - resp, err := a.HTTPClient.Do(req) + resp, err := a.HTTPClient.DoHTTPRequest(ctx, req) if resp != nil { defer func() { err = resp.Body.Close() @@ -115,11 +110,6 @@ func (a *AppServiceQueryAPI) UserIDExists( span, ctx := opentracing.StartSpanFromContext(ctx, "ApplicationServiceUserID") defer span.Finish() - // Create an HTTP client if one does not already exist - if a.HTTPClient == nil { - a.HTTPClient = makeHTTPClient() - } - // Determine which application service should handle this request for _, appservice := range a.Cfg.Derived.ApplicationServices { if appservice.URL != "" && appservice.IsInterestedInUserID(request.UserID) { @@ -134,7 +124,7 @@ func (a *AppServiceQueryAPI) UserIDExists( if err != nil { return err } - resp, err := a.HTTPClient.Do(req.WithContext(ctx)) + resp, err := a.HTTPClient.DoHTTPRequest(ctx, req) if resp != nil { defer func() { err = resp.Body.Close() @@ -169,10 +159,3 @@ func (a *AppServiceQueryAPI) UserIDExists( response.UserIDExists = false return nil } - -// makeHTTPClient creates an HTTP client with certain options that will be used for all query requests to application services -func makeHTTPClient() *http.Client { - return &http.Client{ - Timeout: time.Second * 30, - } -} diff --git a/appservice/workers/transaction_scheduler.go b/appservice/workers/transaction_scheduler.go index 45748c21..47d447c2 100644 --- a/appservice/workers/transaction_scheduler.go +++ b/appservice/workers/transaction_scheduler.go @@ -34,8 +34,6 @@ import ( var ( // Maximum size of events sent in each transaction. transactionBatchSize = 50 - // Timeout for sending a single transaction to an application service. - transactionTimeout = time.Second * 60 ) // SetupTransactionWorkers spawns a separate goroutine for each application @@ -44,6 +42,7 @@ var ( // size), then send that off to the AS's /transactions/{txnID} endpoint. It also // handles exponentially backing off in case the AS isn't currently available. func SetupTransactionWorkers( + client *gomatrixserverlib.Client, appserviceDB storage.Database, workerStates []types.ApplicationServiceWorkerState, ) error { @@ -51,7 +50,7 @@ func SetupTransactionWorkers( for _, workerState := range workerStates { // Don't create a worker if this AS doesn't want to receive events if workerState.AppService.URL != "" { - go worker(appserviceDB, workerState) + go worker(client, appserviceDB, workerState) } } return nil @@ -59,17 +58,12 @@ func SetupTransactionWorkers( // worker is a goroutine that sends any queued events to the application service // it is given. -func worker(db storage.Database, ws types.ApplicationServiceWorkerState) { +func worker(client *gomatrixserverlib.Client, db storage.Database, ws types.ApplicationServiceWorkerState) { log.WithFields(log.Fields{ "appservice": ws.AppService.ID, }).Info("Starting application service") ctx := context.Background() - // Create a HTTP client for sending requests to app services - client := &http.Client{ - Timeout: transactionTimeout, - } - // Initial check for any leftover events to send from last time eventCount, err := db.CountEventsWithAppServiceID(ctx, ws.AppService.ID) if err != nil { @@ -206,7 +200,7 @@ func createTransaction( // send sends events to an application service. Returns an error if an OK was not // received back from the application service or the request timed out. func send( - client *http.Client, + client *gomatrixserverlib.Client, appservice config.ApplicationService, txnID int, transaction []byte, @@ -219,7 +213,7 @@ func send( return err } req.Header.Set("Content-Type", "application/json") - resp, err := client.Do(req) + resp, err := client.DoHTTPRequest(context.TODO(), req) if err != nil { return err } diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index 614e19d5..7d5ddbea 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -496,11 +496,20 @@ func Register( r.Username = strconv.FormatInt(id, 10) } + // Is this an appservice registration? It will be if the access + // token is supplied + accessToken, accessTokenErr := auth.ExtractAccessToken(req) + // Squash username to all lowercase letters r.Username = strings.ToLower(r.Username) - - if resErr = validateUsername(r.Username); resErr != nil { - return *resErr + if r.Type == authtypes.LoginTypeApplicationService && accessTokenErr == nil { + if resErr = validateApplicationServiceUsername(r.Username); resErr != nil { + return *resErr + } + } else { + if resErr = validateUsername(r.Username); resErr != nil { + return *resErr + } } if resErr = validatePassword(r.Password); resErr != nil { return *resErr @@ -513,7 +522,7 @@ func Register( "session_id": r.Auth.Session, }).Info("Processing registration request") - return handleRegistrationFlow(req, r, sessionID, cfg, userAPI) + return handleRegistrationFlow(req, r, sessionID, cfg, userAPI, accessToken, accessTokenErr) } func handleGuestRegistration( @@ -579,6 +588,8 @@ func handleRegistrationFlow( sessionID string, cfg *config.ClientAPI, userAPI userapi.UserInternalAPI, + accessToken string, + accessTokenErr error, ) util.JSONResponse { // TODO: Shared secret registration (create new user scripts) // TODO: Enable registration config flag @@ -588,12 +599,12 @@ func handleRegistrationFlow( // TODO: Handle mapping registrationRequest parameters into session parameters // TODO: email / msisdn auth types. - accessToken, accessTokenErr := auth.ExtractAccessToken(req) // Appservices are special and are not affected by disabled - // registration or user exclusivity. - if r.Auth.Type == authtypes.LoginTypeApplicationService || - (r.Auth.Type == "" && accessTokenErr == nil) { + // registration or user exclusivity. We'll go onto the appservice + // registration flow if a valid access token was provided or if + // the login type specifically requests it. + if r.Type == authtypes.LoginTypeApplicationService && accessTokenErr == nil { return handleApplicationServiceRegistration( accessToken, accessTokenErr, req, r, cfg, userAPI, ) diff --git a/cmd/generate-config/main.go b/cmd/generate-config/main.go index fa0da10c..9ef0f0b4 100644 --- a/cmd/generate-config/main.go +++ b/cmd/generate-config/main.go @@ -61,6 +61,7 @@ func main() { } if *defaultsForCI { + cfg.AppServiceAPI.DisableTLSValidation = true cfg.ClientAPI.RateLimiting.Enabled = false cfg.FederationSender.DisableTLSValidation = true cfg.MSCs.MSCs = []string{"msc2836", "msc2946", "msc2444", "msc2753"} diff --git a/dendrite-config.yaml b/dendrite-config.yaml index a3d1065d..bf604c9d 100644 --- a/dendrite-config.yaml +++ b/dendrite-config.yaml @@ -125,6 +125,11 @@ app_service_api: max_idle_conns: 2 conn_max_lifetime: -1 + # Disable the validation of TLS certificates of appservices. This is + # not recommended in production since it may allow appservice traffic + # to be sent to an unverified endpoint. + disable_tls_validation: false + # Appservice configuration files to load into this homeserver. config_files: [] diff --git a/setup/base.go b/setup/base.go index e9aa2a45..f8a45409 100644 --- a/setup/base.go +++ b/setup/base.go @@ -290,6 +290,22 @@ func (b *BaseDendrite) CreateClient() *gomatrixserverlib.Client { return client } +// CreateAppserviceClient creates a new client for application services. +// It has a specific timeout and obeys TLS validation from the appservice +// config rather than the federation config. +func (b *BaseDendrite) CreateAppserviceClient() *gomatrixserverlib.Client { + opts := []gomatrixserverlib.ClientOption{ + gomatrixserverlib.WithSkipVerify(b.Cfg.AppServiceAPI.DisableTLSValidation), + gomatrixserverlib.WithTimeout(time.Second * 60), + } + if b.Cfg.Global.DNSCache.Enabled { + opts = append(opts, gomatrixserverlib.WithDNSCache(b.DNSCache)) + } + client := gomatrixserverlib.NewClient(opts...) + client.SetUserAgent(fmt.Sprintf("Dendrite/%s", internal.VersionString())) + return client +} + // CreateFederationClient creates a new federation client. Should only be called // once per component. func (b *BaseDendrite) CreateFederationClient() *gomatrixserverlib.FederationClient { diff --git a/setup/config/config_appservice.go b/setup/config/config_appservice.go index a042691d..a6f77abf 100644 --- a/setup/config/config_appservice.go +++ b/setup/config/config_appservice.go @@ -33,6 +33,10 @@ type AppServiceAPI struct { Database DatabaseOptions `yaml:"database"` + // DisableTLSValidation disables the validation of X.509 TLS certs + // on appservice endpoints. This is not recommended in production! + DisableTLSValidation bool `yaml:"disable_tls_validation"` + ConfigFiles []string `yaml:"config_files"` } diff --git a/sytest-whitelist b/sytest-whitelist index 1e4442a0..7ccbaad1 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -510,3 +510,9 @@ Can pass a JSON filter as a query parameter Local room members can get room messages Remote room members can get room messages Guest users can send messages to guest_access rooms if joined +AS can create a user +AS can create a user with an underscore +AS can create a user with inhibit_login +AS can set avatar for ghosted users +AS can set displayname for ghosted users +Ghost user must register before joining room From fe021d374256324cc5394599fc9e018caeadc5bb Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 5 Mar 2021 14:57:42 +0000 Subject: [PATCH 06/10] Treat the sender_localpart as an exclusive namespace of one user (#1790) --- setup/config/config_appservice.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/setup/config/config_appservice.go b/setup/config/config_appservice.go index a6f77abf..724f035f 100644 --- a/setup/config/config_appservice.go +++ b/setup/config/config_appservice.go @@ -197,7 +197,7 @@ func loadAppServices(config *AppServiceAPI, derived *Derived) error { // setupRegexps will create regex objects for exclusive and non-exclusive // usernames, aliases and rooms of all application services, so that other // methods can quickly check if a particular string matches any of them. -func setupRegexps(_ *AppServiceAPI, derived *Derived) (err error) { +func setupRegexps(asAPI *AppServiceAPI, derived *Derived) (err error) { // Combine all exclusive namespaces for later string checking var exclusiveUsernameStrings, exclusiveAliasStrings []string @@ -205,6 +205,16 @@ func setupRegexps(_ *AppServiceAPI, derived *Derived) (err error) { // its contents to the overall exlusive regex string. Room regex // not necessary as we aren't denying exclusive room ID creation for _, appservice := range derived.ApplicationServices { + // The sender_localpart can be considered an exclusive regex for a single user, so let's do that + // to simplify the code + var senderUserIDSlice = []string{fmt.Sprintf("@%s:%s", appservice.SenderLocalpart, asAPI.Matrix.ServerName)} + usersSlice, found := appservice.NamespaceMap["users"] + if !found { + usersSlice = []ApplicationServiceNamespace{} + appservice.NamespaceMap["users"] = usersSlice + } + appendExclusiveNamespaceRegexs(&senderUserIDSlice, usersSlice) + for key, namespaceSlice := range appservice.NamespaceMap { switch key { case "users": From 6aa262ead804529f9e328302b0c2e2653ffffd09 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 5 Mar 2021 16:40:32 +0000 Subject: [PATCH 07/10] Use default transport for AS traffic (#1789) * Use default transport for AS traffic * Update gmsl and use default client * Remove replace * Fix go.sum * Update gomatrixserverlib * Go back to appservices managing their own HTTP clients because argh * Add missing context --- appservice/appservice.go | 13 ++++++++++++- appservice/query/query.go | 7 +++---- appservice/workers/transaction_scheduler.go | 8 ++++---- go.sum | 2 -- setup/base.go | 16 ---------------- 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/appservice/appservice.go b/appservice/appservice.go index f608e8e7..782e090a 100644 --- a/appservice/appservice.go +++ b/appservice/appservice.go @@ -16,7 +16,10 @@ package appservice import ( "context" + "crypto/tls" + "net/http" "sync" + "time" "github.com/gorilla/mux" appserviceAPI "github.com/matrix-org/dendrite/appservice/api" @@ -46,7 +49,15 @@ func NewInternalAPI( userAPI userapi.UserInternalAPI, rsAPI roomserverAPI.RoomserverInternalAPI, ) appserviceAPI.AppServiceQueryAPI { - client := base.CreateAppserviceClient() + client := &http.Client{ + Timeout: time.Second * 30, + Transport: &http.Transport{ + DisableKeepAlives: true, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: base.Cfg.AppServiceAPI.DisableTLSValidation, + }, + }, + } consumer, _ := kafka.SetupConsumerProducer(&base.Cfg.Global.Kafka) // Create a connection to the appservice postgres DB diff --git a/appservice/query/query.go b/appservice/query/query.go index b4c33528..9f6c79a8 100644 --- a/appservice/query/query.go +++ b/appservice/query/query.go @@ -23,7 +23,6 @@ import ( "github.com/matrix-org/dendrite/appservice/api" "github.com/matrix-org/dendrite/setup/config" - "github.com/matrix-org/gomatrixserverlib" opentracing "github.com/opentracing/opentracing-go" log "github.com/sirupsen/logrus" ) @@ -33,7 +32,7 @@ const userIDExistsPath = "/users/" // AppServiceQueryAPI is an implementation of api.AppServiceQueryAPI type AppServiceQueryAPI struct { - HTTPClient *gomatrixserverlib.Client + HTTPClient *http.Client Cfg *config.Dendrite } @@ -63,7 +62,7 @@ func (a *AppServiceQueryAPI) RoomAliasExists( } req = req.WithContext(ctx) - resp, err := a.HTTPClient.DoHTTPRequest(ctx, req) + resp, err := a.HTTPClient.Do(req) if resp != nil { defer func() { err = resp.Body.Close() @@ -124,7 +123,7 @@ func (a *AppServiceQueryAPI) UserIDExists( if err != nil { return err } - resp, err := a.HTTPClient.DoHTTPRequest(ctx, req) + resp, err := a.HTTPClient.Do(req.WithContext(ctx)) if resp != nil { defer func() { err = resp.Body.Close() diff --git a/appservice/workers/transaction_scheduler.go b/appservice/workers/transaction_scheduler.go index 47d447c2..4dab00bd 100644 --- a/appservice/workers/transaction_scheduler.go +++ b/appservice/workers/transaction_scheduler.go @@ -42,7 +42,7 @@ var ( // size), then send that off to the AS's /transactions/{txnID} endpoint. It also // handles exponentially backing off in case the AS isn't currently available. func SetupTransactionWorkers( - client *gomatrixserverlib.Client, + client *http.Client, appserviceDB storage.Database, workerStates []types.ApplicationServiceWorkerState, ) error { @@ -58,7 +58,7 @@ func SetupTransactionWorkers( // worker is a goroutine that sends any queued events to the application service // it is given. -func worker(client *gomatrixserverlib.Client, db storage.Database, ws types.ApplicationServiceWorkerState) { +func worker(client *http.Client, db storage.Database, ws types.ApplicationServiceWorkerState) { log.WithFields(log.Fields{ "appservice": ws.AppService.ID, }).Info("Starting application service") @@ -200,7 +200,7 @@ func createTransaction( // send sends events to an application service. Returns an error if an OK was not // received back from the application service or the request timed out. func send( - client *gomatrixserverlib.Client, + client *http.Client, appservice config.ApplicationService, txnID int, transaction []byte, @@ -213,7 +213,7 @@ func send( return err } req.Header.Set("Content-Type", "application/json") - resp, err := client.DoHTTPRequest(context.TODO(), req) + resp, err := client.Do(req) if err != nil { return err } diff --git a/go.sum b/go.sum index 351acc53..8bfeeee3 100644 --- a/go.sum +++ b/go.sum @@ -866,8 +866,6 @@ github.com/shurcooL/webdavfs v0.0.0-20170829043945-18c3829fa133/go.mod h1:hKmq5k github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= -github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM= -github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/sirupsen/logrus v1.8.0 h1:nfhvjKcUMhBMVqbKHJlk5RPrrfYr/NMo3692g0dwfWU= github.com/sirupsen/logrus v1.8.0/go.mod h1:4GuYW9TZmE769R5STWrRakJc4UqQ3+QQ95fyz7ENv1A= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= diff --git a/setup/base.go b/setup/base.go index f8a45409..e9aa2a45 100644 --- a/setup/base.go +++ b/setup/base.go @@ -290,22 +290,6 @@ func (b *BaseDendrite) CreateClient() *gomatrixserverlib.Client { return client } -// CreateAppserviceClient creates a new client for application services. -// It has a specific timeout and obeys TLS validation from the appservice -// config rather than the federation config. -func (b *BaseDendrite) CreateAppserviceClient() *gomatrixserverlib.Client { - opts := []gomatrixserverlib.ClientOption{ - gomatrixserverlib.WithSkipVerify(b.Cfg.AppServiceAPI.DisableTLSValidation), - gomatrixserverlib.WithTimeout(time.Second * 60), - } - if b.Cfg.Global.DNSCache.Enabled { - opts = append(opts, gomatrixserverlib.WithDNSCache(b.DNSCache)) - } - client := gomatrixserverlib.NewClient(opts...) - client.SetUserAgent(fmt.Sprintf("Dendrite/%s", internal.VersionString())) - return client -} - // CreateFederationClient creates a new federation client. Should only be called // once per component. func (b *BaseDendrite) CreateFederationClient() *gomatrixserverlib.FederationClient { From c3ad2cca49a7ad5890dddf2d8eec3e3cbbff16d1 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 8 Mar 2021 13:18:29 +0000 Subject: [PATCH 08/10] Fix database default connection limits for CI (#1794) --- dendrite-config.yaml | 4 ++-- setup/config/config_appservice.go | 2 +- setup/config/config_federationsender.go | 2 +- setup/config/config_global.go | 4 ++-- setup/config/config_kafka.go | 2 +- setup/config/config_keyserver.go | 2 +- setup/config/config_mediaapi.go | 2 +- setup/config/config_mscs.go | 2 +- setup/config/config_roomserver.go | 2 +- setup/config/config_signingkeyserver.go | 2 +- setup/config/config_syncapi.go | 2 +- setup/config/config_userapi.go | 4 ++-- 12 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dendrite-config.yaml b/dendrite-config.yaml index bf604c9d..22c7b902 100644 --- a/dendrite-config.yaml +++ b/dendrite-config.yaml @@ -240,7 +240,7 @@ media_api: listen: http://[::]:8074 database: connection_string: file:mediaapi.db - max_open_conns: 10 + max_open_conns: 5 max_idle_conns: 2 conn_max_lifetime: -1 @@ -278,7 +278,7 @@ mscs: mscs: [] database: connection_string: file:mscs.db - max_open_conns: 10 + max_open_conns: 5 max_idle_conns: 2 conn_max_lifetime: -1 diff --git a/setup/config/config_appservice.go b/setup/config/config_appservice.go index 724f035f..57cc7be5 100644 --- a/setup/config/config_appservice.go +++ b/setup/config/config_appservice.go @@ -43,7 +43,7 @@ type AppServiceAPI struct { func (c *AppServiceAPI) Defaults() { c.InternalAPI.Listen = "http://localhost:7777" c.InternalAPI.Connect = "http://localhost:7777" - c.Database.Defaults() + c.Database.Defaults(10) c.Database.ConnectionString = "file:appservice.db" } diff --git a/setup/config/config_federationsender.go b/setup/config/config_federationsender.go index 84f5b6f4..67ee6356 100644 --- a/setup/config/config_federationsender.go +++ b/setup/config/config_federationsender.go @@ -25,7 +25,7 @@ type FederationSender struct { func (c *FederationSender) Defaults() { c.InternalAPI.Listen = "http://localhost:7775" c.InternalAPI.Connect = "http://localhost:7775" - c.Database.Defaults() + c.Database.Defaults(10) c.Database.ConnectionString = "file:federationsender.db" c.FederationMaxRetries = 16 diff --git a/setup/config/config_global.go b/setup/config/config_global.go index d4b068db..4b5297ff 100644 --- a/setup/config/config_global.go +++ b/setup/config/config_global.go @@ -122,8 +122,8 @@ type DatabaseOptions struct { ConnMaxLifetimeSeconds int `yaml:"conn_max_lifetime"` } -func (c *DatabaseOptions) Defaults() { - c.MaxOpenConnections = 100 +func (c *DatabaseOptions) Defaults(conns int) { + c.MaxOpenConnections = conns c.MaxIdleConnections = 2 c.ConnMaxLifetimeSeconds = -1 } diff --git a/setup/config/config_kafka.go b/setup/config/config_kafka.go index aa91e558..36191428 100644 --- a/setup/config/config_kafka.go +++ b/setup/config/config_kafka.go @@ -36,7 +36,7 @@ func (k *Kafka) TopicFor(name string) string { func (c *Kafka) Defaults() { c.UseNaffka = true - c.Database.Defaults() + c.Database.Defaults(10) c.Addresses = []string{"localhost:2181"} c.Database.ConnectionString = DataSource("file:naffka.db") c.TopicPrefix = "Dendrite" diff --git a/setup/config/config_keyserver.go b/setup/config/config_keyserver.go index 89162300..62a30dbb 100644 --- a/setup/config/config_keyserver.go +++ b/setup/config/config_keyserver.go @@ -11,7 +11,7 @@ type KeyServer struct { func (c *KeyServer) Defaults() { c.InternalAPI.Listen = "http://localhost:7779" c.InternalAPI.Connect = "http://localhost:7779" - c.Database.Defaults() + c.Database.Defaults(10) c.Database.ConnectionString = "file:keyserver.db" } diff --git a/setup/config/config_mediaapi.go b/setup/config/config_mediaapi.go index a9425b7b..660a508d 100644 --- a/setup/config/config_mediaapi.go +++ b/setup/config/config_mediaapi.go @@ -39,7 +39,7 @@ func (c *MediaAPI) Defaults() { c.InternalAPI.Listen = "http://localhost:7774" c.InternalAPI.Connect = "http://localhost:7774" c.ExternalAPI.Listen = "http://[::]:8074" - c.Database.Defaults() + c.Database.Defaults(5) c.Database.ConnectionString = "file:mediaapi.db" defaultMaxFileSizeBytes := FileSizeBytes(10485760) diff --git a/setup/config/config_mscs.go b/setup/config/config_mscs.go index 764273ec..a94e5dc5 100644 --- a/setup/config/config_mscs.go +++ b/setup/config/config_mscs.go @@ -14,7 +14,7 @@ type MSCs struct { } func (c *MSCs) Defaults() { - c.Database.Defaults() + c.Database.Defaults(5) c.Database.ConnectionString = "file:mscs.db" } diff --git a/setup/config/config_roomserver.go b/setup/config/config_roomserver.go index 2a1fc38b..ffb9b5f9 100644 --- a/setup/config/config_roomserver.go +++ b/setup/config/config_roomserver.go @@ -11,7 +11,7 @@ type RoomServer struct { func (c *RoomServer) Defaults() { c.InternalAPI.Listen = "http://localhost:7770" c.InternalAPI.Connect = "http://localhost:7770" - c.Database.Defaults() + c.Database.Defaults(10) c.Database.ConnectionString = "file:roomserver.db" } diff --git a/setup/config/config_signingkeyserver.go b/setup/config/config_signingkeyserver.go index 51aca38b..c192d1fc 100644 --- a/setup/config/config_signingkeyserver.go +++ b/setup/config/config_signingkeyserver.go @@ -22,7 +22,7 @@ type SigningKeyServer struct { func (c *SigningKeyServer) Defaults() { c.InternalAPI.Listen = "http://localhost:7780" c.InternalAPI.Connect = "http://localhost:7780" - c.Database.Defaults() + c.Database.Defaults(10) c.Database.ConnectionString = "file:signingkeyserver.db" } diff --git a/setup/config/config_syncapi.go b/setup/config/config_syncapi.go index fc08f738..4b9bfb16 100644 --- a/setup/config/config_syncapi.go +++ b/setup/config/config_syncapi.go @@ -15,7 +15,7 @@ func (c *SyncAPI) Defaults() { c.InternalAPI.Listen = "http://localhost:7773" c.InternalAPI.Connect = "http://localhost:7773" c.ExternalAPI.Listen = "http://localhost:8073" - c.Database.Defaults() + c.Database.Defaults(10) c.Database.ConnectionString = "file:syncapi.db" } diff --git a/setup/config/config_userapi.go b/setup/config/config_userapi.go index 2cbd8a45..91b351d1 100644 --- a/setup/config/config_userapi.go +++ b/setup/config/config_userapi.go @@ -16,8 +16,8 @@ type UserAPI struct { func (c *UserAPI) Defaults() { c.InternalAPI.Listen = "http://localhost:7781" c.InternalAPI.Connect = "http://localhost:7781" - c.AccountDatabase.Defaults() - c.DeviceDatabase.Defaults() + c.AccountDatabase.Defaults(10) + c.DeviceDatabase.Defaults(10) c.AccountDatabase.ConnectionString = "file:userapi_accounts.db" c.DeviceDatabase.ConnectionString = "file:userapi_devices.db" } From 850abb1dde2ce6f85554457e3ee94a9837e13897 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Mon, 8 Mar 2021 13:19:02 +0000 Subject: [PATCH 09/10] Make bcrypt cost configurable (#1793) --- cmd/create-account/main.go | 3 ++- cmd/generate-config/main.go | 2 ++ dendrite-config.yaml | 7 +++++++ setup/base.go | 2 +- setup/config/config_userapi.go | 6 ++++++ userapi/storage/accounts/postgres/storage.go | 12 +++++++----- userapi/storage/accounts/sqlite3/storage.go | 12 +++++++----- userapi/storage/accounts/storage.go | 6 +++--- userapi/storage/accounts/storage_wasm.go | 3 ++- userapi/userapi.go | 1 - userapi/userapi_test.go | 3 ++- 11 files changed, 39 insertions(+), 18 deletions(-) diff --git a/cmd/create-account/main.go b/cmd/create-account/main.go index bba2d55d..22732c51 100644 --- a/cmd/create-account/main.go +++ b/cmd/create-account/main.go @@ -24,6 +24,7 @@ import ( "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/userapi/storage/accounts" "github.com/sirupsen/logrus" + "golang.org/x/crypto/bcrypt" ) const usage = `Usage: %s @@ -57,7 +58,7 @@ func main() { accountDB, err := accounts.NewDatabase(&config.DatabaseOptions{ ConnectionString: cfg.UserAPI.AccountDatabase.ConnectionString, - }, cfg.Global.ServerName) + }, cfg.Global.ServerName, bcrypt.DefaultCost) if err != nil { logrus.Fatalln("Failed to connect to the database:", err.Error()) } diff --git a/cmd/generate-config/main.go b/cmd/generate-config/main.go index 9ef0f0b4..bd70cbc5 100644 --- a/cmd/generate-config/main.go +++ b/cmd/generate-config/main.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/matrix-org/dendrite/setup/config" + "golang.org/x/crypto/bcrypt" "gopkg.in/yaml.v2" ) @@ -68,6 +69,7 @@ func main() { cfg.Logging[0].Level = "trace" // don't hit matrix.org when running tests!!! cfg.SigningKeyServer.KeyPerspectives = config.KeyPerspectives{} + cfg.UserAPI.BCryptCost = bcrypt.MinCost } j, err := yaml.Marshal(cfg) diff --git a/dendrite-config.yaml b/dendrite-config.yaml index 22c7b902..13564590 100644 --- a/dendrite-config.yaml +++ b/dendrite-config.yaml @@ -340,6 +340,13 @@ sync_api: # Configuration for the User API. user_api: + # The cost when hashing passwords on registration/login. Default: 10. Min: 4, Max: 31 + # See https://pkg.go.dev/golang.org/x/crypto/bcrypt for more information. + # Setting this lower makes registration/login consume less CPU resources at the cost of security + # should the database be compromised. Setting this higher makes registration/login consume more + # CPU resources but makes it harder to brute force password hashes. + # This value can be low if performing tests or on embedded Dendrite instances (e.g WASM builds) + # bcrypt_cost: 10 internal_api: listen: http://localhost:7781 connect: http://localhost:7781 diff --git a/setup/base.go b/setup/base.go index e9aa2a45..4c50a6be 100644 --- a/setup/base.go +++ b/setup/base.go @@ -263,7 +263,7 @@ func (b *BaseDendrite) KeyServerHTTPClient() keyserverAPI.KeyInternalAPI { // CreateAccountsDB creates a new instance of the accounts database. Should only // be called once per component. func (b *BaseDendrite) CreateAccountsDB() accounts.Database { - db, err := accounts.NewDatabase(&b.Cfg.UserAPI.AccountDatabase, b.Cfg.Global.ServerName) + db, err := accounts.NewDatabase(&b.Cfg.UserAPI.AccountDatabase, b.Cfg.Global.ServerName, b.Cfg.UserAPI.BCryptCost) if err != nil { logrus.WithError(err).Panicf("failed to connect to accounts db") } diff --git a/setup/config/config_userapi.go b/setup/config/config_userapi.go index 91b351d1..e6912384 100644 --- a/setup/config/config_userapi.go +++ b/setup/config/config_userapi.go @@ -1,10 +1,15 @@ package config +import "golang.org/x/crypto/bcrypt" + type UserAPI struct { Matrix *Global `yaml:"-"` InternalAPI InternalAPIOptions `yaml:"internal_api"` + // The cost when hashing passwords. + BCryptCost int `yaml:"bcrypt_cost"` + // The Account database stores the login details and account information // for local users. It is accessed by the UserAPI. AccountDatabase DatabaseOptions `yaml:"account_database"` @@ -20,6 +25,7 @@ func (c *UserAPI) Defaults() { c.DeviceDatabase.Defaults(10) c.AccountDatabase.ConnectionString = "file:userapi_accounts.db" c.DeviceDatabase.ConnectionString = "file:userapi_devices.db" + c.BCryptCost = bcrypt.DefaultCost } func (c *UserAPI) Verify(configErrs *ConfigErrors, isMonolith bool) { diff --git a/userapi/storage/accounts/postgres/storage.go b/userapi/storage/accounts/postgres/storage.go index e6adbfd8..3933fe5b 100644 --- a/userapi/storage/accounts/postgres/storage.go +++ b/userapi/storage/accounts/postgres/storage.go @@ -44,10 +44,11 @@ type Database struct { accountDatas accountDataStatements threepids threepidStatements serverName gomatrixserverlib.ServerName + bcryptCost int } // NewDatabase creates a new accounts and profiles database -func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName) (*Database, error) { +func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName, bcryptCost int) (*Database, error) { db, err := sqlutil.Open(dbProperties) if err != nil { return nil, err @@ -56,6 +57,7 @@ func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserver serverName: serverName, db: db, writer: sqlutil.NewDummyWriter(), + bcryptCost: bcryptCost, } // Create tables before executing migrations so we don't fail if the table is missing, @@ -131,7 +133,7 @@ func (d *Database) SetDisplayName( func (d *Database) SetPassword( ctx context.Context, localpart, plaintextPassword string, ) error { - hash, err := hashPassword(plaintextPassword) + hash, err := d.hashPassword(plaintextPassword) if err != nil { return err } @@ -175,7 +177,7 @@ func (d *Database) createAccount( // Generate a password hash if this is not a password-less user hash := "" if plaintextPassword != "" { - hash, err = hashPassword(plaintextPassword) + hash, err = d.hashPassword(plaintextPassword) if err != nil { return nil, err } @@ -246,8 +248,8 @@ func (d *Database) GetNewNumericLocalpart( return d.accounts.selectNewNumericLocalpart(ctx, nil) } -func hashPassword(plaintext string) (hash string, err error) { - hashBytes, err := bcrypt.GenerateFromPassword([]byte(plaintext), bcrypt.DefaultCost) +func (d *Database) hashPassword(plaintext string) (hash string, err error) { + hashBytes, err := bcrypt.GenerateFromPassword([]byte(plaintext), d.bcryptCost) return string(hashBytes), err } diff --git a/userapi/storage/accounts/sqlite3/storage.go b/userapi/storage/accounts/sqlite3/storage.go index 747be34f..07cc68b3 100644 --- a/userapi/storage/accounts/sqlite3/storage.go +++ b/userapi/storage/accounts/sqlite3/storage.go @@ -42,6 +42,7 @@ type Database struct { accountDatas accountDataStatements threepids threepidStatements serverName gomatrixserverlib.ServerName + bcryptCost int accountsMu sync.Mutex profilesMu sync.Mutex @@ -50,7 +51,7 @@ type Database struct { } // NewDatabase creates a new accounts and profiles database -func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName) (*Database, error) { +func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName, bcryptCost int) (*Database, error) { db, err := sqlutil.Open(dbProperties) if err != nil { return nil, err @@ -59,6 +60,7 @@ func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserver serverName: serverName, db: db, writer: sqlutil.NewExclusiveWriter(), + bcryptCost: bcryptCost, } // Create tables before executing migrations so we don't fail if the table is missing, @@ -143,7 +145,7 @@ func (d *Database) SetDisplayName( func (d *Database) SetPassword( ctx context.Context, localpart, plaintextPassword string, ) error { - hash, err := hashPassword(plaintextPassword) + hash, err := d.hashPassword(plaintextPassword) if err != nil { return err } @@ -208,7 +210,7 @@ func (d *Database) createAccount( // Generate a password hash if this is not a password-less user hash := "" if plaintextPassword != "" { - hash, err = hashPassword(plaintextPassword) + hash, err = d.hashPassword(plaintextPassword) if err != nil { return nil, err } @@ -278,8 +280,8 @@ func (d *Database) GetNewNumericLocalpart( return d.accounts.selectNewNumericLocalpart(ctx, nil) } -func hashPassword(plaintext string) (hash string, err error) { - hashBytes, err := bcrypt.GenerateFromPassword([]byte(plaintext), bcrypt.DefaultCost) +func (d *Database) hashPassword(plaintext string) (hash string, err error) { + hashBytes, err := bcrypt.GenerateFromPassword([]byte(plaintext), d.bcryptCost) return string(hashBytes), err } diff --git a/userapi/storage/accounts/storage.go b/userapi/storage/accounts/storage.go index 3f69e95f..28c437da 100644 --- a/userapi/storage/accounts/storage.go +++ b/userapi/storage/accounts/storage.go @@ -27,12 +27,12 @@ import ( // NewDatabase opens a new Postgres or Sqlite database (based on dataSourceName scheme) // and sets postgres connection parameters -func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName) (Database, error) { +func NewDatabase(dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName, bcryptCost int) (Database, error) { switch { case dbProperties.ConnectionString.IsSQLite(): - return sqlite3.NewDatabase(dbProperties, serverName) + return sqlite3.NewDatabase(dbProperties, serverName, bcryptCost) case dbProperties.ConnectionString.IsPostgres(): - return postgres.NewDatabase(dbProperties, serverName) + return postgres.NewDatabase(dbProperties, serverName, bcryptCost) default: return nil, fmt.Errorf("unexpected database type") } diff --git a/userapi/storage/accounts/storage_wasm.go b/userapi/storage/accounts/storage_wasm.go index dcaf371a..8038326f 100644 --- a/userapi/storage/accounts/storage_wasm.go +++ b/userapi/storage/accounts/storage_wasm.go @@ -25,10 +25,11 @@ import ( func NewDatabase( dbProperties *config.DatabaseOptions, serverName gomatrixserverlib.ServerName, + bcryptCost int, ) (Database, error) { switch { case dbProperties.ConnectionString.IsSQLite(): - return sqlite3.NewDatabase(dbProperties, serverName) + return sqlite3.NewDatabase(dbProperties, serverName, bcryptCost) case dbProperties.ConnectionString.IsPostgres(): return nil, fmt.Errorf("can't use Postgres implementation") default: diff --git a/userapi/userapi.go b/userapi/userapi.go index b8b826bc..74702020 100644 --- a/userapi/userapi.go +++ b/userapi/userapi.go @@ -37,7 +37,6 @@ func AddInternalRoutes(router *mux.Router, intAPI api.UserInternalAPI) { func NewInternalAPI( accountDB accounts.Database, cfg *config.UserAPI, appServices []config.ApplicationService, keyAPI keyapi.KeyInternalAPI, ) api.UserInternalAPI { - deviceDB, err := devices.NewDatabase(&cfg.DeviceDatabase, cfg.Matrix.ServerName) if err != nil { logrus.WithError(err).Panicf("failed to connect to device db") diff --git a/userapi/userapi_test.go b/userapi/userapi_test.go index 25c262ad..9a45a2dc 100644 --- a/userapi/userapi_test.go +++ b/userapi/userapi_test.go @@ -16,6 +16,7 @@ import ( "github.com/matrix-org/dendrite/userapi/inthttp" "github.com/matrix-org/dendrite/userapi/storage/accounts" "github.com/matrix-org/gomatrixserverlib" + "golang.org/x/crypto/bcrypt" ) const ( @@ -25,7 +26,7 @@ const ( func MustMakeInternalAPI(t *testing.T) (api.UserInternalAPI, accounts.Database) { accountDB, err := accounts.NewDatabase(&config.DatabaseOptions{ ConnectionString: "file::memory:", - }, serverName) + }, serverName, bcrypt.MinCost) if err != nil { t.Fatalf("failed to create account DB: %s", err) } From 5acf30cd3caba973318e1233304c5b76c85d815b Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 8 Mar 2021 13:32:21 +0000 Subject: [PATCH 10/10] Update sytest-whitelist --- sytest-whitelist | 1 + 1 file changed, 1 insertion(+) diff --git a/sytest-whitelist b/sytest-whitelist index 7ccbaad1..ed02fdec 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -516,3 +516,4 @@ AS can create a user with inhibit_login AS can set avatar for ghosted users AS can set displayname for ghosted users Ghost user must register before joining room +Inviting an AS-hosted user asks the AS server