From 852168b938da77df95aca5013645a5a895e6e3f2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 Feb 2023 19:27:30 -0600 Subject: [PATCH 1/4] chore: Force license uuids to not be null --- coderd/database/dump.sql | 2 +- .../000094_license_not_null_uuid.down.sql | 1 + .../000094_license_not_null_uuid.up.sql | 4 ++++ coderd/database/models.go | 4 ++-- coderd/database/queries.sql.go | 16 ++++++++-------- coderd/database/sqlc.yaml | 1 + coderd/telemetry/telemetry.go | 2 +- coderd/telemetry/telemetry_test.go | 5 +---- enterprise/coderd/licenses.go | 15 ++++++++++----- enterprise/trialer/trialer.go | 8 ++++---- 10 files changed, 33 insertions(+), 25 deletions(-) create mode 100644 coderd/database/migrations/000094_license_not_null_uuid.down.sql create mode 100644 coderd/database/migrations/000094_license_not_null_uuid.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 664f2648e2123..23df234c895cb 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -209,7 +209,7 @@ CREATE TABLE licenses ( uploaded_at timestamp with time zone NOT NULL, jwt text NOT NULL, exp timestamp with time zone NOT NULL, - uuid uuid + uuid uuid NOT NULL ); COMMENT ON COLUMN licenses.exp IS 'exp tracks the claim of the same name in the JWT, and we include it here so that we can easily query for licenses that have not yet expired.'; diff --git a/coderd/database/migrations/000094_license_not_null_uuid.down.sql b/coderd/database/migrations/000094_license_not_null_uuid.down.sql new file mode 100644 index 0000000000000..3ede10f7725fb --- /dev/null +++ b/coderd/database/migrations/000094_license_not_null_uuid.down.sql @@ -0,0 +1 @@ +ALTER TABLE ONLY licenses ALTER COLUMN uuid DROP NOT NULL; diff --git a/coderd/database/migrations/000094_license_not_null_uuid.up.sql b/coderd/database/migrations/000094_license_not_null_uuid.up.sql new file mode 100644 index 0000000000000..31c9f4f7bd068 --- /dev/null +++ b/coderd/database/migrations/000094_license_not_null_uuid.up.sql @@ -0,0 +1,4 @@ +-- We need to assign uuids to any existing licenses that don't have them. +UPDATE licenses SET uuid = gen_random_uuid() WHERE uuid IS NULL; +-- Assert no licenses have null uuids. +ALTER TABLE ONLY licenses ALTER COLUMN uuid SET NOT NULL; diff --git a/coderd/database/models.go b/coderd/database/models.go index 8496c75df2114..42b144057bd3a 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1282,8 +1282,8 @@ type License struct { UploadedAt time.Time `db:"uploaded_at" json:"uploaded_at"` JWT string `db:"jwt" json:"jwt"` // exp tracks the claim of the same name in the JWT, and we include it here so that we can easily query for licenses that have not yet expired. - Exp time.Time `db:"exp" json:"exp"` - Uuid uuid.NullUUID `db:"uuid" json:"uuid"` + Exp time.Time `db:"exp" json:"exp"` + UUID uuid.UUID `db:"uuid" json:"uuid"` } type Organization struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 7b2b77f753f69..e6ffdb3f8b201 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1363,7 +1363,7 @@ func (q *sqlQuerier) GetLicenses(ctx context.Context) ([]License, error) { &i.UploadedAt, &i.JWT, &i.Exp, - &i.Uuid, + &i.UUID, ); err != nil { return nil, err } @@ -1399,7 +1399,7 @@ func (q *sqlQuerier) GetUnexpiredLicenses(ctx context.Context) ([]License, error &i.UploadedAt, &i.JWT, &i.Exp, - &i.Uuid, + &i.UUID, ); err != nil { return nil, err } @@ -1427,10 +1427,10 @@ VALUES ` type InsertLicenseParams struct { - UploadedAt time.Time `db:"uploaded_at" json:"uploaded_at"` - JWT string `db:"jwt" json:"jwt"` - Exp time.Time `db:"exp" json:"exp"` - Uuid uuid.NullUUID `db:"uuid" json:"uuid"` + UploadedAt time.Time `db:"uploaded_at" json:"uploaded_at"` + JWT string `db:"jwt" json:"jwt"` + Exp time.Time `db:"exp" json:"exp"` + UUID uuid.UUID `db:"uuid" json:"uuid"` } func (q *sqlQuerier) InsertLicense(ctx context.Context, arg InsertLicenseParams) (License, error) { @@ -1438,7 +1438,7 @@ func (q *sqlQuerier) InsertLicense(ctx context.Context, arg InsertLicenseParams) arg.UploadedAt, arg.JWT, arg.Exp, - arg.Uuid, + arg.UUID, ) var i License err := row.Scan( @@ -1446,7 +1446,7 @@ func (q *sqlQuerier) InsertLicense(ctx context.Context, arg InsertLicenseParams) &i.UploadedAt, &i.JWT, &i.Exp, - &i.Uuid, + &i.UUID, ) return i, err } diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index e53fead06d0de..e708a6c4ca065 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -43,6 +43,7 @@ overrides: troubleshooting_url: TroubleshootingURL default_ttl: DefaultTTL motd_file: MOTDFile + uuid: UUID sql: - schema: "./dump.sql" diff --git a/coderd/telemetry/telemetry.go b/coderd/telemetry/telemetry.go index 9d957f40eecb7..2398e05ffa36f 100644 --- a/coderd/telemetry/telemetry.go +++ b/coderd/telemetry/telemetry.go @@ -637,7 +637,7 @@ func ConvertTemplateVersion(version database.TemplateVersion) TemplateVersion { func ConvertLicense(license database.License) License { return License{ UploadedAt: license.UploadedAt, - UUID: license.Uuid.UUID, + UUID: license.UUID, } } diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index 3df1ba9776b3e..ac16cfb5df62d 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -111,10 +111,7 @@ func TestTelemetry(t *testing.T) { UploadedAt: database.Now(), JWT: "", Exp: database.Now().Add(time.Hour), - Uuid: uuid.NullUUID{ - UUID: uuid.New(), - Valid: true, - }, + UUID: uuid.New(), }) assert.NoError(t, err) snapshot := collectSnapshot(t, db) diff --git a/enterprise/coderd/licenses.go b/enterprise/coderd/licenses.go index ef3f6c9d44882..bceddf5e76457 100644 --- a/enterprise/coderd/licenses.go +++ b/enterprise/coderd/licenses.go @@ -8,6 +8,7 @@ import ( _ "embed" "encoding/base64" "encoding/json" + "fmt" "net/http" "strconv" "strings" @@ -98,14 +99,18 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) { } id, err := uuid.Parse(claims.ID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Invalid license ID %q", claims.ID), + Detail: err.Error(), + }) + return + } dl, err := api.Database.InsertLicense(ctx, database.InsertLicenseParams{ UploadedAt: database.Now(), JWT: addLicense.License, Exp: expTime, - Uuid: uuid.NullUUID{ - UUID: id, - Valid: err == nil, - }, + UUID: id, }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -229,7 +234,7 @@ func (api *API) deleteLicense(rw http.ResponseWriter, r *http.Request) { func convertLicense(dl database.License, c jwt.MapClaims) codersdk.License { return codersdk.License{ ID: dl.ID, - UUID: dl.Uuid.UUID, + UUID: dl.UUID, UploadedAt: dl.UploadedAt, Claims: c, } diff --git a/enterprise/trialer/trialer.go b/enterprise/trialer/trialer.go index 48b67a9b9576c..69d395e5a3e16 100644 --- a/enterprise/trialer/trialer.go +++ b/enterprise/trialer/trialer.go @@ -64,14 +64,14 @@ func New(db database.Store, url string, keys map[string]ed25519.PublicKey) func( return xerrors.Errorf("parse claims: %w", err) } id, err := uuid.Parse(claims.ID) + if err != nil { + return xerrors.Errorf("parse uuid: %w", err) + } _, err = db.InsertLicense(ctx, database.InsertLicenseParams{ UploadedAt: database.Now(), JWT: string(raw), Exp: expTime, - Uuid: uuid.NullUUID{ - UUID: id, - Valid: err == nil, - }, + UUID: id, }) if err != nil { return xerrors.Errorf("insert license: %w", err) From 7493130259edadf44a0f85e9ccfb828063094962 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 13 Feb 2023 11:01:28 -0600 Subject: [PATCH 2/4] All unit tests generate uuids for licenses --- enterprise/coderd/coderdenttest/coderdenttest.go | 2 ++ enterprise/coderd/licenses.go | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/enterprise/coderd/coderdenttest/coderdenttest.go b/enterprise/coderd/coderdenttest/coderdenttest.go index 0e5c8e7a1ef93..4dab0efbb31d1 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest.go +++ b/enterprise/coderd/coderdenttest/coderdenttest.go @@ -11,6 +11,7 @@ import ( "time" "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -128,6 +129,7 @@ func GenerateLicense(t *testing.T, options LicenseOptions) string { 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)), diff --git a/enterprise/coderd/licenses.go b/enterprise/coderd/licenses.go index bceddf5e76457..40f45e1b75c17 100644 --- a/enterprise/coderd/licenses.go +++ b/enterprise/coderd/licenses.go @@ -8,7 +8,6 @@ import ( _ "embed" "encoding/base64" "encoding/json" - "fmt" "net/http" "strconv" "strings" @@ -100,11 +99,12 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) { id, err := uuid.Parse(claims.ID) if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Invalid license ID %q", claims.ID), - Detail: err.Error(), - }) - return + // If no uuid is in the license, we generate a random uuid. + // This is not ideal, and this should be fixed to require a uuid + // for all licenses. We require this patch to support older licenses. + // TODO: In the future (April 2023?) we should remove this and reissue + // old licenses with a uuid. + id = uuid.New() } dl, err := api.Database.InsertLicense(ctx, database.InsertLicenseParams{ UploadedAt: database.Now(), From 59a178b63e76427752e11cb10c95c593828513ba Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 13 Feb 2023 11:10:46 -0600 Subject: [PATCH 3/4] Update migration files to new numbers --- ...t_null_uuid.down.sql => 000097_license_not_null_uuid.down.sql} | 0 ...e_not_null_uuid.up.sql => 000097_license_not_null_uuid.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000094_license_not_null_uuid.down.sql => 000097_license_not_null_uuid.down.sql} (100%) rename coderd/database/migrations/{000094_license_not_null_uuid.up.sql => 000097_license_not_null_uuid.up.sql} (100%) diff --git a/coderd/database/migrations/000094_license_not_null_uuid.down.sql b/coderd/database/migrations/000097_license_not_null_uuid.down.sql similarity index 100% rename from coderd/database/migrations/000094_license_not_null_uuid.down.sql rename to coderd/database/migrations/000097_license_not_null_uuid.down.sql diff --git a/coderd/database/migrations/000094_license_not_null_uuid.up.sql b/coderd/database/migrations/000097_license_not_null_uuid.up.sql similarity index 100% rename from coderd/database/migrations/000094_license_not_null_uuid.up.sql rename to coderd/database/migrations/000097_license_not_null_uuid.up.sql From 51d5e41faa315288fc1f5019b8e1a0bb940c9e93 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 13 Feb 2023 14:47:15 -0600 Subject: [PATCH 4/4] Put migration in transaction --- .../database/migrations/000097_license_not_null_uuid.up.sql | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/coderd/database/migrations/000097_license_not_null_uuid.up.sql b/coderd/database/migrations/000097_license_not_null_uuid.up.sql index 31c9f4f7bd068..ca64a6850b021 100644 --- a/coderd/database/migrations/000097_license_not_null_uuid.up.sql +++ b/coderd/database/migrations/000097_license_not_null_uuid.up.sql @@ -1,4 +1,8 @@ +BEGIN; + -- We need to assign uuids to any existing licenses that don't have them. UPDATE licenses SET uuid = gen_random_uuid() WHERE uuid IS NULL; -- Assert no licenses have null uuids. ALTER TABLE ONLY licenses ALTER COLUMN uuid SET NOT NULL; + +COMMIT;