Disable user sso_enable if org is disabling sso (#1331)

* Disable user sso_enable if org is disabling sso * Cleanup test * Add withTx and use it in SaveConfig
2021-07-09 13:12:21 -03:00
parent d90c16f96f
commit 12215bfbbd
4 changed files with 110 additions and 45 deletions
@@ -0,0 +1 @@
+* When disabling SSO at the org level, disable it for all users. Fixes issue 296
@@ -54,6 +54,27 @@ func testOrgInfo(t *testing.T, ds fleet.Datastore) {
 	info4, err := ds.NewAppConfig(info3)
 	assert.Nil(t, err)
 	assert.Equal(t, info3, info4)
+
+	email := "e@mail.com"
+	u := &fleet.User{
+		Password:   []byte("pass"),
+		Email:      email,
+		SSOEnabled: true,
+	}
+	_, err = ds.NewUser(u)
+	assert.Nil(t, err)
+
+	verify, err := ds.UserByEmail(email)
+	assert.Nil(t, err)
+	assert.True(t, verify.SSOEnabled)
+
+	info4.EnableSSO = false
+	err = ds.SaveAppConfig(info4)
+	assert.Nil(t, err)
+
+	verify, err = ds.UserByEmail(email)
+	assert.Nil(t, err)
+	assert.False(t, verify.SSOEnabled)
 }

 func testAdditionalQueries(t *testing.T, ds fleet.Datastore) {
@@ -43,19 +43,19 @@ func (d *Datastore) isEventSchedulerEnabled() (bool, error) {
 	return value == "ON", nil
 }

-func (d *Datastore) ManageHostExpiryEvent(hostExpiryEnabled bool, hostExpiryWindow int) error {
+func (d *Datastore) ManageHostExpiryEvent(tx *sqlx.Tx, hostExpiryEnabled bool, hostExpiryWindow int) error {
 	var err error
 	hostExpiryConfig := struct {
 		Window int `db:"host_expiry_window"`
 	}{}
-	if err = d.db.Get(&hostExpiryConfig, "SELECT host_expiry_window from app_configs LIMIT 1"); err != nil {
+	if err = tx.Get(&hostExpiryConfig, "SELECT host_expiry_window from app_configs LIMIT 1"); err != nil {
 		return errors.Wrap(err, "get expiry window setting")
 	}

 	shouldUpdateWindow := hostExpiryEnabled && hostExpiryConfig.Window != hostExpiryWindow

 	if !hostExpiryEnabled || shouldUpdateWindow {
-		if _, err := d.db.Exec("DROP EVENT IF EXISTS host_expiry"); err != nil {
+		if _, err := tx.Exec("DROP EVENT IF EXISTS host_expiry"); err != nil {
 			if driverErr, ok := err.(*mysql.MySQLError); !ok || driverErr.Number != mysqlerr.ER_DBACCESS_DENIED_ERROR {
 				return errors.Wrap(err, "drop existing host_expiry event")
 			}
@@ -64,7 +64,7 @@ func (d *Datastore) ManageHostExpiryEvent(hostExpiryEnabled bool, hostExpiryWind

 	if shouldUpdateWindow {
 		sql := fmt.Sprintf("CREATE EVENT IF NOT EXISTS host_expiry ON SCHEDULE EVERY 1 HOUR ON COMPLETION PRESERVE DO DELETE FROM hosts WHERE seen_time < DATE_SUB(NOW(), INTERVAL %d DAY)", hostExpiryWindow)
-		if _, err := d.db.Exec(sql); err != nil {
+		if _, err := tx.Exec(sql); err != nil {
 			return errors.Wrap(err, "create new host_expiry event")
 		}
 	}
@@ -81,14 +81,15 @@ func (d *Datastore) SaveAppConfig(info *fleet.AppConfig) error {
 		return errors.New("MySQL Event Scheduler must be enabled to configure Host Expiry.")
 	}

-	if err := d.ManageHostExpiryEvent(info.HostExpiryEnabled, info.HostExpiryWindow); err != nil {
-		return err
-	}
+	return d.withTx(func(tx *sqlx.Tx) error {
+		if err := d.ManageHostExpiryEvent(tx, info.HostExpiryEnabled, info.HostExpiryWindow); err != nil {
+			return err
+		}

-	// Note that we hard code the ID column to 1, insuring that, if no rows
-	// exist, a row will be created with INSERT, if a row does exist the key
-	// will be violate uniqueness constraint and an UPDATE will occur
-	insertStatement := `
+		// Note that we hard code the ID column to 1, insuring that, if no rows
+		// exist, a row will be created with INSERT, if a row does exist the key
+		// will be violate uniqueness constraint and an UPDATE will occur
+		insertStatement := `
 		INSERT INTO app_configs (
 			id,
 			org_name,
@@ -158,41 +159,52 @@ func (d *Datastore) SaveAppConfig(info *fleet.AppConfig) error {
 			enable_analytics = VALUES(enable_analytics)
    `

-	_, err = d.db.Exec(insertStatement,
-		info.OrgName,
-		info.OrgLogoURL,
-		info.ServerURL,
-		info.SMTPConfigured,
-		info.SMTPSenderAddress,
-		info.SMTPServer,
-		info.SMTPPort,
-		info.SMTPAuthenticationType,
-		info.SMTPEnableTLS,
-		info.SMTPAuthenticationMethod,
-		info.SMTPDomain,
-		info.SMTPUserName,
-		info.SMTPPassword,
-		info.SMTPVerifySSLCerts,
-		info.SMTPEnableStartTLS,
-		info.EntityID,
-		info.IssuerURI,
-		info.IDPImageURL,
-		info.Metadata,
-		info.MetadataURL,
-		info.IDPName,
-		info.EnableSSO,
-		info.EnableSSOIdPLogin,
-		info.FIMInterval,
-		info.FIMFileAccesses,
-		info.HostExpiryEnabled,
-		info.HostExpiryWindow,
-		info.LiveQueryDisabled,
-		info.AdditionalQueries,
-		info.AgentOptions,
-		info.EnableAnalytics,
-	)
+		_, err = tx.Exec(insertStatement,
+			info.OrgName,
+			info.OrgLogoURL,
+			info.ServerURL,
+			info.SMTPConfigured,
+			info.SMTPSenderAddress,
+			info.SMTPServer,
+			info.SMTPPort,
+			info.SMTPAuthenticationType,
+			info.SMTPEnableTLS,
+			info.SMTPAuthenticationMethod,
+			info.SMTPDomain,
+			info.SMTPUserName,
+			info.SMTPPassword,
+			info.SMTPVerifySSLCerts,
+			info.SMTPEnableStartTLS,
+			info.EntityID,
+			info.IssuerURI,
+			info.IDPImageURL,
+			info.Metadata,
+			info.MetadataURL,
+			info.IDPName,
+			info.EnableSSO,
+			info.EnableSSOIdPLogin,
+			info.FIMInterval,
+			info.FIMFileAccesses,
+			info.HostExpiryEnabled,
+			info.HostExpiryWindow,
+			info.LiveQueryDisabled,
+			info.AdditionalQueries,
+			info.AgentOptions,
+			info.EnableAnalytics,
+		)
+		if err != nil {
+			return err
+		}

-	return err
+		if !info.EnableSSO {
+			_, err = tx.Exec(`UPDATE users SET sso_enabled=false`)
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
 }

 func (d *Datastore) VerifyEnrollSecret(secret string) (*fleet.EnrollSecret, error) {
@@ -114,6 +114,37 @@ func (d *Datastore) withRetryTxx(fn txFn) (err error) {
 	return backoff.Retry(operation, bo)
 }

+// withTx provides a common way to commit/rollback a txFn
+func (d *Datastore) withTx(fn txFn) (err error) {
+	tx, err := d.db.Beginx()
+	if err != nil {
+		return errors.Wrap(err, "create transaction")
+	}
+
+	defer func() {
+		if p := recover(); p != nil {
+			if err := tx.Rollback(); err != nil {
+				d.logger.Log("err", err, "msg", "error encountered during transaction panic rollback")
+			}
+			panic(p)
+		}
+	}()
+
+	if err := fn(tx); err != nil {
+		rbErr := tx.Rollback()
+		if rbErr != nil && rbErr != sql.ErrTxDone {
+			return errors.Wrapf(err, "got err '%s' rolling back after err", rbErr.Error())
+		}
+		return err
+	}
+
+	if err := tx.Commit(); err != nil {
+		return errors.Wrap(err, "commit transaction")
+	}
+
+	return nil
+}
+
 // New creates an MySQL datastore.
 func New(config config.MysqlConfig, c clock.Clock, opts ...DBOption) (*Datastore, error) {
 	options := &dbOptions{
				`@@ -0,0 +1 @@`
				`* When disabling SSO at the org level, disable it for all users. Fixes issue 296`