cr fixes

Author: mdnight

clientapi/routing/admin.go

@@ -551,12 +551,6 @@ func AdminUserDeviceRetrieveCreate(
 
 	switch req.Method {
 	case http.MethodPost:
-		if err != nil {
-			return util.JSONResponse{
-				Code: http.StatusBadRequest,
-				JSON: spec.InvalidParam(userID),
-			}
-		}
 		var payload struct {
 			DeviceID string `json:"device_id"`
 		}
@@ -980,11 +974,10 @@ func AdminRetrieveAccount(req *http.Request, cfg *config.ClientAPI, userAPI user
 				Code: http.StatusNotFound,
 				JSON: spec.NotFound(err.Error()),
 			}
-		} else if err != nil {
-			return util.JSONResponse{
-				Code: http.StatusInternalServerError,
-				JSON: spec.Unknown(err.Error()),
-			}
+		}
+		return util.JSONResponse{
+			Code: http.StatusInternalServerError,
+			JSON: spec.Unknown(err.Error()),
 		}
 	}
 	body.AvatarURL = profile.AvatarURL

internal/httputil/httpapi.go

@@ -58,6 +58,9 @@ func WithAuth() AuthAPIOption {
 	}
 }
 
+// UserVerifier verifies users by their access tokens. Currently, there are two interface implementations:
+// DefaultUserVerifier and MSC3861UserVerifier. The first one checks if the token exists in the server's database,
+// whereas the latter passes the token for verification to MAS and acts in accordance with MAS's response.
 type UserVerifier interface {
 	// VerifyUserFromRequest authenticates the HTTP request,
 	// on success returns Device of the requester.

setup/mscs/msc3861/msc3861_user_verifier.go

@@ -354,14 +354,13 @@ func (m *MSC3861UserVerifier) introspectToken(ctx context.Context, token string)
 	if err != nil {
 		return nil, err
 	}
-	body := resp.Body
 	defer resp.Body.Close() // nolint: errcheck
 
 	if c := resp.StatusCode; c/100 != 2 {
 		return nil, errors.New(strings.Join([]string{"The introspection endpoint returned a '", resp.Status, "' response"}, ""))
 	}
 	var ir introspectionResponse
-	if err := json.NewDecoder(body).Decode(&ir); err != nil {
+	if err := json.NewDecoder(resp.Body).Decode(&ir); err != nil {
 		return nil, err
 	}
 	return &ir, nil

syncapi/syncapi_test.go

@@ -120,14 +120,14 @@ func (s *syncUserAPI) PerformLastSeenUpdate(ctx context.Context, req *userapi.Pe
 	return nil
 }
 
-type userVerifier struct {
+type mockUserVerifier struct {
 	accessTokenToDeviceAndResponse map[string]struct {
 		Device   *userapi.Device
 		Response *util.JSONResponse
 	}
 }
 
-func (u *userVerifier) VerifyUserFromRequest(req *http.Request) (*userapi.Device, *util.JSONResponse) {
+func (u *mockUserVerifier) VerifyUserFromRequest(req *http.Request) (*userapi.Device, *util.JSONResponse) {
 	if pair, ok := u.accessTokenToDeviceAndResponse[req.URL.Query().Get("access_token")]; ok {
 		return pair.Device, pair.Response
 	}
@@ -161,7 +161,7 @@ func testSyncAccessTokens(t *testing.T, dbType test.DBType) {
 	jsctx, _ := natsInstance.Prepare(processCtx, &cfg.Global.JetStream)
 	defer jetstream.DeleteAllStreams(jsctx, &cfg.Global.JetStream)
 	msgs := toNATSMsgs(t, cfg, room.Events()...)
-	uv := &userVerifier{}
+	uv := &mockUserVerifier{}
 
 	AddPublicRoutes(processCtx, routers, cfg, cm, &natsInstance, &syncUserAPI{accounts: []userapi.Device{alice}}, &syncRoomserverAPI{rooms: []*test.Room{room}}, caches, uv, caching.DisableMetrics)
 	testrig.MustPublishMsgs(t, jsctx, msgs...)
@@ -284,7 +284,7 @@ func testSyncEventFormatPowerLevels(t *testing.T, dbType test.DBType) {
 	cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions)
 	caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics)
 	natsInstance := jetstream.NATSInstance{}
-	uv := userVerifier{
+	uv := mockUserVerifier{
 		accessTokenToDeviceAndResponse: map[string]struct {
 			Device   *userapi.Device
 			Response *util.JSONResponse
@@ -538,7 +538,7 @@ func testSyncAPIUpdatePresenceImmediately(t *testing.T, dbType test.DBType) {
 
 	jsctx, _ := natsInstance.Prepare(processCtx, &cfg.Global.JetStream)
 	defer jetstream.DeleteAllStreams(jsctx, &cfg.Global.JetStream)
-	uv := userVerifier{
+	uv := mockUserVerifier{
 		accessTokenToDeviceAndResponse: map[string]struct {
 			Device   *userapi.Device
 			Response *util.JSONResponse
@@ -668,7 +668,7 @@ func testHistoryVisibility(t *testing.T, dbType test.DBType) {
 		// Use the actual internal roomserver API
 		rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics)
 		rsAPI.SetFederationAPI(nil, nil)
-		uv := userVerifier{
+		uv := mockUserVerifier{
 			accessTokenToDeviceAndResponse: map[string]struct {
 				Device   *userapi.Device
 				Response *util.JSONResponse
@@ -946,7 +946,7 @@ func TestGetMembership(t *testing.T) {
 		// Use an actual roomserver for this
 		rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics)
 		rsAPI.SetFederationAPI(nil, nil)
-		uv := userVerifier{
+		uv := mockUserVerifier{
 			accessTokenToDeviceAndResponse: map[string]struct {
 				Device   *userapi.Device
 				Response *util.JSONResponse
@@ -1023,7 +1023,7 @@ func testSendToDevice(t *testing.T, dbType test.DBType) {
 	caches := caching.NewRistrettoCache(128*1024*1024, time.Hour, caching.DisableMetrics)
 	defer close()
 	natsInstance := jetstream.NATSInstance{}
-	uv := userVerifier{
+	uv := mockUserVerifier{
 		accessTokenToDeviceAndResponse: map[string]struct {
 			Device   *userapi.Device
 			Response *util.JSONResponse
@@ -1257,7 +1257,7 @@ func testContext(t *testing.T, dbType test.DBType) {
 	rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics)
 	rsAPI.SetFederationAPI(nil, nil)
 
-	uv := userVerifier{
+	uv := mockUserVerifier{
 		accessTokenToDeviceAndResponse: map[string]struct {
 			Device   *userapi.Device
 			Response *util.JSONResponse
@@ -1445,7 +1445,7 @@ func TestRemoveEditedEventFromSearchIndex(t *testing.T) {
 
 	rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, caches, caching.DisableMetrics)
 	rsAPI.SetFederationAPI(nil, nil)
-	uv := userVerifier{
+	uv := mockUserVerifier{
 		accessTokenToDeviceAndResponse: map[string]struct {
 			Device   *userapi.Device
 			Response *util.JSONResponse