msc3861: validate access_token uniqueness manually

If MAS is in use, server does not generate access tokens for its client
devices. As a result, the access_token value in device table will always be
empty. The unique constraint is enabled for the access_token it does
not allow storing empty values in the database so we had to drop it.
Since we still have old login logic, the constraint is still required,
so we have to check the uniqueness manually.

Author: mdnight

appservice/appservice.go

@@ -53,6 +53,7 @@ func NewInternalAPI(
 		if err := generateAppServiceAccount(userAPI, appservice, cfg.Global.ServerName); err != nil {
 			logrus.WithFields(logrus.Fields{
 				"appservice": appservice.ID,
+				"as_token":   appservice.ASToken,
 			}).WithError(err).Panicf("failed to generate bot account for appservice")
 		}
 	}
@@ -92,12 +93,13 @@ func generateAppServiceAccount(
 	}
 	var devRes userapi.PerformDeviceCreationResponse
 	err = userAPI.PerformDeviceCreation(context.Background(), &userapi.PerformDeviceCreationRequest{
-		Localpart:          as.SenderLocalpart,
-		ServerName:         serverName,
-		AccessToken:        as.ASToken,
-		DeviceID:           &as.SenderLocalpart,
-		DeviceDisplayName:  &as.SenderLocalpart,
-		NoDeviceListUpdate: true,
+		Localpart:                           as.SenderLocalpart,
+		ServerName:                          serverName,
+		AccessToken:                         as.ASToken,
+		DeviceID:                            &as.SenderLocalpart,
+		DeviceDisplayName:                   &as.SenderLocalpart,
+		NoDeviceListUpdate:                  true,
+		AccessTokenUniqueConstraintDisabled: false,
 	}, &devRes)
 	return err
 }

appservice/appservice_test.go

@@ -139,7 +139,7 @@ func TestAppserviceInternalAPI(t *testing.T) {
 		as := &config.ApplicationService{
 			ID:              "someID",
 			URL:             srv.URL,
-			ASToken:         "",
+			ASToken:         util.RandomString(12),
 			HSToken:         "",
 			SenderLocalpart: "senderLocalPart",
 			NamespaceMap: map[string][]config.ApplicationServiceNamespace{
@@ -233,7 +233,7 @@ func TestAppserviceInternalAPI_UnixSocket_Simple(t *testing.T) {
 	as := &config.ApplicationService{
 		ID:              "someID",
 		URL:             fmt.Sprintf("unix://%s", socket),
-		ASToken:         "",
+		ASToken:         util.RandomString(8),
 		HSToken:         "",
 		SenderLocalpart: "senderLocalPart",
 		NamespaceMap: map[string][]config.ApplicationServiceNamespace{
@@ -377,7 +377,7 @@ func TestRoomserverConsumerOneInvite(t *testing.T) {
 		as := &config.ApplicationService{
 			ID:              "someID",
 			URL:             srv.URL,
-			ASToken:         "",
+			ASToken:         util.RandomString(8),
 			HSToken:         "",
 			SenderLocalpart: "senderLocalPart",
 			NamespaceMap: map[string][]config.ApplicationServiceNamespace{
@@ -510,7 +510,7 @@ func TestOutputAppserviceEvent(t *testing.T) {
 		as := &config.ApplicationService{
 			ID:              "someID",
 			URL:             srv.URL,
-			ASToken:         "",
+			ASToken:         util.RandomString(8),
 			HSToken:         "",
 			SenderLocalpart: "senderLocalPart",
 			NamespaceMap: map[string][]config.ApplicationServiceNamespace{

clientapi/admin_test.go

@@ -1637,8 +1637,9 @@ func TestAdminUserDeviceRetrieveCreate(t *testing.T) {
 		t.Run("Retrieve device", func(t *testing.T) {
 			var deviceRes uapi.PerformDeviceCreationResponse
 			if err := userAPI.PerformDeviceCreation(ctx, &uapi.PerformDeviceCreationRequest{
-				Localpart:  alice.Localpart,
-				ServerName: cfg.Global.ServerName,
+				Localpart:                           alice.Localpart,
+				ServerName:                          cfg.Global.ServerName,
+				AccessTokenUniqueConstraintDisabled: true,
 			}, &deviceRes); err != nil {
 				t.Errorf("failed to create account: %s", err)
 			}
@@ -1747,8 +1748,9 @@ func TestAdminUserDeviceDelete(t *testing.T) {
 		t.Run("Delete existing device", func(t *testing.T) {
 			var deviceRes uapi.PerformDeviceCreationResponse
 			if err := userAPI.PerformDeviceCreation(ctx, &uapi.PerformDeviceCreationRequest{
-				Localpart:  alice.Localpart,
-				ServerName: cfg.Global.ServerName,
+				Localpart:                           alice.Localpart,
+				ServerName:                          cfg.Global.ServerName,
+				AccessTokenUniqueConstraintDisabled: true,
 			}, &deviceRes); err != nil {
 				t.Errorf("failed to create account: %s", err)
 			}
@@ -1844,8 +1846,9 @@ func TestAdminUserDevicesDelete(t *testing.T) {
 		t.Run("Delete existing user's devices", func(t *testing.T) {
 			var deviceRes uapi.PerformDeviceCreationResponse
 			if err := userAPI.PerformDeviceCreation(ctx, &uapi.PerformDeviceCreationRequest{
-				Localpart:  alice.Localpart,
-				ServerName: cfg.Global.ServerName,
+				Localpart:                           alice.Localpart,
+				ServerName:                          cfg.Global.ServerName,
+				AccessTokenUniqueConstraintDisabled: true,
 			}, &deviceRes); err != nil {
 				t.Errorf("failed to create account: %s", err)
 			}

setup/mscs/msc3861/msc3861_user_verifier.go

@@ -96,7 +96,8 @@ func (r *mscError) Error() string {
 
 // VerifyUserFromRequest authenticates the HTTP request, on success returns Device of the requester.
 func (m *MSC3861UserVerifier) VerifyUserFromRequest(req *http.Request) (*api.Device, *util.JSONResponse) {
-	util.GetLogger(req.Context()).Debug("MSC3861.VerifyUserFromRequest")
+	ctx := req.Context()
+	util.GetLogger(ctx).Debug("MSC3861.VerifyUserFromRequest")
 	// Try to find the Application Service user
 	token, err := auth.ExtractAccessToken(req)
 	if err != nil {
@@ -105,8 +106,22 @@ func (m *MSC3861UserVerifier) VerifyUserFromRequest(req *http.Request) (*api.Dev
 			JSON: spec.MissingToken(err.Error()),
 		}
 	}
-	// TODO: try to get appservice user first. See https://github.com/element-hq/synapse/blob/develop/synapse/api/auth/msc3861_delegated.py#L273
-	userData, err := m.getUserByAccessToken(req.Context(), token)
+	if appServiceUserID := req.URL.Query().Get("user_id"); appServiceUserID != "" {
+		var res api.QueryAccessTokenResponse
+		err = m.userAPI.QueryAccessToken(ctx, &api.QueryAccessTokenRequest{
+			AccessToken:      token,
+			AppServiceUserID: req.URL.Query().Get("user_id"),
+		}, &res)
+		if err != nil {
+			util.GetLogger(ctx).WithError(err).Error("userAPI.QueryAccessToken failed")
+			return nil, &util.JSONResponse{
+				Code: http.StatusInternalServerError,
+				JSON: spec.InternalServerError{},
+			}
+		}
+	}
+
+	userData, err := m.getUserByAccessToken(ctx, token)
 	if err != nil {
 		switch e := err.(type) {
 		case (*mscError):
@@ -306,11 +321,12 @@ func (m *MSC3861UserVerifier) getUserByAccessToken(ctx context.Context, token st
 		var rs api.PerformDeviceCreationResponse
 		deviceDisplayName := "OIDC-native client"
 		if err := m.userAPI.PerformDeviceCreation(ctx, &api.PerformDeviceCreationRequest{
-			Localpart:         localpart,
-			ServerName:        m.serverName,
-			AccessToken:       "",
-			DeviceID:          &deviceID,
-			DeviceDisplayName: &deviceDisplayName,
+			Localpart:                           localpart,
+			ServerName:                          m.serverName,
+			AccessToken:                         "",
+			DeviceID:                            &deviceID,
+			DeviceDisplayName:                   &deviceDisplayName,
+			AccessTokenUniqueConstraintDisabled: true,
 			// TODO: Cannot add IPAddr and Useragent values here. Should we care about it here?
 		}, &rs); err != nil {
 			logger.WithError(err).Error("PerformDeviceCreation")

userapi/api/api.go

@@ -381,6 +381,11 @@ type PerformDeviceCreationRequest struct {
 	// FromRegistration determines if this request comes from registering a new account
 	// and is in most cases false.
 	FromRegistration bool
+
+	// AccessTokenUniqueConstraintDisabled determines if unique constraint is applicable for the AccessToken.
+	// It is false if an external auth service is in use (e.g. MAS) and server does not generate its own
+	// auth tokens. Otherwise, if traditional login is in use, the value is true. Default is false.
+	AccessTokenUniqueConstraintDisabled bool
 }
 
 // PerformDeviceCreationResponse is the response for PerformDeviceCreation

userapi/internal/user_api.go

@@ -306,8 +306,15 @@ func (a *UserInternalAPI) PerformDeviceCreation(ctx context.Context, req *api.Pe
 		"device_id":    req.DeviceID,
 		"display_name": req.DeviceDisplayName,
 	}).Info("PerformDeviceCreation")
-	// TODO: Since we have deleted access_token's unique constraint from the db,
-	// we probably should check its uniqueness if msc3861 is disabled
+	if !req.AccessTokenUniqueConstraintDisabled {
+		dev, err := a.DB.GetDeviceByAccessToken(ctx, req.AccessToken)
+		if err != nil && !errors.Is(err, sql.ErrNoRows) {
+			return err
+		}
+		if dev.UserID != "" {
+			return errors.New("unique constraint violation. Access token is not unique" + dev.AccessToken)
+		}
+	}
 	dev, err := a.DB.CreateDevice(ctx, req.Localpart, serverName, req.DeviceID, req.AccessToken, req.DeviceDisplayName, req.IPAddr, req.UserAgent)
 	if err != nil {
 		return err

userapi/userapi_test.go

@@ -445,6 +445,8 @@ func TestAccountData(t *testing.T) {
 func TestDevices(t *testing.T) {
 	ctx := context.Background()
 
+	dupeAccessToken := util.RandomString(8)
+
 	displayName := "testing"
 
 	creationTests := []struct {
@@ -455,25 +457,43 @@ func TestDevices(t *testing.T) {
 	}{
 		{
 			name:      "not a local user",
-			inputData: &api.PerformDeviceCreationRequest{Localpart: "test1", ServerName: "notlocal"},
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test1", ServerName: "notlocal", AccessTokenUniqueConstraintDisabled: true},
 			wantErr:   true,
 		},
 		{
 			name:      "implicit local user",
-			inputData: &api.PerformDeviceCreationRequest{Localpart: "test1", AccessToken: util.RandomString(8), NoDeviceListUpdate: true, DeviceDisplayName: &displayName},
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test1", AccessToken: util.RandomString(8), NoDeviceListUpdate: true, DeviceDisplayName: &displayName, AccessTokenUniqueConstraintDisabled: true},
 		},
 		{
 			name:      "explicit local user",
-			inputData: &api.PerformDeviceCreationRequest{Localpart: "test2", ServerName: "test", AccessToken: util.RandomString(8), NoDeviceListUpdate: true},
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test2", ServerName: "test", AccessToken: util.RandomString(8), NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: true},
 		},
 		{
 			name:      "test3 second device", // used to test deletion later
-			inputData: &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: util.RandomString(8), NoDeviceListUpdate: true},
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: util.RandomString(8), NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: true},
 		},
 		{
 			name:         "test3 third device", // used to test deletion later
 			wantNewDevID: true,
-			inputData:    &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: util.RandomString(8), NoDeviceListUpdate: true},
+			inputData:    &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: util.RandomString(8), NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: true},
+		},
+		{
+			name:      "dupe token - ok (unique constraint enabled)",
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: dupeAccessToken, NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: false},
+		},
+		{
+			name:      "dupe token - not ok (unique constraint enabled)",
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: dupeAccessToken, NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: false},
+			wantErr:   true,
+		},
+		{
+			name:      "dupe token - ok (unique constraint disabled)",
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: dupeAccessToken, NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: true},
+		},
+		{
+			name:      "dupe token - not ok (unique constraint disabled)",
+			inputData: &api.PerformDeviceCreationRequest{Localpart: "test3", ServerName: "test", AccessToken: dupeAccessToken, NoDeviceListUpdate: true, AccessTokenUniqueConstraintDisabled: true},
+			wantErr:   true,
 		},
 	}