Revert "Add webhook secret bootstrap and verification for Gitlab (#4492)" (#4504)

This reverts commit 052daf1.

Co-authored-by: Juan Antonio Osorio <[email protected]>

Author: jhrozek

cmd/dev/app/rule_type/rttst.go

@@ -438,7 +438,7 @@ func getProvider(pstr string, token string, providerConfigFile string) (provifv1
 		}
 
 		// We may pass a "fake" webhook URL here as it is not used in the test
-		client, err := gitlab.New(credentials.NewGitLabTokenCredential(token), cfg, "fake", "fake")
+		client, err := gitlab.New(credentials.NewGitLabTokenCredential(token), cfg, "fake")
 		if err != nil {
 			return nil, fmt.Errorf("error instantiating gitlab provider: %w", err)
 		}

internal/config/server/gitlab.go

@@ -19,12 +19,6 @@ package server
 type GitLabConfig struct {
 	OAuthClientConfig `mapstructure:",squash"`
 
-	// WebhookSecrets is the configuration for the GitLab webhook secrets
-	// setup and verification. This is used to verify incoming webhook requests
-	// from GitLab, as well as to generate the webhook URL for GitLab to send
-	// events to.
-	WebhookSecrets `mapstructure:",squash"`
-
 	// Scopes is the list of scopes to request from the GitLab OAuth provider
 	Scopes []string `mapstructure:"scopes"`
 }

internal/config/server/webhook.go

@@ -23,20 +23,10 @@ import (
 
 // WebhookConfig is the configuration for our webhook capabilities
 type WebhookConfig struct {
-	// WebhookSecrets is the configuration for the webhook secrets.
-	// This is embedded in the WebhookConfig so that the secrets can be
-	// used in the WebhookConfig, as the GitHub provider needs for now.
-	WebhookSecrets `mapstructure:",squash"`
 	// ExternalWebhookURL is the URL that we will send our webhook to
 	ExternalWebhookURL string `mapstructure:"external_webhook_url"`
 	// ExternalPingURL is the URL that we will send our ping to
 	ExternalPingURL string `mapstructure:"external_ping_url"`
-}
-
-// WebhookSecrets is the configuration for the webhook secrets. this is useful
-// to import in whatever provider configuration that needs to use some webhook
-// secrets.
-type WebhookSecrets struct {
 	// WebhookSecret is the secret that we will use to sign our webhook
 	WebhookSecret string `mapstructure:"webhook_secret"`
 	// WebhookSecretFile is the location of the file containing the webhook secret
@@ -49,7 +39,7 @@ type WebhookSecrets struct {
 
 // GetPreviousWebhookSecrets retrieves the previous webhook secrets from a file specified in the WebhookConfig.
 // It reads the contents of the file, splits the data by whitespace, and returns it as a slice of strings.
-func (wc *WebhookSecrets) GetPreviousWebhookSecrets() ([]string, error) {
+func (wc *WebhookConfig) GetPreviousWebhookSecrets() ([]string, error) {
 	data, err := os.ReadFile(wc.PreviousWebhookSecretFile)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read previous webhook secrets from file: %w", err)
@@ -61,6 +51,6 @@ func (wc *WebhookSecrets) GetPreviousWebhookSecrets() ([]string, error) {
 }
 
 // GetWebhookSecret returns the GitHub App's webhook secret
-func (wc *WebhookSecrets) GetWebhookSecret() (string, error) {
+func (wc *WebhookConfig) GetWebhookSecret() (string, error) {
 	return fileOrArg(wc.WebhookSecretFile, wc.WebhookSecret, "webhook secret")
 }

internal/controlplane/fuzz_test.go

@@ -118,11 +118,7 @@ func FuzzGitHubEventParsers(f *testing.F) {
 		}
 		defer os.Remove(whSecretFile.Name())
 
-		whConfig := &server.WebhookConfig{
-			WebhookSecrets: server.WebhookSecrets{
-				WebhookSecretFile: whSecretFile.Name(),
-			},
-		}
+		whConfig := &server.WebhookConfig{WebhookSecretFile: whSecretFile.Name()}
 
 		s := &Server{}
 		ctx := context.Background()

internal/providers/gitlab/gitlab.go

@@ -57,20 +57,11 @@ type gitlabClient struct {
 	glcfg      *minderv1.GitLabProviderConfig
 	webhookURL string
 	gitConfig  config.GitConfig
-
-	// secret for the webhook. This is stored in the
-	// structure to allow efficient fetching.
-	currentWebhookSecret string
 }
 
 // New creates a new GitLab provider
 // Note that the webhook URL should already contain the provider class in the path
-func New(
-	cred provifv1.GitLabCredential,
-	cfg *minderv1.GitLabProviderConfig,
-	webhookURL string,
-	currentWebhookSecret string,
-) (*gitlabClient, error) {
+func New(cred provifv1.GitLabCredential, cfg *minderv1.GitLabProviderConfig, webhookURL string) (*gitlabClient, error) {
 	// TODO: We need a context here.
 	cli := oauth2.NewClient(context.Background(), cred.GetAsOAuth2TokenSource())
 
@@ -83,11 +74,10 @@ func New(
 	}
 
 	return &gitlabClient{
-		cred:                 cred,
-		cli:                  cli,
-		glcfg:                cfg,
-		webhookURL:           webhookURL,
-		currentWebhookSecret: currentWebhookSecret,
+		cred:       cred,
+		cli:        cli,
+		glcfg:      cfg,
+		webhookURL: webhookURL,
 		// TODO: Add git config
 	}, nil
 }

internal/providers/gitlab/manager/manager.go

@@ -24,8 +24,6 @@ import (
 	"net/url"
 	"slices"
 
-	"github.com/rs/zerolog"
-
 	"github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/crypto"
 	"github.com/stacklok/minder/internal/db"
@@ -41,12 +39,6 @@ type providerClassManager struct {
 	glpcfg        *server.GitLabConfig
 	webhookURL    string
 	parentContext context.Context
-
-	// secrets for the webhook. These are stored in the
-	// structure to allow efficient fetching. Rotation
-	// requires a process restart.
-	currentWebhookSecret   string
-	previousWebhookSecrets []string
 }
 
 // NewGitLabProviderClassManager creates a new provider class manager for the dockerhub provider
@@ -58,33 +50,17 @@ func NewGitLabProviderClassManager(
 		return nil, errors.New("webhook URL is required")
 	}
 
-	if cfg == nil {
-		return nil, errors.New("gitlab config is required")
-	}
-
 	webhookURL, err := url.JoinPath(webhookURLBase, url.PathEscape(string(db.ProviderClassGitlab)))
 	if err != nil {
 		return nil, fmt.Errorf("error joining webhook URL: %w", err)
 	}
 
-	whSecret, err := cfg.GetWebhookSecret()
-	if err != nil {
-		return nil, fmt.Errorf("error getting webhook secret: %w", err)
-	}
-
-	previousSecrets, err := cfg.GetPreviousWebhookSecrets()
-	if err != nil {
-		zerolog.Ctx(ctx).Error().Err(err).Msg("previous secrets not loaded")
-	}
-
 	return &providerClassManager{
-		store:                  store,
-		crypteng:               crypteng,
-		glpcfg:                 cfg,
-		webhookURL:             webhookURL,
-		parentContext:          ctx,
-		currentWebhookSecret:   whSecret,
-		previousWebhookSecrets: previousSecrets,
+		store:         store,
+		crypteng:      crypteng,
+		glpcfg:        cfg,
+		webhookURL:    webhookURL,
+		parentContext: ctx,
 	}, nil
 }
 
@@ -115,7 +91,7 @@ func (g *providerClassManager) Build(ctx context.Context, config *db.Provider) (
 		return nil, fmt.Errorf("error parsing gitlab config: %w", err)
 	}
 
-	cli, err := gitlab.New(creds, cfg, g.webhookURL, g.currentWebhookSecret)
+	cli, err := gitlab.New(creds, cfg, g.webhookURL)
 	if err != nil {
 		return nil, fmt.Errorf("error creating gitlab client: %w", err)
 	}

internal/providers/gitlab/manager/webhook.go

@@ -15,22 +15,16 @@
 package manager
 
 import (
-	"errors"
-	"fmt"
 	"net/http"
-	"strings"
 
-	"github.com/google/uuid"
 	"github.com/rs/zerolog"
-
-	"github.com/stacklok/minder/internal/providers/gitlab/webhooksecret"
 )
 
 // GetWebhookHandler implements the ProviderManager interface
 // Note that this is where the whole webhook handler is defined and
 // will live.
 func (m *providerClassManager) GetWebhookHandler() http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
 		l := zerolog.Ctx(m.parentContext).With().
 			Str("webhook", "gitlab").
 			Str("method", r.Method).
@@ -40,57 +34,8 @@ func (m *providerClassManager) GetWebhookHandler() http.Handler {
 			Str("content-type", r.Header.Get("Content-Type")).
 			Logger()
 
-		// Validate the webhook secret
-		if err := m.validateRequest(r); err != nil {
-			l.Error().Err(err).Msg("invalid webhook request")
-			http.Error(w, "invalid webhook request", http.StatusUnauthorized)
-			return
-		}
+		// TODO: Implement webhook handler
 
 		l.Debug().Msg("received webhook")
 	})
 }
-
-func (m *providerClassManager) validateRequest(r *http.Request) error {
-	// Validate the webhook secret
-	gltok := r.Header.Get("X-Gitlab-Token")
-	if gltok == "" {
-		return errors.New("missing X-Gitlab-Token header")
-	}
-
-	if err := m.validateToken(gltok, r); err != nil {
-		return fmt.Errorf("invalid X-Gitlab-Token header: %w", err)
-	}
-
-	return nil
-}
-
-// validateToken validates the incoming GitLab webhook token
-// Validation takes the secret from the GitLab webhook configuration
-// appens the last element of the path to the URL (which is unique per entity)
-func (m *providerClassManager) validateToken(token string, req *http.Request) error {
-	// Extract the unique ID from the URL path
-	path := req.URL.Path
-	uniq := path[strings.LastIndex(path, "/")+1:]
-
-	// uniq must be a valid UUID
-	_, err := uuid.Parse(uniq)
-	if err != nil {
-		return errors.New("invalid unique ID")
-	}
-
-	// Generate the expected secret
-	if valid := webhooksecret.Verify(m.currentWebhookSecret, uniq, token); valid {
-		// If the secret is valid, we can return
-		return nil
-	}
-
-	// Check the previous secrets
-	for _, prev := range m.previousWebhookSecrets {
-		if valid := webhooksecret.Verify(prev, uniq, token); valid {
-			return nil
-		}
-	}
-
-	return errors.New("invalid webhook token")
-}

internal/providers/gitlab/registration.go

@@ -29,7 +29,6 @@ import (
 	"github.com/xanzy/go-gitlab"
 
 	"github.com/stacklok/minder/internal/entities/properties"
-	"github.com/stacklok/minder/internal/providers/gitlab/webhooksecret"
 	"github.com/stacklok/minder/internal/util/ptr"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
@@ -58,11 +57,8 @@ func (c *gitlabClient) RegisterEntity(
 
 	whprops, err := c.createWebhook(ctx, upstreamID)
 	if err != nil {
-		zerolog.Ctx(ctx).Error().
-			Str("upstreamID", upstreamID).
-			Str("provider-class", Class).
-			Err(err).Msg("failed to create webhook")
-		return nil, errors.New("failed to create webhook")
+		// There is already enough context in the error message
+		return nil, err
 	}
 
 	return props.Merge(whprops), nil
@@ -106,20 +102,15 @@ func (c *gitlabClient) createWebhook(ctx context.Context, upstreamID string) (*p
 		return nil, fmt.Errorf("failed to join URL path for webhook: %w", err)
 	}
 
-	sec, err := webhooksecret.New(c.currentWebhookSecret, hookUUID.String())
-	if err != nil {
-		return nil, fmt.Errorf("failed to create webhook secret: %w", err)
-	}
-
 	trve := ptr.Ptr(true)
 	hreq := &gitlab.AddProjectHookOptions{
-		URL:                   &webhookUniqueURL,
-		Token:                 &sec,
-		PushEvents:            trve,
-		TagPushEvents:         trve,
-		MergeRequestsEvents:   trve,
-		ReleasesEvents:        trve,
-		EnableSSLVerification: trve,
+		URL: &webhookUniqueURL,
+		// TODO: Add secret
+		PushEvents:          trve,
+		TagPushEvents:       trve,
+		MergeRequestsEvents: trve,
+		ReleasesEvents:      trve,
+		// TODO: Enable SSL verification
 	}
 
 	hook, err := c.doCreateWebhook(ctx, createHookPath, hreq)