mirror of https://github.com/halo-dev/halo
Fix the problem of enabling 2FA unexpectedly (#6174)
#### What type of PR is this? /kind bug /area core /milestone 2.17.x #### What this PR does / why we need it: Some users encountered 2FA required issue after upgrading Halo 2.16, because they enabled 2FA but didn't configure TOTP before. The issue was introduced by <https://github.com/halo-dev/halo/pull/6005>. This PR checks if TOTP configured to determine whether 2FA is required. #### Does this PR introduce a user-facing change? ```release-note 修复在没有配置 TOTP 验证器的情况下仍被要求二步验证的问题 ```pull/6185/head
parent
4d6450d065
commit
556e65a618
|
@ -22,6 +22,7 @@ import run.halo.app.core.extension.service.UserService;
|
||||||
import run.halo.app.extension.GroupKind;
|
import run.halo.app.extension.GroupKind;
|
||||||
import run.halo.app.infra.exception.UserNotFoundException;
|
import run.halo.app.infra.exception.UserNotFoundException;
|
||||||
import run.halo.app.security.authentication.login.HaloUser;
|
import run.halo.app.security.authentication.login.HaloUser;
|
||||||
|
import run.halo.app.security.authentication.twofactor.TwoFactorUtils;
|
||||||
|
|
||||||
public class DefaultUserDetailService
|
public class DefaultUserDetailService
|
||||||
implements ReactiveUserDetailsService, ReactiveUserDetailsPasswordService {
|
implements ReactiveUserDetailsService, ReactiveUserDetailsPasswordService {
|
||||||
|
@ -63,10 +64,9 @@ public class DefaultUserDetailService
|
||||||
.doOnNext(userBuilder::authorities);
|
.doOnNext(userBuilder::authorities);
|
||||||
|
|
||||||
return setAuthorities.then(Mono.fromSupplier(() -> {
|
return setAuthorities.then(Mono.fromSupplier(() -> {
|
||||||
var twoFactorAuthEnabled =
|
var twoFactorAuthSettings = TwoFactorUtils.getTwoFactorAuthSettings(user);
|
||||||
requireNonNullElse(user.getSpec().getTwoFactorAuthEnabled(), false);
|
|
||||||
return new HaloUser.Builder(userBuilder.build())
|
return new HaloUser.Builder(userBuilder.build())
|
||||||
.twoFactorAuthEnabled(twoFactorAuthEnabled)
|
.twoFactorAuthEnabled(twoFactorAuthSettings.isAvailable())
|
||||||
.totpEncryptedSecret(user.getSpec().getTotpEncryptedSecret())
|
.totpEncryptedSecret(user.getSpec().getTotpEncryptedSecret())
|
||||||
.build();
|
.build();
|
||||||
}));
|
}));
|
||||||
|
|
|
@ -1,6 +1,10 @@
|
||||||
package run.halo.app.security;
|
package run.halo.app.security;
|
||||||
|
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
import static org.mockito.ArgumentMatchers.eq;
|
import static org.mockito.ArgumentMatchers.eq;
|
||||||
import static org.mockito.Mockito.times;
|
import static org.mockito.Mockito.times;
|
||||||
import static org.mockito.Mockito.verify;
|
import static org.mockito.Mockito.verify;
|
||||||
|
@ -107,6 +111,51 @@ class DefaultUserDetailServiceTest {
|
||||||
.verifyComplete();
|
.verifyComplete();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldFindHaloUserDetailsWith2faDisabledWhen2faNotEnabled() {
|
||||||
|
var fakeUser = createFakeUser();
|
||||||
|
when(userService.getUser("faker")).thenReturn(Mono.just(fakeUser));
|
||||||
|
when(roleService.listRoleRefs(any())).thenReturn(Flux.empty());
|
||||||
|
userDetailService.findByUsername("faker")
|
||||||
|
.as(StepVerifier::create)
|
||||||
|
.assertNext(userDetails -> {
|
||||||
|
assertInstanceOf(HaloUserDetails.class, userDetails);
|
||||||
|
assertFalse(((HaloUserDetails) userDetails).isTwoFactorAuthEnabled());
|
||||||
|
})
|
||||||
|
.verifyComplete();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldFindHaloUserDetailsWith2faDisabledWhen2faEnabledButNoTotpConfigured() {
|
||||||
|
var fakeUser = createFakeUser();
|
||||||
|
fakeUser.getSpec().setTwoFactorAuthEnabled(true);
|
||||||
|
when(userService.getUser("faker")).thenReturn(Mono.just(fakeUser));
|
||||||
|
when(roleService.listRoleRefs(any())).thenReturn(Flux.empty());
|
||||||
|
userDetailService.findByUsername("faker")
|
||||||
|
.as(StepVerifier::create)
|
||||||
|
.assertNext(userDetails -> {
|
||||||
|
assertInstanceOf(HaloUserDetails.class, userDetails);
|
||||||
|
assertFalse(((HaloUserDetails) userDetails).isTwoFactorAuthEnabled());
|
||||||
|
})
|
||||||
|
.verifyComplete();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldFindHaloUserDetailsWith2faEnabledWhen2faEnabledAndTotpConfigured() {
|
||||||
|
var fakeUser = createFakeUser();
|
||||||
|
fakeUser.getSpec().setTwoFactorAuthEnabled(true);
|
||||||
|
fakeUser.getSpec().setTotpEncryptedSecret("fake-totp-encrypted-secret");
|
||||||
|
when(userService.getUser("faker")).thenReturn(Mono.just(fakeUser));
|
||||||
|
when(roleService.listRoleRefs(any())).thenReturn(Flux.empty());
|
||||||
|
userDetailService.findByUsername("faker")
|
||||||
|
.as(StepVerifier::create)
|
||||||
|
.assertNext(userDetails -> {
|
||||||
|
assertInstanceOf(HaloUserDetails.class, userDetails);
|
||||||
|
assertTrue(((HaloUserDetails) userDetails).isTwoFactorAuthEnabled());
|
||||||
|
})
|
||||||
|
.verifyComplete();
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldFindUserDetailsByExistingUsernameButKindOfRoleRefIsNotRole() {
|
void shouldFindUserDetailsByExistingUsernameButKindOfRoleRefIsNotRole() {
|
||||||
var foundUser = createFakeUser();
|
var foundUser = createFakeUser();
|
||||||
|
|
Loading…
Reference in New Issue