Skip to content

Commit 3933c45

Browse files
Improve Method Security logging
Closes gh-10247
1 parent 636ac64 commit 3933c45

8 files changed

+58
-23
lines changed

core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationDecision.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,24 @@
1818

1919
import java.util.Collection;
2020

21+
import org.springframework.security.core.GrantedAuthority;
22+
2123
/**
2224
* Represents an {@link AuthorizationDecision} based on a collection of authorities
2325
*
2426
* @author Marcus Da Coregio
2527
* @since 5.6
2628
*/
27-
class AuthorityAuthorizationDecision extends AuthorizationDecision {
29+
public class AuthorityAuthorizationDecision extends AuthorizationDecision {
2830

29-
private final Collection<String> authorities;
31+
private final Collection<GrantedAuthority> authorities;
3032

31-
AuthorityAuthorizationDecision(boolean granted, Collection<String> authorities) {
33+
public AuthorityAuthorizationDecision(boolean granted, Collection<GrantedAuthority> authorities) {
3234
super(granted);
3335
this.authorities = authorities;
3436
}
3537

36-
Collection<String> getAuthorities() {
38+
public Collection<GrantedAuthority> getAuthorities() {
3739
return this.authorities;
3840
}
3941

core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616

1717
package org.springframework.security.authorization;
1818

19-
import java.util.Arrays;
2019
import java.util.HashSet;
2120
import java.util.Set;
2221
import java.util.function.Supplier;
2322

2423
import org.springframework.security.core.Authentication;
2524
import org.springframework.security.core.GrantedAuthority;
25+
import org.springframework.security.core.authority.AuthorityUtils;
2626
import org.springframework.util.Assert;
2727

2828
/**
@@ -37,10 +37,10 @@ public final class AuthorityAuthorizationManager<T> implements AuthorizationMana
3737

3838
private static final String ROLE_PREFIX = "ROLE_";
3939

40-
private final Set<String> authorities;
40+
private final Set<GrantedAuthority> authorities;
4141

4242
private AuthorityAuthorizationManager(String... authorities) {
43-
this.authorities = new HashSet<>(Arrays.asList(authorities));
43+
this.authorities = new HashSet<>(AuthorityUtils.createAuthorityList(authorities));
4444
}
4545

4646
/**
@@ -133,8 +133,7 @@ private boolean isGranted(Authentication authentication) {
133133

134134
private boolean isAuthorized(Authentication authentication) {
135135
for (GrantedAuthority grantedAuthority : authentication.getAuthorities()) {
136-
String authority = grantedAuthority.getAuthority();
137-
if (this.authorities.contains(authority)) {
136+
if (this.authorities.contains(grantedAuthority)) {
138137
return true;
139138
}
140139
}

core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,13 +16,13 @@
1616

1717
package org.springframework.security.authorization;
1818

19-
import java.util.Arrays;
2019
import java.util.List;
2120

2221
import reactor.core.publisher.Mono;
2322

2423
import org.springframework.security.core.Authentication;
2524
import org.springframework.security.core.GrantedAuthority;
25+
import org.springframework.security.core.authority.AuthorityUtils;
2626
import org.springframework.util.Assert;
2727

2828
/**
@@ -35,18 +35,17 @@
3535
*/
3636
public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthorizationManager<T> {
3737

38-
private final List<String> authorities;
38+
private final List<GrantedAuthority> authorities;
3939

4040
AuthorityReactiveAuthorizationManager(String... authorities) {
41-
this.authorities = Arrays.asList(authorities);
41+
this.authorities = AuthorityUtils.createAuthorityList(authorities);
4242
}
4343

4444
@Override
4545
public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
4646
// @formatter:off
4747
return authentication.filter((a) -> a.isAuthenticated())
4848
.flatMapIterable(Authentication::getAuthorities)
49-
.map(GrantedAuthority::getAuthority)
5049
.any(this.authorities::contains)
5150
.map((granted) -> ((AuthorizationDecision) new AuthorityAuthorizationDecision(granted, this.authorities)))
5251
.defaultIfEmpty(new AuthorityAuthorizationDecision(false, this.authorities));

core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java

+19-1
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,18 @@
2121
import org.aopalliance.aop.Advice;
2222
import org.aopalliance.intercept.MethodInterceptor;
2323
import org.aopalliance.intercept.MethodInvocation;
24+
import org.apache.commons.logging.Log;
25+
import org.apache.commons.logging.LogFactory;
2426

2527
import org.springframework.aop.Pointcut;
2628
import org.springframework.aop.PointcutAdvisor;
2729
import org.springframework.aop.framework.AopInfrastructureBean;
2830
import org.springframework.core.Ordered;
31+
import org.springframework.core.log.LogMessage;
2932
import org.springframework.security.access.AccessDeniedException;
3033
import org.springframework.security.access.prepost.PostAuthorize;
3134
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
35+
import org.springframework.security.authorization.AuthorizationDecision;
3236
import org.springframework.security.authorization.AuthorizationManager;
3337
import org.springframework.security.core.Authentication;
3438
import org.springframework.security.core.context.SecurityContextHolder;
@@ -54,6 +58,8 @@ public final class AuthorizationManagerAfterMethodInterceptor
5458
return authentication;
5559
};
5660

61+
private final Log logger = LogFactory.getLog(this.getClass());
62+
5763
private final Pointcut pointcut;
5864

5965
private final AuthorizationManager<MethodInvocationResult> authorizationManager;
@@ -103,7 +109,7 @@ public static AuthorizationManagerAfterMethodInterceptor postAuthorize(
103109
@Override
104110
public Object invoke(MethodInvocation mi) throws Throwable {
105111
Object result = mi.proceed();
106-
this.authorizationManager.verify(AUTHENTICATION_SUPPLIER, new MethodInvocationResult(mi, result));
112+
attemptAuthorization(mi, result);
107113
return result;
108114
}
109115

@@ -134,4 +140,16 @@ public boolean isPerInstance() {
134140
return true;
135141
}
136142

143+
private void attemptAuthorization(MethodInvocation mi, Object result) {
144+
this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
145+
AuthorizationDecision decision = this.authorizationManager.check(AUTHENTICATION_SUPPLIER,
146+
new MethodInvocationResult(mi, result));
147+
if (decision != null && !decision.isGranted()) {
148+
this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
149+
+ this.authorizationManager + " and decision " + decision));
150+
throw new AccessDeniedException("Access Denied");
151+
}
152+
this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
153+
}
154+
137155
}

core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,19 @@
2525
import org.aopalliance.aop.Advice;
2626
import org.aopalliance.intercept.MethodInterceptor;
2727
import org.aopalliance.intercept.MethodInvocation;
28+
import org.apache.commons.logging.Log;
29+
import org.apache.commons.logging.LogFactory;
2830

2931
import org.springframework.aop.Pointcut;
3032
import org.springframework.aop.PointcutAdvisor;
3133
import org.springframework.aop.framework.AopInfrastructureBean;
3234
import org.springframework.core.Ordered;
35+
import org.springframework.core.log.LogMessage;
3336
import org.springframework.security.access.AccessDeniedException;
3437
import org.springframework.security.access.annotation.Secured;
3538
import org.springframework.security.access.prepost.PreAuthorize;
3639
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
40+
import org.springframework.security.authorization.AuthorizationDecision;
3741
import org.springframework.security.authorization.AuthorizationManager;
3842
import org.springframework.security.core.Authentication;
3943
import org.springframework.security.core.context.SecurityContextHolder;
@@ -59,6 +63,8 @@ public final class AuthorizationManagerBeforeMethodInterceptor
5963
return authentication;
6064
};
6165

66+
private final Log logger = LogFactory.getLog(this.getClass());
67+
6268
private final Pointcut pointcut;
6369

6470
private final AuthorizationManager<MethodInvocation> authorizationManager;
@@ -149,7 +155,7 @@ public static AuthorizationManagerBeforeMethodInterceptor jsr250(Jsr250Authoriza
149155
*/
150156
@Override
151157
public Object invoke(MethodInvocation mi) throws Throwable {
152-
this.authorizationManager.verify(AUTHENTICATION_SUPPLIER, mi);
158+
attemptAuthorization(mi);
153159
return mi.proceed();
154160
}
155161

@@ -180,4 +186,15 @@ public boolean isPerInstance() {
180186
return true;
181187
}
182188

189+
private void attemptAuthorization(MethodInvocation mi) {
190+
this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
191+
AuthorizationDecision decision = this.authorizationManager.check(AUTHENTICATION_SUPPLIER, mi);
192+
if (decision != null && !decision.isGranted()) {
193+
this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
194+
+ this.authorizationManager + " and decision " + decision));
195+
throw new AccessDeniedException("Access Denied");
196+
}
197+
this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
198+
}
199+
183200
}

core/src/main/java/org/springframework/security/authorization/method/ExpressionAttributeAuthorizationDecision.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@
2424
* @author Marcus Da Coregio
2525
* @since 5.6
2626
*/
27-
class ExpressionAttributeAuthorizationDecision extends AuthorizationDecision {
27+
public class ExpressionAttributeAuthorizationDecision extends AuthorizationDecision {
2828

2929
private final ExpressionAttribute expressionAttribute;
3030

31-
ExpressionAttributeAuthorizationDecision(boolean granted, ExpressionAttribute expressionAttribute) {
31+
public ExpressionAttributeAuthorizationDecision(boolean granted, ExpressionAttribute expressionAttribute) {
3232
super(granted);
3333
this.expressionAttribute = expressionAttribute;
3434
}
3535

36-
ExpressionAttribute getExpressionAttribute() {
36+
public ExpressionAttribute getExpressionAttribute() {
3737
return this.expressionAttribute;
3838
}
3939

core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public void instantiateWhenAuthorizationManagerNullThenException() {
5353
}
5454

5555
@Test
56-
public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() throws Throwable {
56+
public void beforeWhenMockAuthorizationManagerThenCheckAndReturnedObject() throws Throwable {
5757
MethodInvocation mockMethodInvocation = mock(MethodInvocation.class);
5858
MethodInvocationResult result = new MethodInvocationResult(mockMethodInvocation, new Object());
5959
given(mockMethodInvocation.proceed()).willReturn(result.getResult());
@@ -62,7 +62,7 @@ public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() thro
6262
Pointcut.TRUE, mockAuthorizationManager);
6363
Object returnedObject = advice.invoke(mockMethodInvocation);
6464
assertThat(returnedObject).isEqualTo(result.getResult());
65-
verify(mockAuthorizationManager).verify(eq(AuthorizationManagerAfterMethodInterceptor.AUTHENTICATION_SUPPLIER),
65+
verify(mockAuthorizationManager).check(eq(AuthorizationManagerAfterMethodInterceptor.AUTHENTICATION_SUPPLIER),
6666
any(MethodInvocationResult.class));
6767
}
6868

core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ public void instantiateWhenAuthorizationManagerNullThenException() {
4949
}
5050

5151
@Test
52-
public void beforeWhenMockAuthorizationManagerThenVerify() throws Throwable {
52+
public void beforeWhenMockAuthorizationManagerThenCheck() throws Throwable {
5353
MethodInvocation mockMethodInvocation = mock(MethodInvocation.class);
5454
AuthorizationManager<MethodInvocation> mockAuthorizationManager = mock(AuthorizationManager.class);
5555
AuthorizationManagerBeforeMethodInterceptor advice = new AuthorizationManagerBeforeMethodInterceptor(
5656
Pointcut.TRUE, mockAuthorizationManager);
5757
advice.invoke(mockMethodInvocation);
58-
verify(mockAuthorizationManager).verify(AuthorizationManagerBeforeMethodInterceptor.AUTHENTICATION_SUPPLIER,
58+
verify(mockAuthorizationManager).check(AuthorizationManagerBeforeMethodInterceptor.AUTHENTICATION_SUPPLIER,
5959
mockMethodInvocation);
6060
}
6161

0 commit comments

Comments
 (0)