Merge pull request #939 from iflytek/fix/issue-923-role-grant-check

fix(auth): keep initial grant rules out of SQL checks

server/skillhub-app/src/main/resources/db/migration/V68__system_auth_settings.sql

@@ -1,7 +1,7 @@
 CREATE TABLE system_setting (
     id BIGSERIAL PRIMARY KEY,
     setting_key VARCHAR(128) NOT NULL UNIQUE,
-    value_json JSONB NOT NULL CHECK (jsonb_typeof(value_json) = 'object'),
+    value_json JSONB NOT NULL,
     version BIGINT NOT NULL DEFAULT 0,
     created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
     updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
@@ -13,8 +13,7 @@ CREATE TABLE external_role_grant_rule (
     provider_code VARCHAR(64) NOT NULL,
     normalized_email VARCHAR(256) NOT NULL,
     role_id BIGINT NOT NULL REFERENCES role(id),
-    status VARCHAR(16) NOT NULL DEFAULT 'ACTIVE'
-        CHECK (status IN ('ACTIVE', 'DISABLED', 'CONSUMED')),
+    status VARCHAR(16) NOT NULL DEFAULT 'ACTIVE',
     matched_subject VARCHAR(256),
     granted_user_id VARCHAR(128) REFERENCES user_account(id),
     granted_at TIMESTAMP,
@@ -22,14 +21,7 @@ CREATE TABLE external_role_grant_rule (
     created_by VARCHAR(128),
     updated_by VARCHAR(128),
     created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
-    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
-    CONSTRAINT ck_external_role_grant_consumed CHECK (
-        (status = 'CONSUMED' AND matched_subject IS NOT NULL
-            AND granted_user_id IS NOT NULL AND granted_at IS NOT NULL)
-        OR
-        (status <> 'CONSUMED' AND matched_subject IS NULL
-            AND granted_user_id IS NULL AND granted_at IS NULL)
-    )
+    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
 );
 
 CREATE UNIQUE INDEX uq_external_role_grant_active_identity

server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SystemAuthSettingsPostgresTest.java

@@ -123,6 +123,14 @@ class SystemAuthSettingsPostgresTest {
         assertThat(reloaded.getGrantedAt()).isNotNull();
     }
 
+    @Test
+    void authSettingsBusinessRulesHaveNoSqlCheckConstraints() {
+        Integer count = jdbc.queryForObject("SELECT count(*) FROM pg_constraint "
+                + "WHERE contype = 'c' AND conrelid IN "
+                + "('system_setting'::regclass, 'external_role_grant_rule'::regclass)", Integer.class);
+        assertThat(count).isZero();
+    }
+
     @Test
     @Transactional(propagation = Propagation.NOT_SUPPORTED)
     void sameEmailLocalAccountStaysSeparateFromNewExternalGrant() {

server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/settings/ExternalRoleGrantRule.java

@@ -112,6 +112,9 @@ public class ExternalRoleGrantRule {
 
     public void consume(String subject, String userId) {
         if (status != Status.ACTIVE) throw new IllegalStateException("Only active rules may be consumed");
+        if (subject == null || subject.isBlank() || userId == null || userId.isBlank()) {
+            throw new IllegalArgumentException("Consumed rule requires an external subject and user ID");
+        }
         this.status = Status.CONSUMED;
         this.matchedSubject = subject;
         this.grantedUserId = userId;

server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/ExternalRoleGrantRuleTest.java

@@ -0,0 +1,31 @@
+package com.iflytek.skillhub.auth.settings;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+
+import com.iflytek.skillhub.auth.entity.Role;
+import org.junit.jupiter.api.Test;
+
+class ExternalRoleGrantRuleTest {
+    @Test
+    void consumeRequiresBothIdentityValuesBeforeChangingState() {
+        ExternalRoleGrantRule rule = new ExternalRoleGrantRule(
+                "github", "admin@example.com", mock(Role.class), "admin");
+
+        assertThatThrownBy(() -> rule.consume(" ", "usr_1"))
+                .isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(() -> rule.consume("external_1", null))
+                .isInstanceOf(IllegalArgumentException.class);
+        assertThat(rule.getStatus()).isEqualTo(ExternalRoleGrantRule.Status.ACTIVE);
+        assertThat(rule.getMatchedSubject()).isNull();
+        assertThat(rule.getGrantedUserId()).isNull();
+        assertThat(rule.getGrantedAt()).isNull();
+
+        rule.consume("external_1", "usr_1");
+        assertThat(rule.getStatus()).isEqualTo(ExternalRoleGrantRule.Status.CONSUMED);
+        assertThat(rule.getMatchedSubject()).isEqualTo("external_1");
+        assertThat(rule.getGrantedUserId()).isEqualTo("usr_1");
+        assertThat(rule.getGrantedAt()).isNotNull();
+    }
+}