Skip to content

Commit

Permalink
Fix the problem of not being able to create PAT for OAuth2 user (#6870)
Browse files Browse the repository at this point in the history
#### What type of PR is this?

/kind bug
/area core
/milestone 2.20.x

#### What this PR does / why we need it:

This PR refactors check of whether the current user is a real user to fix the problem of not being able to create PAT for OAuth2 user.

#### Does this PR introduce a user-facing change?

```release-note
修复通过 OAuth2 登录之后无法正常创建和恢复个人令牌的问题
```
  • Loading branch information
JohnNiang authored Oct 15, 2024
1 parent c3020d6 commit b95a83a
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import java.util.List;
import java.util.Objects;
import java.util.function.Predicate;
import org.springframework.security.authentication.AuthenticationTrustResolver;
import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.context.ReactiveSecurityContextHolder;
Expand Down Expand Up @@ -64,6 +66,9 @@ public class UserScopedPatHandlerImpl implements UserScopedPatHandler {

private Clock clock;

private final AuthenticationTrustResolver authTrustResolver =
new AuthenticationTrustResolverImpl();

public UserScopedPatHandlerImpl(ReactiveExtensionClient client,
CryptoService cryptoService,
ExternalUrlSupplier externalUrl,
Expand All @@ -84,8 +89,8 @@ public void setClock(Clock clock) {
this.clock = clock;
}

private static Mono<Authentication> mustBeRealUser(Mono<Authentication> authentication) {
return authentication.filter(AuthorityUtils::isRealUser)
private Mono<Authentication> mustBeAuthenticated(Mono<Authentication> authentication) {
return authentication.filter(authTrustResolver::isAuthenticated)
// Non-username-password authentication could not access the API at any time.
.switchIfEmpty(Mono.error(AccessDeniedException::new));
}
Expand All @@ -94,7 +99,7 @@ private static Mono<Authentication> mustBeRealUser(Mono<Authentication> authenti
public Mono<ServerResponse> create(ServerRequest request) {
return ReactiveSecurityContextHolder.getContext()
.map(SecurityContext::getAuthentication)
.transform(UserScopedPatHandlerImpl::mustBeRealUser)
.transform(this::mustBeAuthenticated)
.flatMap(auth -> request.bodyToMono(PersonalAccessToken.class)
.switchIfEmpty(
Mono.error(() -> new ServerWebInputException("Missing request body.")))
Expand Down Expand Up @@ -222,7 +227,7 @@ public Mono<ServerResponse> delete(ServerRequest request) {
public Mono<ServerResponse> restore(ServerRequest request) {
var restoredPat = ReactiveSecurityContextHolder.getContext()
.map(SecurityContext::getAuthentication)
.transform(UserScopedPatHandlerImpl::mustBeRealUser)
.transform(this::mustBeAuthenticated)
.flatMap(auth -> {
var name = request.pathVariable("name");
return getPat(name, auth.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.springframework.security.authentication.RememberMeAuthenticationToken;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;

/**
Expand Down Expand Up @@ -51,14 +48,4 @@ public static boolean containsSuperRole(Collection<String> roles) {
return roles.contains(SUPER_ROLE_NAME);
}

/**
* Check if the authentication is a real user.
*
* @param authentication current authentication
* @return true if the authentication is a real user; false otherwise
*/
public static boolean isRealUser(Authentication authentication) {
return authentication instanceof UsernamePasswordAuthenticationToken
|| authentication instanceof RememberMeAuthenticationToken;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,12 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static run.halo.app.security.authorization.AuthorityUtils.authoritiesToRoles;
import static run.halo.app.security.authorization.AuthorityUtils.containsSuperRole;
import static run.halo.app.security.authorization.AuthorityUtils.isRealUser;

import java.util.List;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.springframework.security.authentication.RememberMeAuthenticationToken;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.authority.SimpleGrantedAuthority;

class AuthorityUtilsTest {
Expand All @@ -39,9 +35,4 @@ void containsSuperRoleTest() {
assertFalse(containsSuperRole(Set.of("admin")));
}

@Test
void shouldReturnTrueWhenAuthenticationIsRealUser() {
assertTrue(isRealUser(mock(UsernamePasswordAuthenticationToken.class)));
assertTrue(isRealUser(mock(RememberMeAuthenticationToken.class)));
}
}

0 comments on commit b95a83a

Please sign in to comment.