From 9796a1e648a409ae79f7dd930fddb072e1cc27de Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Tue, 12 Jan 2016 17:17:50 -0800 Subject: [PATCH] *: add migration to update JSON fields and require postgres 9.4+ The "redirectURLs" field in the client metadata has been updated to the correct "redirect_uris". To allow backwards compatibility use Postgres' JSON features to update the actual JSON in the text field. json_build_object was introduced in Postgres 9.4. So update the documentations to require at least this version. --- Documentation/getting-started.md | 2 +- README.md | 2 +- db/client.go | 45 +---------- db/migrate_test.go | 81 ++++++++++++++++++- .../0010_client_metadata_field_changed.sql | 9 +++ db/migrations/assets.go | 77 +++++++++++------- 6 files changed, 143 insertions(+), 73 deletions(-) create mode 100644 db/migrations/0010_client_metadata_field_changed.sql diff --git a/Documentation/getting-started.md b/Documentation/getting-started.md index 679959ac..b5c812aa 100644 --- a/Documentation/getting-started.md +++ b/Documentation/getting-started.md @@ -14,7 +14,7 @@ We'll also start the example web app, so we can try registering and logging in. Before continuing, you must have the following installed on your system: * Go 1.4 or greater -* Postgres 9.0 or greater (this guide also assumes that Postgres is up and running) +* Postgres 9.4 or greater (this guide also assumes that Postgres is up and running) In addition, if you wish to try out authenticating against Google's OIDC backend, you must have a new client registered with Google: diff --git a/README.md b/README.md index 8b871908..27ccf988 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ dex consists of multiple components: - configure identity provider connectors - administer OIDC client identities - **database**; a database is used to for persistent storage for keys, users, - OAuth sessions and other data. Currently Postgres is the only supported + OAuth sessions and other data. Currently Postgres (9.4+) is the only supported database. A typical dex deployment consists of N dex-workers behind a load balanacer, and one dex-overlord. diff --git a/db/client.go b/db/client.go index 89a1667f..2912783b 100644 --- a/db/client.go +++ b/db/client.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "net/url" "reflect" "github.com/coreos/go-oidc/oidc" @@ -49,7 +48,7 @@ func newClientIdentityModel(id string, secret []byte, meta *oidc.ClientMetadata) return nil, err } - bmeta, err := json.Marshal(newClientMetadataJSON(meta)) + bmeta, err := json.Marshal(meta) if err != nil { return nil, err } @@ -70,38 +69,6 @@ type clientIdentityModel struct { DexAdmin bool `db:"dex_admin"` } -func newClientMetadataJSON(cm *oidc.ClientMetadata) *clientMetadataJSON { - cmj := clientMetadataJSON{ - RedirectURLs: make([]string, len(cm.RedirectURLs)), - } - - for i, u := range cm.RedirectURLs { - cmj.RedirectURLs[i] = (&u).String() - } - - return &cmj -} - -type clientMetadataJSON struct { - RedirectURLs []string `json:"redirectURLs"` -} - -func (cmj clientMetadataJSON) ClientMetadata() (*oidc.ClientMetadata, error) { - cm := oidc.ClientMetadata{ - RedirectURLs: make([]url.URL, len(cmj.RedirectURLs)), - } - - for i, us := range cmj.RedirectURLs { - up, err := url.Parse(us) - if err != nil { - return nil, err - } - cm.RedirectURLs[i] = *up - } - - return &cm, nil -} - func (m *clientIdentityModel) ClientIdentity() (*oidc.ClientIdentity, error) { ci := oidc.ClientIdentity{ Credentials: oidc.ClientCredentials{ @@ -110,18 +77,10 @@ func (m *clientIdentityModel) ClientIdentity() (*oidc.ClientIdentity, error) { }, } - var cmj clientMetadataJSON - err := json.Unmarshal([]byte(m.Metadata), &cmj) - if err != nil { + if err := json.Unmarshal([]byte(m.Metadata), &ci.Metadata); err != nil { return nil, err } - cm, err := cmj.ClientMetadata() - if err != nil { - return nil, err - } - - ci.Metadata = *cm return &ci, nil } diff --git a/db/migrate_test.go b/db/migrate_test.go index 41cbd74f..25c08968 100644 --- a/db/migrate_test.go +++ b/db/migrate_test.go @@ -3,6 +3,7 @@ package db import ( "fmt" "os" + "strconv" "testing" "github.com/go-gorp/gorp" @@ -25,7 +26,7 @@ func initDB(dsn string) *gorp.DbMap { func TestGetPlannedMigrations(t *testing.T) { dsn := os.Getenv("DEX_TEST_DSN") if dsn == "" { - t.Logf("Test will not run without DEX_TEST_DSN environment variable.") + t.Skip("Test will not run without DEX_TEST_DSN environment variable.") return } dbMap := initDB(dsn) @@ -40,3 +41,81 @@ func TestGetPlannedMigrations(t *testing.T) { t.Fatalf("expected non-empty migrations") } } + +func TestMigrateClientMetadata(t *testing.T) { + dsn := os.Getenv("DEX_TEST_DSN") + if dsn == "" { + t.Skip("Test will not run without DEX_TEST_DSN environment variable.") + return + } + dbMap := initDB(dsn) + + nMigrations := 9 + n, err := MigrateMaxMigrations(dbMap, nMigrations) + if err != nil { + t.Fatalf("failed to perform initial migration: %v", err) + } + if n != nMigrations { + t.Fatalf("expected to perform %d migrations, got %d", nMigrations, n) + } + + tests := []struct { + before string + after string + }{ + // only update rows without a "redirect_uris" key + { + `{"redirectURLs":["foo"]}`, + `{"redirectURLs" : ["foo"], "redirect_uris" : ["foo"]}`, + }, + { + `{"redirectURLs":["foo","bar"]}`, + `{"redirectURLs" : ["foo","bar"], "redirect_uris" : ["foo","bar"]}`, + }, + { + `{"redirect_uris":["foo"],"another_field":8}`, + `{"redirect_uris":["foo"],"another_field":8}`, + }, + { + `{"redirectURLs" : ["foo"], "redirect_uris" : ["foo"]}`, + `{"redirectURLs" : ["foo"], "redirect_uris" : ["foo"]}`, + }, + } + + for i, tt := range tests { + model := &clientIdentityModel{ + ID: strconv.Itoa(i), + Secret: []byte("verysecret"), + Metadata: tt.before, + } + if err := dbMap.Insert(model); err != nil { + t.Fatalf("could not insert model: %v", err) + } + } + + n, err = MigrateMaxMigrations(dbMap, 1) + if err != nil { + t.Fatalf("failed to perform initial migration: %v", err) + } + if n != 1 { + t.Fatalf("expected to perform 1 migration, got %d", n) + } + + for i, tt := range tests { + id := strconv.Itoa(i) + m, err := dbMap.Get(clientIdentityModel{}, id) + if err != nil { + t.Errorf("case %d: failed to get model: %err", i, err) + continue + } + cim, ok := m.(*clientIdentityModel) + if !ok { + t.Errorf("case %d: unrecognized model type: %T", i, m) + continue + } + + if cim.Metadata != tt.after { + t.Errorf("case %d: want=%q, got=%q", i, tt.after, cim.Metadata) + } + } +} diff --git a/db/migrations/0010_client_metadata_field_changed.sql b/db/migrations/0010_client_metadata_field_changed.sql new file mode 100644 index 00000000..a561ae39 --- /dev/null +++ b/db/migrations/0010_client_metadata_field_changed.sql @@ -0,0 +1,9 @@ +-- +migrate Up +UPDATE client_identity +SET metadata = text( + json_build_object( + 'redirectURLs', json(json(metadata)->>'redirectURLs'), + 'redirect_uris', json(json(metadata)->>'redirectURLs') + ) + ) +WHERE (json(metadata)->>'redirect_uris') IS NULL; diff --git a/db/migrations/assets.go b/db/migrations/assets.go index 7da1604b..13b7c535 100644 --- a/db/migrations/assets.go +++ b/db/migrations/assets.go @@ -9,6 +9,7 @@ // 0007_session_scope.sql // 0008_users_active_or_inactive.sql // 0009_key_not_primary_key.sql +// 0010_client_metadata_field_changed.sql // DO NOT EDIT! package migrations @@ -91,7 +92,7 @@ func dbMigrations0001_initial_migrationSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0001_initial_migration.sql", size: 1388, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0001_initial_migration.sql", size: 1388, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -111,7 +112,7 @@ func dbMigrations0002_dex_adminSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0002_dex_admin.sql", size: 126, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0002_dex_admin.sql", size: 126, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -131,7 +132,7 @@ func dbMigrations0003_user_created_atSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0003_user_created_at.sql", size: 111, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0003_user_created_at.sql", size: 111, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -151,7 +152,7 @@ func dbMigrations0004_session_nonceSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0004_session_nonce.sql", size: 60, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0004_session_nonce.sql", size: 60, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -171,7 +172,7 @@ func dbMigrations0005_refresh_token_createSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0005_refresh_token_create.sql", size: 506, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0005_refresh_token_create.sql", size: 506, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -191,7 +192,7 @@ func dbMigrations0006_user_email_uniqueSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0006_user_email_unique.sql", size: 99, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0006_user_email_unique.sql", size: 99, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -211,7 +212,7 @@ func dbMigrations0007_session_scopeSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0007_session_scope.sql", size: 60, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0007_session_scope.sql", size: 60, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -231,7 +232,7 @@ func dbMigrations0008_users_active_or_inactiveSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0008_users_active_or_inactive.sql", size: 110, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0008_users_active_or_inactive.sql", size: 110, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -251,7 +252,27 @@ func dbMigrations0009_key_not_primary_keySql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "db/migrations/0009_key_not_primary_key.sql", size: 182, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + info := bindataFileInfo{name: "db/migrations/0009_key_not_primary_key.sql", size: 182, mode: os.FileMode(436), modTime: time.Unix(1, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var _dbMigrations0010_client_metadata_field_changedSql = []byte("\x1f\x8b\x08\x00\x00\x09\x6e\x88\x00\xff\xd2\xd5\x55\xd0\xce\xcd\x4c\x2f\x4a\x2c\x49\x55\x08\x2d\xe0\x0a\x0d\x70\x71\x0c\x71\x55\x48\xce\xc9\x4c\xcd\x2b\x89\xcf\x4c\x01\x92\x99\x25\x95\x5c\xc1\xae\x21\x0a\xb9\xa9\x25\x89\x29\x89\x25\x89\x0a\xb6\x0a\x25\xa9\x15\x25\x1a\x5c\x0a\x40\x90\x55\x9c\x9f\x17\x9f\x54\x9a\x99\x93\x12\x9f\x9f\x94\x95\x9a\x0c\x15\x06\x01\xf5\xa2\xd4\x94\xcc\x22\xa0\x50\x68\x90\x4f\xb1\xba\x0e\x58\xa9\x06\x98\x80\x99\xa4\xa9\x6b\x67\x87\xaa\x4a\x53\x07\x53\x7b\x7c\x69\x51\x26\xd1\xfa\xc1\xda\x81\xa4\x26\x57\xb8\x87\x6b\x90\xab\x02\x1e\x0d\x10\x73\x35\x15\x3c\x83\x15\xfc\x42\x7d\x7c\xac\xb9\x00\x01\x00\x00\xff\xff\xeb\xe6\x9a\x19\x0b\x01\x00\x00") + +func dbMigrations0010_client_metadata_field_changedSqlBytes() ([]byte, error) { + return bindataRead( + _dbMigrations0010_client_metadata_field_changedSql, + "db/migrations/0010_client_metadata_field_changed.sql", + ) +} + +func dbMigrations0010_client_metadata_field_changedSql() (*asset, error) { + bytes, err := dbMigrations0010_client_metadata_field_changedSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "db/migrations/0010_client_metadata_field_changed.sql", size: 267, mode: os.FileMode(436), modTime: time.Unix(1, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -308,15 +329,16 @@ func AssetNames() []string { // _bindata is a table, holding each asset generator, mapped to its name. var _bindata = map[string]func() (*asset, error){ - "db/migrations/0001_initial_migration.sql": dbMigrations0001_initial_migrationSql, - "db/migrations/0002_dex_admin.sql": dbMigrations0002_dex_adminSql, - "db/migrations/0003_user_created_at.sql": dbMigrations0003_user_created_atSql, - "db/migrations/0004_session_nonce.sql": dbMigrations0004_session_nonceSql, - "db/migrations/0005_refresh_token_create.sql": dbMigrations0005_refresh_token_createSql, - "db/migrations/0006_user_email_unique.sql": dbMigrations0006_user_email_uniqueSql, - "db/migrations/0007_session_scope.sql": dbMigrations0007_session_scopeSql, - "db/migrations/0008_users_active_or_inactive.sql": dbMigrations0008_users_active_or_inactiveSql, - "db/migrations/0009_key_not_primary_key.sql": dbMigrations0009_key_not_primary_keySql, + "db/migrations/0001_initial_migration.sql": dbMigrations0001_initial_migrationSql, + "db/migrations/0002_dex_admin.sql": dbMigrations0002_dex_adminSql, + "db/migrations/0003_user_created_at.sql": dbMigrations0003_user_created_atSql, + "db/migrations/0004_session_nonce.sql": dbMigrations0004_session_nonceSql, + "db/migrations/0005_refresh_token_create.sql": dbMigrations0005_refresh_token_createSql, + "db/migrations/0006_user_email_unique.sql": dbMigrations0006_user_email_uniqueSql, + "db/migrations/0007_session_scope.sql": dbMigrations0007_session_scopeSql, + "db/migrations/0008_users_active_or_inactive.sql": dbMigrations0008_users_active_or_inactiveSql, + "db/migrations/0009_key_not_primary_key.sql": dbMigrations0009_key_not_primary_keySql, + "db/migrations/0010_client_metadata_field_changed.sql": dbMigrations0010_client_metadata_field_changedSql, } // AssetDir returns the file names below a certain @@ -362,15 +384,16 @@ type bintree struct { var _bintree = &bintree{nil, map[string]*bintree{ "db": &bintree{nil, map[string]*bintree{ "migrations": &bintree{nil, map[string]*bintree{ - "0001_initial_migration.sql": &bintree{dbMigrations0001_initial_migrationSql, map[string]*bintree{}}, - "0002_dex_admin.sql": &bintree{dbMigrations0002_dex_adminSql, map[string]*bintree{}}, - "0003_user_created_at.sql": &bintree{dbMigrations0003_user_created_atSql, map[string]*bintree{}}, - "0004_session_nonce.sql": &bintree{dbMigrations0004_session_nonceSql, map[string]*bintree{}}, - "0005_refresh_token_create.sql": &bintree{dbMigrations0005_refresh_token_createSql, map[string]*bintree{}}, - "0006_user_email_unique.sql": &bintree{dbMigrations0006_user_email_uniqueSql, map[string]*bintree{}}, - "0007_session_scope.sql": &bintree{dbMigrations0007_session_scopeSql, map[string]*bintree{}}, - "0008_users_active_or_inactive.sql": &bintree{dbMigrations0008_users_active_or_inactiveSql, map[string]*bintree{}}, - "0009_key_not_primary_key.sql": &bintree{dbMigrations0009_key_not_primary_keySql, map[string]*bintree{}}, + "0001_initial_migration.sql": &bintree{dbMigrations0001_initial_migrationSql, map[string]*bintree{}}, + "0002_dex_admin.sql": &bintree{dbMigrations0002_dex_adminSql, map[string]*bintree{}}, + "0003_user_created_at.sql": &bintree{dbMigrations0003_user_created_atSql, map[string]*bintree{}}, + "0004_session_nonce.sql": &bintree{dbMigrations0004_session_nonceSql, map[string]*bintree{}}, + "0005_refresh_token_create.sql": &bintree{dbMigrations0005_refresh_token_createSql, map[string]*bintree{}}, + "0006_user_email_unique.sql": &bintree{dbMigrations0006_user_email_uniqueSql, map[string]*bintree{}}, + "0007_session_scope.sql": &bintree{dbMigrations0007_session_scopeSql, map[string]*bintree{}}, + "0008_users_active_or_inactive.sql": &bintree{dbMigrations0008_users_active_or_inactiveSql, map[string]*bintree{}}, + "0009_key_not_primary_key.sql": &bintree{dbMigrations0009_key_not_primary_keySql, map[string]*bintree{}}, + "0010_client_metadata_field_changed.sql": &bintree{dbMigrations0010_client_metadata_field_changedSql, map[string]*bintree{}}, }}, }}, }}