dengliming commented on code in PR #670:
URL: https://github.com/apache/shenyu-dashboard/pull/670#discussion_r4111367746
##########
src/models/login.js:
##########
@@ -31,12 +31,19 @@ export default {
effects: {
*login({ payload }, { call, put, select }) {
const { callback } = payload;
- const response = yield call(queryLogin, payload);
+ let response;
+ let requestError;
+ try {
+ response = yield call(queryLogin, payload);
+ } catch (error) {
+ requestError = error;
+ response = error.response?.data ?? error.response;
Review Comment:
[P2] Guard the real page callback when handling rejected or absent responses
For a network rejection without error.response, this catch leaves response
undefined and the next yield call(callback, response) invokes LoginPage's
callback, which still evaluates res.code === 500. That throws a TypeError
before the new message.error fallback can run. The new tests use jest.fn() for
the callback and therefore miss the actual failure path. Using the real page
callback reproduces the crash for rejected requests and undefined/null
responses. Please guard res in the page callback (e.g. res?.code) and test the
model together with that callback.
##########
src/routes/User/Login.js:
##########
@@ -79,7 +53,11 @@ export default class LoginPage extends Component {
this.ChildRef.current.handleChange();
return;
}
- if (secretKey !== "" && secretIv !== "") {
+ if (!(await ensureSecret())) {
Review Comment:
[P1] Allow login when Admin successfully returns sanitized empty secrets
Current apache/shenyu intentionally returns Base64-encoded JSON with key: ""
and iv: "" from /platform/secretInfo (apache/shenyu#6323), and
DashboardUserServiceImpl accepts passwords without client-side AES encryption.
With that valid response, ensureSecret() retries and returns false, so this new
early return prevents every login attempt from being dispatched. I reproduced
this with the actual LoginPage handler: two secret requests, zero login
dispatches. Please distinguish a failed/invalid bootstrap response from a
successful response with empty secrets; the latter must continue login without
AES, while nonempty secrets can retain compatibility with older Admin versions.
Add a regression test using the current Admin response.
Backend implementation:
https://github.com/apache/shenyu/blob/master/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SecretServiceImpl.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
