mirror of
https://github.com/iflytek/skillhub.git
synced 2026-08-27 11:14:59 +00:00
chore(auth): merge big-main into provider registry
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
commit
c6fe19e2c8
30 changed files with 569 additions and 64 deletions
|
|
@ -38,7 +38,7 @@ They did not connect to or modify the shared test-environment database.
|
|||
|
||||
| Scenario | Observable result |
|
||||
|---|---|
|
||||
| Fresh migration | Flyway V44 applied successfully and created `identity_provider_state` |
|
||||
| Fresh migration | Flyway V45 applied successfully and created `identity_provider_state` after the reserved V44 compliance index |
|
||||
| Fixed GitHub authority vector | `oauth2-github`, `https://github.com`, fingerprint `b2a93d58465e3de9e8b6cd127ba18425ae0f80c49c85f18f76086832923ca619`, state `READY` |
|
||||
| Concurrent first pin | Two application instances converged to one READY row with the same fingerprint; no unique-constraint error |
|
||||
| Legacy OAuth binding | Existing `identity_binding` row remained byte-for-byte equivalent while the provider moved through first pin to READY |
|
||||
|
|
@ -48,8 +48,8 @@ They did not connect to or modify the shared test-environment database.
|
|||
| Stale READY mismatch window | Recovery returned 409, persisted `AUTHORITY_MISMATCH`, retained the pinned authority/fingerprint, and wrote no recovery audit |
|
||||
| Transaction rollback | A forced audit insert failure returned 500; the provider state update rolled back and no audit record was added |
|
||||
| Unknown provider routes | Authorization and callback routes returned 403 without an upstream redirect |
|
||||
| V43 to V44 upgrade | A database initialized by `v0.2.15` upgraded successfully and retained its legacy OAuth binding |
|
||||
| Mixed-version and rollback | Current and `v0.2.15` servers were simultaneously healthy against the V44 database; the old provider endpoint returned 200 |
|
||||
| V43 to V45 upgrade | A database initialized by `v0.2.15` upgraded successfully through reserved V44 and retained its legacy OAuth binding |
|
||||
| Mixed-version and rollback | Current and `v0.2.15` servers were simultaneously healthy against the V45 database; the old provider endpoint returned 200 |
|
||||
| Redis session compatibility | A local session created by `v0.2.15` was accepted by the current server for the same user |
|
||||
|
||||
## Remaining integration gate
|
||||
|
|
@ -60,7 +60,7 @@ the merge commit, deploy them to the shared test environment, and verify:
|
|||
1. health, login catalog, and local-password login through the configured test
|
||||
domain;
|
||||
2. unknown provider authorization/callback rejection;
|
||||
3. V44 migration and READY provider state in the shared database;
|
||||
3. reserved V44 compatibility migration, V45 identity migration, and READY provider state in the shared database;
|
||||
4. existing Redis sessions and OAuth bindings;
|
||||
5. recovery authorization and audit behavior;
|
||||
6. logs contain no credentials or unexpected identity errors.
|
||||
|
|
|
|||
|
|
@ -10,15 +10,29 @@ POSTGRES_PASSWORD="identity-v2-test-password"
|
|||
POSTGRES_DB="identity_v2"
|
||||
MAVEN_CACHE_DIR="${MAVEN_CACHE_DIR:-${HOME}/.m2}"
|
||||
|
||||
log() {
|
||||
printf '[identity-binding-v2] %s\n' "$*"
|
||||
}
|
||||
|
||||
cleanup() {
|
||||
exit_code="$?"
|
||||
if [[ "${exit_code}" -ne 0 ]]; then
|
||||
log "failed with exit code ${exit_code}"
|
||||
docker ps -a \
|
||||
--filter "label=skillhub.test.run=${RUN_ID}" \
|
||||
--format 'resource={{.Names}} status={{.Status}}' || true
|
||||
docker logs "${POSTGRES_CONTAINER}" 2>&1 || true
|
||||
fi
|
||||
docker rm -f "${POSTGRES_CONTAINER}" >/dev/null 2>&1 || true
|
||||
docker network rm "${NETWORK}" >/dev/null 2>&1 || true
|
||||
}
|
||||
trap cleanup EXIT
|
||||
|
||||
log "creating isolated Docker network ${NETWORK}"
|
||||
docker network create \
|
||||
--label "skillhub.test.run=${RUN_ID}" \
|
||||
"${NETWORK}" >/dev/null
|
||||
log "starting isolated PostgreSQL container ${POSTGRES_CONTAINER}"
|
||||
docker run -d \
|
||||
--name "${POSTGRES_CONTAINER}" \
|
||||
--label "skillhub.test.run=${RUN_ID}" \
|
||||
|
|
@ -31,28 +45,43 @@ docker run -d \
|
|||
-p 127.0.0.1::5432 \
|
||||
postgres:16-alpine >/dev/null
|
||||
|
||||
log "waiting for PostgreSQL readiness"
|
||||
postgres_ready="false"
|
||||
for _ in $(seq 1 60); do
|
||||
if docker exec "${POSTGRES_CONTAINER}" \
|
||||
pid_one_comm="$(
|
||||
docker exec "${POSTGRES_CONTAINER}" \
|
||||
cat /proc/1/comm 2>/dev/null || true
|
||||
)"
|
||||
if [[ "${pid_one_comm}" == "postgres" ]] \
|
||||
&& docker exec "${POSTGRES_CONTAINER}" \
|
||||
pg_isready -U "${POSTGRES_USER}" -d "${POSTGRES_DB}" \
|
||||
>/dev/null 2>&1; then
|
||||
postgres_ready="true"
|
||||
break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
docker exec "${POSTGRES_CONTAINER}" \
|
||||
pg_isready -U "${POSTGRES_USER}" -d "${POSTGRES_DB}" \
|
||||
>/dev/null
|
||||
if [[ "${postgres_ready}" != "true" ]]; then
|
||||
log "PostgreSQL did not become ready after entrypoint initialization"
|
||||
exit 1
|
||||
fi
|
||||
log "PostgreSQL is ready"
|
||||
|
||||
run_test() {
|
||||
test_class="$1"
|
||||
flyway_target="${2:-}"
|
||||
java_version=""
|
||||
if command -v java >/dev/null 2>&1; then
|
||||
java_version="$(java -version 2>&1 | head -n 1)"
|
||||
java_version="$(java -version 2>&1)"
|
||||
fi
|
||||
if [[ "${java_version}" == *'"21.'* ]]; then
|
||||
host_port="$(docker port "${POSTGRES_CONTAINER}" 5432/tcp \
|
||||
| sed -n 's/.*://p')"
|
||||
if [[ -z "${host_port}" ]]; then
|
||||
log "Docker did not publish a PostgreSQL host port"
|
||||
return 1
|
||||
fi
|
||||
log "running ${test_class} with host Java 21"
|
||||
(
|
||||
cd "${REPO_ROOT}/server"
|
||||
IDENTITY_BINDING_V2_POSTGRES_URL="jdbc:postgresql://127.0.0.1:${host_port}/${POSTGRES_DB}" \
|
||||
|
|
@ -70,6 +99,7 @@ run_test() {
|
|||
return
|
||||
fi
|
||||
|
||||
log "running ${test_class} with containerized Java 21"
|
||||
mkdir -p "${MAVEN_CACHE_DIR}"
|
||||
docker run --rm \
|
||||
--name "${RUN_ID}-java" \
|
||||
|
|
@ -98,7 +128,7 @@ run_test() {
|
|||
}
|
||||
|
||||
run_test IdentityBindingV2MigrationPostgresTest
|
||||
run_test IdentityBindingV2ContractPostgresTest 46
|
||||
run_test IdentityBindingV2ContractPostgresTest 47
|
||||
run_test UserProfileFieldSourceMigrationPostgresTest
|
||||
run_test IdentityBindingV2PostgresIntegrationTest 47
|
||||
run_test IdentityProfileProvisioningPostgresIntegrationTest 47
|
||||
run_test IdentityBindingV2PostgresIntegrationTest 48
|
||||
run_test IdentityProfileProvisioningPostgresIntegrationTest 48
|
||||
|
|
|
|||
|
|
@ -1,14 +1,16 @@
|
|||
package com.iflytek.skillhub.config;
|
||||
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.Executor;
|
||||
import java.util.concurrent.ThreadPoolExecutor;
|
||||
import org.slf4j.MDC;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.core.task.TaskDecorator;
|
||||
import org.springframework.scheduling.annotation.EnableAsync;
|
||||
import org.springframework.scheduling.annotation.EnableScheduling;
|
||||
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
|
||||
|
||||
import java.util.concurrent.Executor;
|
||||
import java.util.concurrent.ThreadPoolExecutor;
|
||||
|
||||
/**
|
||||
* Enables asynchronous event handling and other background execution features used by the
|
||||
* application module.
|
||||
|
|
@ -26,7 +28,30 @@ public class AsyncConfig {
|
|||
executor.setQueueCapacity(100);
|
||||
executor.setThreadNamePrefix("skillhub-event-");
|
||||
executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy());
|
||||
executor.setTaskDecorator(mdcTaskDecorator());
|
||||
executor.initialize();
|
||||
return executor;
|
||||
}
|
||||
|
||||
private TaskDecorator mdcTaskDecorator() {
|
||||
return task -> {
|
||||
Map<String, String> callerContext = MDC.getCopyOfContextMap();
|
||||
return () -> {
|
||||
Map<String, String> executorContext = MDC.getCopyOfContextMap();
|
||||
try {
|
||||
restoreMdc(callerContext);
|
||||
task.run();
|
||||
} finally {
|
||||
restoreMdc(executorContext);
|
||||
}
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
private void restoreMdc(Map<String, String> context) {
|
||||
MDC.clear();
|
||||
if (context != null) {
|
||||
MDC.setContextMap(context);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -60,4 +60,8 @@ public class SkillHubMetrics {
|
|||
"operation", operation
|
||||
).increment();
|
||||
}
|
||||
|
||||
public void incrementSearchRebuildFailure() {
|
||||
meterRegistry.counter("skillhub.search.rebuild.failure").increment();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -133,7 +133,7 @@ public class AdminUserAppService {
|
|||
String status,
|
||||
String actorUserId,
|
||||
AuditRequestContext auditContext) {
|
||||
UserAccount user = loadUser(userId);
|
||||
UserAccount user = loadUserForUpdate(userId);
|
||||
rejectSystemAccountMutation(user);
|
||||
UserStatus nextStatus = parseManageableStatus(status);
|
||||
UserStatus previousStatus = user.getStatus();
|
||||
|
|
@ -258,6 +258,13 @@ public class AdminUserAppService {
|
|||
.orElseThrow(() -> new DomainNotFoundException("error.admin.user.notFound", userId));
|
||||
}
|
||||
|
||||
private UserAccount loadUserForUpdate(String userId) {
|
||||
return userAccountRepository.findByIdForUpdate(userId)
|
||||
.orElseThrow(() -> new DomainNotFoundException(
|
||||
"error.admin.user.notFound",
|
||||
userId));
|
||||
}
|
||||
|
||||
private void rejectSystemAccountMutation(UserAccount user) {
|
||||
if (user.isSystemAccount()) {
|
||||
throw new DomainForbiddenException("error.admin.user.systemAccount.immutable");
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.metrics.SkillHubMetrics;
|
||||
import com.iflytek.skillhub.search.SearchRebuildService;
|
||||
import java.util.List;
|
||||
import org.slf4j.Logger;
|
||||
|
|
@ -15,9 +16,12 @@ public class LabelSearchSyncService {
|
|||
private static final Logger log = LoggerFactory.getLogger(LabelSearchSyncService.class);
|
||||
|
||||
private final SearchRebuildService searchRebuildService;
|
||||
private final SkillHubMetrics metrics;
|
||||
|
||||
public LabelSearchSyncService(SearchRebuildService searchRebuildService) {
|
||||
public LabelSearchSyncService(SearchRebuildService searchRebuildService,
|
||||
SkillHubMetrics metrics) {
|
||||
this.searchRebuildService = searchRebuildService;
|
||||
this.metrics = metrics;
|
||||
}
|
||||
|
||||
@Async("skillhubEventExecutor")
|
||||
|
|
@ -25,6 +29,7 @@ public class LabelSearchSyncService {
|
|||
try {
|
||||
searchRebuildService.rebuildBySkill(skillId);
|
||||
} catch (RuntimeException ex) {
|
||||
metrics.incrementSearchRebuildFailure();
|
||||
log.error("Failed to rebuild search document for skill {}", skillId, ex);
|
||||
}
|
||||
}
|
||||
|
|
@ -41,6 +46,7 @@ public class LabelSearchSyncService {
|
|||
try {
|
||||
searchRebuildService.rebuildBySkill(skillId);
|
||||
} catch (RuntimeException ex) {
|
||||
metrics.incrementSearchRebuildFailure();
|
||||
log.error("Failed to rebuild search document for skill {} after label change", skillId, ex);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,3 @@
|
|||
CREATE INDEX IF NOT EXISTS idx_skill_version_compliance_mappings
|
||||
ON skill_version
|
||||
USING GIN ((parsed_metadata_json -> 'frontmatter' -> 'x-astron-compliance'));
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
-- Binding V2 contract gate.
|
||||
--
|
||||
-- Deploy this migration only after every pre-Binding-V2 application instance
|
||||
-- has exited. Unlike the V45 expand migration, this gate rejects transactions
|
||||
-- has exited. Unlike the V46 expand migration, this gate rejects transactions
|
||||
-- that leave an ACTIVE binding without exactly one ACTIVE primary subject.
|
||||
|
||||
DO $$
|
||||
|
|
@ -37,7 +37,7 @@ class IdentityBindingV2ContractPostgresTest {
|
|||
.schemas(PREFLIGHT_SCHEMA)
|
||||
.defaultSchema(PREFLIGHT_SCHEMA)
|
||||
.createSchemas(true)
|
||||
.target(MigrationVersion.fromVersion("45"))
|
||||
.target(MigrationVersion.fromVersion("46"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
@ -95,7 +95,7 @@ class IdentityBindingV2ContractPostgresTest {
|
|||
WHERE success = TRUE
|
||||
ORDER BY installed_rank DESC
|
||||
LIMIT 1
|
||||
""")).isEqualTo("45");
|
||||
""")).isEqualTo("46");
|
||||
assertThat(singleLong(
|
||||
statement,
|
||||
"""
|
||||
|
|
@ -126,7 +126,7 @@ class IdentityBindingV2ContractPostgresTest {
|
|||
database.username(),
|
||||
database.password())
|
||||
.locations("classpath:db/migration")
|
||||
.target(MigrationVersion.fromVersion("46"))
|
||||
.target(MigrationVersion.fromVersion("47"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@ class IdentityBindingV2MigrationPostgresTest {
|
|||
Flyway.configure()
|
||||
.dataSource(url, username, password)
|
||||
.locations("classpath:db/migration")
|
||||
.target(MigrationVersion.fromVersion("44"))
|
||||
.target(MigrationVersion.fromVersion("45"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
@ -91,7 +91,7 @@ class IdentityBindingV2MigrationPostgresTest {
|
|||
Flyway.configure()
|
||||
.dataSource(url, username, password)
|
||||
.locations("classpath:db/migration")
|
||||
.target(MigrationVersion.fromVersion("45"))
|
||||
.target(MigrationVersion.fromVersion("46"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
@ -237,7 +237,7 @@ class IdentityBindingV2MigrationPostgresTest {
|
|||
.schemas(PREFLIGHT_SCHEMA)
|
||||
.defaultSchema(PREFLIGHT_SCHEMA)
|
||||
.createSchemas(true)
|
||||
.target(MigrationVersion.fromVersion("44"))
|
||||
.target(MigrationVersion.fromVersion("45"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
@ -315,7 +315,7 @@ class IdentityBindingV2MigrationPostgresTest {
|
|||
.schemas(PREFLIGHT_SCHEMA)
|
||||
.defaultSchema(PREFLIGHT_SCHEMA)
|
||||
.createSchemas(true)
|
||||
.target(MigrationVersion.fromVersion("45"))
|
||||
.target(MigrationVersion.fromVersion("46"))
|
||||
.load()
|
||||
.migrate());
|
||||
|
||||
|
|
@ -343,7 +343,7 @@ class IdentityBindingV2MigrationPostgresTest {
|
|||
WHERE success = TRUE
|
||||
ORDER BY installed_rank DESC
|
||||
LIMIT 1
|
||||
""")).isEqualTo("44");
|
||||
""")).isEqualTo("45");
|
||||
assertThat(singleLong(
|
||||
statement,
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ import org.springframework.test.annotation.DirtiesContext;
|
|||
import org.springframework.test.context.ActiveProfiles;
|
||||
import org.springframework.test.context.DynamicPropertyRegistry;
|
||||
import org.springframework.test.context.DynamicPropertySource;
|
||||
import org.springframework.transaction.PlatformTransactionManager;
|
||||
import org.springframework.transaction.support.TransactionTemplate;
|
||||
|
||||
@SpringBootTest
|
||||
@ActiveProfiles("test")
|
||||
|
|
@ -48,6 +50,12 @@ class IdentityProfileProvisioningPostgresIntegrationTest {
|
|||
@Autowired
|
||||
private AdminUserAppService adminUserAppService;
|
||||
|
||||
@Autowired
|
||||
private IdentitySecurityAuditWriter securityAuditWriter;
|
||||
|
||||
@Autowired
|
||||
private PlatformTransactionManager transactionManager;
|
||||
|
||||
@Autowired
|
||||
private JdbcTemplate jdbcTemplate;
|
||||
|
||||
|
|
@ -333,6 +341,13 @@ class IdentityProfileProvisioningPostgresIntegrationTest {
|
|||
'ACTIVE',
|
||||
CURRENT_TIMESTAMP,
|
||||
CURRENT_TIMESTAMP
|
||||
), (
|
||||
'existing-collision-user-2',
|
||||
'Existing User 2',
|
||||
'collision@example.com',
|
||||
'ACTIVE',
|
||||
CURRENT_TIMESTAMP,
|
||||
CURRENT_TIMESTAMP
|
||||
)
|
||||
""");
|
||||
|
||||
|
|
@ -357,7 +372,42 @@ class IdentityProfileProvisioningPostgresIntegrationTest {
|
|||
provider)).isZero();
|
||||
assertThat(count(
|
||||
"SELECT COUNT(*) FROM user_account WHERE email = ?",
|
||||
"collision@example.com")).isEqualTo(1L);
|
||||
"collision@example.com")).isEqualTo(2L);
|
||||
}
|
||||
|
||||
@Test
|
||||
void deniedSecurityAuditSurvivesCallerTransactionRollback() {
|
||||
TransactionTemplate transactionTemplate =
|
||||
new TransactionTemplate(transactionManager);
|
||||
|
||||
assertThatThrownBy(() ->
|
||||
transactionTemplate.executeWithoutResult(status -> {
|
||||
securityAuditWriter.recordDenied(
|
||||
"profile-auto",
|
||||
"oidc",
|
||||
IdentityFailureCode.ACCESS_DENIED,
|
||||
new IdentityLoginContext(
|
||||
"identity-denial-audit-rollback",
|
||||
"127.0.0.1",
|
||||
"identity-profile-test"));
|
||||
throw new IllegalStateException(
|
||||
"force caller rollback");
|
||||
}))
|
||||
.isInstanceOf(IllegalStateException.class)
|
||||
.hasMessage("force caller rollback");
|
||||
|
||||
assertThat(count(
|
||||
"""
|
||||
SELECT COUNT(*)
|
||||
FROM audit_log
|
||||
WHERE action = 'IDENTITY_LOGIN_DENIED'
|
||||
AND request_id =
|
||||
'identity-denial-audit-rollback'
|
||||
AND detail_json ->> 'providerCode' =
|
||||
'profile-auto'
|
||||
AND detail_json ->> 'reason' =
|
||||
'ACCESS_DENIED'
|
||||
""")).isEqualTo(1L);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ class UserProfileFieldSourceMigrationPostgresTest {
|
|||
.schemas(SCHEMA)
|
||||
.defaultSchema(SCHEMA)
|
||||
.createSchemas(true)
|
||||
.target(MigrationVersion.fromVersion("46"))
|
||||
.target(MigrationVersion.fromVersion("47"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
@ -86,7 +86,7 @@ class UserProfileFieldSourceMigrationPostgresTest {
|
|||
.schemas(SCHEMA)
|
||||
.defaultSchema(SCHEMA)
|
||||
.createSchemas(true)
|
||||
.target(MigrationVersion.fromVersion("47"))
|
||||
.target(MigrationVersion.fromVersion("48"))
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
|
|
|
|||
|
|
@ -2,9 +2,13 @@ package com.iflytek.skillhub.config;
|
|||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import java.util.concurrent.CompletableFuture;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.slf4j.MDC;
|
||||
import org.springframework.scheduling.annotation.EnableAsync;
|
||||
import org.springframework.scheduling.annotation.EnableScheduling;
|
||||
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
|
||||
|
||||
class AsyncConfigTest {
|
||||
|
||||
|
|
@ -13,4 +17,26 @@ class AsyncConfigTest {
|
|||
assertThat(AsyncConfig.class).hasAnnotation(EnableAsync.class);
|
||||
assertThat(AsyncConfig.class).hasAnnotation(EnableScheduling.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
void skillhubEventExecutor_propagatesAndClearsMdc() throws Exception {
|
||||
ThreadPoolTaskExecutor executor =
|
||||
(ThreadPoolTaskExecutor) new AsyncConfig().skillhubEventExecutor();
|
||||
try {
|
||||
MDC.put("requestId", "req-597");
|
||||
CompletableFuture<String> propagatedRequestId = new CompletableFuture<>();
|
||||
executor.execute(() -> propagatedRequestId.complete(MDC.get("requestId")));
|
||||
MDC.clear();
|
||||
|
||||
assertThat(propagatedRequestId.get(5, TimeUnit.SECONDS)).isEqualTo("req-597");
|
||||
|
||||
CompletableFuture<String> nextRequestId = new CompletableFuture<>();
|
||||
executor.execute(() -> nextRequestId.complete(MDC.get("requestId")));
|
||||
|
||||
assertThat(nextRequestId.get(5, TimeUnit.SECONDS)).isNull();
|
||||
} finally {
|
||||
MDC.clear();
|
||||
executor.shutdown();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -102,11 +102,24 @@ class FlywayMigrationGuardrailTest {
|
|||
assertThat(migration).contains("bad_namespace.slug <> 'global'");
|
||||
}
|
||||
|
||||
@Test
|
||||
void complianceIndexMigration_mustMatchReservedV44() throws IOException {
|
||||
String migration = Files.readString(
|
||||
migrationPath(
|
||||
"V44__skill_version_compliance_index.sql"));
|
||||
|
||||
assertThat(migration).isEqualTo("""
|
||||
CREATE INDEX IF NOT EXISTS idx_skill_version_compliance_mappings
|
||||
ON skill_version
|
||||
USING GIN ((parsed_metadata_json -> 'frontmatter' -> 'x-astron-compliance'));
|
||||
""");
|
||||
}
|
||||
|
||||
@Test
|
||||
void identityBindingContractGate_mustRemainDeferred() throws IOException {
|
||||
String migration = Files.readString(
|
||||
migrationPath(
|
||||
"V46__identity_binding_v2_contract_gate.sql"));
|
||||
"V47__identity_binding_v2_contract_gate.sql"));
|
||||
|
||||
assertThat(migration)
|
||||
.contains("CREATE CONSTRAINT TRIGGER")
|
||||
|
|
@ -120,7 +133,7 @@ class FlywayMigrationGuardrailTest {
|
|||
throws IOException {
|
||||
String migration = Files.readString(
|
||||
migrationPath(
|
||||
"V47__user_profile_field_source.sql"));
|
||||
"V48__user_profile_field_source.sql"));
|
||||
|
||||
assertThat(migration)
|
||||
.contains("LEGACY_LOCAL")
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ class PrometheusEndpointTest {
|
|||
skillHubMetrics.incrementUserRegister();
|
||||
skillHubMetrics.recordLocalLogin(true);
|
||||
skillHubMetrics.incrementSkillPublish("global", "PENDING_REVIEW");
|
||||
skillHubMetrics.incrementSearchRebuildFailure();
|
||||
|
||||
assertThat(environment.getProperty("management.endpoints.web.exposure.include"))
|
||||
.doesNotContain("prometheus")
|
||||
|
|
@ -54,5 +55,8 @@ class PrometheusEndpointTest {
|
|||
.tag("status", "PENDING_REVIEW")
|
||||
.counter()
|
||||
.count()).isEqualTo(1.0d);
|
||||
assertThat(meterRegistry.get("skillhub.search.rebuild.failure")
|
||||
.counter()
|
||||
.count()).isEqualTo(1.0d);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -154,7 +154,7 @@ class AdminUserAppServiceTest {
|
|||
|
||||
@Test
|
||||
void updateUserStatus_rejectsUnsupportedStatuses() {
|
||||
when(userAccountRepository.findById("user-1"))
|
||||
when(userAccountRepository.findByIdForUpdate("user-1"))
|
||||
.thenReturn(Optional.of(user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE)));
|
||||
|
||||
assertThrows(DomainBadRequestException.class, () -> service.updateUserStatus("user-1", "MERGED"));
|
||||
|
|
@ -163,7 +163,7 @@ class AdminUserAppServiceTest {
|
|||
@Test
|
||||
void updateUserStatus_updatesPersistedStatus() {
|
||||
UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE);
|
||||
when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user));
|
||||
when(userAccountRepository.findByIdForUpdate("user-1")).thenReturn(Optional.of(user));
|
||||
when(userAccountRepository.save(user)).thenReturn(user);
|
||||
|
||||
var response = service.updateUserStatus("user-1", "DISABLED");
|
||||
|
|
@ -177,7 +177,7 @@ class AdminUserAppServiceTest {
|
|||
@Test
|
||||
void updateUserStatus_activatingUserEnsuresGlobalMembership() {
|
||||
UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.PENDING);
|
||||
when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user));
|
||||
when(userAccountRepository.findByIdForUpdate("user-1")).thenReturn(Optional.of(user));
|
||||
when(userAccountRepository.save(user)).thenReturn(user);
|
||||
|
||||
var response = service.updateUserStatus("user-1", "ACTIVE");
|
||||
|
|
@ -195,7 +195,7 @@ class AdminUserAppServiceTest {
|
|||
"alice",
|
||||
"alice@example.com",
|
||||
UserStatus.PENDING);
|
||||
when(userAccountRepository.findById("user-1"))
|
||||
when(userAccountRepository.findByIdForUpdate("user-1"))
|
||||
.thenReturn(Optional.of(user));
|
||||
when(userAccountRepository.save(user)).thenReturn(user);
|
||||
|
||||
|
|
@ -226,7 +226,7 @@ class AdminUserAppServiceTest {
|
|||
@Test
|
||||
void updateUserStatus_rejectsReactivatingMergedAccount() {
|
||||
UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.MERGED);
|
||||
when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user));
|
||||
when(userAccountRepository.findByIdForUpdate("user-1")).thenReturn(Optional.of(user));
|
||||
|
||||
assertThrows(DomainBadRequestException.class,
|
||||
() -> service.updateUserStatus("user-1", "ACTIVE"));
|
||||
|
|
@ -238,7 +238,7 @@ class AdminUserAppServiceTest {
|
|||
|
||||
@Test
|
||||
void updateUserStatus_rejectsSystemAccount() {
|
||||
when(userAccountRepository.findById("builtin-skill-publisher"))
|
||||
when(userAccountRepository.findByIdForUpdate("builtin-skill-publisher"))
|
||||
.thenReturn(Optional.of(systemUser()));
|
||||
|
||||
assertThrows(DomainForbiddenException.class,
|
||||
|
|
@ -249,7 +249,7 @@ class AdminUserAppServiceTest {
|
|||
|
||||
@Test
|
||||
void updateUserStatus_withUnknownUser_throwsNotFound() {
|
||||
when(userAccountRepository.findById("missing")).thenReturn(Optional.empty());
|
||||
when(userAccountRepository.findByIdForUpdate("missing")).thenReturn(Optional.empty());
|
||||
|
||||
assertThrows(DomainNotFoundException.class, () -> service.updateUserStatus("missing", "DISABLED"));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,13 +1,19 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.metrics.SkillHubMetrics;
|
||||
import com.iflytek.skillhub.search.SearchRebuildService;
|
||||
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.Mockito.doThrow;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoInteractions;
|
||||
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||
|
||||
class LabelSearchSyncServiceTest {
|
||||
|
|
@ -15,7 +21,8 @@ class LabelSearchSyncServiceTest {
|
|||
@Test
|
||||
void rebuildSkillsShouldSkipNullsAndDuplicatesWhileProcessingLargeLists() {
|
||||
SearchRebuildService rebuildService = mock(SearchRebuildService.class);
|
||||
LabelSearchSyncService service = new LabelSearchSyncService(rebuildService);
|
||||
SkillHubMetrics metrics = mock(SkillHubMetrics.class);
|
||||
LabelSearchSyncService service = new LabelSearchSyncService(rebuildService, metrics);
|
||||
List<Long> skillIds = new ArrayList<>();
|
||||
skillIds.add(null);
|
||||
for (long i = 1; i <= 120; i++) {
|
||||
|
|
@ -30,5 +37,58 @@ class LabelSearchSyncServiceTest {
|
|||
verify(rebuildService).rebuildBySkill(i);
|
||||
}
|
||||
verifyNoMoreInteractions(rebuildService);
|
||||
verifyNoInteractions(metrics);
|
||||
}
|
||||
|
||||
@Test
|
||||
void rebuildSkillFailureShouldIncrementMetric() {
|
||||
SearchRebuildService rebuildService = mock(SearchRebuildService.class);
|
||||
doThrow(new IllegalStateException("search unavailable"))
|
||||
.when(rebuildService)
|
||||
.rebuildBySkill(42L);
|
||||
|
||||
contextRunner(rebuildService).run(context -> {
|
||||
LabelSearchSyncService service = context.getBean(LabelSearchSyncService.class);
|
||||
SimpleMeterRegistry meterRegistry = context.getBean(SimpleMeterRegistry.class);
|
||||
|
||||
service.rebuildSkill(42L);
|
||||
|
||||
assertThat(meterRegistry.get("skillhub.search.rebuild.failure").counter().count())
|
||||
.isEqualTo(1.0d);
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
void rebuildSkillsShouldCountEachFailureAndContinue() {
|
||||
SearchRebuildService rebuildService = mock(SearchRebuildService.class);
|
||||
doThrow(new IllegalStateException("search unavailable"))
|
||||
.when(rebuildService)
|
||||
.rebuildBySkill(2L);
|
||||
doThrow(new IllegalStateException("search unavailable"))
|
||||
.when(rebuildService)
|
||||
.rebuildBySkill(3L);
|
||||
|
||||
contextRunner(rebuildService).run(context -> {
|
||||
LabelSearchSyncService service = context.getBean(LabelSearchSyncService.class);
|
||||
SimpleMeterRegistry meterRegistry = context.getBean(SimpleMeterRegistry.class);
|
||||
|
||||
service.rebuildSkills(List.of(1L, 2L, 3L, 4L));
|
||||
|
||||
assertThat(meterRegistry.get("skillhub.search.rebuild.failure").counter().count())
|
||||
.isEqualTo(2.0d);
|
||||
verify(rebuildService).rebuildBySkill(1L);
|
||||
verify(rebuildService).rebuildBySkill(2L);
|
||||
verify(rebuildService).rebuildBySkill(3L);
|
||||
verify(rebuildService).rebuildBySkill(4L);
|
||||
verifyNoMoreInteractions(rebuildService);
|
||||
});
|
||||
}
|
||||
|
||||
private ApplicationContextRunner contextRunner(SearchRebuildService rebuildService) {
|
||||
return new ApplicationContextRunner()
|
||||
.withBean(SearchRebuildService.class, () -> rebuildService)
|
||||
.withBean(SimpleMeterRegistry.class)
|
||||
.withBean(SkillHubMetrics.class)
|
||||
.withBean(LabelSearchSyncService.class);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ package com.iflytek.skillhub.auth.identity;
|
|||
|
||||
import java.sql.SQLException;
|
||||
import java.util.Objects;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.dao.DataIntegrityViolationException;
|
||||
import org.springframework.stereotype.Service;
|
||||
|
||||
|
|
@ -9,23 +11,29 @@ import org.springframework.stereotype.Service;
|
|||
class DefaultExternalIdentityLoginService
|
||||
implements ExternalIdentityLoginService {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(
|
||||
DefaultExternalIdentityLoginService.class);
|
||||
|
||||
private final TrustedProviderDescriptorSource descriptorSource;
|
||||
private final ProviderAuthorityLockService authorityLockService;
|
||||
private final IdentityAssertionFactory assertionFactory;
|
||||
private final IdentityResolutionTransaction resolutionTransaction;
|
||||
private final IdentityLoginMetrics metrics;
|
||||
private final IdentitySecurityAuditWriter securityAuditWriter;
|
||||
|
||||
DefaultExternalIdentityLoginService(
|
||||
TrustedProviderDescriptorSource descriptorSource,
|
||||
ProviderAuthorityLockService authorityLockService,
|
||||
IdentityAssertionFactory assertionFactory,
|
||||
IdentityResolutionTransaction resolutionTransaction,
|
||||
IdentityLoginMetrics metrics) {
|
||||
IdentityLoginMetrics metrics,
|
||||
IdentitySecurityAuditWriter securityAuditWriter) {
|
||||
this.descriptorSource = descriptorSource;
|
||||
this.authorityLockService = authorityLockService;
|
||||
this.assertionFactory = assertionFactory;
|
||||
this.resolutionTransaction = resolutionTransaction;
|
||||
this.metrics = metrics;
|
||||
this.securityAuditWriter = securityAuditWriter;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -38,10 +46,12 @@ class DefaultExternalIdentityLoginService
|
|||
Objects.requireNonNull(context, "context");
|
||||
|
||||
String metricProvider = "unresolved";
|
||||
String metricProtocol = "unresolved";
|
||||
try {
|
||||
ProviderDescriptor descriptor =
|
||||
descriptorSource.require(provider);
|
||||
metricProvider = descriptor.providerCode();
|
||||
metricProtocol = descriptor.protocol();
|
||||
authorityLockService.requirePinnedAuthority(descriptor);
|
||||
IdentityAssertion assertion =
|
||||
assertionFactory.create(descriptor, result);
|
||||
|
|
@ -51,19 +61,48 @@ class DefaultExternalIdentityLoginService
|
|||
context);
|
||||
metrics.recordOutcome(
|
||||
descriptor.providerCode(),
|
||||
descriptor.protocol(),
|
||||
outcome);
|
||||
return outcome;
|
||||
} catch (IdentityCoreException exception) {
|
||||
metrics.recordFailure(
|
||||
metricProvider,
|
||||
metricProtocol,
|
||||
exception.getReasonCode());
|
||||
recordDeniedAudit(
|
||||
metricProvider,
|
||||
metricProtocol,
|
||||
exception.getReasonCode(),
|
||||
context);
|
||||
throw exception;
|
||||
} catch (RuntimeException exception) {
|
||||
metrics.recordSystemError(metricProvider);
|
||||
metrics.recordSystemError(
|
||||
metricProvider,
|
||||
metricProtocol);
|
||||
throw exception;
|
||||
}
|
||||
}
|
||||
|
||||
private void recordDeniedAudit(
|
||||
String providerCode,
|
||||
String protocol,
|
||||
IdentityFailureCode failureCode,
|
||||
IdentityLoginContext context) {
|
||||
try {
|
||||
securityAuditWriter.recordDenied(
|
||||
providerCode,
|
||||
protocol,
|
||||
failureCode,
|
||||
context);
|
||||
} catch (RuntimeException auditFailure) {
|
||||
log.error(
|
||||
"Identity denial audit failed for provider '{}' and reason '{}'",
|
||||
providerCode,
|
||||
failureCode,
|
||||
auditFailure);
|
||||
}
|
||||
}
|
||||
|
||||
private IdentityLoginOutcome resolveWithRetry(
|
||||
IdentityAssertion assertion,
|
||||
ProviderDescriptor descriptor,
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ class IdentityLoginMetrics {
|
|||
|
||||
void recordOutcome(
|
||||
String providerCode,
|
||||
String protocol,
|
||||
IdentityLoginOutcome outcome) {
|
||||
String result;
|
||||
if (outcome instanceof
|
||||
|
|
@ -28,28 +29,35 @@ class IdentityLoginMetrics {
|
|||
} else {
|
||||
result = "link_required";
|
||||
}
|
||||
counter(providerCode, result);
|
||||
counter(providerCode, protocol, result);
|
||||
}
|
||||
|
||||
void recordFailure(
|
||||
String providerCode,
|
||||
String protocol,
|
||||
IdentityFailureCode failureCode) {
|
||||
counter(
|
||||
providerCode,
|
||||
protocol,
|
||||
failureCode.name().toLowerCase(Locale.ROOT));
|
||||
}
|
||||
|
||||
void recordSystemError(String providerCode) {
|
||||
counter(providerCode, "system_error");
|
||||
void recordSystemError(
|
||||
String providerCode,
|
||||
String protocol) {
|
||||
counter(providerCode, protocol, "system_error");
|
||||
}
|
||||
|
||||
private void counter(
|
||||
String providerCode,
|
||||
String protocol,
|
||||
String result) {
|
||||
meterRegistry.counter(
|
||||
"skillhub.identity.login",
|
||||
"provider",
|
||||
providerCode,
|
||||
"protocol",
|
||||
protocol,
|
||||
"result",
|
||||
result).increment();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -228,6 +228,11 @@ class IdentityResolutionTransaction {
|
|||
requireAllowed(decision);
|
||||
}
|
||||
|
||||
requireAccessAllowed(
|
||||
assertion,
|
||||
context,
|
||||
IdentityAccessKind.RETURNING_IDENTITY,
|
||||
Optional.of(user.getStatus()));
|
||||
if (decision == AccountLoginDecision.PENDING) {
|
||||
reconcileSubjects(
|
||||
binding,
|
||||
|
|
@ -247,11 +252,6 @@ class IdentityResolutionTransaction {
|
|||
ACCOUNT_PENDING);
|
||||
}
|
||||
|
||||
requireAccessAllowed(
|
||||
assertion,
|
||||
context,
|
||||
IdentityAccessKind.RETURNING_IDENTITY,
|
||||
Optional.of(user.getStatus()));
|
||||
reconcileSubjects(
|
||||
binding,
|
||||
assertion,
|
||||
|
|
@ -378,7 +378,7 @@ class IdentityResolutionTransaction {
|
|||
}
|
||||
|
||||
Optional<String> email = trustedEmail(assertion.profile());
|
||||
if (email.flatMap(userRepository::findByEmailIgnoreCase)
|
||||
if (email.filter(userRepository::existsByEmailIgnoreCase)
|
||||
.isPresent()) {
|
||||
recordAudit(
|
||||
null,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,50 @@
|
|||
package com.iflytek.skillhub.auth.identity;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import org.springframework.stereotype.Service;
|
||||
import org.springframework.transaction.annotation.Propagation;
|
||||
import org.springframework.transaction.annotation.Transactional;
|
||||
|
||||
/**
|
||||
* Persists security denials independently from the identity transaction that
|
||||
* produced them.
|
||||
*/
|
||||
@Service
|
||||
class IdentitySecurityAuditWriter {
|
||||
|
||||
private final AuditLogService auditLogService;
|
||||
|
||||
IdentitySecurityAuditWriter(AuditLogService auditLogService) {
|
||||
this.auditLogService = auditLogService;
|
||||
}
|
||||
|
||||
@Transactional(propagation = Propagation.REQUIRES_NEW)
|
||||
public void recordDenied(
|
||||
String providerCode,
|
||||
String protocol,
|
||||
IdentityFailureCode failureCode,
|
||||
IdentityLoginContext context) {
|
||||
String action = switch (failureCode) {
|
||||
case IDENTITY_IDENTIFIER_CONFLICT ->
|
||||
"IDENTITY_CONFLICT_DETECTED";
|
||||
case PROVIDER_AUTHORITY_MISMATCH ->
|
||||
"PROVIDER_AUTHORITY_MISMATCH";
|
||||
default -> "IDENTITY_LOGIN_DENIED";
|
||||
};
|
||||
auditLogService.record(
|
||||
null,
|
||||
action,
|
||||
"IDENTITY_PROVIDER",
|
||||
null,
|
||||
context.requestId(),
|
||||
context.clientIp(),
|
||||
context.userAgent(),
|
||||
"{\"providerCode\":\""
|
||||
+ providerCode
|
||||
+ "\",\"protocol\":\""
|
||||
+ protocol
|
||||
+ "\",\"reason\":\""
|
||||
+ failureCode.name()
|
||||
+ "\"}");
|
||||
}
|
||||
}
|
||||
|
|
@ -3,6 +3,7 @@ package com.iflytek.skillhub.auth.identity;
|
|||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.doThrow;
|
||||
import static org.mockito.Mockito.inOrder;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.verify;
|
||||
|
|
@ -26,6 +27,7 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
private ProviderAuthorityLockService authorityLockService;
|
||||
private IdentityResolutionTransaction resolutionTransaction;
|
||||
private IdentityLoginMetrics metrics;
|
||||
private IdentitySecurityAuditWriter securityAuditWriter;
|
||||
private DefaultExternalIdentityLoginService service;
|
||||
|
||||
@BeforeEach
|
||||
|
|
@ -37,12 +39,15 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
resolutionTransaction =
|
||||
mock(IdentityResolutionTransaction.class);
|
||||
metrics = mock(IdentityLoginMetrics.class);
|
||||
securityAuditWriter =
|
||||
mock(IdentitySecurityAuditWriter.class);
|
||||
service = new DefaultExternalIdentityLoginService(
|
||||
descriptorSource,
|
||||
authorityLockService,
|
||||
new IdentityAssertionFactory(),
|
||||
resolutionTransaction,
|
||||
metrics);
|
||||
metrics,
|
||||
securityAuditWriter);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -75,7 +80,10 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
any(IdentityAssertion.class),
|
||||
org.mockito.ArgumentMatchers.eq(descriptor),
|
||||
org.mockito.ArgumentMatchers.eq(context));
|
||||
order.verify(metrics).recordOutcome("github", expected);
|
||||
order.verify(metrics).recordOutcome(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
expected);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -98,7 +106,10 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
IdentityLoginContext.empty());
|
||||
|
||||
assertThat(outcome).isSameAs(pending);
|
||||
verify(metrics).recordOutcome("github", pending);
|
||||
verify(metrics).recordOutcome(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
pending);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -126,7 +137,10 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
any(IdentityAssertion.class),
|
||||
org.mockito.ArgumentMatchers.eq(descriptor),
|
||||
any(IdentityLoginContext.class));
|
||||
verify(metrics).recordOutcome("github", expected);
|
||||
verify(metrics).recordOutcome(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
expected);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -152,7 +166,44 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
.IDENTITY_IDENTIFIER_CONFLICT);
|
||||
verify(metrics).recordFailure(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
IdentityFailureCode.IDENTITY_IDENTIFIER_CONFLICT);
|
||||
verify(securityAuditWriter).recordDenied(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
IdentityFailureCode.IDENTITY_IDENTIFIER_CONFLICT,
|
||||
IdentityLoginContext.empty());
|
||||
}
|
||||
|
||||
@Test
|
||||
void auditFailureDoesNotReplaceIdentityDenial() {
|
||||
ResolvedProviderHandle handle =
|
||||
new DefaultResolvedProviderHandle("github");
|
||||
IdentityLoginContext context =
|
||||
IdentityLoginContext.empty();
|
||||
when(descriptorSource.require(handle))
|
||||
.thenReturn(descriptor);
|
||||
when(resolutionTransaction.resolve(
|
||||
any(IdentityAssertion.class),
|
||||
org.mockito.ArgumentMatchers.eq(descriptor),
|
||||
org.mockito.ArgumentMatchers.eq(context)))
|
||||
.thenThrow(new IdentityCoreException(
|
||||
IdentityFailureCode.ACCESS_DENIED));
|
||||
doThrow(new IllegalStateException("audit unavailable"))
|
||||
.when(securityAuditWriter)
|
||||
.recordDenied(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
IdentityFailureCode.ACCESS_DENIED,
|
||||
context);
|
||||
|
||||
assertThatThrownBy(() -> service.authenticate(
|
||||
handle,
|
||||
result(),
|
||||
context))
|
||||
.isInstanceOf(IdentityCoreException.class)
|
||||
.extracting("reasonCode")
|
||||
.isEqualTo(IdentityFailureCode.ACCESS_DENIED);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -182,7 +233,9 @@ class DefaultExternalIdentityLoginServiceTest {
|
|||
any(IdentityAssertion.class),
|
||||
org.mockito.ArgumentMatchers.eq(descriptor),
|
||||
any(IdentityLoginContext.class));
|
||||
verify(metrics).recordSystemError("github");
|
||||
verify(metrics).recordSystemError(
|
||||
"github",
|
||||
"oauth2-github");
|
||||
}
|
||||
|
||||
private static IdentityLoginOutcome authenticated() {
|
||||
|
|
|
|||
|
|
@ -25,20 +25,24 @@ class IdentityLoginMetricsTest {
|
|||
|
||||
metrics.recordOutcome(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
new IdentityLoginOutcome.Authenticated(
|
||||
principal,
|
||||
true,
|
||||
true));
|
||||
metrics.recordOutcome(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
new IdentityLoginOutcome.PendingApproval(
|
||||
"ACCOUNT_PENDING"));
|
||||
metrics.recordOutcome(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
new IdentityLoginOutcome.LinkRequired(
|
||||
"EMAIL_COLLISION"));
|
||||
metrics.recordFailure(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
IdentityFailureCode.ACCESS_DENIED);
|
||||
|
||||
assertThat(counter(
|
||||
|
|
@ -62,6 +66,8 @@ class IdentityLoginMetricsTest {
|
|||
.tags(
|
||||
"provider",
|
||||
"github",
|
||||
"protocol",
|
||||
"oauth2-github",
|
||||
"result",
|
||||
result)
|
||||
.counter()
|
||||
|
|
|
|||
|
|
@ -221,12 +221,9 @@ class IdentityResolutionTransactionTest {
|
|||
|
||||
@Test
|
||||
void verifiedEmailCollisionReturnsOnlyStableLinkReason() {
|
||||
when(userRepository.findByEmailIgnoreCase(
|
||||
when(userRepository.existsByEmailIgnoreCase(
|
||||
"alice@example.com"))
|
||||
.thenReturn(Optional.of(user(
|
||||
"usr_existing",
|
||||
UserStatus.ACTIVE,
|
||||
false)));
|
||||
.thenReturn(true);
|
||||
|
||||
IdentityLoginOutcome outcome = transaction.resolve(
|
||||
githubAssertion(Set.of()),
|
||||
|
|
@ -537,6 +534,58 @@ class IdentityResolutionTransactionTest {
|
|||
.isEqualTo("original@example.com");
|
||||
verify(userRepository, never()).save(user);
|
||||
verify(subjectRepository).saveAll(any());
|
||||
ArgumentCaptor<IdentityAccessContext> accessContext =
|
||||
ArgumentCaptor.forClass(
|
||||
IdentityAccessContext.class);
|
||||
verify(accessPolicy).evaluate(accessContext.capture());
|
||||
assertThat(accessContext.getValue().accessKind())
|
||||
.isEqualTo(
|
||||
IdentityAccessKind.RETURNING_IDENTITY);
|
||||
assertThat(accessContext.getValue()
|
||||
.existingAccountStatus())
|
||||
.contains(UserStatus.PENDING);
|
||||
}
|
||||
|
||||
@Test
|
||||
void deniedLoginPolicyDoesNotMutatePendingBinding() {
|
||||
IdentityBinding binding = binding(
|
||||
1L,
|
||||
"usr_1",
|
||||
"github",
|
||||
"123456");
|
||||
UserAccount user = user("usr_1", UserStatus.PENDING, false);
|
||||
when(bindingRepository.findByProviderCodeAndSubject(
|
||||
"github",
|
||||
"123456")).thenReturn(Optional.of(binding));
|
||||
when(bindingRepository.findByIdAndStatusForUpdate(
|
||||
1L,
|
||||
IdentityBindingStatus.ACTIVE))
|
||||
.thenReturn(Optional.of(binding));
|
||||
when(userRepository.findByIdForUpdate("usr_1"))
|
||||
.thenReturn(Optional.of(user));
|
||||
when(accessPolicy.evaluate(any()))
|
||||
.thenReturn(AccessDecision.DENY);
|
||||
|
||||
assertThatThrownBy(() -> transaction.resolve(
|
||||
githubAssertion(Set.of()),
|
||||
githubDescriptor(ProvisioningMode.APPROVAL),
|
||||
IdentityLoginContext.empty()))
|
||||
.isInstanceOf(IdentityCoreException.class)
|
||||
.extracting("reasonCode")
|
||||
.isEqualTo(IdentityFailureCode.ACCESS_DENIED);
|
||||
|
||||
verify(subjectRepository, never())
|
||||
.demoteActivePrimary(any(), any());
|
||||
verify(bindingRepository, never()).save(any());
|
||||
verify(auditLogService, never()).record(
|
||||
any(),
|
||||
any(),
|
||||
any(),
|
||||
any(),
|
||||
any(),
|
||||
any(),
|
||||
any(),
|
||||
any());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -0,0 +1,65 @@
|
|||
package com.iflytek.skillhub.auth.identity;
|
||||
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.ArgumentMatchers.isNull;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.verify;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
class IdentitySecurityAuditWriterTest {
|
||||
|
||||
private final AuditLogService auditLogService =
|
||||
mock(AuditLogService.class);
|
||||
private final IdentitySecurityAuditWriter writer =
|
||||
new IdentitySecurityAuditWriter(auditLogService);
|
||||
|
||||
@Test
|
||||
void mapsIdentifierConflictToDedicatedAuditAction() {
|
||||
assertAuditAction(
|
||||
IdentityFailureCode.IDENTITY_IDENTIFIER_CONFLICT,
|
||||
"IDENTITY_CONFLICT_DETECTED");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mapsAuthorityMismatchToDedicatedAuditAction() {
|
||||
assertAuditAction(
|
||||
IdentityFailureCode.PROVIDER_AUTHORITY_MISMATCH,
|
||||
"PROVIDER_AUTHORITY_MISMATCH");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mapsOtherDenialsToGenericAuditAction() {
|
||||
assertAuditAction(
|
||||
IdentityFailureCode.ACCESS_DENIED,
|
||||
"IDENTITY_LOGIN_DENIED");
|
||||
}
|
||||
|
||||
private void assertAuditAction(
|
||||
IdentityFailureCode failureCode,
|
||||
String expectedAction) {
|
||||
writer.recordDenied(
|
||||
"github",
|
||||
"oauth2-github",
|
||||
failureCode,
|
||||
new IdentityLoginContext(
|
||||
"request-1",
|
||||
"127.0.0.1",
|
||||
"identity-test"));
|
||||
|
||||
verify(auditLogService).record(
|
||||
isNull(),
|
||||
eq(expectedAction),
|
||||
eq("IDENTITY_PROVIDER"),
|
||||
isNull(),
|
||||
eq("request-1"),
|
||||
eq("127.0.0.1"),
|
||||
eq("identity-test"),
|
||||
eq("{\"providerCode\":\"github\","
|
||||
+ "\"protocol\":\"oauth2-github\","
|
||||
+ "\"reason\":\""
|
||||
+ failureCode.name()
|
||||
+ "\"}"));
|
||||
}
|
||||
}
|
||||
|
|
@ -14,6 +14,7 @@ public interface UserAccountRepository {
|
|||
Optional<UserAccount> findByIdForUpdate(String id);
|
||||
List<UserAccount> findByIdIn(List<String> ids);
|
||||
Optional<UserAccount> findByEmailIgnoreCase(String email);
|
||||
boolean existsByEmailIgnoreCase(String email);
|
||||
Page<UserAccount> search(String keyword, UserStatus status, Pageable pageable);
|
||||
UserAccount save(UserAccount user);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,26 +3,32 @@ package com.iflytek.skillhub.infra.jpa;
|
|||
import com.iflytek.skillhub.domain.user.UserAccount;
|
||||
import com.iflytek.skillhub.domain.user.UserAccountRepository;
|
||||
import com.iflytek.skillhub.domain.user.UserStatus;
|
||||
import jakarta.persistence.LockModeType;
|
||||
import org.springframework.data.domain.Page;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.jpa.repository.JpaRepository;
|
||||
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
|
||||
import org.springframework.data.jpa.repository.Lock;
|
||||
import org.springframework.data.jpa.repository.Query;
|
||||
import org.springframework.data.repository.query.Param;
|
||||
import org.springframework.stereotype.Repository;
|
||||
|
||||
/**
|
||||
* JPA-backed user-account repository that provides filtered admin search over account records.
|
||||
* The native {@code FOR UPDATE} query keeps row-lock behavior portable
|
||||
* between PostgreSQL and the test suite's H2 PostgreSQL compatibility mode.
|
||||
*/
|
||||
@Repository
|
||||
public interface UserAccountJpaRepository
|
||||
extends JpaRepository<UserAccount, String>, JpaSpecificationExecutor<UserAccount>, UserAccountRepository {
|
||||
|
||||
@Override
|
||||
@Lock(LockModeType.PESSIMISTIC_WRITE)
|
||||
@Query("SELECT u FROM UserAccount u WHERE u.id = :id")
|
||||
@Query(
|
||||
value = """
|
||||
SELECT *
|
||||
FROM user_account
|
||||
WHERE id = :id
|
||||
FOR UPDATE
|
||||
""",
|
||||
nativeQuery = true)
|
||||
java.util.Optional<UserAccount> findByIdForUpdate(
|
||||
@Param("id") String id);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue