Skip to content

Commit

Permalink
jwk: Auto-remove old keys when upgrading from < beta.7
Browse files Browse the repository at this point in the history
Closes #921

Signed-off-by: arekkas <aeneas@ory.am>
  • Loading branch information
arekkas committed Jul 14, 2018
1 parent 694b483 commit bf66d62
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 13 deletions.
15 changes: 14 additions & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ before finalizing the upgrade process.
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->


- [1.0.0-beta.7](#100-beta7)
- [Regenerated OpenID Connect ID Token cryptographic keys](#regenerated-openid-connect-id-token-cryptographic-keys)
- [1.0.0-beta.5](#100-beta5)
- [OAuth 2.0 Client Response Type changes](#oauth-20-client-response-type-changes)
- [Schema Changes](#schema-changes)
- [HTTP Error Payload](#http-error-payload)
- [OAuth 2.0 Clients must specify correct `token_endpoint_auth_method`](#oauth-20-clients-must-specify-correct-token_endpoint_auth_method)
Expand All @@ -19,7 +22,7 @@ before finalizing the upgrade process.
- [Breaking Changes](#breaking-changes)
- [Introspection API](#introspection-api)
- [Introspection is now capable of introspecting refresh tokens](#introspection-is-now-capable-of-introspecting-refresh-tokens)
- [Access Control & Warden API](#access-control-&-warden-api)
- [Access Control & Warden API](#access-control--warden-api)
- [Running the backwards compatible set up](#running-the-backwards-compatible-set-up)
- [Warden API](#warden-api)
- [Warden Groups](#warden-groups)
Expand Down Expand Up @@ -88,6 +91,16 @@ before finalizing the upgrade process.

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

## 1.0.0-beta.7

### Regenerated OpenID Connect ID Token cryptographic keys

This patch resolves an issue which caused the migration to fail from beta.4 to beta.5 / beta.6. The reason being that
the keys stored in the data store had mismatching `kid` values if generated by <= beta.5. This patch runs a SQL migration
script which removes the old key and then, after booting up ORY Hydra, regenerates it.

To apply this change, please run you must run `hydra migrate sql` against your database.

## 1.0.0-beta.5

This patch implements the OpenID Connect Dynamic Client registration specification and thus now supports client authentication
Expand Down
120 changes: 120 additions & 0 deletions jwk/manager_0_sql_migrations_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
* Copyright © 2015-2018 Aeneas Rekkas <aeneas+oss@aeneas.io>
*
* 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 <aeneas+oss@aeneas.io>
* @Copyright 2017-2018 Aeneas Rekkas <aeneas+oss@aeneas.io>
* @license Apache-2.0
*/

package jwk_test

import (
"fmt"
"log"
"testing"

"github.com/jmoiron/sqlx"
"github.com/ory/hydra/jwk"
"github.com/ory/sqlcon/dockertest"
"github.com/rubenv/sql-migrate"
"github.com/stretchr/testify/require"
)

var createJWKMigrations = []*migrate.Migration{
{
Id: "1-data",
Up: []string{
`INSERT INTO hydra_jwk (sid, kid, version, keydata) VALUES ('1-sid', '1-kid', 0, 'some-key')`,
},
Down: []string{
`DELETE FROM hydra_jwk WHERE sid='1-sid'`,
},
},
{
Id: "2-data",
Up: []string{
`INSERT INTO hydra_jwk (sid, kid, version, keydata, created_at) VALUES ('2-sid', '2-kid', 0, 'some-key', NOW())`,
},
Down: []string{
`DELETE FROM hydra_jwk WHERE sid='2-sid'`,
},
},
{
Id: "3-data",
Up: []string{
`INSERT INTO hydra_jwk (sid, kid, version, keydata, created_at) VALUES ('3-sid', '3-kid', 0, 'some-key', NOW())`,
},
Down: []string{
`DELETE FROM hydra_jwk WHERE sid='3-sid'`,
},
},
}

var migrations = &migrate.MemoryMigrationSource{
Migrations: []*migrate.Migration{
{Id: "0-data", Up: []string{"DROP TABLE IF EXISTS hydra_jwk"}},
jwk.Migrations.Migrations[0],
createJWKMigrations[0],
jwk.Migrations.Migrations[1],
createJWKMigrations[1],
jwk.Migrations.Migrations[2],
createJWKMigrations[2],
},
}

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_jwk_migration_integration")
for step := range migrations.Migrations {
t.Run(fmt.Sprintf("step=%d", step), func(t *testing.T) {
n, err := migrate.ExecMax(db.DB, db.DriverName(), migrations, migrate.Up, 1)
require.NoError(t, err)
require.Equal(t, n, 1)
})
}

for step := range migrations.Migrations {
t.Run(fmt.Sprintf("step=%d", step), func(t *testing.T) {
n, err := migrate.ExecMax(db.DB, db.DriverName(), migrations, migrate.Down, 1)
require.NoError(t, err)
require.Equal(t, n, 1)
})
}
})
}
}
14 changes: 11 additions & 3 deletions jwk/manager_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type SQLManager struct {
Cipher *AEAD
}

var migrations = &migrate.MemoryMigrationSource{
var Migrations = &migrate.MemoryMigrationSource{
Migrations: []*migrate.Migration{
{
Id: "1",
Expand All @@ -63,6 +63,14 @@ var migrations = &migrate.MemoryMigrationSource{
`ALTER TABLE hydra_jwk DROP COLUMN created_at`,
},
},
// See https://github.com/ory/hydra/issues/921
{
Id: "3",
Up: []string{
`DELETE FROM hydra_jwk WHERE sid='hydra.openid.id-token'`,
},
Down: []string{},
},
},
}

Expand All @@ -76,9 +84,9 @@ type sqlData struct {

func (m *SQLManager) CreateSchemas() (int, error) {
migrate.SetTable("hydra_jwk_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, 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
17 changes: 8 additions & 9 deletions jwk/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ func connectToPG() {
}

s := &SQLManager{DB: db, Cipher: &AEAD{Key: encryptionKey}}
if _, err := s.CreateSchemas(); err != nil {
log.Fatalf("Could not create schema: %v", err)
}

managers["postgres"] = s
}

Expand All @@ -76,11 +72,6 @@ func connectToMySQL() {
}

s := &SQLManager{DB: db, Cipher: &AEAD{Key: encryptionKey}}

if _, err := s.CreateSchemas(); err != nil {
log.Fatalf("Could not create schema: %v", err)
}

managers["mysql"] = s
}

Expand All @@ -89,6 +80,10 @@ func TestManagerKey(t *testing.T) {
require.NoError(t, err)

for name, m := range managers {
if m, ok := m.(*SQLManager); ok {
_, err := m.CreateSchemas()
require.NoError(t, err)
}
t.Run(fmt.Sprintf("case=%s", name), TestHelperManagerKey(m, ks, "TestManagerKey"))
}
}
Expand All @@ -99,6 +94,10 @@ func TestManagerKeySet(t *testing.T) {
ks.Key("private")

for name, m := range managers {
if m, ok := m.(*SQLManager); ok {
_, err := m.CreateSchemas()
require.NoError(t, err)
}
t.Run(fmt.Sprintf("case=%s", name), TestHelperManagerKeySet(m, ks, "TestManagerKeySet"))
}
}

0 comments on commit bf66d62

Please sign in to comment.