Skip to content

Conversation

@gtn1024
Copy link
Contributor

@gtn1024 gtn1024 commented Oct 28, 2025

No description provided.

Copilot AI review requested due to automatic review settings October 28, 2025 02:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a mechanism for custom authentication validation during user login through the IExtraAuthChecker interface, allowing applications to add additional authentication checks beyond standard username/password verification.

  • Added IExtraAuthChecker interface for custom authentication validation
  • Integrated checker execution into the login flow via CDI Instance injection
  • Added comprehensive test coverage with two test checker implementations

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
SsoController.java Injects and executes extra authentication checkers after successful password validation
IExtraAuthChecker.java Defines the interface for custom authentication validation logic
TestLogin.java Adds test cases verifying that custom checkers can block user login and includes two example checker implementations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 6 to 7
void check(IRuntimeUser runtimeUser);
}
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check method lacks documentation explaining its contract. Add a JavaDoc comment describing: (1) the method's purpose in the authentication flow, (2) the expected behavior when validation fails (e.g., throwing an exception), and (3) the parameter's meaning.

Suggested change
void check(IRuntimeUser runtimeUser);
}
/**
* Performs additional authentication checks on the provided runtime user.
* <p>
* This method is intended to be called during the authentication flow to validate
* extra conditions or requirements for the user. If the validation fails,
* an exception should be thrown to indicate authentication failure.
*
* @param runtimeUser the user to be validated by extra authentication checks
* @throws RuntimeException if the user fails the extra authentication checks
*/
void check(IRuntimeUser runtimeUser);

Copilot uses AI. Check for mistakes.
.header("userID", config.superUserId())
.contentType("application/json")
.body(Map.of(
"v_name", "222"
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using magic string '222' for user name. Consider using a descriptive constant like 'TEST_USER_2_NAME' to improve test readability and maintainability.

Copilot uses AI. Check for mistakes.
.header("userID", config.superUserId())
.contentType("application/json")
.body(Map.of(
"v_name", "333"
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using magic string '333' for user name. Consider using a descriptive constant like 'TEST_USER_3_NAME' to improve test readability and maintainability.

Copilot uses AI. Check for mistakes.
@aruis
Copy link
Collaborator

aruis commented Oct 28, 2025

两个想法:

  1. 名字是否叫 IExtraLoginChecker.java 更合适?
  2. 是否要求其声明抛出某个登录失败异常,被平台捕获?

@gtn1024
Copy link
Contributor Author

gtn1024 commented Oct 28, 2025

名字是否叫 IExtraLoginChecker.java 更合适?

done

是否要求其声明抛出某个登录失败异常,被平台捕获?

在 javadoc 还有方法签名上 throws 要求抛出,比如说 MuYunException?这个 checker 我想的使用场景是比如说 2FA 的情况下,错误具体信息需要被用户所看到

@gtn1024 gtn1024 force-pushed the gtn1024/auth-extra-checker branch from b23e266 to 7f6fc42 Compare October 28, 2025 03:18
import net.ximatai.muyun.model.IRuntimeUser;

public interface IExtraLoginChecker {
void check(IRuntimeUser runtimeUser);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要不要声明抛出异常,这样平台会强制捕获,然后再次包装成 muyunException 出去?

@gtn1024 gtn1024 force-pushed the gtn1024/auth-extra-checker branch from 7f6fc42 to ec49de0 Compare October 30, 2025 14:54
@aruis aruis merged commit 1aa3b6a into master Oct 31, 2025
2 checks passed
@gtn1024 gtn1024 deleted the gtn1024/auth-extra-checker branch October 31, 2025 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants