Controller can be configured to allow a predictable URL for tokens (#2371)

Fixes #2182

Author: fgiorgetti

internal/kube/grants/config.go

@@ -15,6 +15,7 @@ type GrantConfig struct {
 	Port                 int
 	TlsCredentialsSecret string
 	Hostname             string
+	RedeemByKey          bool
 }
 
 func BoundGrantConfig(flags *flag.FlagSet) (*GrantConfig, error) {
@@ -32,6 +33,9 @@ func BoundGrantConfig(flags *flag.FlagSet) (*GrantConfig, error) {
 	}
 	iflag.StringVar(flags, &c.TlsCredentialsSecret, "grant-server-tls-credentials", "SKUPPER_GRANT_SERVER_TLS_CREDENTIALS", "skupper-grant-server", "The name of a secret in which TLS credentials for the AccessGrant server are found.")
 	iflag.StringVar(flags, &c.Hostname, "grant-server-podname", "HOSTNAME", "", "The name of the pod in which the AccessGrant server is running (defaults to $HOSTNAME).")
+	if err := iflag.BoolVar(flags, &c.RedeemByKey, "allow-redeem-by-key", "SKUPPER_ALLOW_REDEEM_BY_KEY", false, "Allow AccessGrant redemption using a predictable key."); err != nil {
+		errors = append(errors, err.Error())
+	}
 	if len(errors) > 0 {
 		return c, fmt.Errorf("Invalid environment variable(s): %s", strings.Join(errors, ", "))
 	}

internal/kube/grants/enabled.go

@@ -29,6 +29,9 @@ func enabled(controller *watchers.EventProcessor, currentNamespace string, watch
 		}
 		gc.autoConfigure = ac
 	}
+	if config.RedeemByKey {
+		gc.grants.keyRedeem = true
+	}
 	return gc
 }
 

internal/kube/grants/grant_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -336,7 +337,83 @@ func Test_ServeHttp(t *testing.T) {
 			assert.Equal(t, res.Code, tt.expectedCode)
 		})
 	}
+}
 
+func Test_ServeHttpUIDAndKey(t *testing.T) {
+	skupperObjects := []runtime.Object{
+		&v2alpha1.AccessGrant{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "good",
+				Namespace: "test",
+				UID:       "0bde3bc8-a4a2-404a-bfbe-44fdf7bf3231",
+			},
+			Spec: v2alpha1.AccessGrantSpec{
+				RedemptionsAllowed: 3,
+				Code:               "supersecret",
+			},
+			Status: v2alpha1.AccessGrantStatus{
+				Code:           "supersecret",
+				ExpirationTime: time.Date(2124, time.January, 0, 0, 0, 0, 0, time.UTC).Format(time.RFC3339),
+			},
+		},
+		&v2alpha1.AccessGrant{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "not-bad",
+				Namespace: "test",
+				UID:       "0bde3bc8-a4a2-404a-bfbe-44fdf7bf3232",
+			},
+			Spec: v2alpha1.AccessGrantSpec{
+				RedemptionsAllowed: 3,
+				Code:               "supersecret",
+			},
+			Status: v2alpha1.AccessGrantStatus{
+				Code:           "supersecret",
+				ExpirationTime: time.Date(2124, time.January, 0, 0, 0, 0, 0, time.UTC).Format(time.RFC3339),
+			},
+		},
+	}
+	scenarioKeys := []struct {
+		valid          []string
+		invalid        []string
+		allowKeyRedeem bool
+	}{
+		{
+			valid:          []string{"0bde3bc8-a4a2-404a-bfbe-44fdf7bf3231", "0bde3bc8-a4a2-404a-bfbe-44fdf7bf3232"},
+			invalid:        []string{"test/good", "test/not-bad", "test/invalid", "really-invalid"},
+			allowKeyRedeem: false,
+		},
+		{
+			valid:          []string{"0bde3bc8-a4a2-404a-bfbe-44fdf7bf3231", "0bde3bc8-a4a2-404a-bfbe-44fdf7bf3232", "test/good", "test/not-bad"},
+			invalid:        []string{"test/invalid", "really-invalid"},
+			allowKeyRedeem: true,
+		},
+	}
+	client, err := fake.NewFakeClient("test", nil, skupperObjects, "")
+	if err != nil {
+		t.Error(err)
+	}
+	registry := newGrants(client, dummyGenerator, "https", "")
+	for _, obj := range skupperObjects {
+		grant := obj.(*v2alpha1.AccessGrant)
+		assert.Assert(t, registry.checkGrant(grant.Namespace+"/"+grant.Name, grant))
+	}
+	for _, scenario := range scenarioKeys {
+		registry.keyRedeem = scenario.allowKeyRedeem
+		for _, key := range scenario.valid {
+			t.Run("redeem_valid_key_"+key, func(t *testing.T) {})
+			req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/%s", key), bytes.NewBufferString("supersecret"))
+			res := httptest.NewRecorder()
+			registry.ServeHTTP(res, req)
+			assert.Equal(t, res.Code, http.StatusOK)
+		}
+		for _, key := range scenario.invalid {
+			t.Run("redeem_invalid_key_"+key, func(t *testing.T) {})
+			req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/%s", key), bytes.NewBufferString("supersecret"))
+			res := httptest.NewRecorder()
+			registry.ServeHTTP(res, req)
+			assert.Equal(t, res.Code, http.StatusNotFound)
+		}
+	}
 }
 
 type CheckGrantTestInvocation struct {

internal/kube/grants/grants.go

@@ -28,6 +28,7 @@ type Grants struct {
 	scheme     string
 	grants     map[kubetypes.UID]*skupperv2alpha1.AccessGrant
 	grantIndex map[string]kubetypes.UID
+	keyRedeem  bool
 	lock       sync.Mutex
 	logger     *slog.Logger
 }
@@ -90,6 +91,11 @@ func (g *Grants) get(key string) *skupperv2alpha1.AccessGrant {
 	g.lock.Lock()
 	defer g.lock.Unlock()
 
+	if g.keyRedeem && strings.Contains(key, "/") {
+		if uid, ok := g.grantIndex[key]; ok {
+			key = string(uid)
+		}
+	}
 	if grant, ok := g.grants[kubetypes.UID(key)]; ok {
 		return grant
 	}
@@ -302,7 +308,12 @@ func (g *Grants) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Only POST is supported", http.StatusMethodNotAllowed)
 		return
 	}
-	key := strings.Join(strings.Split(r.URL.Path, "/"), "")
+	var key string
+	if namespaceName, ok := g.keyFromUrl(r.URL.Path); ok {
+		key = namespaceName
+	} else {
+		key = strings.Join(strings.Split(r.URL.Path, "/"), "")
+	}
 	body, err := io.ReadAll(r.Body)
 	if err != nil {
 		g.logger.Error("Error reading body for path", slog.String("path", r.URL.Path), slog.Any("error", err))
@@ -338,6 +349,13 @@ func (g *Grants) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	g.logger.Info("Redemption of access token succeeded", slog.String("namespace", grant.Namespace), slog.String("name", grant.Name))
 }
 
+func (g *Grants) keyFromUrl(url string) (string, bool) {
+	if g.keyRedeem && len(url) > 0 && strings.Contains(url[1:], "/") {
+		return url[1:], true
+	}
+	return "", false
+}
+
 type HttpError struct {
 	text string
 	code int