Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the problem of not being able to create PAT for OAuth2 user #6870

Merged
merged 1 commit into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)));
}
}
Loading