diff --git a/appservice/api/query.go b/appservice/api/query.go index 8ce3b4e04..9542df565 100644 --- a/appservice/api/query.go +++ b/appservice/api/query.go @@ -20,13 +20,13 @@ package api import ( "context" "database/sql" - "errors" "net/http" "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/dendrite/clientapi/auth/storage/accounts" "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/dendrite/common" commonHTTP "github.com/matrix-org/dendrite/common/http" opentracing "github.com/opentracing/opentracing-go" ) @@ -164,7 +164,7 @@ func RetrieveUserProfile( // If no user exists, return if !userResp.UserIDExists { - return nil, errors.New("no known profile for given user ID") + return nil, common.ErrProfileNoExists } // Try to query the user from the local database again diff --git a/clientapi/auth/authtypes/profile.go b/clientapi/auth/authtypes/profile.go index 6cf508f4f..0bc49658b 100644 --- a/clientapi/auth/authtypes/profile.go +++ b/clientapi/auth/authtypes/profile.go @@ -14,7 +14,7 @@ package authtypes -// Profile represents the profile for a Matrix account on this home server. +// Profile represents the profile for a Matrix account. type Profile struct { Localpart string DisplayName string diff --git a/clientapi/routing/profile.go b/clientapi/routing/profile.go index 8d28b3660..e8ea6cf13 100644 --- a/clientapi/routing/profile.go +++ b/clientapi/routing/profile.go @@ -30,43 +30,61 @@ import ( "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/gomatrix" "github.com/matrix-org/util" ) // GetProfile implements GET /profile/{userID} func GetProfile( - req *http.Request, accountDB *accounts.Database, userID string, asAPI appserviceAPI.AppServiceQueryAPI, + req *http.Request, accountDB *accounts.Database, cfg *config.Dendrite, + userID string, + asAPI appserviceAPI.AppServiceQueryAPI, + federation *gomatrixserverlib.FederationClient, ) util.JSONResponse { - profile, err := appserviceAPI.RetrieveUserProfile(req.Context(), userID, asAPI, accountDB) + profile, err := getProfile(req.Context(), accountDB, cfg, userID, asAPI, federation) if err != nil { + if err == common.ErrProfileNoExists { + return util.JSONResponse{ + Code: http.StatusNotFound, + JSON: jsonerror.NotFound("The user does not exist or does not have a profile"), + } + } + return httputil.LogThenError(req, err) } - res := common.ProfileResponse{ - AvatarURL: profile.AvatarURL, - DisplayName: profile.DisplayName, - } return util.JSONResponse{ Code: http.StatusOK, - JSON: res, + JSON: common.ProfileResponse{ + AvatarURL: profile.AvatarURL, + DisplayName: profile.DisplayName, + }, } } // GetAvatarURL implements GET /profile/{userID}/avatar_url func GetAvatarURL( - req *http.Request, accountDB *accounts.Database, userID string, asAPI appserviceAPI.AppServiceQueryAPI, + req *http.Request, accountDB *accounts.Database, cfg *config.Dendrite, + userID string, asAPI appserviceAPI.AppServiceQueryAPI, + federation *gomatrixserverlib.FederationClient, ) util.JSONResponse { - profile, err := appserviceAPI.RetrieveUserProfile(req.Context(), userID, asAPI, accountDB) + profile, err := getProfile(req.Context(), accountDB, cfg, userID, asAPI, federation) if err != nil { + if err == common.ErrProfileNoExists { + return util.JSONResponse{ + Code: http.StatusNotFound, + JSON: jsonerror.NotFound("The user does not exist or does not have a profile"), + } + } + return httputil.LogThenError(req, err) } - res := common.AvatarURL{ - AvatarURL: profile.AvatarURL, - } return util.JSONResponse{ Code: http.StatusOK, - JSON: res, + JSON: common.AvatarURL{ + AvatarURL: profile.AvatarURL, + }, } } @@ -152,18 +170,27 @@ func SetAvatarURL( // GetDisplayName implements GET /profile/{userID}/displayname func GetDisplayName( - req *http.Request, accountDB *accounts.Database, userID string, asAPI appserviceAPI.AppServiceQueryAPI, + req *http.Request, accountDB *accounts.Database, cfg *config.Dendrite, + userID string, asAPI appserviceAPI.AppServiceQueryAPI, + federation *gomatrixserverlib.FederationClient, ) util.JSONResponse { - profile, err := appserviceAPI.RetrieveUserProfile(req.Context(), userID, asAPI, accountDB) + profile, err := getProfile(req.Context(), accountDB, cfg, userID, asAPI, federation) if err != nil { + if err == common.ErrProfileNoExists { + return util.JSONResponse{ + Code: http.StatusNotFound, + JSON: jsonerror.NotFound("The user does not exist or does not have a profile"), + } + } + return httputil.LogThenError(req, err) } - res := common.DisplayName{ - DisplayName: profile.DisplayName, - } + return util.JSONResponse{ Code: http.StatusOK, - JSON: res, + JSON: common.DisplayName{ + DisplayName: profile.DisplayName, + }, } } @@ -247,6 +274,48 @@ func SetDisplayName( } } +// getProfile gets the full profile of a user by querying the database or a +// remote homeserver. +// Returns an error when something goes wrong or specifically +// common.ErrProfileNoExists when the profile doesn't exist. +func getProfile( + ctx context.Context, accountDB *accounts.Database, cfg *config.Dendrite, + userID string, + asAPI appserviceAPI.AppServiceQueryAPI, + federation *gomatrixserverlib.FederationClient, +) (*authtypes.Profile, error) { + localpart, domain, err := gomatrixserverlib.SplitID('@', userID) + if err != nil { + return nil, err + } + + if domain != cfg.Matrix.ServerName { + profile, fedErr := federation.LookupProfile(ctx, domain, userID, "") + if fedErr != nil { + if x, ok := fedErr.(gomatrix.HTTPError); ok { + if x.Code == http.StatusNotFound { + return nil, common.ErrProfileNoExists + } + } + + return nil, fedErr + } + + return &authtypes.Profile{ + Localpart: localpart, + DisplayName: profile.DisplayName, + AvatarURL: profile.AvatarURL, + }, nil + } + + profile, err := appserviceAPI.RetrieveUserProfile(ctx, userID, asAPI, accountDB) + if err != nil { + return nil, err + } + + return profile, nil +} + func buildMembershipEvents( ctx context.Context, memberships []authtypes.Membership, diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 469b5b964..d36ed6957 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -292,7 +292,7 @@ func Setup( if err != nil { return util.ErrorResponse(err) } - return GetProfile(req, accountDB, vars["userID"], asAPI) + return GetProfile(req, accountDB, &cfg, vars["userID"], asAPI, federation) }), ).Methods(http.MethodGet, http.MethodOptions) @@ -302,7 +302,7 @@ func Setup( if err != nil { return util.ErrorResponse(err) } - return GetAvatarURL(req, accountDB, vars["userID"], asAPI) + return GetAvatarURL(req, accountDB, &cfg, vars["userID"], asAPI, federation) }), ).Methods(http.MethodGet, http.MethodOptions) @@ -324,7 +324,7 @@ func Setup( if err != nil { return util.ErrorResponse(err) } - return GetDisplayName(req, accountDB, vars["userID"], asAPI) + return GetDisplayName(req, accountDB, &cfg, vars["userID"], asAPI, federation) }), ).Methods(http.MethodGet, http.MethodOptions) diff --git a/clientapi/routing/sendevent.go b/clientapi/routing/sendevent.go index e916e451e..9696b360e 100644 --- a/clientapi/routing/sendevent.go +++ b/clientapi/routing/sendevent.go @@ -50,7 +50,7 @@ func SendEvent( ) util.JSONResponse { if txnID != nil { // Try to fetch response from transactionsCache - if res, ok := txnCache.FetchTransaction(*txnID); ok { + if res, ok := txnCache.FetchTransaction(device.AccessToken, *txnID); ok { return *res } } @@ -83,7 +83,7 @@ func SendEvent( } // Add response to transactionsCache if txnID != nil { - txnCache.AddTransaction(*txnID, &res) + txnCache.AddTransaction(device.AccessToken, *txnID, &res) } return res diff --git a/common/transactions/transactions.go b/common/transactions/transactions.go index febcb9a75..80b403a98 100644 --- a/common/transactions/transactions.go +++ b/common/transactions/transactions.go @@ -22,7 +22,14 @@ import ( // DefaultCleanupPeriod represents the default time duration after which cacheCleanService runs. const DefaultCleanupPeriod time.Duration = 30 * time.Minute -type txnsMap map[string]*util.JSONResponse +type txnsMap map[CacheKey]*util.JSONResponse + +// CacheKey is the type for the key in a transactions cache. +// This is needed because the spec requires transaction IDs to have a per-access token scope. +type CacheKey struct { + AccessToken string + TxnID string +} // Cache represents a temporary store for response entries. // Entries are evicted after a certain period, defined by cleanupPeriod. @@ -50,14 +57,14 @@ func NewWithCleanupPeriod(cleanupPeriod time.Duration) *Cache { return &t } -// FetchTransaction looks up an entry for txnID in Cache. +// FetchTransaction looks up an entry for the (accessToken, txnID) tuple in Cache. // Looks in both the txnMaps. // Returns (JSON response, true) if txnID is found, else the returned bool is false. -func (t *Cache) FetchTransaction(txnID string) (*util.JSONResponse, bool) { +func (t *Cache) FetchTransaction(accessToken, txnID string) (*util.JSONResponse, bool) { t.RLock() defer t.RUnlock() for _, txns := range t.txnsMaps { - res, ok := txns[txnID] + res, ok := txns[CacheKey{accessToken, txnID}] if ok { return res, true } @@ -65,13 +72,13 @@ func (t *Cache) FetchTransaction(txnID string) (*util.JSONResponse, bool) { return nil, false } -// AddTransaction adds an entry for txnID in Cache for later access. +// AddTransaction adds an entry for the (accessToken, txnID) tuple in Cache. // Adds to the front txnMap. -func (t *Cache) AddTransaction(txnID string, res *util.JSONResponse) { +func (t *Cache) AddTransaction(accessToken, txnID string, res *util.JSONResponse) { t.Lock() defer t.Unlock() - t.txnsMaps[0][txnID] = res + t.txnsMaps[0][CacheKey{accessToken, txnID}] = res } // cacheCleanService is responsible for cleaning up entries after cleanupPeriod. diff --git a/common/transactions/transactions_test.go b/common/transactions/transactions_test.go index 0cdb776cc..f565e4846 100644 --- a/common/transactions/transactions_test.go +++ b/common/transactions/transactions_test.go @@ -24,27 +24,54 @@ type fakeType struct { } var ( - fakeTxnID = "aRandomTxnID" - fakeResponse = &util.JSONResponse{Code: http.StatusOK, JSON: fakeType{ID: "0"}} + fakeAccessToken = "aRandomAccessToken" + fakeAccessToken2 = "anotherRandomAccessToken" + fakeTxnID = "aRandomTxnID" + fakeResponse = &util.JSONResponse{ + Code: http.StatusOK, JSON: fakeType{ID: "0"}, + } + fakeResponse2 = &util.JSONResponse{ + Code: http.StatusOK, JSON: fakeType{ID: "1"}, + } ) // TestCache creates a New Cache and tests AddTransaction & FetchTransaction func TestCache(t *testing.T) { fakeTxnCache := New() - fakeTxnCache.AddTransaction(fakeTxnID, fakeResponse) + fakeTxnCache.AddTransaction(fakeAccessToken, fakeTxnID, fakeResponse) // Add entries for noise. for i := 1; i <= 100; i++ { fakeTxnCache.AddTransaction( + fakeAccessToken, fakeTxnID+string(i), &util.JSONResponse{Code: http.StatusOK, JSON: fakeType{ID: string(i)}}, ) } - testResponse, ok := fakeTxnCache.FetchTransaction(fakeTxnID) + testResponse, ok := fakeTxnCache.FetchTransaction(fakeAccessToken, fakeTxnID) if !ok { t.Error("Failed to retrieve entry for txnID: ", fakeTxnID) } else if testResponse.JSON != fakeResponse.JSON { t.Error("Fetched response incorrect. Expected: ", fakeResponse.JSON, " got: ", testResponse.JSON) } } + +// TestCacheScope ensures transactions with the same transaction ID are not shared +// across multiple access tokens. +func TestCacheScope(t *testing.T) { + cache := New() + cache.AddTransaction(fakeAccessToken, fakeTxnID, fakeResponse) + cache.AddTransaction(fakeAccessToken2, fakeTxnID, fakeResponse2) + + if res, ok := cache.FetchTransaction(fakeAccessToken, fakeTxnID); !ok { + t.Errorf("failed to retrieve entry for (%s, %s)", fakeAccessToken, fakeTxnID) + } else if res.JSON != fakeResponse.JSON { + t.Errorf("Wrong cache entry for (%s, %s). Expected: %v; got: %v", fakeAccessToken, fakeTxnID, fakeResponse.JSON, res.JSON) + } + if res, ok := cache.FetchTransaction(fakeAccessToken2, fakeTxnID); !ok { + t.Errorf("failed to retrieve entry for (%s, %s)", fakeAccessToken, fakeTxnID) + } else if res.JSON != fakeResponse2.JSON { + t.Errorf("Wrong cache entry for (%s, %s). Expected: %v; got: %v", fakeAccessToken, fakeTxnID, fakeResponse2.JSON, res.JSON) + } +} diff --git a/common/types.go b/common/types.go index 6888d3806..91765be00 100644 --- a/common/types.go +++ b/common/types.go @@ -15,9 +15,14 @@ package common import ( + "errors" "strconv" ) +// ErrProfileNoExists is returned when trying to lookup a user's profile that +// doesn't exist locally. +var ErrProfileNoExists = errors.New("no known profile for given user ID") + // AccountData represents account data sent from the client API server to the // sync API server type AccountData struct { diff --git a/testfile b/testfile index 7f6547e88..d04ab731c 100644 --- a/testfile +++ b/testfile @@ -159,6 +159,7 @@ Inbound federation rejects remote attempts to kick local users to rooms An event which redacts itself should be ignored A pair of events which redact each other should be ignored Full state sync includes joined rooms +A message sent after an initial sync appears in the timeline of an incremental sync. Can add tag Can remove tag Can list tags for a room @@ -166,5 +167,6 @@ Tags appear in an initial v2 /sync Newly updated tags appear in an incremental v2 /sync Deleted tags appear in an incremental v2 /sync /event/ on non world readable room does not work +Outbound federation can query profile data /event/ on joined room works /event/ does not allow access to events before the user joined