Fixes related to comments in PR

pkg/bastion/acl.go

@@ -1,7 +1,9 @@
 package bastion
 
 import (
+	"context"
 	"encoding/json"
+	"fmt"
 	"log"
 	"os/exec"
 	"sort"
@@ -11,6 +13,9 @@ import (
 	"moul.io/sshportal/pkg/dbmodels"
 )
 
+// AclHookTimeout is timeout for external ACL hook execution
+const AclHookTimeout = 2 * time.Second
+
 type byWeight []*dbmodels.ACL
 
 func (a byWeight) Len() int           { return len(a) }
@@ -38,9 +43,13 @@ func checkACLs(user dbmodels.User, host dbmodels.Host, aclCheckCmd string) strin
 	}
 	// FIXME: add ACLs that match host pattern
 
-	// deny by default if no shared ACL
+	// if no shared ACL then execute ACLs hook if it exists and return its result
 	if len(aclMap) == 0 {
-		return checkACLsHook(aclCheckCmd, string(dbmodels.ACLActionDeny), user, host) // default action
+		action, err := checkACLsHook(aclCheckCmd, string(dbmodels.ACLActionDeny), user, host)
+		if err != nil {
+			log.Println(err)
+		}
+		return action
 	}
 
 	// transform map to slice and sort it
@@ -50,51 +59,62 @@ func checkACLs(user dbmodels.User, host dbmodels.Host, aclCheckCmd string) strin
 	}
 	sort.Sort(byWeight(acls))
 
-	return checkACLsHook(aclCheckCmd, acls[0].Action, user, host)
+	action, err := checkACLsHook(aclCheckCmd, acls[0].Action, user, host)
+	if err != nil {
+		log.Println(err)
+	}
+
+	return action
 }
 
 // checkACLsHook executes external command to check ACL and passes following parameters:
-// $1 - SSH Portal action (`allow` or `deny`)
+// $1 - SSH Portal `action` (`allow` or `deny`)
 // $2 - User as JSON string
 // $3 - Host as JSON string
 // External program has to return `allow` or `deny` in stdout.
-func checkACLsHook(aclCheckCmd string, action string, user dbmodels.User, host dbmodels.Host) string {
-	if aclCheckCmd != "" {
-		jsonUser, err := json.Marshal(user)
-		if err != nil {
-			log.Printf("Error: %v", err)
-			return action
-		}
-
-		jsonHost, err := json.Marshal(host)
-		if err != nil {
-			log.Printf("Error: %v", err)
-			return action
-		}
-
-		args := []string{
-			action,
-			string(jsonUser),
-			string(jsonHost),
-		}
-
-		out, err := exec.Command(aclCheckCmd, args...).CombinedOutput()
-		if err != nil {
-			log.Printf("Error: %v", err)
-			return action
-		}
-
-		outStr := strings.TrimSuffix(string(out), "\n")
-
-		switch outStr {
-		case string(dbmodels.ACLActionAllow):
-			return string(dbmodels.ACLActionAllow)
-		case string(dbmodels.ACLActionDeny):
-			return string(dbmodels.ACLActionDeny)
-		default:
-			log.Printf("Error: acl-check-cmd wrong output '%s'\n", outStr)
-			return action
-		}
+// In case of any error function returns `action`.
+func checkACLsHook(aclCheckCmd string, action string, user dbmodels.User, host dbmodels.Host) (string, error) {
+	if aclCheckCmd == "" {
+		return action, nil
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), AclHookTimeout)
+	defer cancel()
+
+	jsonUser, err := json.Marshal(user)
+	if err != nil {
+		return action, err
+	}
+
+	jsonHost, err := json.Marshal(host)
+	if err != nil {
+		return action, err
+	}
+
+	args := []string{
+		action,
+		string(jsonUser),
+		string(jsonHost),
+	}
+
+	cmd := exec.CommandContext(ctx, aclCheckCmd, args...)
+	out, err := cmd.Output()
+	if err != nil {
+		return action, err
+	}
+
+	if ctx.Err() == context.DeadlineExceeded {
+		return action, fmt.Errorf("external ACL hook command timed out")
+	}
+
+	outStr := strings.TrimSuffix(string(out), "\n")
+
+	switch outStr {
+	case string(dbmodels.ACLActionAllow):
+		return string(dbmodels.ACLActionAllow), nil
+	case string(dbmodels.ACLActionDeny):
+		return string(dbmodels.ACLActionDeny), nil
+	default:
+		return action, fmt.Errorf("acl-check-cmd wrong output '%s'", outStr)
 	}
-	return action
 }