Skip to content

Commit 5c6d7f4

Browse files
stirbycode-asherspikecurtisjohnstcnBrunoQuaresma
authored
fix: patch mainline improvements back to 2.14.3 (coder#14566)
* fix: increase group name limit to 36 from 32 (coder#14443) (cherry picked from commit 4997691) * fix: allow posting licenses that will be valid in future (coder#14491) (cherry picked from commit 5bd5801) * fix: stop reporting future licenses as errors (coder#14492) (cherry picked from commit 4eac2ac) * fix(provisionerd/runner): do not log entire resources (coder#14538) fix(coderd/workspaceagentsrpc): do not log entire agent fix(provisionerd/runner): do not log entire resources (cherry picked from commit 5366f25) * fix(site): fix agent logs streaming for third party apps (coder#14541) (cherry picked from commit 242b1ea) * Fix TestLicenseEntitlements/CurrentAndFuture test --------- Co-authored-by: Asher <ash@coder.com> Co-authored-by: Spike Curtis <spike@coder.com> Co-authored-by: Cian Johnston <cian@coder.com> Co-authored-by: Bruno Quaresma <bruno@coder.com>
1 parent d1cd784 commit 5c6d7f4

File tree

12 files changed

+220
-45
lines changed

12 files changed

+220
-45
lines changed

‎coderd/httpapi/httpapi.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func init() {
4646
valid := NameValid(str)
4747
return valid == nil
4848
}
49-
for _, tag := range []string{"username", "organization_name", "template_name", "group_name", "workspace_name", "oauth2_app_name"} {
49+
for _, tag := range []string{"username", "organization_name", "template_name", "workspace_name", "oauth2_app_name"} {
5050
err := Validate.RegisterValidation(tag, nameValidator)
5151
if err != nil {
5252
panic(err)
@@ -96,6 +96,20 @@ func init() {
9696
if err != nil {
9797
panic(err)
9898
}
99+
100+
groupNameValidator := func(fl validator.FieldLevel) bool {
101+
f := fl.Field().Interface()
102+
str, ok := f.(string)
103+
if !ok {
104+
return false
105+
}
106+
valid := GroupNameValid(str)
107+
return valid == nil
108+
}
109+
err = Validate.RegisterValidation("group_name", groupNameValidator)
110+
if err != nil {
111+
panic(err)
112+
}
99113
}
100114

101115
// Is404Error returns true if the given error should return a 404 status code.

‎coderd/httpapi/name.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,23 @@ func UserRealNameValid(str string) error {
9696
return nil
9797
}
9898

99+
// GroupNameValid returns whether the input string is a valid group name.
100+
func GroupNameValid(str string) error {
101+
// 36 is to support using UUIDs as the group name.
102+
if len(str) > 36 {
103+
return xerrors.New("must be <= 36 characters")
104+
}
105+
// Avoid conflicts with routes like /groups/new and /groups/create.
106+
if str == "new" || str == "create" {
107+
return xerrors.Errorf("cannot use %q as a name", str)
108+
}
109+
matched := UsernameValidRegex.MatchString(str)
110+
if !matched {
111+
return xerrors.New("must be alphanumeric with hyphens")
112+
}
113+
return nil
114+
}
115+
99116
// NormalizeUserRealName normalizes a user name such that it will pass
100117
// validation by UserRealNameValid. This is done to avoid blocking
101118
// little Bobby Whitespace from using Coder.

‎coderd/workspaceagentsrpc.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,19 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) {
116116
}
117117
defer mux.Close()
118118

119-
logger.Debug(ctx, "accepting agent RPC connection", slog.F("agent", workspaceAgent))
119+
logger.Debug(ctx, "accepting agent RPC connection",
120+
slog.F("agent_id", workspaceAgent.ID),
121+
slog.F("agent_created_at", workspaceAgent.CreatedAt),
122+
slog.F("agent_updated_at", workspaceAgent.UpdatedAt),
123+
slog.F("agent_name", workspaceAgent.Name),
124+
slog.F("agent_first_connected_at", workspaceAgent.FirstConnectedAt.Time),
125+
slog.F("agent_last_connected_at", workspaceAgent.LastConnectedAt.Time),
126+
slog.F("agent_disconnected_at", workspaceAgent.DisconnectedAt.Time),
127+
slog.F("agent_version", workspaceAgent.Version),
128+
slog.F("agent_last_connected_replica_id", workspaceAgent.LastConnectedReplicaID),
129+
slog.F("agent_connection_timeout_seconds", workspaceAgent.ConnectionTimeoutSeconds),
130+
slog.F("agent_api_version", workspaceAgent.APIVersion),
131+
slog.F("agent_resource_id", workspaceAgent.ResourceID))
120132

121133
closeCtx, closeCtxCancel := context.WithCancel(ctx)
122134
defer closeCtxCancel()

‎enterprise/coderd/coderdenttest/coderdenttest.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ type LicenseOptions struct {
174174
// ExpiresAt is the time at which the license will hard expire.
175175
// ExpiresAt should always be greater then GraceAt.
176176
ExpiresAt time.Time
177+
// NotBefore is the time at which the license becomes valid. If set to the
178+
// zero value, the `nbf` claim on the license is set to 1 minute in the
179+
// past.
180+
NotBefore time.Time
177181
Features license.Features
178182
}
179183

@@ -195,6 +199,13 @@ func (opts *LicenseOptions) Valid(now time.Time) *LicenseOptions {
195199
return opts
196200
}
197201

202+
func (opts *LicenseOptions) FutureTerm(now time.Time) *LicenseOptions {
203+
opts.NotBefore = now.Add(time.Hour * 24)
204+
opts.ExpiresAt = now.Add(time.Hour * 24 * 60)
205+
opts.GraceAt = now.Add(time.Hour * 24 * 53)
206+
return opts
207+
}
208+
198209
func (opts *LicenseOptions) UserLimit(limit int64) *LicenseOptions {
199210
return opts.Feature(codersdk.FeatureUserLimit, limit)
200211
}
@@ -233,13 +244,16 @@ func GenerateLicense(t *testing.T, options LicenseOptions) string {
233244
if options.GraceAt.IsZero() {
234245
options.GraceAt = time.Now().Add(time.Hour)
235246
}
247+
if options.NotBefore.IsZero() {
248+
options.NotBefore = time.Now().Add(-time.Minute)
249+
}
236250

237251
c := &license.Claims{
238252
RegisteredClaims: jwt.RegisteredClaims{
239253
ID: uuid.NewString(),
240254
Issuer: "test@testing.test",
241255
ExpiresAt: jwt.NewNumericDate(options.ExpiresAt),
242-
NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Minute)),
256+
NotBefore: jwt.NewNumericDate(options.NotBefore),
243257
IssuedAt: jwt.NewNumericDate(time.Now().Add(-time.Minute)),
244258
},
245259
LicenseExpires: jwt.NewNumericDate(options.GraceAt),

‎enterprise/coderd/groups_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func TestPatchGroup(t *testing.T) {
156156
const displayName = "foobar"
157157
ctx := testutil.Context(t, testutil.WaitLong)
158158
group, err := userAdminClient.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
159-
Name: "hi",
159+
Name: "ff7dcee2-e7c4-4bc4-a9e4-84870770e4c5", // GUID should fit.
160160
AvatarURL: "https://example.com",
161161
QuotaAllowance: 10,
162162
DisplayName: "",
@@ -165,14 +165,14 @@ func TestPatchGroup(t *testing.T) {
165165
require.Equal(t, 10, group.QuotaAllowance)
166166

167167
group, err = userAdminClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
168-
Name: "bye",
168+
Name: "ddd502d2-2984-4724-b5bf-1109a4d7462d", // GUID should fit.
169169
AvatarURL: ptr.Ref("https://google.com"),
170170
QuotaAllowance: ptr.Ref(20),
171171
DisplayName: ptr.Ref(displayName),
172172
})
173173
require.NoError(t, err)
174174
require.Equal(t, displayName, group.DisplayName)
175-
require.Equal(t, "bye", group.Name)
175+
require.Equal(t, "ddd502d2-2984-4724-b5bf-1109a4d7462d", group.Name)
176176
require.Equal(t, "https://google.com", group.AvatarURL)
177177
require.Equal(t, 20, group.QuotaAllowance)
178178
})

‎enterprise/coderd/license/license.go

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ func LicensesEntitlements(
100100
// 'Entitlements' group as a whole.
101101
for _, license := range licenses {
102102
claims, err := ParseClaims(license.JWT, keys)
103+
var vErr *jwt.ValidationError
104+
if xerrors.As(err, &vErr) && vErr.Is(jwt.ErrTokenNotValidYet) {
105+
// The license isn't valid yet. We don't consider any entitlements contained in it, but
106+
// it's also not an error. Just skip it silently. This can happen if an administrator
107+
// uploads a license for a new term that hasn't started yet.
108+
continue
109+
}
103110
if err != nil {
104111
entitlements.Errors = append(entitlements.Errors,
105112
fmt.Sprintf("Invalid license (%s) parsing claims: %s", license.UUID.String(), err.Error()))
@@ -287,6 +294,8 @@ var (
287294
ErrInvalidVersion = xerrors.New("license must be version 3")
288295
ErrMissingKeyID = xerrors.Errorf("JOSE header must contain %s", HeaderKeyID)
289296
ErrMissingLicenseExpires = xerrors.New("license missing license_expires")
297+
ErrMissingExp = xerrors.New("exp claim missing or not parsable")
298+
ErrMultipleIssues = xerrors.New("license has multiple issues; contact support")
290299
)
291300

292301
type Features map[codersdk.FeatureName]int64
@@ -336,7 +345,7 @@ func ParseRaw(l string, keys map[string]ed25519.PublicKey) (jwt.MapClaims, error
336345
return nil, xerrors.New("unable to parse Claims")
337346
}
338347

339-
// ParseClaims validates a database.License record, and if valid, returns the claims. If
348+
// ParseClaims validates a raw JWT, and if valid, returns the claims. If
340349
// unparsable or invalid, it returns an error
341350
func ParseClaims(rawJWT string, keys map[string]ed25519.PublicKey) (*Claims, error) {
342351
tok, err := jwt.ParseWithClaims(
@@ -348,18 +357,53 @@ func ParseClaims(rawJWT string, keys map[string]ed25519.PublicKey) (*Claims, err
348357
if err != nil {
349358
return nil, err
350359
}
351-
if claims, ok := tok.Claims.(*Claims); ok && tok.Valid {
360+
return validateClaims(tok)
361+
}
362+
363+
func validateClaims(tok *jwt.Token) (*Claims, error) {
364+
if claims, ok := tok.Claims.(*Claims); ok {
352365
if claims.Version != uint64(CurrentVersion) {
353366
return nil, ErrInvalidVersion
354367
}
355368
if claims.LicenseExpires == nil {
356369
return nil, ErrMissingLicenseExpires
357370
}
371+
if claims.ExpiresAt == nil {
372+
return nil, ErrMissingExp
373+
}
358374
return claims, nil
359375
}
360376
return nil, xerrors.New("unable to parse Claims")
361377
}
362378

379+
// ParseClaimsIgnoreNbf validates a raw JWT, but ignores `nbf` claim. If otherwise valid, it returns
380+
// the claims. If unparsable or invalid, it returns an error. Ignoring the `nbf` (not before) is
381+
// useful to determine if a JWT _will_ become valid at any point now or in the future.
382+
func ParseClaimsIgnoreNbf(rawJWT string, keys map[string]ed25519.PublicKey) (*Claims, error) {
383+
tok, err := jwt.ParseWithClaims(
384+
rawJWT,
385+
&Claims{},
386+
keyFunc(keys),
387+
jwt.WithValidMethods(ValidMethods),
388+
)
389+
var vErr *jwt.ValidationError
390+
if xerrors.As(err, &vErr) {
391+
// zero out the NotValidYet error to check if there were other problems
392+
vErr.Errors = vErr.Errors & (^jwt.ValidationErrorNotValidYet)
393+
if vErr.Errors != 0 {
394+
// There are other errors besides not being valid yet. We _could_ go
395+
// through all the jwt.ValidationError bits and try to work out the
396+
// correct error, but if we get here something very strange is
397+
// going on so let's just return a generic error that says to get in
398+
// touch with our support team.
399+
return nil, ErrMultipleIssues
400+
}
401+
} else if err != nil {
402+
return nil, err
403+
}
404+
return validateClaims(tok)
405+
}
406+
363407
func keyFunc(keys map[string]ed25519.PublicKey) func(*jwt.Token) (interface{}, error) {
364408
return func(j *jwt.Token) (interface{}, error) {
365409
keyID, ok := j.Header[HeaderKeyID].(string)

‎enterprise/coderd/license/license_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,23 @@ func TestLicenseEntitlements(t *testing.T) {
824824
assert.True(t, entitlements.Features[codersdk.FeatureMultipleOrganizations].Enabled, "multi-org enabled for premium")
825825
},
826826
},
827+
{
828+
Name: "CurrentAndFuture",
829+
Licenses: []*coderdenttest.LicenseOptions{
830+
enterpriseLicense().UserLimit(100),
831+
premiumLicense().UserLimit(200).FutureTerm(time.Now()),
832+
},
833+
Enablements: defaultEnablements,
834+
AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) {
835+
assertEnterpriseFeatures(t, entitlements)
836+
assertNoErrors(t, entitlements)
837+
assertNoWarnings(t, entitlements)
838+
userFeature := entitlements.Features[codersdk.FeatureUserLimit]
839+
assert.Equalf(t, int64(100), *userFeature.Limit, "user limit")
840+
assert.Equal(t, codersdk.EntitlementNotEntitled,
841+
entitlements.Features[codersdk.FeatureMultipleOrganizations].Entitlement)
842+
},
843+
},
827844
}
828845

829846
for _, tc := range testCases {

‎enterprise/coderd/licenses.go

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,25 +86,7 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) {
8686
return
8787
}
8888

89-
rawClaims, err := license.ParseRaw(addLicense.License, api.LicenseKeys)
90-
if err != nil {
91-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
92-
Message: "Invalid license",
93-
Detail: err.Error(),
94-
})
95-
return
96-
}
97-
exp, ok := rawClaims["exp"].(float64)
98-
if !ok {
99-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
100-
Message: "Invalid license",
101-
Detail: "exp claim missing or not parsable",
102-
})
103-
return
104-
}
105-
expTime := time.Unix(int64(exp), 0)
106-
107-
claims, err := license.ParseClaims(addLicense.License, api.LicenseKeys)
89+
claims, err := license.ParseClaimsIgnoreNbf(addLicense.License, api.LicenseKeys)
10890
if err != nil {
10991
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
11092
Message: "Invalid license",
@@ -134,7 +116,7 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) {
134116
dl, err := api.Database.InsertLicense(ctx, database.InsertLicenseParams{
135117
UploadedAt: dbtime.Now(),
136118
JWT: addLicense.License,
137-
Exp: expTime,
119+
Exp: claims.ExpiresAt.Time,
138120
UUID: id,
139121
})
140122
if err != nil {
@@ -160,7 +142,15 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) {
160142
// don't fail the HTTP request, since we did write it successfully to the database
161143
}
162144

163-
httpapi.Write(ctx, rw, http.StatusCreated, convertLicense(dl, rawClaims))
145+
c, err := decodeClaims(dl)
146+
if err != nil {
147+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
148+
Message: "Failed to decode database response",
149+
Detail: err.Error(),
150+
})
151+
return
152+
}
153+
httpapi.Write(ctx, rw, http.StatusCreated, convertLicense(dl, c))
164154
}
165155

166156
// postRefreshEntitlements forces an `updateEntitlements` call and publishes

‎enterprise/coderd/licenses_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"net/http"
66
"testing"
7+
"time"
78

89
"github.com/google/uuid"
910
"github.com/stretchr/testify/assert"
@@ -82,6 +83,53 @@ func TestPostLicense(t *testing.T) {
8283
t.Error("expected to get error status 400")
8384
}
8485
})
86+
87+
// Test a license that isn't yet valid, but will be in the future. We should allow this so that
88+
// operators can upload a license ahead of time.
89+
t.Run("NotYet", func(t *testing.T) {
90+
t.Parallel()
91+
client, _ := coderdenttest.New(t, &coderdenttest.Options{DontAddLicense: true})
92+
respLic := coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
93+
AccountType: license.AccountTypeSalesforce,
94+
AccountID: "testing",
95+
Features: license.Features{
96+
codersdk.FeatureAuditLog: 1,
97+
},
98+
NotBefore: time.Now().Add(time.Hour),
99+
GraceAt: time.Now().Add(2 * time.Hour),
100+
ExpiresAt: time.Now().Add(3 * time.Hour),
101+
})
102+
assert.GreaterOrEqual(t, respLic.ID, int32(0))
103+
// just a couple spot checks for sanity
104+
assert.Equal(t, "testing", respLic.Claims["account_id"])
105+
features, err := respLic.FeaturesClaims()
106+
require.NoError(t, err)
107+
assert.EqualValues(t, 1, features[codersdk.FeatureAuditLog])
108+
})
109+
110+
// Test we still reject a license that isn't valid yet, but has other issues (e.g. expired
111+
// before it starts).
112+
t.Run("NotEver", func(t *testing.T) {
113+
t.Parallel()
114+
client, _ := coderdenttest.New(t, &coderdenttest.Options{DontAddLicense: true})
115+
lic := coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{
116+
AccountType: license.AccountTypeSalesforce,
117+
AccountID: "testing",
118+
Features: license.Features{
119+
codersdk.FeatureAuditLog: 1,
120+
},
121+
NotBefore: time.Now().Add(time.Hour),
122+
GraceAt: time.Now().Add(2 * time.Hour),
123+
ExpiresAt: time.Now().Add(-time.Hour),
124+
})
125+
_, err := client.AddLicense(context.Background(), codersdk.AddLicenseRequest{
126+
License: lic,
127+
})
128+
errResp := &codersdk.Error{}
129+
require.ErrorAs(t, err, &errResp)
130+
require.Equal(t, http.StatusBadRequest, errResp.StatusCode())
131+
require.Contains(t, errResp.Detail, license.ErrMultipleIssues.Error())
132+
})
85133
}
86134

87135
func TestGetLicense(t *testing.T) {

0 commit comments

Comments
 (0)