From c9a3933677f9284c1bbc958988c67495a7d5a927 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 30 Jul 2026 20:59:56 +0800 Subject: [PATCH] fix(auth): reject unsafe identity binding migrations Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../V45__identity_binding_v2_expand.sql | 79 +++++++- ...dentityBindingV2MigrationPostgresTest.java | 169 ++++++++++++++++++ 2 files changed, 246 insertions(+), 2 deletions(-) diff --git a/server/skillhub-app/src/main/resources/db/migration/V45__identity_binding_v2_expand.sql b/server/skillhub-app/src/main/resources/db/migration/V45__identity_binding_v2_expand.sql index 58c62980..6ae1a59c 100644 --- a/server/skillhub-app/src/main/resources/db/migration/V45__identity_binding_v2_expand.sql +++ b/server/skillhub-app/src/main/resources/db/migration/V45__identity_binding_v2_expand.sql @@ -30,6 +30,9 @@ ALTER TABLE identity_binding DO $$ DECLARE duplicate_summary TEXT; + account_summary TEXT; + identifier_summary TEXT; + violations TEXT[] := ARRAY[]::TEXT[]; BEGIN SELECT string_agg( format('%s/%s (%s bindings)', user_id, provider_code, binding_count), @@ -40,13 +43,85 @@ BEGIN FROM identity_binding GROUP BY user_id, provider_code HAVING COUNT(*) > 1 + ORDER BY user_id, provider_code LIMIT 20 ) duplicates; IF duplicate_summary IS NOT NULL THEN + violations := array_append( + violations, + format( + 'multiple active bindings for user/provider: %s', + duplicate_summary + ) + ); + END IF; + + SELECT string_agg( + format( + '%s/%s -> %s (%s)', + binding_id, + provider_code, + user_id, + account_state + ), + ', ' ORDER BY binding_id + ) + INTO account_summary + FROM ( + SELECT + binding.id AS binding_id, + binding.provider_code, + binding.user_id, + COALESCE(account.status, 'MISSING') AS account_state + FROM identity_binding binding + LEFT JOIN user_account account + ON account.id = binding.user_id + WHERE account.id IS NULL + OR account.status = 'MERGED' + ORDER BY binding.id + LIMIT 20 + ) invalid_accounts; + + IF account_summary IS NOT NULL THEN + violations := array_append( + violations, + format( + 'bindings reference missing or MERGED accounts: %s', + account_summary + ) + ); + END IF; + + SELECT string_agg( + format('%s/%s', binding_id, provider_code), + ', ' ORDER BY binding_id + ) + INTO identifier_summary + FROM ( + SELECT id AS binding_id, provider_code + FROM identity_binding + WHERE provider_code !~ '^[a-z0-9][a-z0-9._-]{0,63}$' + OR btrim(subject) = '' + OR subject ~ '[[:cntrl:]]' + ORDER BY id + LIMIT 20 + ) invalid_identifiers; + + IF identifier_summary IS NOT NULL THEN + violations := array_append( + violations, + format( + 'bindings contain invalid provider/subject identifiers: %s', + identifier_summary + ) + ); + END IF; + + IF cardinality(violations) > 0 THEN RAISE EXCEPTION - 'Binding V2 preflight failed: multiple active bindings for user/provider: %', - duplicate_summary; + 'Binding V2 preflight failed: %', + array_to_string(violations, '; '); END IF; END $$; diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2MigrationPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2MigrationPostgresTest.java index 8e3c905f..023bbcbe 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2MigrationPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2MigrationPostgresTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.auth.identity; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import java.sql.Connection; import java.sql.DriverManager; @@ -16,6 +17,9 @@ import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; matches = "jdbc:postgresql:.*") class IdentityBindingV2MigrationPostgresTest { + private static final String PREFLIGHT_SCHEMA = + "identity_v2_expand_preflight"; + static final String PRE_EXPAND_USER = "identity-v2-pre-expand-user"; static final String PRE_EXPAND_SUBJECT = @@ -166,6 +170,146 @@ class IdentityBindingV2MigrationPostgresTest { } } + @Test + void preflightReportsAllUnsafeLegacyBindingClasses() + throws Exception { + String url = requiredEnvironment( + "IDENTITY_BINDING_V2_POSTGRES_URL"); + String username = requiredEnvironment( + "IDENTITY_BINDING_V2_POSTGRES_USERNAME"); + String password = requiredEnvironment( + "IDENTITY_BINDING_V2_POSTGRES_PASSWORD"); + dropSchema(url, username, password); + try { + Flyway.configure() + .dataSource(url, username, password) + .locations("classpath:db/migration") + .schemas(PREFLIGHT_SCHEMA) + .defaultSchema(PREFLIGHT_SCHEMA) + .createSchemas(true) + .target(MigrationVersion.fromVersion("44")) + .load() + .migrate(); + + try (Connection connection = DriverManager.getConnection( + url, + username, + password); + Statement statement = connection.createStatement()) { + statement.execute( + "SET search_path TO " + PREFLIGHT_SCHEMA); + statement.executeUpdate(""" + INSERT INTO user_account ( + id, + display_name, + status, + created_at, + updated_at + ) VALUES + ('identity-v2-duplicate-user', + 'Duplicate User', + 'ACTIVE', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP), + ('identity-v2-merged-user', + 'Merged User', + 'MERGED', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP), + ('identity-v2-invalid-user', + 'Invalid Identifier User', + 'ACTIVE', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP) + """); + statement.executeUpdate(""" + INSERT INTO identity_binding ( + user_id, + provider_code, + subject, + login_name, + created_at, + updated_at + ) VALUES + ('identity-v2-duplicate-user', + 'github', + '910000000001', + 'duplicate-one', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP), + ('identity-v2-duplicate-user', + 'github', + '910000000002', + 'duplicate-two', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP), + ('identity-v2-merged-user', + 'gitlab', + '920000000001', + 'merged', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP), + ('identity-v2-invalid-user', + 'Invalid Provider', + ' ', + 'invalid', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP) + """); + } + + Throwable failure = catchThrowable(() -> + Flyway.configure() + .dataSource(url, username, password) + .locations("classpath:db/migration") + .schemas(PREFLIGHT_SCHEMA) + .defaultSchema(PREFLIGHT_SCHEMA) + .createSchemas(true) + .target(MigrationVersion.fromVersion("45")) + .load() + .migrate()); + + assertThat(failure).isNotNull(); + assertThat(rootCause(failure).getMessage()) + .contains( + "multiple active bindings for user/provider") + .contains( + "bindings reference missing or MERGED accounts") + .contains( + "bindings contain invalid provider/subject identifiers"); + + try (Connection connection = DriverManager.getConnection( + url, + username, + password); + Statement statement = connection.createStatement()) { + statement.execute( + "SET search_path TO " + PREFLIGHT_SCHEMA); + assertThat(singleString( + statement, + """ + SELECT version + FROM flyway_schema_history + WHERE success = TRUE + ORDER BY installed_rank DESC + LIMIT 1 + """)).isEqualTo("44"); + assertThat(singleLong( + statement, + """ + SELECT COUNT(*) + FROM information_schema.columns + WHERE table_schema = + 'identity_v2_expand_preflight' + AND table_name = 'identity_binding' + AND column_name = 'status' + """)).isZero(); + } + } finally { + dropSchema(url, username, password); + } + } + private static String singleString( Statement statement, String sql) throws Exception { @@ -192,4 +336,29 @@ class IdentityBindingV2MigrationPostgresTest { } return value; } + + private static void dropSchema( + String url, + String username, + String password) throws Exception { + try (Connection connection = DriverManager.getConnection( + url, + username, + password); + Statement statement = connection.createStatement()) { + statement.execute( + "DROP SCHEMA IF EXISTS " + + PREFLIGHT_SCHEMA + + " CASCADE"); + } + } + + private static Throwable rootCause(Throwable failure) { + Throwable current = failure; + while (current.getCause() != null + && current.getCause() != current) { + current = current.getCause(); + } + return current; + } }