From b0cecb37d20468b7b9f01176951580138354c0aa Mon Sep 17 00:00:00 2001 From: Evgeniy Cheban Date: Fri, 18 Apr 2025 13:11:17 +0300 Subject: [PATCH] Replace deprecated #check calls with #authorize Closes gh-16936 Signed-off-by: Evgeniy Cheban --- .../AuthorizeHttpRequestsConfigurerTests.java | 6 ++++- .../authorization/AuthorizationManager.java | 8 +++---- .../ObservationAuthorizationManager.java | 23 +++++++++++++++---- .../ObservationAuthorizationManagerTests.java | 5 +++- ...MatcherDelegatingAuthorizationManager.java | 18 +++++++++++++-- ...MatcherDelegatingAuthorizationManager.java | 18 +++++++++++++-- 6 files changed, 63 insertions(+), 15 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java index 057deea40c..7d57cae201 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -85,6 +85,7 @@ import org.springframework.web.servlet.handler.HandlerMappingIntrospector; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doCallRealMethod; @@ -153,6 +154,7 @@ public class AuthorizeHttpRequestsConfigurerTests { @Test public void configureMvcMatcherAccessAuthorizationManagerWhenNotNullThenVerifyUse() throws Exception { CustomAuthorizationManagerConfig.authorizationManager = mock(AuthorizationManager.class); + given(CustomAuthorizationManagerConfig.authorizationManager.authorize(any(), any())).willCallRealMethod(); this.spring.register(CustomAuthorizationManagerConfig.class, BasicController.class).autowire(); this.mvc.perform(get("/")).andExpect(status().isOk()); verify(CustomAuthorizationManagerConfig.authorizationManager).check(any(), any()); @@ -161,6 +163,8 @@ public class AuthorizeHttpRequestsConfigurerTests { @Test public void configureNoParameterMvcMatcherAccessAuthorizationManagerWhenNotNullThenVerifyUse() throws Exception { CustomAuthorizationManagerNoParameterConfig.authorizationManager = mock(AuthorizationManager.class); + given(CustomAuthorizationManagerNoParameterConfig.authorizationManager.authorize(any(), any())) + .willCallRealMethod(); this.spring.register(CustomAuthorizationManagerNoParameterConfig.class, BasicController.class).autowire(); this.mvc.perform(get("/")).andExpect(status().isOk()); verify(CustomAuthorizationManagerNoParameterConfig.authorizationManager).check(any(), any()); diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/AuthorizationManager.java index 758abce2c7..b039068b22 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorizationManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,9 +39,9 @@ public interface AuthorizationManager { * @throws AccessDeniedException if access is not granted */ default void verify(Supplier authentication, T object) { - AuthorizationDecision decision = check(authentication, object); - if (decision != null && !decision.isGranted()) { - throw new AuthorizationDeniedException("Access Denied", decision); + AuthorizationResult result = authorize(authentication, object); + if (result != null && !result.isGranted()) { + throw new AuthorizationDeniedException("Access Denied", result); } } diff --git a/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java index 0494dbfecb..2c4a785f30 100644 --- a/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -67,6 +67,19 @@ public final class ObservationAuthorizationManager @Deprecated @Override public AuthorizationDecision check(Supplier authentication, T object) { + AuthorizationResult result = authorize(authentication, object); + if (result == null) { + return null; + } + if (result instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( + "Please call #authorize or ensure that the returned result is of type AuthorizationDecision"); + } + + @Override + public AuthorizationResult authorize(Supplier authentication, T object) { AuthorizationObservationContext context = new AuthorizationObservationContext<>(object); Supplier wrapped = () -> { context.setAuthentication(authentication.get()); @@ -74,13 +87,13 @@ public final class ObservationAuthorizationManager }; Observation observation = Observation.createNotStarted(this.convention, () -> context, this.registry).start(); try (Observation.Scope scope = observation.openScope()) { - AuthorizationDecision decision = this.delegate.check(wrapped, object); - context.setAuthorizationResult(decision); - if (decision != null && !decision.isGranted()) { + AuthorizationResult result = this.delegate.authorize(wrapped, object); + context.setAuthorizationResult(result); + if (result != null && !result.isGranted()) { observation.error(new AccessDeniedException( this.messages.getMessage("AbstractAccessDecisionManager.accessDenied", "Access Denied"))); } - return decision; + return result; } catch (Throwable ex) { observation.error(ex); diff --git a/core/src/test/java/org/springframework/security/authorization/ObservationAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/ObservationAuthorizationManagerTests.java index 1921f7dc1a..bb2ccd8faa 100644 --- a/core/src/test/java/org/springframework/security/authorization/ObservationAuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/ObservationAuthorizationManagerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -74,6 +74,7 @@ public class ObservationAuthorizationManagerTests { void verifyWhenDefaultsThenObserves() { given(this.handler.supportsContext(any())).willReturn(true); given(this.authorizationManager.check(any(), any())).willReturn(this.grant); + given(this.authorizationManager.authorize(any(), any())).willCallRealMethod(); this.tested.verify(this.token, this.object); ArgumentCaptor captor = ArgumentCaptor.forClass(Observation.Context.class); verify(this.handler).onStart(captor.capture()); @@ -92,6 +93,7 @@ public class ObservationAuthorizationManagerTests { this.tested.setMessageSource(source); given(this.handler.supportsContext(any())).willReturn(true); given(this.authorizationManager.check(any(), any())).willReturn(this.deny); + given(this.authorizationManager.authorize(any(), any())).willCallRealMethod(); given(source.getMessage(eq("AbstractAccessDecisionManager.accessDenied"), any(), any(), any())) .willReturn("accessDenied"); assertThatExceptionOfType(AccessDeniedException.class) @@ -116,6 +118,7 @@ public class ObservationAuthorizationManagerTests { ((Supplier) invocation.getArgument(0)).get(); return this.grant; }); + given(this.authorizationManager.authorize(any(), any())).willCallRealMethod(); this.tested.verify(this.token, this.object); ArgumentCaptor captor = ArgumentCaptor.forClass(Observation.Context.class); verify(this.handler).onStart(captor.capture()); diff --git a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java index e7ff32718a..c24bb1b7b5 100644 --- a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java +++ b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java @@ -30,6 +30,7 @@ import org.springframework.security.authorization.AuthenticatedAuthorizationMana import org.springframework.security.authorization.AuthorityAuthorizationManager; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; import org.springframework.security.authorization.SingleResultAuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.messaging.util.matcher.MessageMatcher; @@ -63,11 +64,24 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho * @return an {@link AuthorizationDecision}. If there is no {@link MessageMatcher} * matching the message, or the {@link AuthorizationManager} could not decide, then * null is returned - * @deprecated please use {@link #authorize(Supplier, Object)} instead + * @deprecated please use {@link #authorize(Supplier, Message)} instead */ @Deprecated @Override public AuthorizationDecision check(Supplier authentication, Message message) { + AuthorizationResult result = authorize(authentication, message); + if (result == null) { + return null; + } + if (result instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( + "Please call #authorize or ensure that the returned result is of type AuthorizationDecision"); + } + + @Override + public AuthorizationResult authorize(Supplier authentication, Message message) { if (this.logger.isTraceEnabled()) { this.logger.trace(LogMessage.format("Authorizing message")); } @@ -79,7 +93,7 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho if (this.logger.isTraceEnabled()) { this.logger.trace(LogMessage.format("Checking authorization on message using %s", manager)); } - return manager.check(authentication, authorizationContext); + return manager.authorize(authentication, authorizationContext); } } this.logger.trace("Abstaining since did not find matching MessageMatcher"); diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java b/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java index b01ab43bbd..de660a3f97 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java @@ -30,6 +30,7 @@ import org.springframework.security.authorization.AuthenticatedAuthorizationMana import org.springframework.security.authorization.AuthorityAuthorizationManager; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; import org.springframework.security.authorization.SingleResultAuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.web.util.UrlUtils; @@ -69,11 +70,24 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho * @return an {@link AuthorizationDecision}. If there is no {@link RequestMatcher} * matching the request, or the {@link AuthorizationManager} could not decide, then * null is returned - * @deprecated please use {@link #authorize(Supplier, Object)} instead + * @deprecated please use {@link #authorize(Supplier, HttpServletRequest)} instead */ @Deprecated @Override public AuthorizationDecision check(Supplier authentication, HttpServletRequest request) { + AuthorizationResult result = authorize(authentication, request); + if (result == null) { + return null; + } + if (result instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( + "Please call #authorize or ensure that the returned result is of type AuthorizationDecision"); + } + + @Override + public AuthorizationResult authorize(Supplier authentication, HttpServletRequest request) { if (this.logger.isTraceEnabled()) { this.logger.trace(LogMessage.format("Authorizing %s", requestLine(request))); } @@ -87,7 +101,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho this.logger.trace( LogMessage.format("Checking authorization on %s using %s", requestLine(request), manager)); } - return manager.check(authentication, + return manager.authorize(authentication, new RequestAuthorizationContext(request, matchResult.getVariables())); } }