Add auditing metadata in tests #190
Conversation
NielsCW
force-pushed
the
auditing-metadata
branch
from
74a97d1
to
5cda685
Compare
| public static void loginUser(String username) { | ||
| var user = new User(username, "password", Set.of()); | ||
| var authentication = new UsernamePasswordAuthenticationToken(user, user.getPassword(), user.getAuthorities()); | ||
| var context = SecurityContextHolder.getContext(); | ||
| context.setAuthentication(authentication); | ||
| } |
There was a problem hiding this comment.
can you explain how this is related to @WithMockUser on the class-level ?
There was a problem hiding this comment.
@WithMockUser sets the SecurityContext to contain an authenticated User with the given username. All HTTP requests and repository.save() calls will update lastModifiedBy to contain this mockuser. This loginUser() overrides SecurityContext.authentication to contain a new authenticated User. So that the new user is used in the auditing of the following requests. This is used to test whether lastModifiedBy can be altered when a different user makes a PUT request for example.
There was a problem hiding this comment.
so the "problem" is that if you override @WithMockUser("other") on the test-method level, the before-each also runs with user "other" ?
There was a problem hiding this comment.
With @WithMockUser alone, you can only test one user per test:
If a test is marked with @WithMockUser("John") and it calls a function that modifies an object in a repository with @WithMockUser("Bob"), lastModifiedDate of that object will be updated, but lastModifiedBy will contain "John".
There was a problem hiding this comment.
and something like this does not work: https://docs.spring.io/spring-security/reference/servlet/test/mockmvc/authentication.html#test-mockmvc-securitycontextholder-rpp ?
There was a problem hiding this comment.
The "problem" with this is that you can't use mocks and have to add extra org.springframework.security:spring-security-config dependency and implement security filtering in your configuration. I have not yet succeeded in getting it to work in demo application.
|
|
||
| @Bean | ||
| public AuditorAware<UserMetadata> auditorProvider() { | ||
| return new AuditorAwareImpl(); |
There was a problem hiding this comment.
to keep this test-fixture as compact as possible, I would prefer to inline the implementation as an anonymous class
| public static void loginUser(String username) { | ||
| var user = new User(username, "password", Set.of()); | ||
| var authentication = new UsernamePasswordAuthenticationToken(user, user.getPassword(), user.getAuthorities()); | ||
| var context = SecurityContextHolder.getContext(); | ||
| context.setAuthentication(authentication); | ||
| } |
There was a problem hiding this comment.
so the "problem" is that if you override @WithMockUser("other") on the test-method level, the before-each also runs with user "other" ?
| public static void loginUser(String username) { | ||
| var user = new User(username, "password", Set.of()); | ||
| var authentication = new UsernamePasswordAuthenticationToken(user, user.getPassword(), user.getAuthorities()); | ||
| var context = SecurityContextHolder.getContext(); | ||
| context.setAuthentication(authentication); | ||
| } |
There was a problem hiding this comment.
and something like this does not work: https://docs.spring.io/spring-security/reference/servlet/test/mockmvc/authentication.html#test-mockmvc-securitycontextholder-rpp ?
NielsCW
force-pushed
the
auditing-metadata
branch
from
df3dba4
to
7d2f63d
Compare
NielsCW
force-pushed
the
auditing-metadata
branch
from
494fd25
to
50b9514
Compare
|
We need to figure out what the implications are of putting |
NielsCW
force-pushed
the
auditing-metadata
branch
from
5257128
to
84701fa
Compare
|
|
||
| private static ServerRequestObservationContext createContext() { | ||
| var request = new MockHttpServletRequest(); | ||
| request.setAttribute(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE, "true"); |
There was a problem hiding this comment.
The tests used to work because the handler in BasicAuditEventExtractor.createEventBuilder is null and it was missing a null check. Since the null-check is added and the requests are crafted manually, we add this attribute to make the handler non-null and the request isn't ignored.
There was a problem hiding this comment.
isn't it fishy that all the code expect that it contains a request-handler-method and we're putting the string "true" in there ? 🤔
…ng metadata tests for relations
…..)) in invoicing-application-tests to perform requests with new user
NielsCW
force-pushed
the
auditing-metadata
branch
from
6ce0ba9
to
b9a4c85
Compare
|
No description provided.