-
Notifications
You must be signed in to change notification settings - Fork 5
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 access rules order and add integration tests #16
Conversation
Access rules were being applied in the wrong order, global rules first and service rules after, leading to the global /** rule matching service URLs that it shouldn't. Adds integration tests using WireMock.
interesting lib, I did not know about it |
I've read about it, first time I used it though. Really nice. |
public @Test void testService_requires_specific_roles_redirects_unauthenticated_to_login() { | ||
mockService.stubFor(get(urlMatching("/analytics(/.*)?")).willReturn(ok())); | ||
|
||
testClient.get().uri("/analytics")// | ||
.exchange()// | ||
.expectStatus().isFound()// | ||
.expectHeader().location("/login"); | ||
|
||
testClient.get().uri("/analytics/any/thing")// | ||
.exchange()// | ||
.expectStatus().isFound()// | ||
.expectHeader().location("/login"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the test's intent: if "willReturn(ok())", then why are we expecting a redirect to "/login" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because we're not expecting a 200, or the backend service to be hit at all. If it returns 200 means the backend service was hit and the access rule wasn't applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which mimics what would happen in real life. If you hit /analytics/anything in the actual analytics app, it'd return OK. But we want to assert the request doesn't even get there because the access rule requires a given role, and there's no authenticated user, so it gets redirected to the login page?
Access rules were being applied in the wrong order,
global rules first and service rules after, leading
to the global
/**
rule matching service URLs thatit shouldn't.
Adds integration tests using WireMock.