Improve reliability generally

Author: the-maldridge

go.mod

@@ -3,7 +3,7 @@ module github.com/NetAuth/plugin-okta
 go 1.12
 
 require (
-	github.com/NetAuth/NetAuth v0.1.5-0.20190908061829-d33cf3914de8
+	github.com/NetAuth/NetAuth v0.1.5-0.20190910041404-507f94268003
 	github.com/NetAuth/Protocol v0.0.0-20190423042654-b6296098bf96
 	github.com/golang/protobuf v1.3.2 // indirect
 	github.com/hashicorp/go-hclog v0.9.2

go.sum

@@ -12,6 +12,8 @@ github.com/NetAuth/NetAuth v0.1.5-0.20190908040708-99f89c21459d h1:iVa8DbFqvzhxP
 github.com/NetAuth/NetAuth v0.1.5-0.20190908040708-99f89c21459d/go.mod h1:7RO3RQKSobRoghtojZ8hA/SBylqEGteZ+xhYU/IexC8=
 github.com/NetAuth/NetAuth v0.1.5-0.20190908061829-d33cf3914de8 h1:kXkjpX4l8wZvQ6N/EElUKgA2wUbRSTFHAlXFXOE7bWo=
 github.com/NetAuth/NetAuth v0.1.5-0.20190908061829-d33cf3914de8/go.mod h1:7RO3RQKSobRoghtojZ8hA/SBylqEGteZ+xhYU/IexC8=
+github.com/NetAuth/NetAuth v0.1.5-0.20190910041404-507f94268003 h1:Z3A9fbRGDl/RWHcTOPdZL2PevJgNGjcA8qEF9LTpbnk=
+github.com/NetAuth/NetAuth v0.1.5-0.20190910041404-507f94268003/go.mod h1:7RO3RQKSobRoghtojZ8hA/SBylqEGteZ+xhYU/IexC8=
 github.com/NetAuth/Protocol v0.0.0-20190121213421-fa32b3772b23/go.mod h1:CO+2Q2H4PqOcK9PO0fBxaZrygsIjTHn3k+f24c+Qmj0=
 github.com/NetAuth/Protocol v0.0.0-20190423042654-b6296098bf96 h1:sA8zUayQTmfIbRd98R8KS/AewfohWiwTWt5K+m+dPLk=
 github.com/NetAuth/Protocol v0.0.0-20190423042654-b6296098bf96/go.mod h1:CO+2Q2H4PqOcK9PO0fBxaZrygsIjTHn3k+f24c+Qmj0=

impl/entity.go

@@ -118,10 +118,10 @@ func (o OktaPlugin) EntityUnlock(e pb.Entity) (pb.Entity, error) {
 // EntityDestroy should never be used, deleting users is generally
 // bad, but if you must, then this function will ensure that users in
 // Okta have also been wiped.
-func (o OktaPlugin) EntityDestroy(e pb.Entity) error {
+func (o OktaPlugin) EntityDestroy(e pb.Entity) (pb.Entity, error) {
 	oktaID := getEntityOktaID(e)
 	if oktaID == "" {
-		return nil
+		return e, nil
 	}
 
 	_, err := o.c.User.DeactivateUser(oktaID, nil)
@@ -134,5 +134,5 @@ func (o OktaPlugin) EntityDestroy(e pb.Entity) error {
 		appLogger.Warn("Failed to delete Okta user", "entity", e.GetID(), "error", err)
 	}
 
-	return nil
+	return e, nil
 }

impl/group.go

@@ -64,17 +64,29 @@ func (o OktaPlugin) GroupUpdate(g pb.Group) (pb.Group, error) {
 // GroupDestroy pushes the destruction of groups to Okta.  It is
 // recommended to never destroy a group, but if this is desired this
 // function will ensure the group is removed in Okta as well.
-func (o OktaPlugin) GroupDestroy(g pb.Group) error {
+func (o OktaPlugin) GroupDestroy(g pb.Group) (pb.Group, error) {
 	appLogger.Info("Attempting to remove group from Okta", "group", g.GetName())
 	oktaID := getGroupOktaID(g)
 	if oktaID == "" {
-		return nil
+		return g, nil
 	}
-	resp, err := o.c.Group.DeleteGroup(oktaID)
-	if err != nil {
-		appLogger.Warn("Failed to delete Okta Group", "group", g.GetName(), "oktaID", oktaID, "error", err)
+
+	// Deleting groups in Okta appears to be very racy, and this
+	// often leads to groups not actually being deleted.  The fix
+	// is to keep trying to get the group until it goes away since
+	// that is the only way Okta provides to be sure that a group
+	// is really gone.
+	var err error
+	err = nil
+	for err == nil {
+		_, err = o.c.Group.DeleteGroup(oktaID)
+		if err != nil {
+			appLogger.Warn("Failed to delete Okta Group", "group", g.GetName(), "oktaID", oktaID, "error", err)
+		}
+
+		_, _, err = o.c.Group.GetGroup(oktaID, nil)
+		appLogger.Debug("Error after getting group", "error", err)
 	}
 
-	appLogger.Debug("Okta Response", "response", resp)
-	return nil
+	return g, nil
 }

impl/main.go

@@ -30,7 +30,7 @@ func init() {
 
 	viper.SetDefault("log.level", "INFO")
 	appLogger = hclog.New(&hclog.LoggerOptions{
-		Name:  "fail2lock",
+		Name:  "okta",
 		Level: hclog.LevelFromString(viper.GetString("log.level")),
 	})
 	hclog.SetDefault(appLogger)