coder · stirby · Sep 5, 2024 · Aug 26, 2024 · Sep 3, 2024 · Sep 3, 2024
diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
@@ -46,7 +46,7 @@ func init() {
 		valid := NameValid(str)
 		return valid == nil
 	}
-	for _, tag := range []string{"username", "organization_name", "template_name", "group_name", "workspace_name", "oauth2_app_name"} {
+	for _, tag := range []string{"username", "organization_name", "template_name", "workspace_name", "oauth2_app_name"} {
 		err := Validate.RegisterValidation(tag, nameValidator)
 		if err != nil {
 			panic(err)
@@ -96,6 +96,20 @@ func init() {
 	if err != nil {
 		panic(err)
 	}
+
+	groupNameValidator := func(fl validator.FieldLevel) bool {
+		f := fl.Field().Interface()
+		str, ok := f.(string)
+		if !ok {
+			return false
+		}
+		valid := GroupNameValid(str)
+		return valid == nil
+	}
+	err = Validate.RegisterValidation("group_name", groupNameValidator)
+	if err != nil {
+		panic(err)
+	}
 }
 
 // Is404Error returns true if the given error should return a 404 status code.

diff --git a/coderd/httpapi/name.go b/coderd/httpapi/name.go
@@ -96,6 +96,23 @@ func UserRealNameValid(str string) error {
 	return nil
 }
 
+// GroupNameValid returns whether the input string is a valid group name.
+func GroupNameValid(str string) error {
+	// 36 is to support using UUIDs as the group name.
+	if len(str) > 36 {
+		return xerrors.New("must be <= 36 characters")
+	}
+	// Avoid conflicts with routes like /groups/new and /groups/create.
+	if str == "new" || str == "create" {
+		return xerrors.Errorf("cannot use %q as a name", str)
+	}
+	matched := UsernameValidRegex.MatchString(str)
+	if !matched {
+		return xerrors.New("must be alphanumeric with hyphens")
+	}
+	return nil
+}
+
 // NormalizeUserRealName normalizes a user name such that it will pass
 // validation by UserRealNameValid. This is done to avoid blocking
 // little  Bobby  Whitespace  from using Coder.

diff --git a/coderd/workspaceagentsrpc.go b/coderd/workspaceagentsrpc.go
@@ -116,7 +116,19 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) {
 	}
 	defer mux.Close()
 
-	logger.Debug(ctx, "accepting agent RPC connection", slog.F("agent", workspaceAgent))
+	logger.Debug(ctx, "accepting agent RPC connection",
+		slog.F("agent_id", workspaceAgent.ID),
+		slog.F("agent_created_at", workspaceAgent.CreatedAt),
+		slog.F("agent_updated_at", workspaceAgent.UpdatedAt),
+		slog.F("agent_name", workspaceAgent.Name),
+		slog.F("agent_first_connected_at", workspaceAgent.FirstConnectedAt.Time),
+		slog.F("agent_last_connected_at", workspaceAgent.LastConnectedAt.Time),
+		slog.F("agent_disconnected_at", workspaceAgent.DisconnectedAt.Time),
+		slog.F("agent_version", workspaceAgent.Version),
+		slog.F("agent_last_connected_replica_id", workspaceAgent.LastConnectedReplicaID),
+		slog.F("agent_connection_timeout_seconds", workspaceAgent.ConnectionTimeoutSeconds),
+		slog.F("agent_api_version", workspaceAgent.APIVersion),
+		slog.F("agent_resource_id", workspaceAgent.ResourceID))
 
 	closeCtx, closeCtxCancel := context.WithCancel(ctx)
 	defer closeCtxCancel()

diff --git a/enterprise/coderd/coderdenttest/coderdenttest.go b/enterprise/coderd/coderdenttest/coderdenttest.go
@@ -174,6 +174,10 @@ type LicenseOptions struct {
 	// ExpiresAt is the time at which the license will hard expire.
 	// ExpiresAt should always be greater then GraceAt.
 	ExpiresAt time.Time
+	// NotBefore is the time at which the license becomes valid. If set to the
+	// zero value, the `nbf` claim on the license is set to 1 minute in the
+	// past.
+	NotBefore time.Time
 	Features  license.Features
 }
 
@@ -195,6 +199,13 @@ func (opts *LicenseOptions) Valid(now time.Time) *LicenseOptions {
 	return opts
 }
 
+func (opts *LicenseOptions) FutureTerm(now time.Time) *LicenseOptions {
+	opts.NotBefore = now.Add(time.Hour * 24)
+	opts.ExpiresAt = now.Add(time.Hour * 24 * 60)
+	opts.GraceAt = now.Add(time.Hour * 24 * 53)
+	return opts
+}
+
 func (opts *LicenseOptions) UserLimit(limit int64) *LicenseOptions {
 	return opts.Feature(codersdk.FeatureUserLimit, limit)
 }
@@ -233,13 +244,16 @@ func GenerateLicense(t *testing.T, options LicenseOptions) string {
 	if options.GraceAt.IsZero() {
 		options.GraceAt = time.Now().Add(time.Hour)
 	}
+	if options.NotBefore.IsZero() {
+		options.NotBefore = time.Now().Add(-time.Minute)
+	}
 
 	c := &license.Claims{
 		RegisteredClaims: jwt.RegisteredClaims{
 			ID:        uuid.NewString(),
 			Issuer:    "test@testing.test",
 			ExpiresAt: jwt.NewNumericDate(options.ExpiresAt),
-			NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Minute)),
+			NotBefore: jwt.NewNumericDate(options.NotBefore),
 			IssuedAt:  jwt.NewNumericDate(time.Now().Add(-time.Minute)),
 		},
 		LicenseExpires: jwt.NewNumericDate(options.GraceAt),

diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go
@@ -156,7 +156,7 @@ func TestPatchGroup(t *testing.T) {
 		const displayName = "foobar"
 		ctx := testutil.Context(t, testutil.WaitLong)
 		group, err := userAdminClient.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
-			Name:           "hi",
+			Name:           "ff7dcee2-e7c4-4bc4-a9e4-84870770e4c5", // GUID should fit.
 			AvatarURL:      "https://example.com",
 			QuotaAllowance: 10,
 			DisplayName:    "",
@@ -165,14 +165,14 @@ func TestPatchGroup(t *testing.T) {
 		require.Equal(t, 10, group.QuotaAllowance)
 
 		group, err = userAdminClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
-			Name:           "bye",
+			Name:           "ddd502d2-2984-4724-b5bf-1109a4d7462d", // GUID should fit.
 			AvatarURL:      ptr.Ref("https://google.com"),
 			QuotaAllowance: ptr.Ref(20),
 			DisplayName:    ptr.Ref(displayName),
 		})
 		require.NoError(t, err)
 		require.Equal(t, displayName, group.DisplayName)
-		require.Equal(t, "bye", group.Name)
+		require.Equal(t, "ddd502d2-2984-4724-b5bf-1109a4d7462d", group.Name)
 		require.Equal(t, "https://google.com", group.AvatarURL)
 		require.Equal(t, 20, group.QuotaAllowance)
 	})

diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go
@@ -100,6 +100,13 @@ func LicensesEntitlements(
 	//   'Entitlements' group as a whole.
 	for _, license := range licenses {
 		claims, err := ParseClaims(license.JWT, keys)
+		var vErr *jwt.ValidationError
+		if xerrors.As(err, &vErr) && vErr.Is(jwt.ErrTokenNotValidYet) {
+			// The license isn't valid yet.  We don't consider any entitlements contained in it, but
+			// it's also not an error.  Just skip it silently.  This can happen if an administrator
+			// uploads a license for a new term that hasn't started yet.
+			continue
+		}
 		if err != nil {
 			entitlements.Errors = append(entitlements.Errors,
 				fmt.Sprintf("Invalid license (%s) parsing claims: %s", license.UUID.String(), err.Error()))
@@ -287,6 +294,8 @@ var (
 	ErrInvalidVersion        = xerrors.New("license must be version 3")
 	ErrMissingKeyID          = xerrors.Errorf("JOSE header must contain %s", HeaderKeyID)
 	ErrMissingLicenseExpires = xerrors.New("license missing license_expires")
+	ErrMissingExp            = xerrors.New("exp claim missing or not parsable")
+	ErrMultipleIssues        = xerrors.New("license has multiple issues; contact support")
 )
 
 type Features map[codersdk.FeatureName]int64
@@ -336,7 +345,7 @@ func ParseRaw(l string, keys map[string]ed25519.PublicKey) (jwt.MapClaims, error
 	return nil, xerrors.New("unable to parse Claims")
 }
 
-// ParseClaims validates a database.License record, and if valid, returns the claims.  If
+// ParseClaims validates a raw JWT, and if valid, returns the claims.  If
 // unparsable or invalid, it returns an error
 func ParseClaims(rawJWT string, keys map[string]ed25519.PublicKey) (*Claims, error) {
 	tok, err := jwt.ParseWithClaims(
@@ -348,18 +357,53 @@ func ParseClaims(rawJWT string, keys map[string]ed25519.PublicKey) (*Claims, err
 	if err != nil {
 		return nil, err
 	}
-	if claims, ok := tok.Claims.(*Claims); ok && tok.Valid {
+	return validateClaims(tok)
+}
+
+func validateClaims(tok *jwt.Token) (*Claims, error) {
+	if claims, ok := tok.Claims.(*Claims); ok {
 		if claims.Version != uint64(CurrentVersion) {
 			return nil, ErrInvalidVersion
 		}
 		if claims.LicenseExpires == nil {
 			return nil, ErrMissingLicenseExpires
 		}
+		if claims.ExpiresAt == nil {
+			return nil, ErrMissingExp
+		}
 		return claims, nil
 	}
 	return nil, xerrors.New("unable to parse Claims")
 }
 
+// ParseClaimsIgnoreNbf validates a raw JWT, but ignores `nbf` claim. If otherwise valid, it returns
+// the claims.  If unparsable or invalid, it returns an error. Ignoring the `nbf` (not before) is
+// useful to determine if a JWT _will_ become valid at any point now or in the future.
+func ParseClaimsIgnoreNbf(rawJWT string, keys map[string]ed25519.PublicKey) (*Claims, error) {
+	tok, err := jwt.ParseWithClaims(
+		rawJWT,
+		&Claims{},
+		keyFunc(keys),
+		jwt.WithValidMethods(ValidMethods),
+	)
+	var vErr *jwt.ValidationError
+	if xerrors.As(err, &vErr) {
+		// zero out the NotValidYet error to check if there were other problems
+		vErr.Errors = vErr.Errors & (^jwt.ValidationErrorNotValidYet)
+		if vErr.Errors != 0 {
+			// There are other errors besides not being valid yet. We _could_ go
+			// through all the jwt.ValidationError bits and try to work out the
+			// correct error, but if we get here something very strange is
+			// going on so let's just return a generic error that says to get in
+			// touch with our support team.
+			return nil, ErrMultipleIssues
+		}
+	} else if err != nil {
+		return nil, err
+	}
+	return validateClaims(tok)
+}
+
 func keyFunc(keys map[string]ed25519.PublicKey) func(*jwt.Token) (interface{}, error) {
 	return func(j *jwt.Token) (interface{}, error) {
 		keyID, ok := j.Header[HeaderKeyID].(string)

diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go
@@ -824,6 +824,25 @@ func TestLicenseEntitlements(t *testing.T) {
 				assert.True(t, entitlements.Features[codersdk.FeatureMultipleOrganizations].Enabled, "multi-org enabled for premium")
 			},
 		},
+		{
+			Name: "CurrentAndFuture",
+			Licenses: []*coderdenttest.LicenseOptions{
+				enterpriseLicense().UserLimit(100),
+				premiumLicense().UserLimit(200).FutureTerm(time.Now()),
+			},
+			Enablements: defaultEnablements,
+			AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) {
+				assertEnterpriseFeatures(t, entitlements)
+				assertNoErrors(t, entitlements)
+				assertNoWarnings(t, entitlements)
+				userFeature := entitlements.Features[codersdk.FeatureUserLimit]
+				assert.Equalf(t, int64(100), *userFeature.Limit, "user limit")
+				assert.Equal(t, codersdk.EntitlementNotEntitled,
+					entitlements.Features[codersdk.FeatureMultipleOrganizations].Entitlement)
+				assert.Equal(t, codersdk.EntitlementNotEntitled,
+					entitlements.Features[codersdk.FeatureCustomRoles].Entitlement)
+			},
+		},
 	}
 
 	for _, tc := range testCases {

diff --git a/enterprise/coderd/licenses.go b/enterprise/coderd/licenses.go
@@ -86,25 +86,7 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	rawClaims, err := license.ParseRaw(addLicense.License, api.LicenseKeys)
-	if err != nil {
-		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-			Message: "Invalid license",
-			Detail:  err.Error(),
-		})
-		return
-	}
-	exp, ok := rawClaims["exp"].(float64)
-	if !ok {
-		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-			Message: "Invalid license",
-			Detail:  "exp claim missing or not parsable",
-		})
-		return
-	}
-	expTime := time.Unix(int64(exp), 0)
-
-	claims, err := license.ParseClaims(addLicense.License, api.LicenseKeys)
+	claims, err := license.ParseClaimsIgnoreNbf(addLicense.License, api.LicenseKeys)
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message: "Invalid license",
@@ -134,7 +116,7 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) {
 	dl, err := api.Database.InsertLicense(ctx, database.InsertLicenseParams{
 		UploadedAt: dbtime.Now(),
 		JWT:        addLicense.License,
-		Exp:        expTime,
+		Exp:        claims.ExpiresAt.Time,
 		UUID:       id,
 	})
 	if err != nil {
@@ -160,7 +142,15 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) {
 		// don't fail the HTTP request, since we did write it successfully to the database
 	}
 
-	httpapi.Write(ctx, rw, http.StatusCreated, convertLicense(dl, rawClaims))
+	c, err := decodeClaims(dl)
+	if err != nil {
+		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Failed to decode database response",
+			Detail:  err.Error(),
+		})
+		return
+	}
+	httpapi.Write(ctx, rw, http.StatusCreated, convertLicense(dl, c))
 }
 
 // postRefreshEntitlements forces an `updateEntitlements` call and publishes

diff --git a/enterprise/coderd/licenses_test.go b/enterprise/coderd/licenses_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
@@ -82,6 +83,53 @@ func TestPostLicense(t *testing.T) {
 			t.Error("expected to get error status 400")
 		}
 	})
+
+	// Test a license that isn't yet valid, but will be in the future.  We should allow this so that
+	// operators can upload a license ahead of time.
+	t.Run("NotYet", func(t *testing.T) {
+		t.Parallel()
+		client, _ := coderdenttest.New(t, &coderdenttest.Options{DontAddLicense: true})
+		respLic := coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
+			AccountType: license.AccountTypeSalesforce,
+			AccountID:   "testing",
+			Features: license.Features{
+				codersdk.FeatureAuditLog: 1,
+			},
+			NotBefore: time.Now().Add(time.Hour),
+			GraceAt:   time.Now().Add(2 * time.Hour),
+			ExpiresAt: time.Now().Add(3 * time.Hour),
+		})
+		assert.GreaterOrEqual(t, respLic.ID, int32(0))
+		// just a couple spot checks for sanity
+		assert.Equal(t, "testing", respLic.Claims["account_id"])
+		features, err := respLic.FeaturesClaims()
+		require.NoError(t, err)
+		assert.EqualValues(t, 1, features[codersdk.FeatureAuditLog])
+	})
+
+	// Test we still reject a license that isn't valid yet, but has other issues (e.g. expired
+	// before it starts).
+	t.Run("NotEver", func(t *testing.T) {
+		t.Parallel()
+		client, _ := coderdenttest.New(t, &coderdenttest.Options{DontAddLicense: true})
+		lic := coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{
+			AccountType: license.AccountTypeSalesforce,
+			AccountID:   "testing",
+			Features: license.Features{
+				codersdk.FeatureAuditLog: 1,
+			},
+			NotBefore: time.Now().Add(time.Hour),
+			GraceAt:   time.Now().Add(2 * time.Hour),
+			ExpiresAt: time.Now().Add(-time.Hour),
+		})
+		_, err := client.AddLicense(context.Background(), codersdk.AddLicenseRequest{
+			License: lic,
+		})
+		errResp := &codersdk.Error{}
+		require.ErrorAs(t, err, &errResp)
+		require.Equal(t, http.StatusBadRequest, errResp.StatusCode())
+		require.Contains(t, errResp.Detail, license.ErrMultipleIssues.Error())
+	})
 }
 
 func TestGetLicense(t *testing.T) {