mvysny / karibu-testing

Vaadin Server-Side Browserless Containerless Unit Testing
Apache License 2.0
111 stars 14 forks source link

Vaadin 24.3.0 breaks existing tests with exception in ErrorStateRenderer #161

Closed klauss42 closed 2 months ago

klauss42 commented 9 months ago

Karibu tests are failing after upgrading to Vaadin 24.3.0 with the following exception:

...
c.v.f.r.internal.ErrorStateRenderer      : The same exception com.vaadin.flow.router.AccessDeniedException has been thrown several times during navigation. Can't use any HasErrorParameter view for this error.

com.vaadin.flow.router.internal.ErrorStateRenderer$ExceptionsTrace: Exceptions handled by HasErrorParameter views are :com.vaadin.flow.router.NotFoundException, com.vaadin.flow.router.AccessDeniedException
    at com.vaadin.flow.router.internal.ErrorStateRenderer.handle(ErrorStateRenderer.java:93) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.router.internal.AbstractNavigationStateRenderer.reroute(AbstractNavigationStateRenderer.java:722) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.router.internal.AbstractNavigationStateRenderer.handleTriggeredBeforeEvent(AbstractNavigationStateRenderer.java:680) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.component.internal.JavaScriptNavigationStateRenderer.handleTriggeredBeforeEvent(JavaScriptNavigationStateRenderer.java:99) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.router.internal.AbstractNavigationStateRenderer.sendBeforeEnterEvent(AbstractNavigationStateRenderer.java:603) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.router.internal.AbstractNavigationStateRenderer.sendBeforeEnterEvent(AbstractNavigationStateRenderer.java:578) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.router.internal.AbstractNavigationStateRenderer.createChainIfEmptyAndExecuteBeforeEnterNavigation(AbstractNavigationStateRenderer.java:454) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.router.internal.AbstractNavigationStateRenderer.handle(AbstractNavigationStateRenderer.java:211) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.component.internal.JavaScriptNavigationStateRenderer.handle(JavaScriptNavigationStateRenderer.java:78) ~[flow-server-24.3.0.jar:24.3.0]
    at com.vaadin.flow.component.UI.handleNavigation(UI.java:1853) ~[flow-server-24.3.0.jar:24.3.0]

Our application is using Spring Security and view based permissions and the Karibu tests try to verify proper role checks per view.

As it is quite complicated to explain and reproduce, I prepared a small sample app, which reproduces the problem. There are 2 tests in the sample app:

 @Test
  fun `User with valid role is allowed to access page`() {
    integrationTestPreparer.withUserForRole( "valid", "USER")

    UI.getCurrent().navigate("/")
    // simulate a button click as if clicked by the user
    LocatorJ._click(LocatorJ._get(Button::class.java) { spec: SearchSpecJ<Button?> -> spec.withCaption("Say hello") })
  }

  @Test
  fun `User with invalid role is not allowed to access page`() {
    integrationTestPreparer.withUserForRole("invalid", "INVALID_ROLE")

    val ex = assertThrows<NotFoundException> {
      UI.getCurrent().navigate(HelloWorldView::class.java)
    }
    Assertions.assertThat(ex.message).contains("No route found for 'hello'")
  }

These tests run successful with Vaadin 24.2.6. With 24.3.0 the second test dealing with an AccessDenied error fails. You can reproduce by changing the version in the pom.xml

Here is the sample app: vaadin24-karibu-sample-app.zip

mvysny commented 6 months ago

Hi, I also noticed the following exception in karibu-tests themselves:

The same exception com.vaadin.flow.router.AccessDeniedException has been thrown several times during navigation. Can't use any HasErrorParameter view for this error.

I think this is the reason why the test fails, but unfortunately I haven't got the chance to analyze this change of Vaadin's behavior in depth.

mvysny commented 2 months ago

Looks like the problem is as follows:

  1. Navigation to a security-protected route fails. NavigationAccessControl:259 reroutes to exception AccessDeniedException
  2. RouteAccessDeniedError picks up the call and redirects to NotFoundException at line 39
  3. Vaadin tries to navigate to MockRouteNotFoundError, but it lacks any security annotations
  4. NavigationAccessControl is triggered and prevents access to MockRouteNotFoundError by re-routing to AccessDeniedException
  5. Vaadin decides it has seen enough and breaks the navigation cycle.

Let me write a test for this, and hopefully fix this issue by adding appropriate security annotations to MockRouteNotFoundError

mvysny commented 2 months ago

Alrighty, I've fixed the issue by introducing MockRouteAccessDeniedError which simply throws the AccessDeniedException immediately instead of redirecting to RouteNotFoundError. I've also added @AnonymousAllowed to MockRouteAccessDeniedError and MockRouteNotFoundError, so hopefully this bug should be fixed.

Fixed in Karibu-Testing 2.1.7, will be released shortly.

mvysny commented 2 months ago

Please use Karibu-Testing 2.1.8, there was a bug in 2.1.7.