From ba72710e2d11295287a9816a6d3dbd2b544ff075 Mon Sep 17 00:00:00 2001
From: Herman Slatman <hermanslatman@hotmail.com>
Date: Fri, 22 Sep 2023 12:40:14 +0200
Subject: [PATCH] Address code review remarks

---
 authority/admin/api/webhook.go |  4 +---
 authority/provisioner/scep.go  | 16 ++++------------
 scep/api/api.go                |  5 +----
 scep/context.go                |  1 -
 webhook/types.go               |  2 +-
 5 files changed, 7 insertions(+), 21 deletions(-)
 delete mode 100644 scep/context.go

diff --git a/authority/admin/api/webhook.go b/authority/admin/api/webhook.go
index 574cbf18..c83c85e7 100644
--- a/authority/admin/api/webhook.go
+++ b/authority/admin/api/webhook.go
@@ -56,9 +56,7 @@ func validateWebhook(webhook *linkedca.Webhook) error {
 	}
 
 	// kind
-	switch webhook.Kind {
-	case linkedca.Webhook_ENRICHING, linkedca.Webhook_AUTHORIZING, linkedca.Webhook_SCEPCHALLENGE, linkedca.Webhook_NOTIFYING:
-	default:
+	if _, ok := linkedca.Webhook_Kind_name[int32(webhook.Kind)]; !ok || webhook.Kind == linkedca.Webhook_NO_KIND {
 		return admin.NewError(admin.ErrorBadRequestType, "webhook kind %q is invalid", webhook.Kind)
 	}
 
diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go
index 0c514275..0c106927 100644
--- a/authority/provisioner/scep.go
+++ b/authority/provisioner/scep.go
@@ -197,12 +197,8 @@ func (c *notificationController) Success(ctx context.Context, csr *x509.Certific
 		}
 		req.X509Certificate.Raw = cert.Raw // adding the full certificate DER bytes
 		req.SCEPTransactionID = transactionID
-		resp, err := wh.DoWithContext(ctx, c.client, req, nil)
-		if err != nil {
-			return fmt.Errorf("failed executing webhook request: %w", err)
-		}
-		if !resp.Allow { // TODO(hs): different response for notifying?
-			return ErrSCEPNotificationFailed // return early
+		if _, err = wh.DoWithContext(ctx, c.client, req, nil); err != nil {
+			return fmt.Errorf("failed executing webhook request: %w: %w", ErrSCEPNotificationFailed, err)
 		}
 	}
 
@@ -218,12 +214,8 @@ func (c *notificationController) Failure(ctx context.Context, csr *x509.Certific
 		req.SCEPTransactionID = transactionID
 		req.SCEPErrorCode = errorCode
 		req.SCEPErrorDescription = errorDescription
-		resp, err := wh.DoWithContext(ctx, c.client, req, nil)
-		if err != nil {
-			return fmt.Errorf("failed executing webhook request: %w", err)
-		}
-		if !resp.Allow { // TODO(hs): different response for notifying?
-			return ErrSCEPNotificationFailed // return early
+		if _, err = wh.DoWithContext(ctx, c.client, req, nil); err != nil {
+			return fmt.Errorf("failed executing webhook request: %w: %w", ErrSCEPNotificationFailed, err)
 		}
 	}
 
diff --git a/scep/api/api.go b/scep/api/api.go
index a321f59f..b8ec5bea 100644
--- a/scep/api/api.go
+++ b/scep/api/api.go
@@ -333,10 +333,7 @@ func PKIOperation(ctx context.Context, req request) (Response, error) {
 
 	certRep, err := auth.SignCSR(ctx, csr, msg)
 	if err != nil {
-		// default to ERROR_INTERNAL_ERROR: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/18d8fbe8-a967-4f1c-ae50-99ca8e491d2d
-		errorCode := 0x0000054F
-		errorDescription := err.Error()
-		if notifyErr := auth.NotifyFailure(ctx, csr, transactionID, errorCode, errorDescription); notifyErr != nil {
+		if notifyErr := auth.NotifyFailure(ctx, csr, transactionID, 0, err.Error()); notifyErr != nil {
 			// TODO(hs): ignore this error case? It's not critical if the notification fails; but logging it might be good
 			_ = notifyErr
 		}
diff --git a/scep/context.go b/scep/context.go
deleted file mode 100644
index ce73569b..00000000
--- a/scep/context.go
+++ /dev/null
@@ -1 +0,0 @@
-package scep
diff --git a/webhook/types.go b/webhook/types.go
index 4c443969..2d7832b8 100644
--- a/webhook/types.go
+++ b/webhook/types.go
@@ -80,7 +80,7 @@ type RequestBody struct {
 	X509Certificate        *X509Certificate        `json:"x509Certificate,omitempty"`
 	SSHCertificateRequest  *SSHCertificateRequest  `json:"sshCertificateRequest,omitempty"`
 	SSHCertificate         *SSHCertificate         `json:"sshCertificate,omitempty"`
-	// Only set for SCEP challenge validation requests
+	// Only set for SCEP webhook requests
 	SCEPChallenge        string `json:"scepChallenge,omitempty"`
 	SCEPTransactionID    string `json:"scepTransactionID,omitempty"`
 	SCEPErrorCode        int    `json:"scepErrorCode,omitempty"`