From 66b65fdd27a71ed6d134f23286a54bfa9269971e Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Sat, 28 Aug 2021 14:30:59 +0200 Subject: [PATCH] Fix failing tests --- clientapi/auth/auth.go | 6 +++--- internal/httputil/http.go | 4 ++++ setup/mscs/msc2836/msc2836_test.go | 3 +-- setup/mscs/msc2946/msc2946_test.go | 3 +-- userapi/api/api.go | 4 ++-- userapi/api/api_trace.go | 7 +++++-- userapi/internal/api.go | 31 ++++++++++++++++++++++++------ userapi/inthttp/client.go | 3 ++- userapi/inthttp/server.go | 6 +++--- 9 files changed, 46 insertions(+), 21 deletions(-) diff --git a/clientapi/auth/auth.go b/clientapi/auth/auth.go index b4c39ae3..c850bf91 100644 --- a/clientapi/auth/auth.go +++ b/clientapi/auth/auth.go @@ -70,11 +70,11 @@ func VerifyUserFromRequest( jsonErr := jsonerror.InternalServerError() return nil, &jsonErr } - if res.Err != nil { - if forbidden, ok := res.Err.(*api.ErrorForbidden); ok { + if res.Err != "" { + if strings.HasPrefix(strings.ToLower(res.Err), "forbidden:") { // TODO: use actual error and no string comparison return nil, &util.JSONResponse{ Code: http.StatusForbidden, - JSON: jsonerror.Forbidden(forbidden.Message), + JSON: jsonerror.Forbidden(res.Err), } } } diff --git a/internal/httputil/http.go b/internal/httputil/http.go index 9197371a..b103d5b1 100644 --- a/internal/httputil/http.go +++ b/internal/httputil/http.go @@ -23,6 +23,7 @@ import ( "net/url" "strings" + "github.com/matrix-org/dendrite/userapi/api" opentracing "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" ) @@ -72,6 +73,9 @@ func PostJSON( var errorBody struct { Message string `json:"message"` } + if _, ok := response.(*api.PerformKeyBackupResponse); ok { // TODO: remove this, once cross-boundary errors are a thing + return nil + } if msgerr := json.NewDecoder(res.Body).Decode(&errorBody); msgerr == nil { return fmt.Errorf("Internal API: %d from %s: %s", res.StatusCode, apiURL, errorBody.Message) } diff --git a/setup/mscs/msc2836/msc2836_test.go b/setup/mscs/msc2836/msc2836_test.go index 51fde691..78ae1115 100644 --- a/setup/mscs/msc2836/msc2836_test.go +++ b/setup/mscs/msc2836/msc2836_test.go @@ -7,7 +7,6 @@ import ( "crypto/sha256" "encoding/base64" "encoding/json" - "fmt" "io/ioutil" "net/http" "sort" @@ -504,7 +503,7 @@ type testUserAPI struct { func (u *testUserAPI) QueryAccessToken(ctx context.Context, req *userapi.QueryAccessTokenRequest, res *userapi.QueryAccessTokenResponse) error { dev, ok := u.accessTokens[req.AccessToken] if !ok { - res.Err = fmt.Errorf("unknown token") + res.Err = "unknown token" return nil } res.Device = &dev diff --git a/setup/mscs/msc2946/msc2946_test.go b/setup/mscs/msc2946/msc2946_test.go index c362d9fb..4cda123f 100644 --- a/setup/mscs/msc2946/msc2946_test.go +++ b/setup/mscs/msc2946/msc2946_test.go @@ -19,7 +19,6 @@ import ( "context" "crypto/ed25519" "encoding/json" - "fmt" "io/ioutil" "net/http" "net/url" @@ -347,7 +346,7 @@ type testUserAPI struct { func (u *testUserAPI) QueryAccessToken(ctx context.Context, req *userapi.QueryAccessTokenRequest, res *userapi.QueryAccessTokenResponse) error { dev, ok := u.accessTokens[req.AccessToken] if !ok { - res.Err = fmt.Errorf("unknown token") + res.Err = "unknown token" return nil } res.Device = &dev diff --git a/userapi/api/api.go b/userapi/api/api.go index 75d06dd6..0c760483 100644 --- a/userapi/api/api.go +++ b/userapi/api/api.go @@ -33,7 +33,7 @@ type UserInternalAPI interface { PerformDeviceUpdate(ctx context.Context, req *PerformDeviceUpdateRequest, res *PerformDeviceUpdateResponse) error PerformAccountDeactivation(ctx context.Context, req *PerformAccountDeactivationRequest, res *PerformAccountDeactivationResponse) error PerformOpenIDTokenCreation(ctx context.Context, req *PerformOpenIDTokenCreationRequest, res *PerformOpenIDTokenCreationResponse) error - PerformKeyBackup(ctx context.Context, req *PerformKeyBackupRequest, res *PerformKeyBackupResponse) + PerformKeyBackup(ctx context.Context, req *PerformKeyBackupRequest, res *PerformKeyBackupResponse) error QueryKeyBackup(ctx context.Context, req *QueryKeyBackupRequest, res *QueryKeyBackupResponse) QueryProfile(ctx context.Context, req *QueryProfileRequest, res *QueryProfileResponse) error QueryAccessToken(ctx context.Context, req *QueryAccessTokenRequest, res *QueryAccessTokenResponse) error @@ -181,7 +181,7 @@ type QueryAccessTokenRequest struct { // QueryAccessTokenResponse is the response for QueryAccessToken type QueryAccessTokenResponse struct { Device *Device - Err error // e.g ErrorForbidden + Err string // e.g ErrorForbidden } // QueryAccountDataRequest is the request for QueryAccountData diff --git a/userapi/api/api_trace.go b/userapi/api/api_trace.go index 84dcb309..aa069f40 100644 --- a/userapi/api/api_trace.go +++ b/userapi/api/api_trace.go @@ -74,11 +74,14 @@ func (t *UserInternalAPITrace) PerformOpenIDTokenCreation(ctx context.Context, r util.GetLogger(ctx).Infof("PerformOpenIDTokenCreation req=%+v res=%+v", js(req), js(res)) return err } -func (t *UserInternalAPITrace) PerformKeyBackup(ctx context.Context, req *PerformKeyBackupRequest, res *PerformKeyBackupResponse) { - t.Impl.PerformKeyBackup(ctx, req, res) +func (t *UserInternalAPITrace) PerformKeyBackup(ctx context.Context, req *PerformKeyBackupRequest, res *PerformKeyBackupResponse) error { + err := t.Impl.PerformKeyBackup(ctx, req, res) + util.GetLogger(ctx).Infof("PerformKeyBackup req=%+v res=%+v", js(req), js(res)) + return err } func (t *UserInternalAPITrace) QueryKeyBackup(ctx context.Context, req *QueryKeyBackupRequest, res *QueryKeyBackupResponse) { t.Impl.QueryKeyBackup(ctx, req, res) + util.GetLogger(ctx).Infof("QueryKeyBackup req=%+v res=%+v", js(req), js(res)) } func (t *UserInternalAPITrace) QueryProfile(ctx context.Context, req *QueryProfileRequest, res *QueryProfileResponse) error { err := t.Impl.QueryProfile(ctx, req, res) diff --git a/userapi/internal/api.go b/userapi/internal/api.go index 518edef4..c9cd1ca3 100644 --- a/userapi/internal/api.go +++ b/userapi/internal/api.go @@ -358,8 +358,11 @@ func (a *UserInternalAPI) QueryAccountData(ctx context.Context, req *api.QueryAc func (a *UserInternalAPI) QueryAccessToken(ctx context.Context, req *api.QueryAccessTokenRequest, res *api.QueryAccessTokenResponse) error { if req.AppServiceUserID != "" { appServiceDevice, err := a.queryAppServiceToken(ctx, req.AccessToken, req.AppServiceUserID) + if err != nil { + res.Err = err.Error() + } res.Device = appServiceDevice - res.Err = err + return nil } device, err := a.DeviceDB.GetDeviceByAccessToken(ctx, req.AccessToken) @@ -455,13 +458,16 @@ func (a *UserInternalAPI) QueryOpenIDToken(ctx context.Context, req *api.QueryOp return nil } -func (a *UserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.PerformKeyBackupRequest, res *api.PerformKeyBackupResponse) { +func (a *UserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.PerformKeyBackupRequest, res *api.PerformKeyBackupResponse) error { // Delete metadata if req.DeleteBackup { if req.Version == "" { res.BadInput = true res.Error = "must specify a version to delete" - return + if res.Error != "" { + return fmt.Errorf(res.Error) + } + return nil } exists, err := a.AccountDB.DeleteKeyBackup(ctx, req.UserID, req.Version) if err != nil { @@ -469,7 +475,10 @@ func (a *UserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.Perform } res.Exists = exists res.Version = req.Version - return + if res.Error != "" { + return fmt.Errorf(res.Error) + } + return nil } // Create metadata if req.Version == "" { @@ -479,7 +488,10 @@ func (a *UserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.Perform } res.Exists = err == nil res.Version = version - return + if res.Error != "" { + return fmt.Errorf(res.Error) + } + return nil } // Update metadata if len(req.Keys.Rooms) == 0 { @@ -489,10 +501,17 @@ func (a *UserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.Perform } res.Exists = err == nil res.Version = req.Version - return + if res.Error != "" { + return fmt.Errorf(res.Error) + } + return nil } // Upload Keys for a specific version metadata a.uploadBackupKeys(ctx, req, res) + if res.Error != "" { + return fmt.Errorf(res.Error) + } + return nil } func (a *UserInternalAPI) uploadBackupKeys(ctx context.Context, req *api.PerformKeyBackupRequest, res *api.PerformKeyBackupResponse) { diff --git a/userapi/inthttp/client.go b/userapi/inthttp/client.go index a89d1a26..1599d463 100644 --- a/userapi/inthttp/client.go +++ b/userapi/inthttp/client.go @@ -228,7 +228,7 @@ func (h *httpUserInternalAPI) QueryOpenIDToken(ctx context.Context, req *api.Que return httputil.PostJSON(ctx, span, h.httpClient, apiURL, req, res) } -func (h *httpUserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.PerformKeyBackupRequest, res *api.PerformKeyBackupResponse) { +func (h *httpUserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.PerformKeyBackupRequest, res *api.PerformKeyBackupResponse) error { span, ctx := opentracing.StartSpanFromContext(ctx, "PerformKeyBackup") defer span.Finish() @@ -237,6 +237,7 @@ func (h *httpUserInternalAPI) PerformKeyBackup(ctx context.Context, req *api.Per if err != nil { res.Error = err.Error() } + return nil } func (h *httpUserInternalAPI) QueryKeyBackup(ctx context.Context, req *api.QueryKeyBackupRequest, res *api.QueryKeyBackupResponse) { span, ctx := opentracing.StartSpanFromContext(ctx, "QueryKeyBackup") diff --git a/userapi/inthttp/server.go b/userapi/inthttp/server.go index 61f85318..ac05bcd0 100644 --- a/userapi/inthttp/server.go +++ b/userapi/inthttp/server.go @@ -256,9 +256,9 @@ func AddRoutes(internalAPIMux *mux.Router, s api.UserInternalAPI) { if err := json.NewDecoder(req.Body).Decode(&request); err != nil { return util.MessageResponse(http.StatusBadRequest, err.Error()) } - s.PerformKeyBackup(req.Context(), &request, &response) - if response.Error != "" { - return util.ErrorResponse(fmt.Errorf("PerformKeyBackup: %s", response.Error)) + err := s.PerformKeyBackup(req.Context(), &request, &response) + if err != nil { + return util.JSONResponse{Code: http.StatusBadRequest, JSON: &response} } return util.JSONResponse{Code: http.StatusOK, JSON: &response} }),