[AMBARI-22837] AmbariAuthorizationFilterTest.testDoFilter_userNoPermissionsAccess fails unexpectedly#176
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
| public class AmbariAuthorizationFilterTest { | ||
| @Before | ||
| public void setUp() { | ||
| SecurityContextHolder.getContext().setAuthentication(null); |
There was a problem hiding this comment.
Technically, this is not the correct solution since the test case appears to use a mocked SecurityContext, not the one from the SecurityContextHolder. See org.apache.ambari.server.security.authorization.AmbariAuthorizationFilterTest#performGeneralDoFilterTest.
I think we need to remove the mocked SecurityContext and use the one provided by Spring. For example, see org.apache.ambari.server.controller.internal.UserResourceProviderTest#deleteResourcesTest.
In cases like the referenced method, we can do the following rather than create a mocked SecurityContext:
SecurityContextHolder.getContext().setAuthentication(authentication);
Since the test case explictly sets the authenticated user before performing the test, there is no real need to explicitly clear it in the before method.
|
I have made the requested changes. |
|
Refer to this link for build results (access rights to CI server needed): |
|
@rlevas @echekanskiy is this pull request ready to be merged? Feel free to let me know if any additional changes are necessary. |
3 participants
What changes were proposed in this pull request?
This test requires that the security context not be set up when it runs to pass. If a test that sets up the security context improperly is run before this test, it will fail despite there being no changes in the code.
This can be fixed by clearing the security context.
How was this patch tested?
Patch was tested manually.
@adoroszlai This issue is similar to another pull request I submitted (#133)