From 26aa3738507a0a77ae63e87cdc2900e4a3924f29 Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Thu, 7 Oct 2021 17:21:57 +0200 Subject: [PATCH] feat: deprecate autoincrement primary key in hydra_client This is the first step towards resolving #2781. The issue will be resolved when we remove the deprecated column. --- client/client.go | 6 +- go.mod | 1 + persistence/sql/migratest/exptected_data.go | 5 +- persistence/sql/migratest/migration_test.go | 4 + .../testdata/20211004110001_testdata.sql | 75 +++++++++++ ...ange_client_primary_key.cockroach.down.sql | 1 + ...change_client_primary_key.cockroach.up.sql | 2 + ...0_change_client_primary_key.mysql.down.sql | 3 + ...000_change_client_primary_key.mysql.up.sql | 7 + ...hange_client_primary_key.postgres.down.sql | 4 + ..._change_client_primary_key.postgres.up.sql | 16 +++ ..._change_client_primary_key.sqlite.down.sql | 115 ++++++++++++++++ ...00_change_client_primary_key.sqlite.up.sql | 125 ++++++++++++++++++ ...ange_client_primary_key.cockroach.down.sql | 1 + ...change_client_primary_key.cockroach.up.sql | 1 + ...ange_client_primary_key.cockroach.down.sql | 2 + ...change_client_primary_key.cockroach.up.sql | 2 + 17 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 persistence/sql/migratest/testdata/20211004110001_testdata.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.down.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.up.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.down.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.up.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.down.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.up.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.down.sql create mode 100644 persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.up.sql create mode 100644 persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.down.sql create mode 100644 persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.up.sql create mode 100644 persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.down.sql create mode 100644 persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.up.sql diff --git a/client/client.go b/client/client.go index feb140e002c..638aff60b4f 100644 --- a/client/client.go +++ b/client/client.go @@ -25,6 +25,7 @@ import ( "time" "github.com/gobuffalo/pop/v5" + "github.com/gofrs/uuid" jose "gopkg.in/square/go-jose.v2" // Naming the dependency jose is important for go-swagger to work, see https://github.com/go-swagger/go-swagger/issues/1587 @@ -37,7 +38,10 @@ import ( // // swagger:model oAuth2Client type Client struct { - ID int64 `json:"-" db:"pk"` + ID uuid.UUID `json:"-" db:"pk_new"` + + // This field is deprecated and will be removed + PKDeprecated int64 `json:"-" db:"pk_deprecated"` // ID is the id for this client. OutfacingID string `json:"client_id" db:"id"` diff --git a/go.mod b/go.mod index 8e20cf36a05..50dc259e5e9 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( github.com/gobuffalo/pop/v5 v5.3.4 github.com/gobuffalo/x v0.0.0-20181007152206-913e47c59ca7 github.com/gobwas/glob v0.2.3 + github.com/gofrs/uuid v3.2.0+incompatible github.com/golang/mock v1.6.0 github.com/google/uuid v1.2.0 github.com/gorilla/securecookie v1.1.1 diff --git a/persistence/sql/migratest/exptected_data.go b/persistence/sql/migratest/exptected_data.go index 37e00ec79c2..cf93023b6ad 100644 --- a/persistence/sql/migratest/exptected_data.go +++ b/persistence/sql/migratest/exptected_data.go @@ -6,6 +6,8 @@ import ( "gopkg.in/square/go-jose.v2" + "github.com/gofrs/uuid" + "github.com/ory/hydra/client" "github.com/ory/hydra/consent" "github.com/ory/hydra/jwk" @@ -17,7 +19,8 @@ import ( func expectedClient(i int) *client.Client { c := &client.Client{ - ID: int64(i), + ID: uuid.Nil, + PKDeprecated: int64(i), OutfacingID: fmt.Sprintf("client-%04d", i), Name: fmt.Sprintf("Client %04d", i), Secret: fmt.Sprintf("secret-%04d", i), diff --git a/persistence/sql/migratest/migration_test.go b/persistence/sql/migratest/migration_test.go index 533fdcbb075..8507aa4cbe4 100644 --- a/persistence/sql/migratest/migration_test.go +++ b/persistence/sql/migratest/migration_test.go @@ -5,6 +5,8 @@ import ( "fmt" "testing" + "github.com/gofrs/uuid" + "github.com/ory/x/configx" "github.com/ory/hydra/persistence/sql" @@ -75,6 +77,8 @@ func TestMigrations(t *testing.T) { expected := expectedClient(i) actual := &client.Client{} require.NoError(t, c.Where("id = ?", expected.OutfacingID).First(actual)) + require.NotEqual(t, uuid.Nil.String(), actual.ID.String()) + expected.ID = actual.ID assertEqualClients(t, expected, actual) lastClient = actual }) diff --git a/persistence/sql/migratest/testdata/20211004110001_testdata.sql b/persistence/sql/migratest/testdata/20211004110001_testdata.sql new file mode 100644 index 00000000000..4ce475f1db5 --- /dev/null +++ b/persistence/sql/migratest/testdata/20211004110001_testdata.sql @@ -0,0 +1,75 @@ +INSERT INTO hydra_client +( + pk_new, + pk_deprecated, + id, + client_name, + client_secret, + redirect_uris, + grant_types, + response_types, + scope, + owner, + policy_uri, + tos_uri, + client_uri, + logo_uri, + contacts, + client_secret_expires_at, + sector_identifier_uri, + jwks, + jwks_uri, + request_uris, + token_endpoint_auth_method, + request_object_signing_alg, + userinfo_signed_response_alg, + subject_type, + allowed_cors_origins, + audience, + created_at, + updated_at, + frontchannel_logout_uri, + frontchannel_logout_session_required, + post_logout_redirect_uris, + backchannel_logout_uri, + backchannel_logout_session_required, + metadata, + token_endpoint_auth_signing_alg +) +VALUES +('08f4a4b7-6601-4fd7-bb7f-29ec0681b86d', + 0, + 'client-20', + 'Client 20', + 'secret-20', + 'http://redirect/20_1', + 'grant-20_1', + 'response-20_1', + 'scope-20', + 'owner-20', + 'http://policy/20', + 'http://tos/20', + 'http://client/20', + 'http://logo/20', + 'contact-20_1', + 0, + 'http://sector_id/20', + '', + 'http://jwks/20', + 'http://request/20_1', + 'token_auth-20', + 'r_alg-20', + 'u_alg-20', + 'subject-20', + 'http://cors/20_1', + 'autdience-20_1', + now(), + now(), + 'http://front_logout/20', + true, + 'http://post_redirect/20_1', + 'http://back_logout/20', + true, + '{"migration": "20"}', + '' +); diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.down.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.down.sql new file mode 100644 index 00000000000..d2981774ecf --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.down.sql @@ -0,0 +1 @@ +ALTER TABLE hydra_client DROP CONSTRAINT "primary", ADD CONSTRAINT "primary" PRIMARY KEY (pk_deprecated); diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.up.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.up.sql new file mode 100644 index 00000000000..433ed783793 --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.cockroach.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE hydra_client RENAME pk TO pk_deprecated; +ALTER TABLE hydra_client ADD pk_new UUID NOT NULL DEFAULT gen_random_uuid(); diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.down.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.down.sql new file mode 100644 index 00000000000..44fbaf4a99d --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE hydra_client CHANGE COLUMN pk_deprecated pk INT UNSIGNED AUTO_INCREMENT; +ALTER TABLE hydra_client DROP PRIMARY KEY, ADD PRIMARY KEY (pk); +ALTER TABLE hydra_client DROP pk_new; diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.up.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.up.sql new file mode 100644 index 00000000000..6ae2b6e1722 --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.mysql.up.sql @@ -0,0 +1,7 @@ +ALTER TABLE hydra_client CHANGE COLUMN pk pk_deprecated INT UNSIGNED; +ALTER TABLE hydra_client ADD COLUMN pk_new CHAR(36); +UPDATE hydra_client SET pk_new = (SELECT uuid()); +ALTER TABLE hydra_client ALTER pk_new DROP DEFAULT; +ALTER TABLE hydra_client DROP PRIMARY KEY, ADD PRIMARY KEY (pk_new); +ALTER TABLE hydra_client ADD KEY (pk_deprecated); +ALTER TABLE hydra_client CHANGE COLUMN pk_deprecated pk_deprecated INT UNSIGNED AUTO_INCREMENT; diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.down.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.down.sql new file mode 100644 index 00000000000..85eba62f80c --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.down.sql @@ -0,0 +1,4 @@ +ALTER TABLE hydra_client RENAME pk_deprecated TO pk; +ALTER TABLE hydra_client DROP CONSTRAINT hydra_client_pkey; +ALTER TABLE hydra_client ADD PRIMARY KEY (pk); +ALTER TABLE hydra_client DROP pk_new; diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.up.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.up.sql new file mode 100644 index 00000000000..38949da329b --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.postgres.up.sql @@ -0,0 +1,16 @@ +ALTER TABLE hydra_client RENAME pk TO pk_deprecated; +-- UUID generation based on https://stackoverflow.com/a/21327318/12723442 +ALTER TABLE hydra_client ADD COLUMN pk_new UUID DEFAULT uuid_in( + overlay( + overlay( + md5(random()::text || ':' || clock_timestamp()::text) + placing '4' + from 13 + ) + placing to_hex(floor(random()*(11-8+1) + 8)::int)::text + from 17 + )::cstring +); +ALTER TABLE hydra_client ALTER pk_new DROP DEFAULT; +ALTER TABLE hydra_client DROP CONSTRAINT hydra_client_pkey; +ALTER TABLE hydra_client ADD PRIMARY KEY (pk_new); diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.down.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.down.sql new file mode 100644 index 00000000000..8bd8b2c99a1 --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.down.sql @@ -0,0 +1,115 @@ +CREATE TABLE "_hydra_client_tmp" +( + id VARCHAR(255) NOT NULL, + client_name TEXT NOT NULL, + client_secret TEXT NOT NULL, + redirect_uris TEXT NOT NULL, + grant_types TEXT NOT NULL, + response_types TEXT NOT NULL, + scope TEXT NOT NULL, + owner TEXT NOT NULL, + policy_uri TEXT NOT NULL, + tos_uri TEXT NOT NULL, + client_uri TEXT NOT NULL, + logo_uri TEXT NOT NULL, + contacts TEXT NOT NULL, + client_secret_expires_at INTEGER NOT NULL DEFAULT 0, + sector_identifier_uri TEXT NOT NULL, + jwks TEXT NOT NULL, + jwks_uri TEXT NOT NULL, + request_uris TEXT NOT NULL, + token_endpoint_auth_method VARCHAR(25) NOT NULL DEFAULT '', + request_object_signing_alg VARCHAR(10) NOT NULL DEFAULT '', + userinfo_signed_response_alg VARCHAR(10) NOT NULL DEFAULT '', + subject_type VARCHAR(15) NOT NULL DEFAULT '', + allowed_cors_origins TEXT NOT NULL, + pk INTEGER PRIMARY KEY, + audience TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + frontchannel_logout_uri TEXT NOT NULL DEFAULT '', + frontchannel_logout_session_required INTEGER NOT NULL DEFAULT false, + post_logout_redirect_uris TEXT NOT NULL DEFAULT '', + backchannel_logout_uri TEXT NOT NULL DEFAULT '', + backchannel_logout_session_required INTEGER NOT NULL DEFAULT false, + metadata TEXT NOT NULL DEFAULT '{}', + token_endpoint_auth_signing_alg VARCHAR(10) NOT NULL DEFAULT '' +); + +INSERT INTO "_hydra_client_tmp" ( + id, + client_name, + client_secret, + redirect_uris, + grant_types, + response_types, + scope, + owner, + policy_uri, + tos_uri, + client_uri, + logo_uri, + contacts, + client_secret_expires_at, + sector_identifier_uri, + jwks, + jwks_uri, + request_uris, + token_endpoint_auth_method, + request_object_signing_alg, + userinfo_signed_response_alg, + subject_type, + allowed_cors_origins, + pk, + audience, + created_at, + updated_at, + frontchannel_logout_uri, + frontchannel_logout_session_required, + post_logout_redirect_uris, + backchannel_logout_uri, + backchannel_logout_session_required, + metadata, + token_endpoint_auth_signing_alg +) SELECT + id, + client_name, + client_secret, + redirect_uris, + grant_types, + response_types, + scope, + owner, + policy_uri, + tos_uri, + client_uri, + logo_uri, + contacts, + client_secret_expires_at, + sector_identifier_uri, + jwks, + jwks_uri, + request_uris, + token_endpoint_auth_method, + request_object_signing_alg, + userinfo_signed_response_alg, + subject_type, + allowed_cors_origins, + pk_deprecated, + audience, + created_at, + updated_at, + frontchannel_logout_uri, + frontchannel_logout_session_required, + post_logout_redirect_uris, + backchannel_logout_uri, + backchannel_logout_session_required, + metadata, + token_endpoint_auth_signing_alg +FROM "hydra_client"; + +DROP INDEX hydra_client_id_idx; +DROP TABLE "hydra_client"; +ALTER TABLE "_hydra_client_tmp" RENAME TO "hydra_client"; + +CREATE UNIQUE INDEX hydra_client_id_idx ON hydra_client (id); diff --git a/persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.up.sql b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.up.sql new file mode 100644 index 00000000000..5f96c98de35 --- /dev/null +++ b/persistence/sql/migrations/20211004110001000000_change_client_primary_key.sqlite.up.sql @@ -0,0 +1,125 @@ +CREATE TABLE "_hydra_client_tmp" +( + id VARCHAR(255) NOT NULL, + client_name TEXT NOT NULL, + client_secret TEXT NOT NULL, + redirect_uris TEXT NOT NULL, + grant_types TEXT NOT NULL, + response_types TEXT NOT NULL, + scope TEXT NOT NULL, + owner TEXT NOT NULL, + policy_uri TEXT NOT NULL, + tos_uri TEXT NOT NULL, + client_uri TEXT NOT NULL, + logo_uri TEXT NOT NULL, + contacts TEXT NOT NULL, + client_secret_expires_at INTEGER NOT NULL DEFAULT 0, + sector_identifier_uri TEXT NOT NULL, + jwks TEXT NOT NULL, + jwks_uri TEXT NOT NULL, + request_uris TEXT NOT NULL, + token_endpoint_auth_method VARCHAR(25) NOT NULL DEFAULT '', + request_object_signing_alg VARCHAR(10) NOT NULL DEFAULT '', + userinfo_signed_response_alg VARCHAR(10) NOT NULL DEFAULT '', + subject_type VARCHAR(15) NOT NULL DEFAULT '', + allowed_cors_origins TEXT NOT NULL, + pk_deprecated INTEGER NULL DEFAULT NULL, + pk_new TEXT PRIMARY KEY, + audience TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + frontchannel_logout_uri TEXT NOT NULL DEFAULT '', + frontchannel_logout_session_required INTEGER NOT NULL DEFAULT false, + post_logout_redirect_uris TEXT NOT NULL DEFAULT '', + backchannel_logout_uri TEXT NOT NULL DEFAULT '', + backchannel_logout_session_required INTEGER NOT NULL DEFAULT false, + metadata TEXT NOT NULL DEFAULT '{}', + token_endpoint_auth_signing_alg VARCHAR(10) NOT NULL DEFAULT '' +); + +-- UUID generation based on https://stackoverflow.com/a/61000724/12723442 +INSERT INTO "_hydra_client_tmp" ( + id, + client_name, + client_secret, + redirect_uris, + grant_types, + response_types, + scope, + owner, + policy_uri, + tos_uri, + client_uri, + logo_uri, + contacts, + client_secret_expires_at, + sector_identifier_uri, + jwks, + jwks_uri, + request_uris, + token_endpoint_auth_method, + request_object_signing_alg, + userinfo_signed_response_alg, + subject_type, + allowed_cors_origins, + pk_deprecated, + pk_new, + audience, + created_at, + updated_at, + frontchannel_logout_uri, + frontchannel_logout_session_required, + post_logout_redirect_uris, + backchannel_logout_uri, + backchannel_logout_session_required, + metadata, + token_endpoint_auth_signing_alg +) SELECT + id, + client_name, + client_secret, + redirect_uris, + grant_types, + response_types, + scope, + owner, + policy_uri, + tos_uri, + client_uri, + logo_uri, + contacts, + client_secret_expires_at, + sector_identifier_uri, + jwks, + jwks_uri, + request_uris, + token_endpoint_auth_method, + request_object_signing_alg, + userinfo_signed_response_alg, + subject_type, + allowed_cors_origins, + pk, + lower( + hex(randomblob(4)) || + '-' || hex(randomblob(2)) || + '-' || '4' || substr(hex(randomblob(2)), 2) || + '-' || substr('AB89', 1 + (abs(random()) % 4) , 1) || substr(hex(randomblob(2)), 2) || + '-' || hex(randomblob(6)) + ), + audience, + created_at, + updated_at, + frontchannel_logout_uri, + frontchannel_logout_session_required, + post_logout_redirect_uris, + backchannel_logout_uri, + backchannel_logout_session_required, + metadata, + token_endpoint_auth_signing_alg +FROM "hydra_client"; + +DROP INDEX hydra_client_id_idx; +DROP TABLE "hydra_client"; +ALTER TABLE "_hydra_client_tmp" RENAME TO "hydra_client"; + +CREATE UNIQUE INDEX hydra_client_id_idx ON hydra_client (id); diff --git a/persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.down.sql b/persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.down.sql new file mode 100644 index 00000000000..ab8b6cef150 --- /dev/null +++ b/persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.down.sql @@ -0,0 +1 @@ +ALTER TABLE hydra_client ALTER pk_new SET DEFAULT gen_random_uuid(); \ No newline at end of file diff --git a/persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.up.sql b/persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.up.sql new file mode 100644 index 00000000000..fdcc9e5f74a --- /dev/null +++ b/persistence/sql/migrations/20211004110002000000_change_client_primary_key.cockroach.up.sql @@ -0,0 +1 @@ +ALTER TABLE hydra_client ALTER pk_new DROP DEFAULT; diff --git a/persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.down.sql b/persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.down.sql new file mode 100644 index 00000000000..768746d74c2 --- /dev/null +++ b/persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE hydra_client DROP pk_new; +ALTER TABLE hydra_client RENAME pk_deprecated TO pk; diff --git a/persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.up.sql b/persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.up.sql new file mode 100644 index 00000000000..adb7b50679d --- /dev/null +++ b/persistence/sql/migrations/20211004110003000000_change_client_primary_key.cockroach.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE hydra_client DROP CONSTRAINT "primary"; +ALTER TABLE hydra_client ADD CONSTRAINT "primary" PRIMARY KEY (pk_new);