Skip to content

Commit

Permalink
client: Fix sql migration step for oidc (ory#919)
Browse files Browse the repository at this point in the history
A bug was introduced in beta.5 which caused the SQL migrations to fail if data existed in the database already. This patch resolves that and adds test cases for the migration steps by adding data after each migration.

Closes ory#918

Signed-off-by: arekkas <[email protected]>
  • Loading branch information
arekkas authored Jul 11, 2018
1 parent fccbfac commit ad5e8bc
Show file tree
Hide file tree
Showing 4 changed files with 305 additions and 59 deletions.
157 changes: 157 additions & 0 deletions client/manager_0_sql_migrations_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
})
}
}
107 changes: 78 additions & 29 deletions client/manager_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
22 changes: 13 additions & 9 deletions client/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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
}

Expand All @@ -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))
}
}
Loading

0 comments on commit ad5e8bc

Please sign in to comment.