Merge pull request #77 from alphagov/fix_unbind

Try to drop username generated with MD5

sqlengine/mysql_engine.go

@@ -115,7 +115,20 @@ func (d *MySQLEngine) DropUser(bindingID string) error {
 	dropUserStatement := "DROP USER '" + username + "'@'%';"
 	d.logger.Debug("drop-user", lager.Data{"statement": dropUserStatement})
 
-	if _, err := d.db.Exec(dropUserStatement); err != nil {
+	_, err := d.db.Exec(dropUserStatement)
+	if err == nil {
+		return nil
+	}
+
+	// Try to drop the username generated the old way
+
+	username = generateUsernameOld(bindingID)
+
+	dropUserStatement = "DROP USER '" + username + "'@'%';"
+	d.logger.Debug("drop-user", lager.Data{"statement": dropUserStatement})
+
+	_, err = d.db.Exec(dropUserStatement)
+	if err != nil {
 		d.logger.Error("sql-error", err)
 		return err
 	}

sqlengine/postgres_engine.go

@@ -144,19 +144,33 @@ func (d *PostgresEngine) DropUser(bindingID string) error {
 	username := generateUsername(bindingID)
 	dropUserStatement := fmt.Sprintf(`drop role "%s"`, username)
 
-	if _, err := d.db.Exec(dropUserStatement); err != nil {
-		// When handling unbinds for bindings created before the switch to
-		// event-triggers based permissions the `username` won't exist. We
-		// swallow the error to prevent unbinding from failing.
-		if pqErr, ok := err.(*pq.Error); ok && pqErr.Code == "42704" {
-			d.logger.Info("warning", lager.Data{"warning": "User " + username + " does not exist"})
-			return nil
+	_, err := d.db.Exec(dropUserStatement)
+	if err == nil {
+		return nil
+	}
+
+	// When handling unbinds for bindings created before the switch to
+	// event-triggers based permissions the `username` won't exist.
+	// Also we changed how we generate usernames so we have to try to drop the username generated
+	// the old way. If none of the usernames exist then we swallow the error
+	if pqErr, ok := err.(*pq.Error); ok && pqErr.Code == "42704" {
+		d.logger.Info("warning", lager.Data{"warning": "User " + username + " does not exist"})
+
+		username = generateUsernameOld(bindingID)
+		dropUserStatement = fmt.Sprintf(`drop role "%s"`, username)
+		if _, err = d.db.Exec(dropUserStatement); err != nil {
+			if pqErr, ok := err.(*pq.Error); ok && pqErr.Code == "42704" {
+				d.logger.Info("warning", lager.Data{"warning": "User " + username + " does not exist"})
+				return nil
+			}
+			d.logger.Error("sql-error", err)
+			return err
 		}
-		d.logger.Error("sql-error", err)
-		return err
+
+		return nil
 	}
 
-	return nil
+	return err
 }
 
 func (d *PostgresEngine) ResetState() error {

sqlengine/sql_engine.go

@@ -30,6 +30,11 @@ func generateUsername(seed string) string {
 	return "u" + strings.Replace(usernameString, "-", "_", -1)
 }
 
+func generateUsernameOld(seed string) string {
+	usernameString := strings.ToLower(utils.GetMD5B64(seed, usernameLength-1))
+	return "u" + strings.Replace(usernameString, "-", "_", -1)
+}
+
 func generatePassword() string {
 	return utils.RandomAlphaNum(passwordLength)
 }

utils/utils.go

@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"crypto/md5"
 	"crypto/rand"
 	"crypto/sha256"
 	"encoding/base64"
@@ -51,3 +52,13 @@ func GenerateHash(text string, maxLength int) string {
 		return encoded
 	}
 }
+
+func GetMD5B64(text string, maxLength int) string {
+	md5 := md5.Sum([]byte(text))
+	encoded := base64.URLEncoding.EncodeToString(md5[:])
+	if len(encoded) > maxLength {
+		return encoded[0:maxLength]
+	} else {
+		return encoded
+	}
+}