Use native EZproxy support to terminate sessions

BACKSTORY

Not long after giving a demo on using brick + fail2ban to timeout
existing sessions, I stumbled across another project on GitHub which
uses `ezproxy kill` to terminate an active user session:

- https://github.com/calvinm/ezproxy-abuse-checker/blob/d7202e617305745cf272df9918b1e95ff030f63f/block_user.pl#L32
- https://github.com/calvinm/ezproxy-abuse-checker/blob/master/block_user.pl

Despite OCLC Support indicating otherwise, native support for
terminating user sessions was clearly available. This built-in support
offers a much safer/localized effect vs banning the IP associated
with a reported account; banning the IP has the potential to have a
much larger splash radius blocking legitimate user accounts.

The primary developer for the `calvinm/ezproxy-abuse-checker` project
confirmed that they learned of this support on the EZProxy mailing
list: calvinm/ezproxy-abuse-checker#1

This is encouraging, as I am (at least a little) optimistic that this
unpublished support will persist in future versions, at least until
official, fully documented and supported functionality for terminating
sessions is included.

IMPLEMENTATION

This commit adds (optional) support for terminating user sessions
using the official EZProxy binary already present on EZproxy servers.
The fully-qualified location of this binary is configurable via
command-line flag, environment variable or TOML-format configuration
file. Additional settings are provided to tune the sessions search
process such as configurable retries and search delay.

The session termination process involves reading current sessions from
the Active Users and Hosts "state" file that EZproxy uses to track
sessions and hosts managed by EZProxy and then using the EZproxy
binary to terminate each session via the `kill` subcommand.

As part of the early development efforts I initially tried to use the
latest Audit logs to pull session IDs, but quickly realized that while
the format was easier to parse, it was far less stable due to log
rotation and the need to resolve the active state ourselves (logins
minus logouts, minus timeouts, etc). Some of that support still
remains as of this commit, but may be removed in a future release if
found to not be needed. As of this writing I believe it can still be
used as part of interacting with a future endpoint.

The aforementioned search delay and retry settings are provided to
work around an observed race condition between EZproxy recording state
changes and other applications (such as ours) attempting to read the
current state. The delay in EZproxy writing the changes to disk (or
kernel settings?) may result in our application attempting to
terminate sessions related to a monitoring system report and not
finding them within the Active Users and Hosts "state" file.

The defaults attempt to strike a balance between waiting a little
longer in order to "find" and terminate those sessions vs moving on
with current findings. The defaults may need to be adjusted further
depending on the production environment.

TEAMS NOTIFICATIONS

Minor changes in an effort to better clarify the purpose of the lead-in
content:

- explicit `step X of Y` labeling to notification titles
- consistent use of Note (preferred) and Error (fallback) field values
  to generate primary "summary" text
- rename "Request Annotations" to "Request Errors" to reflect
  dedicated single purpose vs blend of Note and Error field values as
  before

ADDITIONAL SUPPORT

During development, the necessary code to interact with EZProxy was
first created as a local package, but was eventually moved to a
separate module in order to break out the changes for separate use.

See the `atc0005/go-ezproxy` project for further information on that
module.

Two separate binaries were created during testing:

- mock `ezproxy` binary which returns known return codes and results +
  some extra to help condition validation checks for what I believe
  will be unknown/unpublished return codes I've yet to encounter

- `es` binary used to search for and optionally terminate active user
  sessions for a specified username. This binary supports the same
  search retry and delay settings as `brick`

refs GH-31, GH-59

.github/workflows/lint-and-build-code.yml

@@ -92,7 +92,10 @@ jobs:
       - name: Build with (mostly) default options
         # Note: We use the `-mod=vendor` flag to explicitly request that our
         # top-level vendor folder be used instead of fetching remote packages
-        run: go build -v -mod=vendor ./cmd/brick
+        run: |
+          go build -v -mod=vendor ./cmd/brick
+          go build -v -mod=vendor ./cmd/es
+          go build -v -mod=vendor ./cmd/ezproxy
 
       - name: Build using project Makefile
         run: make all

.gitignore

@@ -19,6 +19,9 @@
 
 # Linux binaries
 /brick
+/es
+/ezproxy
+/ezproxy-mock
 
 # Local Visual Studio Code editor settings (e.g., ignored words for Spelling extension)
 /.vscode

Makefile

@@ -32,7 +32,7 @@
 SHELL = /bin/bash
 
 # Space-separated list of cmd/BINARY_NAME directories to build
-WHAT 					= brick
+WHAT 					= brick es ezproxy
 
 # TODO: This will need to be standardized across all cmd files in order to
 # work as intended.
@@ -53,7 +53,7 @@ GOLANGCI_LINT_VERSION		= v1.27.0
 
 # The default `go build` process embeds debugging information. Building
 # without that debugging information reduces the binary size by around 28%.
-BUILDCMD				=	go build -mod=vendor -a -ldflags="-s -w -X $(VERSION_VAR_PKG).version=$(VERSION)"
+BUILDCMD				=	go build -mod=vendor -a -ldflags="-s -w -X $(VERSION_VAR_PKG).Version=$(VERSION)"
 GOCLEANCMD				=	go clean -mod=vendor ./...
 GITCLEANCMD				= 	git clean -xfd
 CHECKSUMCMD				=	sha256sum -b
@@ -149,11 +149,11 @@ windows:
 
 	@for target in $(WHAT); do \
 		mkdir -p $(OUTPUTDIR)/$$target && \
-		echo "Building 386 binaries" && \
+		echo "Building $$target 386 binary" && \
 		env GOOS=windows GOARCH=386 $(BUILDCMD) -o $(OUTPUTDIR)/$$target/$$target-$(VERSION)-windows-386.exe ${PWD}/cmd/$$target && \
-		echo "Building amd64 binaries" && \
+		echo "Building $$target amd64 binary" && \
 		env GOOS=windows GOARCH=amd64 $(BUILDCMD) -o $(OUTPUTDIR)/$$target/$$target-$(VERSION)-windows-amd64.exe ${PWD}/cmd/$$target && \
-		echo "Generating checksum files" && \
+		echo "Generating $$target checksum files" && \
 		$(CHECKSUMCMD) $(OUTPUTDIR)/$$target/$$target-$(VERSION)-windows-386.exe > $(OUTPUTDIR)/$$target/$$target-$(VERSION)-windows-386.exe.sha256 && \
 		$(CHECKSUMCMD) $(OUTPUTDIR)/$$target/$$target-$(VERSION)-windows-amd64.exe > $(OUTPUTDIR)/$$target/$$target-$(VERSION)-windows-amd64.exe.sha256; \
 	done
@@ -167,11 +167,11 @@ linux:
 
 	@for target in $(WHAT); do \
 		mkdir -p $(OUTPUTDIR)/$$target && \
-		echo "Building 386 binaries" && \
+		echo "Building $$target 386 binary" && \
 		env GOOS=linux GOARCH=386 $(BUILDCMD) -o $(OUTPUTDIR)/$$target/$$target-$(VERSION)-linux-386 ${PWD}/cmd/$$target && \
-		echo "Building amd64 binaries" && \
+		echo "Building $$target amd64 binary" && \
 		env GOOS=linux GOARCH=amd64 $(BUILDCMD) -o $(OUTPUTDIR)/$$target/$$target-$(VERSION)-linux-amd64 ${PWD}/cmd/$$target && \
-		echo "Generating checksum files" && \
+		echo "Generating $$target checksum files" && \
 		$(CHECKSUMCMD) $(OUTPUTDIR)/$$target/$$target-$(VERSION)-linux-386 > $(OUTPUTDIR)/$$target/$$target-$(VERSION)-linux-386.sha256 && \
 		$(CHECKSUMCMD) $(OUTPUTDIR)/$$target/$$target-$(VERSION)-linux-amd64 > $(OUTPUTDIR)/$$target/$$target-$(VERSION)-linux-amd64.sha256; \
 	done

README.md

@@ -13,9 +13,9 @@ Automatically disable EZproxy user accounts via incoming webhook requests.
 <!-- omit in toc -->
 ## Table of contents
 
+- [Project home](#project-home)
 - [Status](#status)
 - [Overview](#overview)
-- [Project home](#project-home)
 - [Features](#features)
   - [Current](#current)
   - [Missing](#missing)
@@ -28,6 +28,12 @@ Automatically disable EZproxy user accounts via incoming webhook requests.
 - [License](#license)
 - [References](#references)
 
+## Project home
+
+See [our GitHub repo](https://github.com/atc0005/brick) for the latest code,
+to file an issue or submit improvements for review and potential inclusion
+into the project.
+
 ## Status
 
 Alpha quality; most
@@ -63,12 +69,6 @@ See also:
 - [High-level overview](docs/start-here.md)
 - our other [documentation](#documentation) for further instructions
 
-## Project home
-
-See [our GitHub repo](https://github.com/atc0005/brick) for the latest code,
-to file an issue or submit improvements for review and potential inclusion
-into the project.
-
 ## Features
 
 ### Current

cmd/brick/handlers.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"time"
@@ -90,6 +91,11 @@ func disableUserHandler(
 	disabledUsers *files.DisabledUsers,
 	ignoredSources files.IgnoredSources,
 	notifyWorkQueue chan<- events.Record,
+	terminateSessions bool,
+	ezproxyActiveFilePath string,
+	ezproxySessionsSearchDelay int,
+	ezproxySessionSearchRetries int,
+	ezproxyExecutable string,
 ) http.HandlerFunc {
 
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -121,9 +127,9 @@ func disableUserHandler(
 		// have the option of displaying it in a raw format (e.g.,
 		// troubleshooting), replace the Body with a new io.ReadCloser to
 		// allow later access to r.Body for JSON-decoding purposes
-		requestBody, err := ioutil.ReadAll(r.Body)
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+		requestBody, requestBodyReadErr := ioutil.ReadAll(r.Body)
+		if requestBodyReadErr != nil {
+			http.Error(w, requestBodyReadErr.Error(), http.StatusBadRequest)
 			return
 		}
 
@@ -135,8 +141,7 @@ func disableUserHandler(
 		// error, respond to the client with the error message and appropriate
 		// status code.
 		var payloadV2 events.SplunkAlertPayloadV2
-		err = json.NewDecoder(r.Body).Decode(&payloadV2)
-		if err != nil {
+		if err := json.NewDecoder(r.Body).Decode(&payloadV2); err != nil {
 			log.Errorf("Error decoding r.Body into payloadV2:\n%v\n\n", err)
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
@@ -154,8 +159,25 @@ func disableUserHandler(
 
 		}
 
-		// inform sender that the payload was received
-		fmt.Fprintln(w, "OK: Payload received")
+		// Explicitly confirm that the payload was received so that the sender
+		// can go ahead and disconnect. This prevents holding up the sender
+		// while this application performs further (unrelated from the
+		// sender's perspective) processing.
+		//
+		// FIXME: Is having a newline here best practice, or no?
+		if _, err := io.WriteString(w, "OK: Payload received\n"); err != nil {
+			log.Error("disableUserHandler: Failed to send OK status response to payload sender")
+		}
+
+		// Manually flush http.ResponseWriter in an additional effort to
+		// prevent undue wait time for payload sender
+		if f, ok := w.(http.Flusher); ok {
+			log.Debug("disableUserHandler: Manually flushing http.ResponseWriter")
+			f.Flush()
+		} else {
+			log.Warn("disableUserHandler: http.Flusher interface not available, cannot flush http.ResponseWriter")
+			log.Warn("disableUserHandler: Not flushing http.ResponseWriter may cause a noticeable delay between requests")
+		}
 
 		// if we made it this far, the payload checks out and we should be
 		// able to safely retrieve values that we need. We will also append
@@ -174,50 +196,27 @@ func disableUserHandler(
 			Headers:         r.Header,
 		}
 
-		log.Debugf(
-			"disableUserHandler: Received disable request from %q for user %q from IP %q.",
-			alert.PayloadSenderIP,
-			alert.Username,
-			alert.UserIP,
-		)
-
-		// Send to Notification Manager for processing
-		record := events.Record{
-			Alert: alert,
-			Note:  "disable request received",
-		}
-		go func() { notifyWorkQueue <- record }()
-
-		if err := files.ProcessEvent(
+		// All return values from subfunction calls are dropped into the
+		// notifyWorkQueue channel; nothing is returned here for further
+		// processing.
+		//
+		// NOTE: Because this is executed in a goroutine, the client (e.g.,
+		// monitoring system) gets a near-immediate response back and the
+		// connection is closed. There are probably other/better ways to
+		// achieve that specific result without using a goroutine, but the
+		// effect is worth noting for further exploration later.
+		go files.ProcessDisableEvent(
 			alert,
 			disabledUsers,
 			reportedUserEventsLog,
 			ignoredSources,
 			notifyWorkQueue,
-		); err != nil {
-
-			// TODO: Ensure that any errors bubble-up to *something* that will
-			// get the errors in front of us. A later step in this app's
-			// design is to expose errors via an endpoint so that monitoring
-			// systems like Nagios can poll for unresolved errors. Perhaps
-			// CRITICAL for failed writes, WARNING for writes pending a
-			// delayed retry (based on the assumption that a later re-check in
-			// the monitoring system will find the pending write no longer
-			// present, signaling an OK state).
-			log.Error(err.Error())
-
-			// FIXME: This is likely a duplicate of something handled further
-			// down?
-			go func() {
-				notifyWorkQueue <- events.Record{
-					Alert: alert,
-					Error: err,
-					Note:  "ProcessEvent() failure",
-				}
-			}()
-
-			return
-		}
+			terminateSessions,
+			ezproxyActiveFilePath,
+			ezproxySessionsSearchDelay,
+			ezproxySessionSearchRetries,
+			ezproxyExecutable,
+		)
 
 	}
 }

cmd/brick/main.go

@@ -105,9 +105,8 @@ func main() {
 		syscall.SIGTERM, // full restart
 	)
 
-	// Where events.EventRecord values will be sent for processing. We
-	// use a buffered channel in an effort to reduce the delay for client
-	// requests.
+	// Where events will be sent for processing. We use a buffered channel in
+	// an effort to reduce the delay for client requests.
 	notifyWorkQueue := make(chan events.Record, config.NotifyMgrQueueDepth)
 
 	// Create "notifications manager" function as persistent goroutine to
@@ -155,6 +154,11 @@ func main() {
 			disabledUsers,
 			ignoredSources,
 			notifyWorkQueue,
+			appConfig.EZproxyTerminateSessions(),
+			appConfig.EZproxyActiveFilePath(),
+			appConfig.EZproxySearchDelay(),
+			appConfig.EZproxySearchRetries(),
+			appConfig.EZproxyExecutablePath(),
 		),
 	)