client: Fix sql migration step for oidc

client/manager_0_sql_migrations_test.go

@@ -0,0 +1,157 @@
+/*
+ * Copyright © 2015-2018 Aeneas Rekkas <[email protected]>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * @author		Aeneas Rekkas <[email protected]>
+ * @Copyright 	2017-2018 Aeneas Rekkas <[email protected]>
+ * @license 	Apache-2.0
+ */
+
+package client_test
+
+import (
+	"fmt"
+	"log"
+	"testing"
+
+	"github.com/jmoiron/sqlx"
+	"github.com/ory/fosite"
+	"github.com/ory/hydra/client"
+	"github.com/ory/sqlcon/dockertest"
+	"github.com/rubenv/sql-migrate"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+var createClientMigrations = []*migrate.Migration{
+	{
+		Id: "1-data",
+		Up: []string{
+			`INSERT INTO hydra_client (id, client_name, client_secret, redirect_uris, grant_types, response_types, scope, owner, policy_uri, tos_uri, client_uri, logo_uri, contacts, public) VALUES ('1-data', 'some-client', 'abcdef', 'http://localhost|http://google', 'authorize_code|implicit', 'token|id_token', 'foo|bar', 'aeneas', 'http://policy', 'http://tos', 'http://client', 'http://logo', 'aeneas|foo', true)`,
+		},
+		Down: []string{
+			`DELETE FROM hydra_client WHERE id='1-data'`,
+		},
+	},
+	{
+		Id: "2-data",
+		Up: []string{
+			`INSERT INTO hydra_client (id, client_name, client_secret, redirect_uris, grant_types, response_types, scope, owner, policy_uri, tos_uri, client_uri, logo_uri, contacts, public, client_secret_expires_at) VALUES ('2-data', 'some-client', 'abcdef', 'http://localhost|http://google', 'authorize_code|implicit', 'token|id_token', 'foo|bar', 'aeneas', 'http://policy', 'http://tos', 'http://client', 'http://logo', 'aeneas|foo', true, 0)`,
+		},
+		Down: []string{
+			`DELETE FROM hydra_client WHERE id='2-data'`,
+		},
+	},
+	{
+		Id: "3-data",
+		Up: []string{
+			`INSERT INTO hydra_client (id, client_name, client_secret, redirect_uris, grant_types, response_types, scope, owner, policy_uri, tos_uri, client_uri, logo_uri, contacts, public, client_secret_expires_at, sector_identifier_uri, jwks, jwks_uri, token_endpoint_auth_method, request_uris, request_object_signing_alg, userinfo_signed_response_alg) VALUES ('3-data', 'some-client', 'abcdef', 'http://localhost|http://google', 'authorize_code|implicit', 'token|id_token', 'foo|bar', 'aeneas', 'http://policy', 'http://tos', 'http://client', 'http://logo', 'aeneas|foo', true, 0, 'http://sector', '{"keys": []}', 'http://jwks', 'client_secret', 'http://uri1|http://uri2', 'rs256', 'rs526')`,
+		},
+		Down: []string{
+			`DELETE FROM hydra_client WHERE id='3-data'`,
+		},
+	},
+	{
+		Id: "4-data",
+		Up: []string{
+			`INSERT INTO hydra_client (id, client_name, client_secret, redirect_uris, grant_types, response_types, scope, owner, policy_uri, tos_uri, client_uri, logo_uri, contacts, public, client_secret_expires_at, sector_identifier_uri, jwks, jwks_uri, token_endpoint_auth_method, request_uris, request_object_signing_alg, userinfo_signed_response_alg) VALUES ('4-data', 'some-client', 'abcdef', 'http://localhost|http://google', 'authorize_code|implicit', 'token|id_token', 'foo|bar', 'aeneas', 'http://policy', 'http://tos', 'http://client', 'http://logo', 'aeneas|foo', true, 0, 'http://sector', '{"keys": []}', 'http://jwks', 'client_secret', 'http://uri1|http://uri2', 'rs256', 'rs526')`,
+		},
+		Down: []string{
+			`DELETE FROM hydra_client WHERE id='4-data'`,
+		},
+	},
+}
+
+var migrations = map[string]*migrate.MemoryMigrationSource{
+	"mysql": {
+		Migrations: []*migrate.Migration{
+			{Id: "0-data", Up: []string{"DROP TABLE IF EXISTS hydra_client"}},
+			client.Migrations["mysql"].Migrations[0],
+			createClientMigrations[0],
+			client.Migrations["mysql"].Migrations[1],
+			createClientMigrations[1],
+			client.Migrations["mysql"].Migrations[2],
+			createClientMigrations[2],
+			client.Migrations["mysql"].Migrations[3],
+			createClientMigrations[3],
+		},
+	},
+	"postgres": {
+		Migrations: []*migrate.Migration{
+			{Id: "0-data", Up: []string{"DROP TABLE IF EXISTS hydra_client"}},
+			client.Migrations["postgres"].Migrations[0],
+			createClientMigrations[0],
+			client.Migrations["postgres"].Migrations[1],
+			createClientMigrations[1],
+			client.Migrations["postgres"].Migrations[2],
+			createClientMigrations[2],
+			client.Migrations["postgres"].Migrations[3],
+			createClientMigrations[3],
+		},
+	},
+}
+
+func TestMigrations(t *testing.T) {
+	var dbs = map[string]*sqlx.DB{}
+	if testing.Short() {
+		return
+	}
+
+	dockertest.Parallel([]func(){
+		func() {
+			db, err := dockertest.ConnectToTestPostgreSQL()
+			if err != nil {
+				log.Fatalf("Could not connect to database: %v", err)
+			}
+			dbs["postgres"] = db
+		},
+		func() {
+			db, err := dockertest.ConnectToTestMySQL()
+			if err != nil {
+				log.Fatalf("Could not connect to database: %v", err)
+			}
+			dbs["mysql"] = db
+		},
+	})
+
+	for k, db := range dbs {
+		t.Run(fmt.Sprintf("database=%s", k), func(t *testing.T) {
+			migrate.SetTable("hydra_client_migration_integration")
+			for step := range migrations[k].Migrations {
+				t.Run(fmt.Sprintf("step=%d", step), func(t *testing.T) {
+					n, err := migrate.ExecMax(db.DB, db.DriverName(), migrations[k], migrate.Up, 1)
+					require.NoError(t, err)
+					require.Equal(t, n, 1)
+				})
+			}
+
+			for _, key := range []string{"1-data", "2-data", "3-data", "4-data"} {
+				t.Run("client="+key, func(t *testing.T) {
+					s := &client.SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
+					c, err := s.GetConcreteClient(key)
+					require.NoError(t, err)
+					assert.EqualValues(t, c.ID, key)
+				})
+			}
+
+			for step := range migrations[k].Migrations {
+				t.Run(fmt.Sprintf("step=%d", step), func(t *testing.T) {
+					n, err := migrate.ExecMax(db.DB, db.DriverName(), migrations[k], migrate.Down, 1)
+					require.NoError(t, err)
+					require.Equal(t, n, 1)
+				})
+			}
+		})
+	}
+}

client/manager_sql.go

@@ -35,11 +35,10 @@ import (
 	"gopkg.in/square/go-jose.v2"
 )
 
-var migrations = &migrate.MemoryMigrationSource{
-	Migrations: []*migrate.Migration{
-		{
-			Id: "1",
-			Up: []string{`CREATE TABLE IF NOT EXISTS hydra_client (
+var sharedMigrations = []*migrate.Migration{
+	{
+		Id: "1",
+		Up: []string{`CREATE TABLE IF NOT EXISTS hydra_client (
 	id      	varchar(255) NOT NULL PRIMARY KEY,
 	client_name  	text NOT NULL,
 	client_secret  	text NOT NULL,
@@ -55,41 +54,85 @@ var migrations = &migrate.MemoryMigrationSource{
 	contacts  		text NOT NULL,
 	public  		boolean NOT NULL
 )`},
-			Down: []string{
-				"DROP TABLE hydra_client",
-			},
+		Down: []string{
+			"DROP TABLE hydra_client",
+		},
+	},
+	{
+		Id: "2",
+		Up: []string{
+			`ALTER TABLE hydra_client ADD client_secret_expires_at INTEGER NOT NULL DEFAULT 0`,
+		},
+		Down: []string{
+			`ALTER TABLE hydra_client DROP COLUMN client_secret_expires_at`,
+		},
+	},
+	{
+		Id: "3",
+		Up: []string{
+			`ALTER TABLE hydra_client ADD sector_identifier_uri TEXT`,
+			`ALTER TABLE hydra_client ADD jwks TEXT`,
+			`ALTER TABLE hydra_client ADD jwks_uri TEXT`,
+			`ALTER TABLE hydra_client ADD request_uris TEXT`,
+			`ALTER TABLE hydra_client ADD token_endpoint_auth_method VARCHAR(25) NOT NULL DEFAULT ''`,
+			`ALTER TABLE hydra_client ADD request_object_signing_alg  VARCHAR(10) NOT NULL DEFAULT ''`,
+			`ALTER TABLE hydra_client ADD userinfo_signed_response_alg VARCHAR(10) NOT NULL DEFAULT ''`,
 		},
+		Down: []string{
+			`ALTER TABLE hydra_client DROP COLUMN sector_identifier_uri`,
+			`ALTER TABLE hydra_client DROP COLUMN jwks`,
+			`ALTER TABLE hydra_client DROP COLUMN jwks_uri`,
+			`ALTER TABLE hydra_client DROP COLUMN token_endpoint_auth_method`,
+			`ALTER TABLE hydra_client DROP COLUMN request_uris`,
+			`ALTER TABLE hydra_client DROP COLUMN request_object_signing_alg`,
+			`ALTER TABLE hydra_client DROP COLUMN userinfo_signed_response_alg`,
+		},
+	},
+}
+
+var Migrations = map[string]*migrate.MemoryMigrationSource{
+	"mysql": {Migrations: []*migrate.Migration{
+		sharedMigrations[0],
+		sharedMigrations[1],
+		sharedMigrations[2],
 		{
-			Id: "2",
+			Id: "4",
 			Up: []string{
-				`ALTER TABLE hydra_client ADD client_secret_expires_at INTEGER NOT NULL DEFAULT 0`,
+				`UPDATE hydra_client SET sector_identifier_uri='', jwks='', jwks_uri='', request_uris=''`,
+				`ALTER TABLE hydra_client MODIFY sector_identifier_uri TEXT NOT NULL`,
+				`ALTER TABLE hydra_client MODIFY jwks TEXT NOT NULL`,
+				`ALTER TABLE hydra_client MODIFY jwks_uri TEXT NOT NULL`,
+				`ALTER TABLE hydra_client MODIFY request_uris TEXT NOT NULL`,
 			},
 			Down: []string{
-				`ALTER TABLE hydra_client DROP COLUMN client_secret_expires_at`,
+				`ALTER TABLE hydra_client MODIFY sector_identifier_uri TEXT`,
+				`ALTER TABLE hydra_client MODIFY jwks TEXT`,
+				`ALTER TABLE hydra_client MODIFY jwks_uri TEXT`,
+				`ALTER TABLE hydra_client MODIFY request_uris TEXT`,
 			},
 		},
+	}},
+	"postgres": {Migrations: []*migrate.Migration{
+		sharedMigrations[0],
+		sharedMigrations[1],
+		sharedMigrations[2],
 		{
-			Id: "3",
+			Id: "4",
 			Up: []string{
-				`ALTER TABLE hydra_client ADD sector_identifier_uri TEXT NOT NULL`,
-				`ALTER TABLE hydra_client ADD jwks TEXT NOT NULL`,
-				`ALTER TABLE hydra_client ADD jwks_uri TEXT NOT NULL`,
-				`ALTER TABLE hydra_client ADD token_endpoint_auth_method  VARCHAR(25) NOT NULL`,
-				`ALTER TABLE hydra_client ADD request_uris TEXT NOT NULL`,
-				`ALTER TABLE hydra_client ADD request_object_signing_alg  VARCHAR(10) NOT NULL`,
-				`ALTER TABLE hydra_client ADD userinfo_signed_response_alg VARCHAR(10) NOT NULL`,
+				`UPDATE hydra_client SET sector_identifier_uri='', jwks='', jwks_uri='', request_uris=''`,
+				`ALTER TABLE hydra_client ALTER COLUMN sector_identifier_uri SET NOT NULL`,
+				`ALTER TABLE hydra_client ALTER COLUMN jwks SET NOT NULL`,
+				`ALTER TABLE hydra_client ALTER COLUMN jwks_uri SET NOT NULL`,
+				`ALTER TABLE hydra_client ALTER COLUMN request_uris SET NOT NULL`,
 			},
 			Down: []string{
-				`ALTER TABLE hydra_client DROP COLUMN sector_identifier_uri`,
-				`ALTER TABLE hydra_client DROP COLUMN jwks`,
-				`ALTER TABLE hydra_client DROP COLUMN jwks_uri`,
-				`ALTER TABLE hydra_client DROP COLUMN token_endpoint_auth_method`,
-				`ALTER TABLE hydra_client DROP COLUMN request_uris`,
-				`ALTER TABLE hydra_client DROP COLUMN request_object_signing_alg`,
-				`ALTER TABLE hydra_client DROP COLUMN userinfo_signed_response_alg`,
+				`ALTER TABLE hydra_client ALTER COLUMN sector_identifier_uri DROP NOT NULL`,
+				`ALTER TABLE hydra_client ALTER COLUMN jwks DROP NOT NULL`,
+				`ALTER TABLE hydra_client ALTER COLUMN jwks_uri DROP NOT NULL`,
+				`ALTER TABLE hydra_client ALTER COLUMN request_uris DROP NOT NULL`,
 			},
 		},
-	},
+	}},
 }
 
 type SQLManager struct {
@@ -221,10 +264,16 @@ func (d *sqlData) ToClient() (*Client, error) {
 }
 
 func (m *SQLManager) CreateSchemas() (int, error) {
+	database := m.DB.DriverName()
+	switch database {
+	case "pgx", "pq":
+		database = "postgres"
+	}
+
 	migrate.SetTable("hydra_client_migration")
-	n, err := migrate.Exec(m.DB.DB, m.DB.DriverName(), migrations, migrate.Up)
+	n, err := migrate.Exec(m.DB.DB, m.DB.DriverName(), Migrations[database], migrate.Up)
 	if err != nil {
-		return 0, errors.Wrapf(err, "Could not migrate sql schema, applied %d migrations", n)
+		return 0, errors.Wrapf(err, "Could not migrate sql schema, applied %d Migrations", n)
 	}
 	return n, nil
 }

client/manager_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/ory/fosite"
 	. "github.com/ory/hydra/client"
 	"github.com/ory/sqlcon/dockertest"
+	"github.com/stretchr/testify/require"
 )
 
 var clientManagers = map[string]Manager{}
@@ -60,10 +61,6 @@ func connectToMySQL() {
 	}
 
 	s := &SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
-	if _, err := s.CreateSchemas(); err != nil {
-		log.Fatalf("Could not create schema: %v", err)
-	}
-
 	clientManagers["mysql"] = s
 }
 
@@ -74,28 +71,35 @@ func connectToPG() {
 	}
 
 	s := &SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
-
-	if _, err := s.CreateSchemas(); err != nil {
-		log.Fatalf("Could not create schema: %v", err)
-	}
-
 	clientManagers["postgres"] = s
 }
 
 func TestCreateGetDeleteClient(t *testing.T) {
 	for k, m := range clientManagers {
+		if s, ok := m.(*SQLManager); ok {
+			_, err := s.CreateSchemas()
+			require.NoError(t, err)
+		}
 		t.Run(fmt.Sprintf("case=%s", k), TestHelperCreateGetDeleteClient(k, m))
 	}
 }
 
 func TestClientAutoGenerateKey(t *testing.T) {
 	for k, m := range clientManagers {
+		if s, ok := m.(*SQLManager); ok {
+			_, err := s.CreateSchemas()
+			require.NoError(t, err)
+		}
 		t.Run(fmt.Sprintf("case=%s", k), TestHelperClientAutoGenerateKey(k, m))
 	}
 }
 
 func TestAuthenticateClient(t *testing.T) {
 	for k, m := range clientManagers {
+		if s, ok := m.(*SQLManager); ok {
+			_, err := s.CreateSchemas()
+			require.NoError(t, err)
+		}
 		t.Run(fmt.Sprintf("case=%s", k), TestHelperClientAuthenticate(k, m))
 	}
 }