diff --git a/.github/workflows/pr-e2e.yml b/.github/workflows/pr-e2e.yml index 29fb694a..c8e9a3b9 100644 --- a/.github/workflows/pr-e2e.yml +++ b/.github/workflows/pr-e2e.yml @@ -62,6 +62,9 @@ jobs: - name: Ensure Maven wrapper is executable run: chmod +x server/mvnw + - name: Verify Binding V2 migration and concurrency on PostgreSQL + run: bash scripts/tests/identity-binding-v2-postgres-test.sh + - name: Start full dev stack run: make dev-all diff --git a/scripts/tests/identity-binding-v2-postgres-test.sh b/scripts/tests/identity-binding-v2-postgres-test.sh new file mode 100755 index 00000000..af2d85e6 --- /dev/null +++ b/scripts/tests/identity-binding-v2-postgres-test.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +RUN_ID="skillhub-identity-v2-$$" +POSTGRES_CONTAINER="${RUN_ID}-postgres" +NETWORK="${RUN_ID}-network" +POSTGRES_USER="identity_v2" +POSTGRES_PASSWORD="identity-v2-test-password" +POSTGRES_DB="identity_v2" +MAVEN_CACHE_DIR="${MAVEN_CACHE_DIR:-${HOME}/.m2}" + +cleanup() { + docker rm -f "${POSTGRES_CONTAINER}" >/dev/null 2>&1 || true + docker network rm "${NETWORK}" >/dev/null 2>&1 || true +} +trap cleanup EXIT + +docker network create \ + --label "skillhub.test.run=${RUN_ID}" \ + "${NETWORK}" >/dev/null +docker run -d \ + --name "${POSTGRES_CONTAINER}" \ + --label "skillhub.test.run=${RUN_ID}" \ + --network "${NETWORK}" \ + --memory=1g \ + --cpus=1 \ + -e "POSTGRES_USER=${POSTGRES_USER}" \ + -e "POSTGRES_PASSWORD=${POSTGRES_PASSWORD}" \ + -e "POSTGRES_DB=${POSTGRES_DB}" \ + -p 127.0.0.1::5432 \ + postgres:16-alpine >/dev/null + +for _ in $(seq 1 60); do + if docker exec "${POSTGRES_CONTAINER}" \ + pg_isready -U "${POSTGRES_USER}" -d "${POSTGRES_DB}" \ + >/dev/null 2>&1; then + break + fi + sleep 1 +done +docker exec "${POSTGRES_CONTAINER}" \ + pg_isready -U "${POSTGRES_USER}" -d "${POSTGRES_DB}" \ + >/dev/null + +run_test() { + test_class="$1" + java_version="" + if command -v java >/dev/null 2>&1; then + java_version="$(java -version 2>&1 | head -n 1)" + fi + if [[ "${java_version}" == *'"21.'* ]]; then + host_port="$(docker port "${POSTGRES_CONTAINER}" 5432/tcp \ + | sed -n 's/.*://p')" + ( + cd "${REPO_ROOT}/server" + IDENTITY_BINDING_V2_POSTGRES_URL="jdbc:postgresql://127.0.0.1:${host_port}/${POSTGRES_DB}" \ + IDENTITY_BINDING_V2_POSTGRES_USERNAME="${POSTGRES_USER}" \ + IDENTITY_BINDING_V2_POSTGRES_PASSWORD="${POSTGRES_PASSWORD}" \ + MAVEN_OPTS="-Xmx2g -XX:MaxMetaspaceSize=512m" \ + ./mvnw \ + -pl skillhub-app \ + -am \ + "-Dtest=${test_class}" \ + -Dsurefire.failIfNoSpecifiedTests=false \ + test + ) + return + fi + + mkdir -p "${MAVEN_CACHE_DIR}" + docker run --rm \ + --name "${RUN_ID}-java" \ + --label "skillhub.test.run=${RUN_ID}" \ + --network "${NETWORK}" \ + --memory=4g \ + --cpus=2 \ + --user "$(id -u):$(id -g)" \ + -e MAVEN_USER_HOME=/tmp/skillhub-maven-home/.m2 \ + -e MAVEN_OPTS="-Xmx2g -XX:MaxMetaspaceSize=512m" \ + -e "IDENTITY_BINDING_V2_POSTGRES_URL=jdbc:postgresql://${POSTGRES_CONTAINER}:5432/${POSTGRES_DB}" \ + -e "IDENTITY_BINDING_V2_POSTGRES_USERNAME=${POSTGRES_USER}" \ + -e "IDENTITY_BINDING_V2_POSTGRES_PASSWORD=${POSTGRES_PASSWORD}" \ + -v "${REPO_ROOT}:/workspace" \ + -v "${MAVEN_CACHE_DIR}:/tmp/skillhub-maven-home/.m2" \ + -w /workspace/server \ + eclipse-temurin:21-jdk-alpine \ + ./mvnw \ + -Dmaven.repo.local=/tmp/skillhub-maven-home/.m2/repository \ + -pl skillhub-app \ + -am \ + "-Dtest=${test_class}" \ + -Dsurefire.failIfNoSpecifiedTests=false \ + test +} + +run_test IdentityBindingV2MigrationPostgresTest +run_test IdentityBindingV2PostgresIntegrationTest 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 new file mode 100644 index 00000000..58c62980 --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V45__identity_binding_v2_expand.sql @@ -0,0 +1,131 @@ +-- Binding V2 expand migration. +-- +-- This migration intentionally does not install the deferred "at least one +-- active primary subject" trigger. During a rolling upgrade, PR 1 pods must +-- remain able to insert identity_binding rows without writing the new subject +-- table. The trigger is a separate contract-gate migration after all old pods +-- have exited and the preflight has passed. + +ALTER TABLE identity_binding + ADD COLUMN status VARCHAR(16) NOT NULL DEFAULT 'ACTIVE', + ADD COLUMN last_authenticated_at TIMESTAMPTZ, + ADD COLUMN last_synchronized_at TIMESTAMPTZ, + ADD COLUMN revoked_at TIMESTAMPTZ, + ADD COLUMN revoked_by VARCHAR(128), + ADD COLUMN revocation_reason VARCHAR(256), + ADD CONSTRAINT chk_identity_binding_status + CHECK (status IN ('ACTIVE', 'REVOKED')), + ADD CONSTRAINT chk_identity_binding_revocation + CHECK ( + (status = 'ACTIVE' + AND revoked_at IS NULL + AND revoked_by IS NULL + AND revocation_reason IS NULL) + OR + (status = 'REVOKED' AND revoked_at IS NOT NULL) + ), + ADD CONSTRAINT uq_identity_binding_id_provider + UNIQUE (id, provider_code); + +DO $$ +DECLARE + duplicate_summary TEXT; +BEGIN + SELECT string_agg( + format('%s/%s (%s bindings)', user_id, provider_code, binding_count), + ', ' ORDER BY user_id, provider_code) + INTO duplicate_summary + FROM ( + SELECT user_id, provider_code, COUNT(*) AS binding_count + FROM identity_binding + GROUP BY user_id, provider_code + HAVING COUNT(*) > 1 + LIMIT 20 + ) duplicates; + + IF duplicate_summary IS NOT NULL THEN + RAISE EXCEPTION + 'Binding V2 preflight failed: multiple active bindings for user/provider: %', + duplicate_summary; + END IF; +END +$$; + +CREATE UNIQUE INDEX uq_identity_binding_active_user_provider + ON identity_binding(user_id, provider_code) + WHERE status = 'ACTIVE'; + +CREATE TABLE identity_binding_subject ( + id BIGSERIAL PRIMARY KEY, + binding_id BIGINT NOT NULL, + provider_code VARCHAR(64) NOT NULL, + subject_type VARCHAR(64) NOT NULL, + subject_value VARCHAR(512) NOT NULL, + is_primary BOOLEAN NOT NULL, + status VARCHAR(16) NOT NULL DEFAULT 'ACTIVE', + created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, + last_seen_at TIMESTAMPTZ, + revoked_at TIMESTAMPTZ, + CONSTRAINT fk_identity_binding_subject_binding_provider + FOREIGN KEY (binding_id, provider_code) + REFERENCES identity_binding(id, provider_code) + ON DELETE CASCADE, + CONSTRAINT chk_identity_binding_subject_type + CHECK (subject_type ~ '^[a-z][a-z0-9_]{0,63}$'), + CONSTRAINT chk_identity_binding_subject_value + CHECK (subject_value <> ''), + CONSTRAINT chk_identity_binding_subject_status + CHECK (status IN ('ACTIVE', 'REVOKED')), + CONSTRAINT chk_identity_binding_subject_revocation + CHECK ( + (status = 'ACTIVE' AND revoked_at IS NULL) + OR + (status = 'REVOKED' + AND revoked_at IS NOT NULL + AND is_primary = FALSE) + ) +); + +INSERT INTO identity_binding_subject ( + binding_id, + provider_code, + subject_type, + subject_value, + is_primary, + status, + created_at, + last_seen_at +) +SELECT + id, + provider_code, + 'legacy_subject', + subject, + TRUE, + 'ACTIVE', + created_at, + updated_at +FROM identity_binding; + +CREATE UNIQUE INDEX uq_identity_binding_subject_active_identity + ON identity_binding_subject( + provider_code, + subject_type, + subject_value + ) + WHERE status = 'ACTIVE'; + +CREATE UNIQUE INDEX uq_identity_binding_subject_active_primary + ON identity_binding_subject(binding_id) + WHERE status = 'ACTIVE' AND is_primary = TRUE; + +CREATE INDEX idx_identity_binding_subject_binding + ON identity_binding_subject(binding_id, status); + +CREATE INDEX idx_identity_binding_subject_lookup + ON identity_binding_subject( + provider_code, + subject_type, + subject_value, + status + ); 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 new file mode 100644 index 00000000..8e3c905f --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2MigrationPostgresTest.java @@ -0,0 +1,195 @@ +package com.iflytek.skillhub.auth.identity; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.Statement; +import org.flywaydb.core.Flyway; +import org.flywaydb.core.api.MigrationVersion; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; + +@EnabledIfEnvironmentVariable( + named = "IDENTITY_BINDING_V2_POSTGRES_URL", + matches = "jdbc:postgresql:.*") +class IdentityBindingV2MigrationPostgresTest { + + static final String PRE_EXPAND_USER = + "identity-v2-pre-expand-user"; + static final String PRE_EXPAND_SUBJECT = + "900000000001"; + static final String MIXED_VERSION_USER = + "identity-v2-mixed-version-user"; + static final String MIXED_VERSION_SUBJECT = + "900000000002"; + + @Test + void migratesLegacyDataAndKeepsOldWritesValidDuringExpand() 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"); + + Flyway.configure() + .dataSource(url, username, password) + .locations("classpath:db/migration") + .target(MigrationVersion.fromVersion("44")) + .load() + .migrate(); + + try (Connection connection = + DriverManager.getConnection( + url, + username, + password); + Statement statement = + connection.createStatement()) { + statement.executeUpdate(""" + INSERT INTO user_account ( + id, + display_name, + email, + status, + created_at, + updated_at + ) VALUES ( + 'identity-v2-pre-expand-user', + 'Pre Expand User', + 'pre-expand@example.com', + '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-pre-expand-user', + 'github', + '900000000001', + 'pre-expand', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP + ) + """); + } + + Flyway.configure() + .dataSource(url, username, password) + .locations("classpath:db/migration") + .load() + .migrate(); + + try (Connection connection = + DriverManager.getConnection( + url, + username, + password); + Statement statement = + connection.createStatement()) { + assertThat(singleString( + statement, + """ + SELECT status + FROM identity_binding + WHERE user_id = 'identity-v2-pre-expand-user' + """)).isEqualTo("ACTIVE"); + assertThat(singleLong( + statement, + """ + SELECT COUNT(*) + FROM identity_binding_subject subject + JOIN identity_binding binding + ON binding.id = subject.binding_id + WHERE binding.user_id = + 'identity-v2-pre-expand-user' + AND subject.subject_type = 'legacy_subject' + AND subject.subject_value = '900000000001' + AND subject.is_primary = TRUE + AND subject.status = 'ACTIVE' + """)).isEqualTo(1L); + + statement.executeUpdate(""" + INSERT INTO user_account ( + id, + display_name, + email, + status, + created_at, + updated_at + ) VALUES ( + 'identity-v2-mixed-version-user', + 'Mixed Version User', + 'mixed-version@example.com', + '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-mixed-version-user', + 'github', + '900000000002', + 'mixed-version', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP + ) + """); + assertThat(singleLong( + statement, + """ + SELECT COUNT(*) + FROM identity_binding_subject subject + JOIN identity_binding binding + ON binding.id = subject.binding_id + WHERE binding.user_id = + 'identity-v2-mixed-version-user' + """)).isZero(); + } + } + + private static String singleString( + Statement statement, + String sql) throws Exception { + try (ResultSet result = statement.executeQuery(sql)) { + assertThat(result.next()).isTrue(); + return result.getString(1); + } + } + + private static long singleLong( + Statement statement, + String sql) throws Exception { + try (ResultSet result = statement.executeQuery(sql)) { + assertThat(result.next()).isTrue(); + return result.getLong(1); + } + } + + private static String requiredEnvironment(String name) { + String value = System.getenv(name); + if (value == null || value.isBlank()) { + throw new IllegalStateException( + "Missing required environment variable " + name); + } + return value; + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2PostgresIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2PostgresIntegrationTest.java new file mode 100644 index 00000000..1065dd57 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingV2PostgresIntegrationTest.java @@ -0,0 +1,264 @@ +package com.iflytek.skillhub.auth.identity; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +@SpringBootTest +@ActiveProfiles("test") +@EnabledIfEnvironmentVariable( + named = "IDENTITY_BINDING_V2_POSTGRES_URL", + matches = "jdbc:postgresql:.*") +class IdentityBindingV2PostgresIntegrationTest { + + private static final String CONCURRENT_SUBJECT = + "900000000003"; + + @Autowired + private ExternalIdentityLoginService loginService; + + @Autowired + private TrustedProviderRouteResolver routeResolver; + + @Autowired + private ClientRegistrationRepository registrationRepository; + + @Autowired + private JdbcTemplate jdbcTemplate; + + @DynamicPropertySource + static void postgresProperties( + DynamicPropertyRegistry registry) { + registry.add( + "spring.datasource.url", + () -> requiredEnvironment( + "IDENTITY_BINDING_V2_POSTGRES_URL")); + registry.add( + "spring.datasource.username", + () -> requiredEnvironment( + "IDENTITY_BINDING_V2_POSTGRES_USERNAME")); + registry.add( + "spring.datasource.password", + () -> requiredEnvironment( + "IDENTITY_BINDING_V2_POSTGRES_PASSWORD")); + registry.add( + "spring.datasource.driver-class-name", + () -> "org.postgresql.Driver"); + registry.add( + "spring.jpa.database-platform", + () -> "org.hibernate.dialect.PostgreSQLDialect"); + registry.add( + "spring.jpa.hibernate.ddl-auto", + () -> "validate"); + registry.add( + "spring.flyway.enabled", + () -> "true"); + } + + @Test + void upgradesMixedVersionWriteAndPreservesLegacyReadColumn() { + IdentityLoginOutcome outcome = authenticate( + IdentityBindingV2MigrationPostgresTest + .MIXED_VERSION_SUBJECT); + + assertThat(outcome) + .isInstanceOf( + IdentityLoginOutcome.Authenticated.class); + IdentityLoginOutcome.Authenticated authenticated = + (IdentityLoginOutcome.Authenticated) outcome; + assertThat(authenticated.accountCreated()).isFalse(); + assertThat(authenticated.bindingCreated()).isFalse(); + + Long bindingId = jdbcTemplate.queryForObject( + """ + SELECT id + FROM identity_binding + WHERE user_id = ? + AND provider_code = 'github' + AND subject = ? + AND status = 'ACTIVE' + """, + Long.class, + IdentityBindingV2MigrationPostgresTest + .MIXED_VERSION_USER, + IdentityBindingV2MigrationPostgresTest + .MIXED_VERSION_SUBJECT); + assertThat(bindingId).isNotNull(); + + List> subjects = + jdbcTemplate.queryForList( + """ + SELECT + subject_type, + subject_value, + is_primary, + status + FROM identity_binding_subject + WHERE binding_id = ? + ORDER BY subject_type + """, + bindingId); + assertThat(subjects) + .extracting( + row -> row.get("subject_type"), + row -> row.get("subject_value"), + row -> row.get("is_primary"), + row -> row.get("status")) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple( + "github_user_id", + IdentityBindingV2MigrationPostgresTest + .MIXED_VERSION_SUBJECT, + true, + "ACTIVE"), + org.assertj.core.groups.Tuple.tuple( + "legacy_subject", + IdentityBindingV2MigrationPostgresTest + .MIXED_VERSION_SUBJECT, + false, + "ACTIVE")); + + assertThatThrownBy(() -> jdbcTemplate.update( + """ + INSERT INTO identity_binding_subject ( + binding_id, + provider_code, + subject_type, + subject_value, + is_primary, + status + ) VALUES (?, 'github', 'other_primary', + 'other-primary-value', TRUE, 'ACTIVE') + """, + bindingId)) + .isInstanceOf(DataIntegrityViolationException.class); + } + + @Test + void concurrentFirstLoginConvergesOnOneBinding() throws Exception { + ResolvedProviderHandle provider = githubProvider(); + ProviderAuthenticationResult result = + providerResult(CONCURRENT_SUBJECT); + CountDownLatch start = new CountDownLatch(1); + List> futures = + new ArrayList<>(); + + try (var executor = + Executors.newVirtualThreadPerTaskExecutor()) { + for (int index = 0; index < 6; index++) { + futures.add(executor.submit(() -> { + start.await(); + return loginService.authenticate( + provider, + result, + IdentityLoginContext.empty()); + })); + } + start.countDown(); + List outcomes = + new ArrayList<>(); + for (Future future : futures) { + outcomes.add(future.get()); + } + assertThat(outcomes) + .allSatisfy(outcome -> assertThat(outcome) + .isInstanceOf( + IdentityLoginOutcome + .Authenticated.class)); + assertThat(outcomes.stream() + .map(IdentityLoginOutcome.Authenticated.class::cast) + .filter(IdentityLoginOutcome.Authenticated + ::accountCreated) + .count()).isEqualTo(1L); + } + + assertThat(jdbcTemplate.queryForObject( + """ + SELECT COUNT(*) + FROM identity_binding + WHERE provider_code = 'github' + AND subject = ? + AND status = 'ACTIVE' + """, + Long.class, + CONCURRENT_SUBJECT)).isEqualTo(1L); + assertThat(jdbcTemplate.queryForObject( + """ + SELECT COUNT(*) + FROM identity_binding_subject + WHERE provider_code = 'github' + AND subject_type = 'github_user_id' + AND subject_value = ? + AND status = 'ACTIVE' + AND is_primary = TRUE + """, + Long.class, + CONCURRENT_SUBJECT)).isEqualTo(1L); + } + + private IdentityLoginOutcome authenticate(String subject) { + return loginService.authenticate( + githubProvider(), + providerResult(subject), + IdentityLoginContext.empty()); + } + + private ResolvedProviderHandle githubProvider() { + ClientRegistration registration = + registrationRepository.findByRegistrationId( + "github"); + assertThat(registration).isNotNull(); + return routeResolver.resolve(registration); + } + + private static ProviderAuthenticationResult providerResult( + String subject) { + return new ProviderAuthenticationResult( + new SubjectCandidate( + "github_user_id", + subject), + List.of(), + Map.of( + "login", + List.of(new ProviderAttributeValue( + "identity-v2-user", + ProviderAttributeTrust.ASSERTED)), + "email", + List.of(new ProviderAttributeValue( + subject + "@example.com", + ProviderAttributeTrust.VERIFIED))), + new ProtocolAuthenticationEvidence( + "oauth2-github", + Instant.now(), + Set.of( + "oauth2_authorization_code"))); + } + + private static String requiredEnvironment(String name) { + String value = System.getenv(name); + if (value == null || value.isBlank()) { + throw new IllegalStateException( + "Missing required environment variable " + name); + } + return value; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBinding.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBinding.java index 6e28aa68..edfbe887 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBinding.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBinding.java @@ -8,6 +8,8 @@ import org.hibernate.type.SqlTypes; import jakarta.persistence.Column; import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; import jakarta.persistence.GeneratedValue; import jakarta.persistence.GenerationType; import jakarta.persistence.Id; @@ -40,6 +42,25 @@ public class IdentityBinding { @Column(name = "extra_json", columnDefinition = "jsonb") private String extraJson; + @Enumerated(EnumType.STRING) + @Column(nullable = false, length = 16) + private IdentityBindingStatus status = IdentityBindingStatus.ACTIVE; + + @Column(name = "last_authenticated_at") + private Instant lastAuthenticatedAt; + + @Column(name = "last_synchronized_at") + private Instant lastSynchronizedAt; + + @Column(name = "revoked_at") + private Instant revokedAt; + + @Column(name = "revoked_by", length = 128) + private String revokedBy; + + @Column(name = "revocation_reason", length = 256) + private String revocationReason; + @Column(name = "created_at", nullable = false, updatable = false) private Instant createdAt; @@ -53,6 +74,7 @@ public class IdentityBinding { this.providerCode = providerCode; this.subject = subject; this.loginName = loginName; + this.status = IdentityBindingStatus.ACTIVE; } @PrePersist @@ -77,6 +99,28 @@ public class IdentityBinding { public void setLoginName(String loginName) { this.loginName = loginName; } public String getExtraJson() { return extraJson; } public void setExtraJson(String extraJson) { this.extraJson = extraJson; } + public IdentityBindingStatus getStatus() { return status; } + public Instant getLastAuthenticatedAt() { return lastAuthenticatedAt; } + public Instant getLastSynchronizedAt() { return lastSynchronizedAt; } + public Instant getRevokedAt() { return revokedAt; } + public String getRevokedBy() { return revokedBy; } + public String getRevocationReason() { return revocationReason; } public Instant getCreatedAt() { return createdAt; } public Instant getUpdatedAt() { return updatedAt; } + + public void recordAuthentication(Instant authenticatedAt) { + if (authenticatedAt != null + && (lastAuthenticatedAt == null + || authenticatedAt.isAfter(lastAuthenticatedAt))) { + lastAuthenticatedAt = authenticatedAt; + } + } + + public void recordSynchronization(Instant synchronizedAt) { + if (synchronizedAt != null + && (lastSynchronizedAt == null + || synchronizedAt.isAfter(lastSynchronizedAt))) { + lastSynchronizedAt = synchronizedAt; + } + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingStatus.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingStatus.java new file mode 100644 index 00000000..485c4c31 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingStatus.java @@ -0,0 +1,6 @@ +package com.iflytek.skillhub.auth.entity; + +public enum IdentityBindingStatus { + ACTIVE, + REVOKED +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingSubject.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingSubject.java new file mode 100644 index 00000000..46c22100 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingSubject.java @@ -0,0 +1,110 @@ +package com.iflytek.skillhub.auth.entity; + +import java.time.Clock; +import java.time.Instant; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.PrePersist; +import jakarta.persistence.Table; + +@Entity +@Table(name = "identity_binding_subject") +public class IdentityBindingSubject { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @Column(name = "binding_id", nullable = false) + private Long bindingId; + + @Column(name = "provider_code", nullable = false, length = 64) + private String providerCode; + + @Column(name = "subject_type", nullable = false, length = 64) + private String subjectType; + + @Column(name = "subject_value", nullable = false, length = 512) + private String subjectValue; + + @Column(name = "is_primary", nullable = false) + private boolean primary; + + @Enumerated(EnumType.STRING) + @Column(nullable = false, length = 16) + private IdentityBindingSubjectStatus status = + IdentityBindingSubjectStatus.ACTIVE; + + @Column(name = "created_at", nullable = false, updatable = false) + private Instant createdAt; + + @Column(name = "last_seen_at") + private Instant lastSeenAt; + + @Column(name = "revoked_at") + private Instant revokedAt; + + protected IdentityBindingSubject() {} + + public IdentityBindingSubject( + Long bindingId, + String providerCode, + String subjectType, + String subjectValue, + boolean primary, + Instant lastSeenAt) { + this.bindingId = bindingId; + this.providerCode = providerCode; + this.subjectType = subjectType; + this.subjectValue = subjectValue; + this.primary = primary; + this.status = IdentityBindingSubjectStatus.ACTIVE; + this.lastSeenAt = lastSeenAt; + } + + @PrePersist + void prePersist() { + createdAt = Instant.now(Clock.systemUTC()); + } + + public Long getId() { return id; } + public Long getBindingId() { return bindingId; } + public String getProviderCode() { return providerCode; } + public String getSubjectType() { return subjectType; } + public String getSubjectValue() { return subjectValue; } + public boolean isPrimary() { return primary; } + public IdentityBindingSubjectStatus getStatus() { return status; } + public Instant getCreatedAt() { return createdAt; } + public Instant getLastSeenAt() { return lastSeenAt; } + public Instant getRevokedAt() { return revokedAt; } + + public void makePrimary() { + requireActive(); + primary = true; + } + + public void makeAlias() { + requireActive(); + primary = false; + } + + public void markSeen(Instant seenAt) { + requireActive(); + if (seenAt != null + && (lastSeenAt == null || seenAt.isAfter(lastSeenAt))) { + lastSeenAt = seenAt; + } + } + + private void requireActive() { + if (status != IdentityBindingSubjectStatus.ACTIVE) { + throw new IllegalStateException( + "Revoked identity subject cannot be changed"); + } + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingSubjectStatus.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingSubjectStatus.java new file mode 100644 index 00000000..97bf645d --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/entity/IdentityBindingSubjectStatus.java @@ -0,0 +1,6 @@ +package com.iflytek.skillhub.auth.entity; + +public enum IdentityBindingSubjectStatus { + ACTIVE, + REVOKED +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginService.java index f815af81..eef0953e 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginService.java @@ -4,7 +4,9 @@ import com.iflytek.skillhub.auth.policy.AccessDecision; import com.iflytek.skillhub.auth.policy.AccessPolicy; import com.iflytek.skillhub.auth.policy.IdentityAccessContext; import com.iflytek.skillhub.domain.user.UserStatus; +import java.sql.SQLException; import java.util.Objects; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; @Service @@ -54,7 +56,47 @@ class DefaultExternalIdentityLoginService decision == AccessDecision.PENDING_APPROVAL ? UserStatus.PENDING : UserStatus.ACTIVE; - return resolutionTransaction.resolve(assertion, initialStatus); + try { + return resolutionTransaction.resolve( + assertion, + initialStatus, + descriptor.legacyPrimarySubjectType()); + } catch (DataIntegrityViolationException firstConflict) { + if (!isUniqueConstraintViolation(firstConflict)) { + throw firstConflict; + } + try { + return resolutionTransaction.resolve( + assertion, + initialStatus, + descriptor.legacyPrimarySubjectType()); + } catch (DataIntegrityViolationException repeatedConflict) { + if (!isUniqueConstraintViolation(repeatedConflict)) { + repeatedConflict.addSuppressed(firstConflict); + throw repeatedConflict; + } + repeatedConflict.addSuppressed(firstConflict); + throw new IdentityCoreException( + IdentityFailureCode.IDENTITY_IDENTIFIER_CONFLICT, + repeatedConflict); + } + } + } + + private boolean isUniqueConstraintViolation(Throwable failure) { + Throwable current = failure; + while (current != null) { + if (current instanceof SQLException sqlException + && "23505".equals(sqlException.getSQLState())) { + return true; + } + Throwable cause = current.getCause(); + if (cause == current) { + break; + } + current = cause; + } + return false; } private IdentityAccessContext toAccessContext( diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ExternalSubject.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ExternalSubject.java index 8950d22e..27513ac2 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ExternalSubject.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ExternalSubject.java @@ -1,18 +1,25 @@ package com.iflytek.skillhub.auth.identity; import java.util.Objects; +import java.util.regex.Pattern; record ExternalSubject( String type, String value ) { + private static final Pattern TYPE_PATTERN = + Pattern.compile("[a-z][a-z0-9_]{0,63}"); + ExternalSubject { Objects.requireNonNull(type, "type"); Objects.requireNonNull(value, "value"); - if (type.isBlank() || type.length() > 64) { + if (!TYPE_PATTERN.matcher(type).matches()) { throw new IllegalArgumentException("Invalid external subject type"); } - if (value.isBlank() || value.length() > ProviderAssertionLimits.MAX_SUBJECT_VALUE_LENGTH) { + if (value.isBlank() + || value.length() + > ProviderAssertionLimits.MAX_SUBJECT_VALUE_LENGTH + || value.chars().anyMatch(Character::isISOControl)) { throw new IllegalArgumentException("Invalid external subject value"); } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertion.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertion.java index 2e110883..bb2055a8 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertion.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertion.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.auth.identity; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -22,9 +23,39 @@ record IdentityAssertion( Objects.requireNonNull(mappedAttributes, "mappedAttributes"); Objects.requireNonNull(evidence, "evidence"); + if (alternateSubjects.contains(primarySubject)) { + throw new IllegalArgumentException( + "Primary subject must not also be an alias"); + } alternateSubjects = Set.copyOf(alternateSubjects); LinkedHashMap> copied = new LinkedHashMap<>(); mappedAttributes.forEach((key, values) -> copied.put(key, List.copyOf(values))); mappedAttributes = Map.copyOf(copied); } + + Set allSubjects() { + LinkedHashSet subjects = new LinkedHashSet<>(); + subjects.add(primarySubject); + subjects.addAll(alternateSubjects); + return Set.copyOf(subjects); + } + + ExternalSubject requireUniqueSubject(String subjectType) { + ExternalSubject resolved = null; + for (ExternalSubject subject : allSubjects()) { + if (!subject.type().equals(subjectType)) { + continue; + } + if (resolved != null) { + throw new IdentityCoreException( + IdentityFailureCode.INVALID_IDENTITY_ASSERTION); + } + resolved = subject; + } + if (resolved == null) { + throw new IdentityCoreException( + IdentityFailureCode.IDENTITY_SUBJECT_MISSING); + } + return resolved; + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactory.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactory.java index 219f71fe..e49dd74b 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactory.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactory.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.auth.identity; import java.net.URI; import java.net.URISyntaxException; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -18,20 +19,27 @@ final class IdentityAssertionFactory { throw invalidAssertion(); } validatePayload(result); - if (!result.alternateSubjects().isEmpty()) { - throw invalidAssertion(); - } SubjectCandidate primary = result.primarySubject(); if (!descriptor.primarySubjectType().equals(primary.type()) - || !descriptor.allowedSubjectTypes().contains(primary.type())) { + || !descriptor.subjectCanonicalizers() + .containsKey(primary.type())) { throw invalidAssertion(); } - String canonicalValue = descriptor.subjectCanonicalizer() + String canonicalValue = descriptor.canonicalizerFor(primary.type()) .canonicalize(primary.value()); ExternalSubject primarySubject = new ExternalSubject(primary.type(), canonicalValue); + Set alternateSubjects = + canonicalizeAlternates( + descriptor, + result.alternateSubjects(), + primarySubject); + validateLegacySubject( + descriptor, + primarySubject, + alternateSubjects); ExternalProfile profile = createProfile(descriptor, result, primarySubject); AuthenticationEvidence evidence = new AuthenticationEvidence( descriptor.protocol(), @@ -44,12 +52,59 @@ final class IdentityAssertionFactory { descriptor.protocol(), descriptor.canonicalAuthority()), primarySubject, - Set.of(), + alternateSubjects, profile, Map.of(), evidence); } + private Set canonicalizeAlternates( + ProviderDescriptor descriptor, + List candidates, + ExternalSubject primarySubject) { + if (candidates.size() + > ProviderAssertionLimits.MAX_ALTERNATE_SUBJECT_COUNT) { + throw invalidAssertion(); + } + LinkedHashSet canonical = new LinkedHashSet<>(); + for (SubjectCandidate candidate : candidates) { + ExternalSubject subject = new ExternalSubject( + candidate.type(), + descriptor.canonicalizerFor(candidate.type()) + .canonicalize(candidate.value())); + if (subject.equals(primarySubject) || !canonical.add(subject)) { + throw invalidAssertion(); + } + } + return Set.copyOf(canonical); + } + + private void validateLegacySubject( + ProviderDescriptor descriptor, + ExternalSubject primarySubject, + Set alternateSubjects) { + ExternalSubject legacy = null; + if (primarySubject.type().equals( + descriptor.legacyPrimarySubjectType())) { + legacy = primarySubject; + } + for (ExternalSubject subject : alternateSubjects) { + if (!subject.type().equals( + descriptor.legacyPrimarySubjectType())) { + continue; + } + if (legacy != null) { + throw invalidAssertion(); + } + legacy = subject; + } + if (legacy == null + || legacy.value().length() + > ProviderAssertionLimits.MAX_LEGACY_SUBJECT_VALUE_LENGTH) { + throw invalidAssertion(); + } + } + private ExternalProfile createProfile( ProviderDescriptor descriptor, ProviderAuthenticationResult result, diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransaction.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransaction.java index 1c06079b..6fd7db7d 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransaction.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransaction.java @@ -1,26 +1,42 @@ package com.iflytek.skillhub.auth.identity; import com.iflytek.skillhub.auth.entity.IdentityBinding; +import com.iflytek.skillhub.auth.entity.IdentityBindingStatus; +import com.iflytek.skillhub.auth.entity.IdentityBindingSubject; +import com.iflytek.skillhub.auth.entity.IdentityBindingSubjectStatus; import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; +import com.iflytek.skillhub.auth.repository.IdentityBindingSubjectRepository; import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; +import java.time.Instant; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; /** - * Short database transaction that preserves the existing identity-binding and - * provisioning behavior behind the unified core facade. + * Short database transaction for Binding V2 resolution, legacy compatibility, + * and provisioning. Protocol I/O has already completed before this service is + * invoked. */ @Service class IdentityResolutionTransaction { private static final String ACCOUNT_PENDING = "ACCOUNT_PENDING"; + private static final String LEGACY_SUBJECT_TYPE = "legacy_subject"; private final IdentityBindingRepository bindingRepository; + private final IdentityBindingSubjectRepository subjectRepository; private final UserAccountRepository userRepository; private final GlobalNamespaceMembershipService membershipService; private final AccountLoginGuard accountLoginGuard; @@ -28,11 +44,13 @@ class IdentityResolutionTransaction { IdentityResolutionTransaction( IdentityBindingRepository bindingRepository, + IdentityBindingSubjectRepository subjectRepository, UserAccountRepository userRepository, GlobalNamespaceMembershipService membershipService, AccountLoginGuard accountLoginGuard, PlatformPrincipalFactory principalFactory) { this.bindingRepository = bindingRepository; + this.subjectRepository = subjectRepository; this.userRepository = userRepository; this.membershipService = membershipService; this.accountLoginGuard = accountLoginGuard; @@ -42,33 +60,156 @@ class IdentityResolutionTransaction { @Transactional public IdentityLoginOutcome resolve( IdentityAssertion assertion, - UserStatus initialStatus) { + UserStatus initialStatus, + String legacyPrimarySubjectType) { + ExternalSubject legacySubject = + assertion.requireUniqueSubject(legacyPrimarySubjectType); + MatchResolution initialMatches = + resolveMatches(assertion, legacySubject); + if (initialMatches.bindingId() == null) { + return createAccount( + assertion, + legacySubject, + initialStatus); + } + IdentityBinding binding = bindingRepository + .findByIdAndStatusForUpdate( + initialMatches.bindingId(), + IdentityBindingStatus.ACTIVE) + .orElseThrow(this::identifierConflict); + MatchResolution lockedMatches = + resolveMatches(assertion, legacySubject); + if (!binding.getId().equals(lockedMatches.bindingId())) { + throw identifierConflict(); + } + return resolveExisting( + assertion, + legacySubject, + binding, + lockedMatches.revokedAliases()); + } + + private MatchResolution resolveMatches( + IdentityAssertion assertion, + ExternalSubject legacySubject) { + List typedMatches = + subjectRepository.findMatchingSubjects( + assertion.provider().providerCode(), + subjectValuesByType(assertion.allSubjects())); + IdentityBinding legacyMatch = bindingRepository .findByProviderCodeAndSubject( assertion.provider().providerCode(), - assertion.primarySubject().value()) + legacySubject.value()) .orElse(null); - if (binding != null) { - return resolveExisting(assertion, binding); + + LinkedHashSet activeBindingIds = typedMatches.stream() + .filter(subject -> subject.getStatus() + == IdentityBindingSubjectStatus.ACTIVE) + .map(IdentityBindingSubject::getBindingId) + .collect(Collectors.toCollection(LinkedHashSet::new)); + if (legacyMatch != null) { + if (legacyMatch.getStatus() == IdentityBindingStatus.ACTIVE) { + activeBindingIds.add(legacyMatch.getId()); + } else if (activeBindingIds.isEmpty()) { + throw accessDenied(); + } else { + throw identifierConflict(); + } } - return createAccount(assertion, initialStatus); + if (activeBindingIds.size() > 1) { + throw identifierConflict(); + } + + Map> matchesBySubject = + typedMatches.stream().collect(Collectors.groupingBy( + this::externalSubject, + LinkedHashMap::new, + Collectors.toList())); + if (activeBindingIds.isEmpty()) { + if (matchesBySubject.values().stream() + .flatMap(List::stream) + .anyMatch(subject -> subject.getStatus() + == IdentityBindingSubjectStatus.REVOKED)) { + throw accessDenied(); + } + return new MatchResolution(null, Set.of()); + } + + Long bindingId = activeBindingIds.getFirst(); + LinkedHashSet revokedAliases = + new LinkedHashSet<>(); + for (ExternalSubject assertedSubject : assertion.allSubjects()) { + List matches = + matchesBySubject.getOrDefault( + assertedSubject, + List.of()); + boolean hasActive = matches.stream().anyMatch(subject -> + subject.getStatus() + == IdentityBindingSubjectStatus.ACTIVE); + if (hasActive) { + continue; + } + List revoked = matches.stream() + .filter(subject -> subject.getStatus() + == IdentityBindingSubjectStatus.REVOKED) + .toList(); + if (revoked.isEmpty()) { + continue; + } + if (assertedSubject.equals(assertion.primarySubject())) { + throw accessDenied(); + } + if (revoked.stream().anyMatch(subject -> + !bindingId.equals(subject.getBindingId()))) { + throw identifierConflict(); + } + revokedAliases.add(assertedSubject); + } + return new MatchResolution( + bindingId, + Set.copyOf(revokedAliases)); } private IdentityLoginOutcome resolveExisting( IdentityAssertion assertion, - IdentityBinding binding) { + ExternalSubject legacySubject, + IdentityBinding binding, + Set revokedAliases) { + if (!binding.getProviderCode().equals( + assertion.provider().providerCode()) + || !binding.getSubject().equals(legacySubject.value())) { + throw identifierConflict(); + } + UserAccount user = userRepository.findById(binding.getUserId()) .orElseThrow(() -> new IllegalStateException( "User not found for identity binding")); AccountLoginDecision decision = accountLoginGuard.evaluateInteractive(user); - if (decision == AccountLoginDecision.PENDING) { - return new IdentityLoginOutcome.PendingApproval(ACCOUNT_PENDING); + if (decision != AccountLoginDecision.ALLOWED + && decision != AccountLoginDecision.PENDING) { + requireAllowed(decision); + } + + reconcileSubjects( + binding, + assertion, + revokedAliases); + binding.recordAuthentication( + assertion.evidence().authenticatedAt()); + bindingRepository.save(binding); + + if (decision == AccountLoginDecision.PENDING) { + return new IdentityLoginOutcome.PendingApproval( + ACCOUNT_PENDING); } - requireAllowed(decision); synchronizeCompatibilityProfile(user, assertion.profile()); user = userRepository.save(user); + binding.recordSynchronization( + assertion.evidence().authenticatedAt()); + bindingRepository.save(binding); return new IdentityLoginOutcome.Authenticated( principalFactory.create( user, @@ -77,31 +218,143 @@ class IdentityResolutionTransaction { false); } + private void reconcileSubjects( + IdentityBinding binding, + IdentityAssertion assertion, + Set revokedAliases) { + subjectRepository.demoteActivePrimary( + binding.getId(), + IdentityBindingSubjectStatus.ACTIVE); + List activeSubjects = + new ArrayList<>( + subjectRepository + .findByBindingIdAndStatusForUpdate( + binding.getId(), + IdentityBindingSubjectStatus.ACTIVE)); + boolean legacyManaged = activeSubjects.isEmpty() + || activeSubjects.stream().anyMatch(subject -> + LEGACY_SUBJECT_TYPE.equals( + subject.getSubjectType())); + Map existing = + new HashMap<>(); + for (IdentityBindingSubject subject : activeSubjects) { + IdentityBindingSubject duplicate = existing.put( + externalSubject(subject), + subject); + if (duplicate != null) { + throw identifierConflict(); + } + subject.makeAlias(); + } + + Instant authenticatedAt = + assertion.evidence().authenticatedAt(); + if (legacyManaged) { + ExternalSubject compatibilitySubject = + new ExternalSubject( + LEGACY_SUBJECT_TYPE, + binding.getSubject()); + existing.computeIfAbsent( + compatibilitySubject, + ignored -> { + IdentityBindingSubject created = + new IdentityBindingSubject( + binding.getId(), + binding.getProviderCode(), + compatibilitySubject.type(), + compatibilitySubject.value(), + false, + authenticatedAt); + activeSubjects.add(created); + return created; + }); + } + + for (ExternalSubject assertedSubject : + assertion.allSubjects()) { + if (revokedAliases.contains(assertedSubject)) { + continue; + } + IdentityBindingSubject subject = existing.computeIfAbsent( + assertedSubject, + ignored -> { + IdentityBindingSubject created = + new IdentityBindingSubject( + binding.getId(), + binding.getProviderCode(), + assertedSubject.type(), + assertedSubject.value(), + false, + authenticatedAt); + activeSubjects.add(created); + return created; + }); + subject.markSeen(authenticatedAt); + } + + IdentityBindingSubject primary = + existing.get(assertion.primarySubject()); + if (primary == null) { + throw accessDenied(); + } + primary.makePrimary(); + subjectRepository.saveAll(activeSubjects); + } + private IdentityLoginOutcome createAccount( IdentityAssertion assertion, + ExternalSubject legacySubject, UserStatus initialStatus) { ExternalProfile profile = assertion.profile(); UserAccount user = new UserAccount( "usr_" + UUID.randomUUID(), profile.displayName(), trustedEmail(profile).orElse(null), - profile.avatarUrl().map(Object::toString).orElse(null)); + profile.avatarUrl() + .map(Object::toString) + .orElse(null)); user.setStatus(initialStatus); user = userRepository.save(user); if (initialStatus == UserStatus.ACTIVE) { membershipService.ensureMember(user.getId()); } - bindingRepository.save(new IdentityBinding( + IdentityBinding binding = new IdentityBinding( user.getId(), assertion.provider().providerCode(), - assertion.primarySubject().value(), - profile.displayName())); + legacySubject.value(), + profile.displayName()); + binding.recordAuthentication( + assertion.evidence().authenticatedAt()); + IdentityBinding savedBinding = + bindingRepository.save(binding); + if (savedBinding.getId() == null) { + throw new IllegalStateException( + "Identity binding id was not assigned"); + } + + List subjects = + assertion.allSubjects().stream() + .map(subject -> new IdentityBindingSubject( + savedBinding.getId(), + savedBinding.getProviderCode(), + subject.type(), + subject.value(), + subject.equals( + assertion.primarySubject()), + assertion.evidence() + .authenticatedAt())) + .toList(); + subjectRepository.saveAll(subjects); if (initialStatus == UserStatus.PENDING) { - return new IdentityLoginOutcome.PendingApproval(ACCOUNT_PENDING); + return new IdentityLoginOutcome.PendingApproval( + ACCOUNT_PENDING); } requireAllowed(accountLoginGuard.evaluateInteractive(user)); + savedBinding.recordSynchronization( + assertion.evidence().authenticatedAt()); + bindingRepository.save(savedBinding); return new IdentityLoginOutcome.Authenticated( principalFactory.create( user, @@ -110,6 +363,26 @@ class IdentityResolutionTransaction { true); } + private Map> subjectValuesByType( + Set subjects) { + LinkedHashMap> valuesByType = + new LinkedHashMap<>(); + for (ExternalSubject subject : subjects) { + valuesByType.computeIfAbsent( + subject.type(), + ignored -> new LinkedHashSet<>()) + .add(subject.value()); + } + return Map.copyOf(valuesByType); + } + + private ExternalSubject externalSubject( + IdentityBindingSubject subject) { + return new ExternalSubject( + subject.getSubjectType(), + subject.getSubjectValue()); + } + private void synchronizeCompatibilityProfile( UserAccount user, ExternalProfile profile) { @@ -122,7 +395,8 @@ class IdentityResolutionTransaction { private Optional trustedEmail(ExternalProfile profile) { return profile.email() - .filter(claim -> claim.assurance().isVerifiedOrAuthoritative()) + .filter(claim -> claim.assurance() + .isVerifiedOrAuthoritative()) .map(EmailClaim::value); } @@ -139,4 +413,19 @@ class IdentityResolutionTransaction { throw new IdentityCoreException(failureCode); } } + + private IdentityCoreException identifierConflict() { + return new IdentityCoreException( + IdentityFailureCode.IDENTITY_IDENTIFIER_CONFLICT); + } + + private IdentityCoreException accessDenied() { + return new IdentityCoreException( + IdentityFailureCode.ACCESS_DENIED); + } + + private record MatchResolution( + Long bindingId, + Set revokedAliases) { + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderAssertionLimits.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderAssertionLimits.java index 21b19f96..c422c7bb 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderAssertionLimits.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderAssertionLimits.java @@ -1,7 +1,9 @@ package com.iflytek.skillhub.auth.identity; final class ProviderAssertionLimits { - static final int MAX_SUBJECT_VALUE_LENGTH = 256; + static final int MAX_SUBJECT_VALUE_LENGTH = 512; + static final int MAX_LEGACY_SUBJECT_VALUE_LENGTH = 256; + static final int MAX_ALTERNATE_SUBJECT_COUNT = 16; static final int MAX_ATTRIBUTE_COUNT = 64; static final int MAX_VALUES_PER_ATTRIBUTE = 16; static final int MAX_ATTRIBUTE_VALUE_LENGTH = 2_048; diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderDescriptor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderDescriptor.java index 3d960446..053a791e 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderDescriptor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/ProviderDescriptor.java @@ -1,8 +1,8 @@ package com.iflytek.skillhub.auth.identity; import java.util.List; +import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.regex.Pattern; record ProviderDescriptor( @@ -11,8 +11,8 @@ record ProviderDescriptor( String canonicalAuthority, String displayName, String primarySubjectType, - Set allowedSubjectTypes, - SubjectCanonicalizer subjectCanonicalizer, + String legacyPrimarySubjectType, + Map subjectCanonicalizers, List displayNameAttributes, List emailAttributes, List avatarAttributes, @@ -29,8 +29,12 @@ record ProviderDescriptor( Objects.requireNonNull(canonicalAuthority, "canonicalAuthority"); Objects.requireNonNull(displayName, "displayName"); Objects.requireNonNull(primarySubjectType, "primarySubjectType"); - Objects.requireNonNull(allowedSubjectTypes, "allowedSubjectTypes"); - Objects.requireNonNull(subjectCanonicalizer, "subjectCanonicalizer"); + Objects.requireNonNull( + legacyPrimarySubjectType, + "legacyPrimarySubjectType"); + Objects.requireNonNull( + subjectCanonicalizers, + "subjectCanonicalizers"); Objects.requireNonNull(displayNameAttributes, "displayNameAttributes"); Objects.requireNonNull(emailAttributes, "emailAttributes"); Objects.requireNonNull(avatarAttributes, "avatarAttributes"); @@ -48,12 +52,26 @@ record ProviderDescriptor( if (displayName.isBlank() || displayName.length() > 128) { throw new IllegalArgumentException("Invalid provider display name"); } - allowedSubjectTypes = Set.copyOf(allowedSubjectTypes); - if (!allowedSubjectTypes.contains(primarySubjectType)) { + subjectCanonicalizers = Map.copyOf(subjectCanonicalizers); + if (!subjectCanonicalizers.containsKey(primarySubjectType)) { throw new IllegalArgumentException("Primary subject type is not allowed"); } + if (!subjectCanonicalizers.containsKey(legacyPrimarySubjectType)) { + throw new IllegalArgumentException( + "Legacy primary subject type is not allowed"); + } displayNameAttributes = List.copyOf(displayNameAttributes); emailAttributes = List.copyOf(emailAttributes); avatarAttributes = List.copyOf(avatarAttributes); } + + SubjectCanonicalizer canonicalizerFor(String subjectType) { + SubjectCanonicalizer canonicalizer = + subjectCanonicalizers.get(subjectType); + if (canonicalizer == null) { + throw new IdentityCoreException( + IdentityFailureCode.INVALID_IDENTITY_ASSERTION); + } + return canonicalizer; + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSource.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSource.java index 1a27a0ac..b9ded715 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSource.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSource.java @@ -245,8 +245,8 @@ class StaticTrustedProviderDescriptorSource authority, displayName, subjectType, - Set.of(subjectType), - canonicalizer, + subjectType, + Map.of(subjectType, canonicalizer), displayNameAttributes, emailAttributes, avatarAttributes, diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingRepository.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingRepository.java index 94dc3cfe..0d6067d8 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingRepository.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingRepository.java @@ -1,9 +1,15 @@ package com.iflytek.skillhub.auth.repository; import com.iflytek.skillhub.auth.entity.IdentityBinding; -import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.stereotype.Repository; +import com.iflytek.skillhub.auth.entity.IdentityBindingStatus; +import jakarta.persistence.LockModeType; +import java.util.List; import java.util.Optional; +import org.springframework.data.jpa.repository.Lock; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; /** * JPA repository for links between platform users and external identity-provider subjects. @@ -11,6 +17,19 @@ import java.util.Optional; @Repository public interface IdentityBindingRepository extends JpaRepository { Optional findByProviderCodeAndSubject(String providerCode, String subject); + + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query(""" + select binding + from IdentityBinding binding + where binding.id = :bindingId + and binding.status = :status + """) + Optional findByIdAndStatusForUpdate( + @Param("bindingId") Long bindingId, + @Param("status") IdentityBindingStatus status); + boolean existsByProviderCode(String providerCode); - java.util.List findByUserId(String userId); + + List findByUserId(String userId); } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectLookupRepository.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectLookupRepository.java new file mode 100644 index 00000000..af2140d0 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectLookupRepository.java @@ -0,0 +1,18 @@ +package com.iflytek.skillhub.auth.repository; + +import com.iflytek.skillhub.auth.entity.IdentityBindingSubject; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Exact typed-subject lookup. Subject type and value remain paired so the + * query cannot accidentally match the Cartesian product of independent IN + * clauses. + */ +public interface IdentityBindingSubjectLookupRepository { + + List findMatchingSubjects( + String providerCode, + Map> subjectValuesByType); +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectRepository.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectRepository.java new file mode 100644 index 00000000..e3f11b7c --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectRepository.java @@ -0,0 +1,44 @@ +package com.iflytek.skillhub.auth.repository; + +import com.iflytek.skillhub.auth.entity.IdentityBindingSubject; +import com.iflytek.skillhub.auth.entity.IdentityBindingSubjectStatus; +import jakarta.persistence.LockModeType; +import java.util.List; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Lock; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; + +@Repository +public interface IdentityBindingSubjectRepository + extends JpaRepository, + IdentityBindingSubjectLookupRepository { + + @Modifying( + flushAutomatically = true, + clearAutomatically = true) + @Query(""" + update IdentityBindingSubject subject + set subject.primary = false + where subject.bindingId = :bindingId + and subject.status = :status + and subject.primary = true + """) + int demoteActivePrimary( + @Param("bindingId") Long bindingId, + @Param("status") IdentityBindingSubjectStatus status); + + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query(""" + select subject + from IdentityBindingSubject subject + where subject.bindingId = :bindingId + and subject.status = :status + order by subject.id + """) + List findByBindingIdAndStatusForUpdate( + @Param("bindingId") Long bindingId, + @Param("status") IdentityBindingSubjectStatus status); +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectRepositoryImpl.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectRepositoryImpl.java new file mode 100644 index 00000000..13bc3fa8 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/IdentityBindingSubjectRepositoryImpl.java @@ -0,0 +1,60 @@ +package com.iflytek.skillhub.auth.repository; + +import com.iflytek.skillhub.auth.entity.IdentityBindingSubject; +import jakarta.persistence.EntityManager; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Predicate; +import jakarta.persistence.criteria.Root; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.Set; + +class IdentityBindingSubjectRepositoryImpl + implements IdentityBindingSubjectLookupRepository { + + private final EntityManager entityManager; + + IdentityBindingSubjectRepositoryImpl(EntityManager entityManager) { + this.entityManager = entityManager; + } + + @Override + public List findMatchingSubjects( + String providerCode, + Map> subjectValuesByType) { + if (subjectValuesByType.isEmpty()) { + return List.of(); + } + + CriteriaBuilder builder = entityManager.getCriteriaBuilder(); + CriteriaQuery query = + builder.createQuery(IdentityBindingSubject.class); + Root subject = + query.from(IdentityBindingSubject.class); + + List exactPairs = new ArrayList<>(); + subjectValuesByType.entrySet().stream() + .sorted(Map.Entry.comparingByKey()) + .forEach(entry -> entry.getValue().stream() + .sorted(Comparator.naturalOrder()) + .forEach(value -> exactPairs.add(builder.and( + builder.equal( + subject.get("subjectType"), + entry.getKey()), + builder.equal( + subject.get("subjectValue"), + value))))); + + query.select(subject) + .where(builder.and( + builder.equal( + subject.get("providerCode"), + providerCode), + builder.or(exactPairs.toArray(Predicate[]::new)))) + .orderBy(builder.asc(subject.get("id"))); + return entityManager.createQuery(query).getResultList(); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginServiceTest.java index 2b0d59d3..d53af467 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultExternalIdentityLoginServiceTest.java @@ -13,6 +13,7 @@ import com.iflytek.skillhub.auth.policy.AccessDecision; import com.iflytek.skillhub.auth.policy.AccessPolicy; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.user.UserStatus; +import java.sql.SQLException; import java.time.Instant; import java.util.List; import java.util.Map; @@ -21,6 +22,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.InOrder; import org.mockito.ArgumentCaptor; +import org.springframework.dao.DataIntegrityViolationException; class DefaultExternalIdentityLoginServiceTest { @@ -67,7 +69,8 @@ class DefaultExternalIdentityLoginServiceTest { when(accessPolicy.evaluate(any())).thenReturn(AccessDecision.ALLOW); when(resolutionTransaction.resolve( any(IdentityAssertion.class), - org.mockito.ArgumentMatchers.eq(UserStatus.ACTIVE))) + org.mockito.ArgumentMatchers.eq(UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq("github_user_id"))) .thenReturn(expected); IdentityLoginOutcome outcome = @@ -90,7 +93,8 @@ class DefaultExternalIdentityLoginServiceTest { order.verify(accessPolicy).evaluate(any()); order.verify(resolutionTransaction).resolve( any(IdentityAssertion.class), - org.mockito.ArgumentMatchers.eq(UserStatus.ACTIVE)); + org.mockito.ArgumentMatchers.eq(UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq("github_user_id")); } @Test @@ -104,7 +108,8 @@ class DefaultExternalIdentityLoginServiceTest { new IdentityLoginOutcome.PendingApproval("ACCOUNT_PENDING"); when(resolutionTransaction.resolve( any(IdentityAssertion.class), - org.mockito.ArgumentMatchers.eq(UserStatus.PENDING))) + org.mockito.ArgumentMatchers.eq(UserStatus.PENDING), + org.mockito.ArgumentMatchers.eq("github_user_id"))) .thenReturn(pending); IdentityLoginOutcome outcome = @@ -131,7 +136,131 @@ class DefaultExternalIdentityLoginServiceTest { .extracting("reasonCode") .isEqualTo(IdentityFailureCode.ACCESS_DENIED); - verify(resolutionTransaction, never()).resolve(any(), any()); + verify(resolutionTransaction, never()).resolve( + any(), + any(), + any()); + } + + @Test + void retriesConcurrentFirstLoginInANewResolutionTransaction() { + ResolvedProviderHandle handle = + new DefaultResolvedProviderHandle("github"); + PlatformPrincipal principal = new PlatformPrincipal( + "usr_1", + "alice", + "alice@example.com", + null, + "github", + Set.of("USER")); + IdentityLoginOutcome expected = + new IdentityLoginOutcome.Authenticated( + principal, + false, + false); + when(descriptorSource.require(handle)) + .thenReturn(descriptor); + when(accessPolicy.evaluate(any())) + .thenReturn(AccessDecision.ALLOW); + when(resolutionTransaction.resolve( + any(IdentityAssertion.class), + org.mockito.ArgumentMatchers.eq( + UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq( + "github_user_id"))) + .thenThrow(uniqueViolation()) + .thenReturn(expected); + + IdentityLoginOutcome outcome = service.authenticate( + handle, + result(), + IdentityLoginContext.empty()); + + assertThat(outcome).isSameAs(expected); + verify(resolutionTransaction, + org.mockito.Mockito.times(2)).resolve( + any(IdentityAssertion.class), + org.mockito.ArgumentMatchers.eq( + UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq( + "github_user_id")); + } + + @Test + void repeatedUniqueConflictFailsClosed() { + ResolvedProviderHandle handle = + new DefaultResolvedProviderHandle("github"); + when(descriptorSource.require(handle)) + .thenReturn(descriptor); + when(accessPolicy.evaluate(any())) + .thenReturn(AccessDecision.ALLOW); + when(resolutionTransaction.resolve( + any(IdentityAssertion.class), + org.mockito.ArgumentMatchers.eq( + UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq( + "github_user_id"))) + .thenThrow(uniqueViolation(), uniqueViolation()); + + assertThatThrownBy(() -> service.authenticate( + handle, + result(), + IdentityLoginContext.empty())) + .isInstanceOf(IdentityCoreException.class) + .extracting("reasonCode") + .isEqualTo( + IdentityFailureCode + .IDENTITY_IDENTIFIER_CONFLICT); + verify(resolutionTransaction, + org.mockito.Mockito.times(2)).resolve( + any(IdentityAssertion.class), + org.mockito.ArgumentMatchers.eq( + UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq( + "github_user_id")); + } + + @Test + void nonUniqueIntegrityFailureIsNotRetriedOrMisclassified() { + ResolvedProviderHandle handle = + new DefaultResolvedProviderHandle("github"); + DataIntegrityViolationException checkViolation = + new DataIntegrityViolationException( + "check violation", + new SQLException( + "check violation", + "23514")); + when(descriptorSource.require(handle)) + .thenReturn(descriptor); + when(accessPolicy.evaluate(any())) + .thenReturn(AccessDecision.ALLOW); + when(resolutionTransaction.resolve( + any(IdentityAssertion.class), + org.mockito.ArgumentMatchers.eq( + UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq( + "github_user_id"))) + .thenThrow(checkViolation); + + assertThatThrownBy(() -> service.authenticate( + handle, + result(), + IdentityLoginContext.empty())) + .isSameAs(checkViolation); + verify(resolutionTransaction).resolve( + any(IdentityAssertion.class), + org.mockito.ArgumentMatchers.eq( + UserStatus.ACTIVE), + org.mockito.ArgumentMatchers.eq( + "github_user_id")); + } + + private static DataIntegrityViolationException uniqueViolation() { + return new DataIntegrityViolationException( + "unique violation", + new SQLException( + "unique violation", + "23505")); } private static ProviderAuthenticationResult result() { @@ -160,8 +289,10 @@ class DefaultExternalIdentityLoginServiceTest { "https://github.com", "GitHub", "github_user_id", - Set.of("github_user_id"), - SubjectCanonicalizer.DECIMAL, + "github_user_id", + Map.of( + "github_user_id", + SubjectCanonicalizer.DECIMAL), List.of("login"), List.of("email"), List.of("avatar_url"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderAuthorityOperationsTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderAuthorityOperationsTest.java index 334d60af..817b6d47 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderAuthorityOperationsTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderAuthorityOperationsTest.java @@ -108,8 +108,10 @@ class DefaultIdentityProviderAuthorityOperationsTest { "https://github.com", "GitHub", "github_user_id", - Set.of("github_user_id"), - SubjectCanonicalizer.DECIMAL, + "github_user_id", + java.util.Map.of( + "github_user_id", + SubjectCanonicalizer.DECIMAL), List.of("login"), List.of("email"), List.of("avatar_url"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderReadinessServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderReadinessServiceTest.java index 9e7bc8d1..ed840634 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderReadinessServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/DefaultIdentityProviderReadinessServiceTest.java @@ -52,8 +52,10 @@ class DefaultIdentityProviderReadinessServiceTest { "https://github.com", "GitHub", "github_user_id", - Set.of("github_user_id"), - SubjectCanonicalizer.DECIMAL, + "github_user_id", + java.util.Map.of( + "github_user_id", + SubjectCanonicalizer.DECIMAL), List.of("login"), List.of("email"), List.of("avatar_url"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactoryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactoryTest.java index 51700e7e..8280baef 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactoryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAssertionFactoryTest.java @@ -60,8 +60,8 @@ class IdentityAssertionFactoryTest { "https://id.example.com", "Corporate OIDC", "oidc_sub", - Set.of("oidc_sub"), - SubjectCanonicalizer.EXACT, + "oidc_sub", + Map.of("oidc_sub", SubjectCanonicalizer.EXACT), List.of("preferred_username", "name", "sub"), List.of("email"), List.of("picture"), @@ -130,15 +130,71 @@ class IdentityAssertionFactoryTest { } @Test - void rejectsAliasesUntilBindingV2IsAvailable() { + void canonicalizesTypedAliasesForBindingV2() { + ProviderDescriptor descriptor = new ProviderDescriptor( + "corp", + "oidc", + "https://id.example.com", + "Corporate Identity", + "stable_id", + "legacy_id", + Map.of( + "stable_id", + SubjectCanonicalizer.EXACT, + "legacy_id", + SubjectCanonicalizer.EXACT, + "alias_id", + SubjectCanonicalizer.EXACT), + List.of("name"), + List.of("email"), + List.of("picture"), + EmailAssurance.VERIFIED); ProviderAuthenticationResult result = result( - new SubjectCandidate("github_user_id", "123456"), - List.of(new SubjectCandidate("github_user_id", "654321")), + new SubjectCandidate("stable_id", "stable-123"), + List.of( + new SubjectCandidate( + "alias_id", + "alias-123"), + new SubjectCandidate( + "legacy_id", + "legacy-123")), Map.of("login", values("alice", ProviderAttributeTrust.ASSERTED)), - "oauth2-github" + "oidc" ); - assertThatThrownBy(() -> factory.create(githubDescriptor(), result)) + IdentityAssertion assertion = + factory.create(descriptor, result); + + assertThat(assertion.primarySubject()).isEqualTo( + new ExternalSubject("stable_id", "stable-123")); + assertThat(assertion.alternateSubjects()) + .containsExactlyInAnyOrder( + new ExternalSubject( + "alias_id", + "alias-123"), + new ExternalSubject( + "legacy_id", + "legacy-123")); + assertThat(assertion.requireUniqueSubject("legacy_id")) + .isEqualTo(new ExternalSubject( + "legacy_id", + "legacy-123")); + } + + @Test + void rejectsDuplicateLegacySubjectCandidates() { + ProviderAuthenticationResult result = result( + new SubjectCandidate("github_user_id", "123456"), + List.of(new SubjectCandidate( + "github_user_id", + "654321")), + Map.of("login", values( + "alice", + ProviderAttributeTrust.ASSERTED)), + "oauth2-github"); + + assertThatThrownBy(() -> + factory.create(githubDescriptor(), result)) .isInstanceOf(IdentityCoreException.class) .extracting("reasonCode") .isEqualTo(IdentityFailureCode.INVALID_IDENTITY_ASSERTION); @@ -193,8 +249,10 @@ class IdentityAssertionFactoryTest { "https://github.com", "GitHub", "github_user_id", - Set.of("github_user_id"), - SubjectCanonicalizer.DECIMAL, + "github_user_id", + Map.of( + "github_user_id", + SubjectCanonicalizer.DECIMAL), List.of("login"), List.of("email"), List.of("avatar_url"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransactionTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransactionTest.java index dd3f5c6b..60469e02 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransactionTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityResolutionTransactionTest.java @@ -9,24 +9,34 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.iflytek.skillhub.auth.entity.IdentityBinding; +import com.iflytek.skillhub.auth.entity.IdentityBindingStatus; +import com.iflytek.skillhub.auth.entity.IdentityBindingSubject; +import com.iflytek.skillhub.auth.entity.IdentityBindingSubjectStatus; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; +import com.iflytek.skillhub.auth.repository.IdentityBindingSubjectRepository; import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; import java.net.URI; import java.time.Instant; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import org.springframework.test.util.ReflectionTestUtils; class IdentityResolutionTransactionTest { + private static final Instant AUTHENTICATED_AT = + Instant.parse("2026-07-30T08:00:00Z"); + private IdentityBindingRepository bindingRepository; + private IdentityBindingSubjectRepository subjectRepository; private UserAccountRepository userRepository; private GlobalNamespaceMembershipService membershipService; private AccountLoginGuard accountLoginGuard; @@ -36,235 +46,474 @@ class IdentityResolutionTransactionTest { @BeforeEach void setUp() { bindingRepository = mock(IdentityBindingRepository.class); + subjectRepository = + mock(IdentityBindingSubjectRepository.class); userRepository = mock(UserAccountRepository.class); - membershipService = mock(GlobalNamespaceMembershipService.class); + membershipService = + mock(GlobalNamespaceMembershipService.class); accountLoginGuard = new AccountLoginGuard(); principalFactory = mock(PlatformPrincipalFactory.class); transaction = new IdentityResolutionTransaction( bindingRepository, + subjectRepository, userRepository, membershipService, accountLoginGuard, principalFactory); + when(subjectRepository.findMatchingSubjects(any(), any())) + .thenReturn(List.of()); + when(bindingRepository.findByProviderCodeAndSubject( + any(), + any())).thenReturn(Optional.empty()); + when(userRepository.save(any(UserAccount.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + when(bindingRepository.save(any(IdentityBinding.class))) + .thenAnswer(invocation -> { + IdentityBinding binding = + invocation.getArgument(0); + if (binding.getId() == null) { + ReflectionTestUtils.setField( + binding, + "id", + 100L); + } + return binding; + }); } @Test - void createsActiveAccountBindingMembershipAndPrincipal() { + void createsAccountAndDualWritesLegacyAndTypedSubjects() { IdentityAssertion assertion = assertion( - EmailAssurance.VERIFIED, - "alice@example.com"); - when(bindingRepository.findByProviderCodeAndSubject( - "github", - "123456")).thenReturn(Optional.empty()); - when(userRepository.save(any(UserAccount.class))) - .thenAnswer(invocation -> invocation.getArgument(0)); + new ExternalSubject("stable_id", "stable-123"), + Set.of( + new ExternalSubject( + "legacy_id", + "legacy-123"), + new ExternalSubject( + "alias_id", + "alias-123"))); PlatformPrincipal principal = principal("generated"); - when(principalFactory.create(any(UserAccount.class), org.mockito.ArgumentMatchers.eq("github"))) + when(principalFactory.create( + any(UserAccount.class), + org.mockito.ArgumentMatchers.eq("provider"))) .thenReturn(principal); - IdentityLoginOutcome outcome = - transaction.resolve(assertion, UserStatus.ACTIVE); - - assertThat(outcome) - .isEqualTo(new IdentityLoginOutcome.Authenticated( - principal, - true, - true)); - ArgumentCaptor userCaptor = - ArgumentCaptor.forClass(UserAccount.class); - verify(userRepository).save(userCaptor.capture()); - UserAccount created = userCaptor.getValue(); - assertThat(created.getId()).startsWith("usr_"); - assertThat(created.getDisplayName()).isEqualTo("alice"); - assertThat(created.getEmail()).isEqualTo("alice@example.com"); - assertThat(created.getAvatarUrl()) - .isEqualTo("https://avatars.example/alice.png"); - verify(membershipService).ensureMember(created.getId()); - ArgumentCaptor bindingCaptor = - ArgumentCaptor.forClass(IdentityBinding.class); - verify(bindingRepository).save(bindingCaptor.capture()); - assertThat(bindingCaptor.getValue().getProviderCode()) - .isEqualTo("github"); - assertThat(bindingCaptor.getValue().getSubject()) - .isEqualTo("123456"); - } - - @Test - void createsPendingAccountWithoutMembershipOrSessionPrincipal() { - IdentityAssertion assertion = assertion( - EmailAssurance.VERIFIED, - "alice@example.com"); - when(bindingRepository.findByProviderCodeAndSubject( - "github", - "123456")).thenReturn(Optional.empty()); - when(userRepository.save(any(UserAccount.class))) - .thenAnswer(invocation -> invocation.getArgument(0)); - - IdentityLoginOutcome outcome = - transaction.resolve(assertion, UserStatus.PENDING); - - assertThat(outcome).isEqualTo( - new IdentityLoginOutcome.PendingApproval("ACCOUNT_PENDING")); - verify(membershipService, never()).ensureMember(any()); - verify(principalFactory, never()).create(any(), any()); - verify(bindingRepository).save(any(IdentityBinding.class)); - } - - @Test - void existingApprovedAccountIgnoresPendingProvisioningDefault() { - IdentityBinding binding = - new IdentityBinding("usr_1", "github", "123456", "alice"); - UserAccount user = new UserAccount( - "usr_1", - "old", - "old@example.com", - null); - when(bindingRepository.findByProviderCodeAndSubject( - "github", - "123456")).thenReturn(Optional.of(binding)); - when(userRepository.findById("usr_1")).thenReturn(Optional.of(user)); - when(userRepository.save(user)).thenReturn(user); - PlatformPrincipal principal = principal("usr_1"); - when(principalFactory.create(user, "github")).thenReturn(principal); - - IdentityLoginOutcome outcome = - transaction.resolve( - assertion(EmailAssurance.VERIFIED, "alice@example.com"), - UserStatus.PENDING); + IdentityLoginOutcome outcome = transaction.resolve( + assertion, + UserStatus.ACTIVE, + "legacy_id"); assertThat(outcome).isEqualTo( new IdentityLoginOutcome.Authenticated( principal, - false, - false)); - assertThat(user.getDisplayName()).isEqualTo("alice"); - assertThat(user.getEmail()).isEqualTo("alice@example.com"); - verify(membershipService, never()).ensureMember(any()); - verify(bindingRepository, never()).save(binding); + true, + true)); + ArgumentCaptor bindingCaptor = + ArgumentCaptor.forClass(IdentityBinding.class); + verify(bindingRepository, org.mockito.Mockito.atLeastOnce()) + .save(bindingCaptor.capture()); + IdentityBinding binding = + bindingCaptor.getAllValues().getFirst(); + assertThat(binding.getSubject()).isEqualTo("legacy-123"); + assertThat(binding.getStatus()) + .isEqualTo(IdentityBindingStatus.ACTIVE); + assertThat(binding.getLastAuthenticatedAt()) + .isEqualTo(AUTHENTICATED_AT); + + @SuppressWarnings("unchecked") + ArgumentCaptor> subjectsCaptor = + ArgumentCaptor.forClass(List.class); + verify(subjectRepository).saveAll( + subjectsCaptor.capture()); + assertThat(subjectsCaptor.getValue()) + .extracting( + IdentityBindingSubject::getSubjectType, + IdentityBindingSubject::getSubjectValue, + IdentityBindingSubject::isPrimary) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple( + "stable_id", + "stable-123", + true), + org.assertj.core.groups.Tuple.tuple( + "legacy_id", + "legacy-123", + false), + org.assertj.core.groups.Tuple.tuple( + "alias_id", + "alias-123", + false)); + verify(membershipService).ensureMember(any()); } @Test - void existingPendingAccountReturnsPendingBeforeProfileMutation() { - IdentityBinding binding = - new IdentityBinding("usr_1", "github", "123456", "alice"); - UserAccount user = new UserAccount( + void upgradesLegacyPrimaryInOneTransaction() { + IdentityBinding binding = binding( + 1L, "usr_1", - "original", - "original@example.com", - null); - user.setStatus(UserStatus.PENDING); + "github", + "123456"); + IdentityBindingSubject legacy = + subject( + 1L, + "github", + "legacy_subject", + "123456", + true); + UserAccount user = user("usr_1", UserStatus.ACTIVE, false); when(bindingRepository.findByProviderCodeAndSubject( "github", "123456")).thenReturn(Optional.of(binding)); - when(userRepository.findById("usr_1")).thenReturn(Optional.of(user)); + when(bindingRepository.findByIdAndStatusForUpdate( + 1L, + IdentityBindingStatus.ACTIVE)) + .thenReturn(Optional.of(binding)); + when(subjectRepository.findByBindingIdAndStatusForUpdate( + 1L, + IdentityBindingSubjectStatus.ACTIVE)) + .thenReturn(List.of(legacy)); + when(userRepository.findById("usr_1")) + .thenReturn(Optional.of(user)); + when(principalFactory.create(user, "github")) + .thenReturn(principal("usr_1")); - IdentityLoginOutcome outcome = - transaction.resolve( - assertion(EmailAssurance.VERIFIED, "changed@example.com"), - UserStatus.ACTIVE); + transaction.resolve( + githubAssertion(Set.of()), + UserStatus.ACTIVE, + "github_user_id"); - assertThat(outcome).isEqualTo( - new IdentityLoginOutcome.PendingApproval("ACCOUNT_PENDING")); - assertThat(user.getDisplayName()).isEqualTo("original"); - assertThat(user.getEmail()).isEqualTo("original@example.com"); - verify(userRepository, never()).save(user); - verify(principalFactory, never()).create(any(), any()); + @SuppressWarnings("unchecked") + ArgumentCaptor> subjectsCaptor = + ArgumentCaptor.forClass(List.class); + verify(subjectRepository).saveAll( + subjectsCaptor.capture()); + assertThat(subjectsCaptor.getValue()) + .extracting( + IdentityBindingSubject::getSubjectType, + IdentityBindingSubject::getSubjectValue, + IdentityBindingSubject::isPrimary) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple( + "legacy_subject", + "123456", + false), + org.assertj.core.groups.Tuple.tuple( + "github_user_id", + "123456", + true)); } @Test - void blockedExistingAccountFailsBeforeProfileMutation() { - assertBlocked(UserStatus.DISABLED, false, IdentityFailureCode.ACCOUNT_DISABLED); - assertBlocked(UserStatus.MERGED, false, IdentityFailureCode.ACCOUNT_MERGED); - assertBlocked(UserStatus.ACTIVE, true, IdentityFailureCode.SYSTEM_ACCOUNT_FORBIDDEN); - } - - @Test - void unverifiedEmailNeverPopulatesOrOverwritesTrustedProfile() { + void resolvesMultipleAliasesOnlyWhenTheyBelongToOneBinding() { IdentityAssertion assertion = assertion( - EmailAssurance.UNVERIFIED, - "unverified@example.com"); + new ExternalSubject("stable_id", "stable-123"), + Set.of( + new ExternalSubject( + "legacy_id", + "legacy-123"), + new ExternalSubject( + "alias_id", + "alias-123"))); + IdentityBinding binding = binding( + 1L, + "usr_1", + "provider", + "legacy-123"); + IdentityBindingSubject alias = + subject( + 1L, + "provider", + "alias_id", + "alias-123", + true); + IdentityBindingSubject stable = + subject( + 1L, + "provider", + "stable_id", + "stable-123", + false); + when(subjectRepository.findMatchingSubjects( + org.mockito.ArgumentMatchers.eq("provider"), + any())).thenReturn(List.of(alias, stable)); + when(bindingRepository.findByProviderCodeAndSubject( + "provider", + "legacy-123")).thenReturn(Optional.of(binding)); + when(bindingRepository.findByIdAndStatusForUpdate( + 1L, + IdentityBindingStatus.ACTIVE)) + .thenReturn(Optional.of(binding)); + when(subjectRepository.findByBindingIdAndStatusForUpdate( + 1L, + IdentityBindingSubjectStatus.ACTIVE)) + .thenReturn(List.of(alias, stable)); + UserAccount user = user("usr_1", UserStatus.ACTIVE, false); + when(userRepository.findById("usr_1")) + .thenReturn(Optional.of(user)); + when(principalFactory.create(user, "provider")) + .thenReturn(principal("usr_1")); + + transaction.resolve( + assertion, + UserStatus.ACTIVE, + "legacy_id"); + + assertThat(alias.isPrimary()).isFalse(); + assertThat(stable.isPrimary()).isTrue(); + } + + @Test + void aliasesResolvingToDifferentBindingsFailClosed() { + IdentityAssertion assertion = assertion( + new ExternalSubject("stable_id", "stable-123"), + Set.of( + new ExternalSubject( + "legacy_id", + "legacy-123"), + new ExternalSubject( + "alias_id", + "alias-123"))); + when(subjectRepository.findMatchingSubjects( + org.mockito.ArgumentMatchers.eq("provider"), + any())).thenReturn(List.of( + subject( + 1L, + "provider", + "stable_id", + "stable-123", + true), + subject( + 2L, + "provider", + "alias_id", + "alias-123", + true))); + + assertThatThrownBy(() -> transaction.resolve( + assertion, + UserStatus.ACTIVE, + "legacy_id")) + .isInstanceOf(IdentityCoreException.class) + .extracting("reasonCode") + .isEqualTo( + IdentityFailureCode + .IDENTITY_IDENTIFIER_CONFLICT); + verify(userRepository, never()).save(any()); + } + + @Test + void revokedPrimaryCannotBeAutomaticallyReactivated() { + IdentityBindingSubject revoked = + subject( + 1L, + "github", + "github_user_id", + "123456", + false); + ReflectionTestUtils.setField( + revoked, + "status", + IdentityBindingSubjectStatus.REVOKED); + ReflectionTestUtils.setField( + revoked, + "revokedAt", + AUTHENTICATED_AT.minusSeconds(60)); + when(subjectRepository.findMatchingSubjects( + org.mockito.ArgumentMatchers.eq("github"), + any())).thenReturn(List.of(revoked)); + + assertThatThrownBy(() -> transaction.resolve( + githubAssertion(Set.of()), + UserStatus.ACTIVE, + "github_user_id")) + .isInstanceOf(IdentityCoreException.class) + .extracting("reasonCode") + .isEqualTo(IdentityFailureCode.ACCESS_DENIED); + verify(bindingRepository, never()) + .findByIdAndStatusForUpdate(any(), any()); + } + + @Test + void pendingAccountUpgradesSubjectsWithoutMutatingProfile() { + IdentityBinding binding = binding( + 1L, + "usr_1", + "github", + "123456"); + UserAccount user = user("usr_1", UserStatus.PENDING, false); when(bindingRepository.findByProviderCodeAndSubject( "github", - "123456")).thenReturn(Optional.empty()); - when(userRepository.save(any(UserAccount.class))) - .thenAnswer(invocation -> invocation.getArgument(0)); - when(principalFactory.create(any(), any())).thenReturn(principal("new")); + "123456")).thenReturn(Optional.of(binding)); + when(bindingRepository.findByIdAndStatusForUpdate( + 1L, + IdentityBindingStatus.ACTIVE)) + .thenReturn(Optional.of(binding)); + when(subjectRepository.findByBindingIdAndStatusForUpdate( + 1L, + IdentityBindingSubjectStatus.ACTIVE)) + .thenReturn(List.of()); + when(userRepository.findById("usr_1")) + .thenReturn(Optional.of(user)); - transaction.resolve(assertion, UserStatus.ACTIVE); + IdentityLoginOutcome outcome = transaction.resolve( + githubAssertion(Set.of()), + UserStatus.ACTIVE, + "github_user_id"); - ArgumentCaptor userCaptor = - ArgumentCaptor.forClass(UserAccount.class); - verify(userRepository).save(userCaptor.capture()); - assertThat(userCaptor.getValue().getEmail()).isNull(); + assertThat(outcome).isEqualTo( + new IdentityLoginOutcome.PendingApproval( + "ACCOUNT_PENDING")); + assertThat(user.getDisplayName()).isEqualTo("original"); + assertThat(user.getEmail()) + .isEqualTo("original@example.com"); + verify(userRepository, never()).save(user); + verify(subjectRepository).saveAll(any()); + } + + @Test + void blockedAccountFailsBeforeAnyBindingMutation() { + assertBlocked( + UserStatus.DISABLED, + false, + IdentityFailureCode.ACCOUNT_DISABLED); + assertBlocked( + UserStatus.MERGED, + false, + IdentityFailureCode.ACCOUNT_MERGED); + assertBlocked( + UserStatus.ACTIVE, + true, + IdentityFailureCode.SYSTEM_ACCOUNT_FORBIDDEN); } private void assertBlocked( UserStatus status, boolean system, IdentityFailureCode expectedCode) { - IdentityBindingRepository localBindingRepository = - mock(IdentityBindingRepository.class); - UserAccountRepository localUserRepository = - mock(UserAccountRepository.class); - IdentityResolutionTransaction localTransaction = - new IdentityResolutionTransaction( - localBindingRepository, - localUserRepository, - membershipService, - accountLoginGuard, - principalFactory); - IdentityBinding binding = - new IdentityBinding("usr_blocked", "github", "123456", "old"); - UserAccount user = system - ? UserAccount.systemAccount( - "usr_blocked", - "original", - "original@example.com", - null) - : new UserAccount( - "usr_blocked", - "original", - "original@example.com", - null); - user.setStatus(status); - when(localBindingRepository.findByProviderCodeAndSubject( + IdentityBinding binding = binding( + 1L, + "usr_blocked", + "github", + "123456"); + UserAccount user = user("usr_blocked", status, system); + when(bindingRepository.findByProviderCodeAndSubject( "github", "123456")).thenReturn(Optional.of(binding)); - when(localUserRepository.findById("usr_blocked")) + when(bindingRepository.findByIdAndStatusForUpdate( + 1L, + IdentityBindingStatus.ACTIVE)) + .thenReturn(Optional.of(binding)); + when(userRepository.findById("usr_blocked")) .thenReturn(Optional.of(user)); - assertThatThrownBy(() -> localTransaction.resolve( - assertion(EmailAssurance.VERIFIED, "changed@example.com"), - UserStatus.ACTIVE)) + assertThatThrownBy(() -> transaction.resolve( + githubAssertion(Set.of()), + UserStatus.ACTIVE, + "github_user_id")) .isInstanceOf(IdentityCoreException.class) .extracting("reasonCode") .isEqualTo(expectedCode); - - assertThat(user.getDisplayName()).isEqualTo("original"); - assertThat(user.getEmail()).isEqualTo("original@example.com"); - verify(localUserRepository, never()).save(user); + verify(subjectRepository, never()) + .findByBindingIdAndStatusForUpdate( + any(), + any()); } - private static IdentityAssertion assertion( - EmailAssurance assurance, - String email) { + private static IdentityAssertion githubAssertion( + Set aliases) { return new IdentityAssertion( new ProviderReference( "github", "oauth2-github", "https://github.com"), - new ExternalSubject("github_user_id", "123456"), - Set.of(), - new ExternalProfile( - "alice", - Optional.of(new EmailClaim(email, assurance)), - Optional.of(URI.create( - "https://avatars.example/alice.png"))), + new ExternalSubject( + "github_user_id", + "123456"), + aliases, + profile(), Map.of(), - new AuthenticationEvidence( - "oauth2-github", - Instant.parse("2026-07-30T08:00:00Z"), - Set.of("oauth2_authorization_code"))); + evidence("oauth2-github")); + } + + private static IdentityAssertion assertion( + ExternalSubject primary, + Set aliases) { + return new IdentityAssertion( + new ProviderReference( + "provider", + "oidc", + "https://id.example.com"), + primary, + aliases, + profile(), + Map.of(), + evidence("oidc")); + } + + private static ExternalProfile profile() { + return new ExternalProfile( + "alice", + Optional.of(new EmailClaim( + "alice@example.com", + EmailAssurance.VERIFIED)), + Optional.of(URI.create( + "https://avatars.example/alice.png"))); + } + + private static AuthenticationEvidence evidence( + String protocol) { + return new AuthenticationEvidence( + protocol, + AUTHENTICATED_AT, + Set.of("oauth2_authorization_code")); + } + + private static IdentityBinding binding( + long id, + String userId, + String providerCode, + String subject) { + IdentityBinding binding = new IdentityBinding( + userId, + providerCode, + subject, + "alice"); + ReflectionTestUtils.setField(binding, "id", id); + return binding; + } + + private static IdentityBindingSubject subject( + long bindingId, + String providerCode, + String type, + String value, + boolean primary) { + return new IdentityBindingSubject( + bindingId, + providerCode, + type, + value, + primary, + AUTHENTICATED_AT); + } + + private static UserAccount user( + String userId, + UserStatus status, + boolean system) { + UserAccount user = system + ? UserAccount.systemAccount( + userId, + "original", + "original@example.com", + null) + : new UserAccount( + userId, + "original", + "original@example.com", + null); + user.setStatus(status); + return user; } private static PlatformPrincipal principal(String userId) { @@ -273,7 +522,7 @@ class IdentityResolutionTransactionTest { "alice", "alice@example.com", null, - "github", + "provider", Set.of("USER")); } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityLockServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityLockServiceTest.java index 7048c02a..6b1a4691 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityLockServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityLockServiceTest.java @@ -36,8 +36,10 @@ class ProviderAuthorityLockServiceTest { "https://github.com", "GitHub", "github_user_id", - java.util.Set.of("github_user_id"), - SubjectCanonicalizer.DECIMAL, + "github_user_id", + java.util.Map.of( + "github_user_id", + SubjectCanonicalizer.DECIMAL), java.util.List.of("login"), java.util.List.of("email"), java.util.List.of("avatar_url"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityStateTransactionTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityStateTransactionTest.java index 8a180a0a..ce6faf57 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityStateTransactionTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ProviderAuthorityStateTransactionTest.java @@ -406,8 +406,10 @@ class ProviderAuthorityStateTransactionTest { "https://github.com", "GitHub", "github_user_id", - java.util.Set.of("github_user_id"), - SubjectCanonicalizer.DECIMAL, + "github_user_id", + java.util.Map.of( + "github_user_id", + SubjectCanonicalizer.DECIMAL), java.util.List.of("login"), java.util.List.of("email"), java.util.List.of("avatar_url"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ReconciledIdentityProviderCatalogTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ReconciledIdentityProviderCatalogTest.java index bf7cae67..954ff17a 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ReconciledIdentityProviderCatalogTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/ReconciledIdentityProviderCatalogTest.java @@ -111,8 +111,10 @@ class ReconciledIdentityProviderCatalogTest { "https://" + providerCode + ".example", displayName, "oidc_sub", - Set.of("oidc_sub"), - SubjectCanonicalizer.EXACT, + "oidc_sub", + java.util.Map.of( + "oidc_sub", + SubjectCanonicalizer.EXACT), List.of("name"), List.of("email"), List.of("picture"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSourceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSourceTest.java index a7318f0e..64d3b1a3 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSourceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/StaticTrustedProviderDescriptorSourceTest.java @@ -82,7 +82,9 @@ class StaticTrustedProviderDescriptorSourceTest { assertThat(descriptor.canonicalAuthority()) .isEqualTo("https://id.example.com/tenant"); assertThat(descriptor.primarySubjectType()).isEqualTo("oidc_sub"); - assertThat(descriptor.subjectCanonicalizer()) + assertThat(descriptor.legacyPrimarySubjectType()) + .isEqualTo("oidc_sub"); + assertThat(descriptor.canonicalizerFor("oidc_sub")) .isEqualTo(SubjectCanonicalizer.EXACT); }