Added authorization checks for BrokerService's JSON RPC methods

[sleshchenko]

What does this PR do?

Adds authorization checks for BrokerService's JSON RPC methods. Only user who has run permission is allowed to call broker/result and broker/statusChanged workspace related methods.

What issues does this PR fix or reference?

#11080

Release Notes

N/A

Docs PR

N/A

[garagatyi]

Looks good!
Please, take a look at my inlined comments.

[garagatyi]

I would add operations that would throw some custom exception when unexpected permissions check is executed in tests. This would ensure that there are no similar bugs in the code and tests at the same time. WDYT?

[sleshchenko]

Makes sense. I'll do it.

[sleshchenko]

I'll try to implement what you suggested but it fails

...
public class BrokerServicePermissionFilterTest {
  ...

  @BeforeMethod
  public void setUp() {
    EnvironmentContext.getCurrent().setSubject(subject);
    when(subject.hasPermission(any(), any(), any())).thenThrow(new IllegalArgumentException());
    permissionFilter = new BrokerServicePermissionFilter();
  }

...
  @Test(
      dataProvider = "coveredMethods",
      expectedExceptions = ForbiddenException.class,
      expectedExceptionsMessageRegExp =
          "User doesn't have the required permissions to the specified workspace")
  public void shouldThrowExceptionIfUserDoesNotHaveRunPermission(String method) throws Exception {

    // !!!!!!!!!!!!!!!!
    //  java.lang.IllegalArgumentException is thrown here

    // given
    when(subject.hasPermission(eq(WorkspaceDomain.DOMAIN_ID), eq("ws123"), eq(WorkspaceDomain.RUN)))
        .thenReturn(false);

    // when
    permissionFilter.doAccept(
        method, DtoFactory.newDto(BrokerResultEvent.class).withWorkspaceId("ws123"));
  }
  ...
}

So, looks like mockito make invocation method on defining new method behavior

    when(subject.hasPermission(eq(WorkspaceDomain.DOMAIN_ID), eq("ws123"), eq(WorkspaceDomain.RUN)))
        .thenReturn(false);

And java.lang.IllegalArgumentException is thrown since there is defined behavior for any values of parameters.

If you know how to implement it in another simple way - I'll be happy to do it.

[garagatyi]

ok

[garagatyi]

Can you elaborate why this is specific to the infra implementation?

[sleshchenko]

Because BrokerService is kubernetes specific component that is bound only for Kubernetes and OpenShift infrastructures. Does it make sense?

[garagatyi]

yes, not sure it should be really specific to the k8s in general, but I'm fine with that for the time being

[sleshchenko]

ci-test

[riuvshin]

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:11092
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

[sleshchenko]

Maven build is failed because of build error in Che Dockerfiles :: Theia module that is not changed in my PR. Will rerun tests again

[sleshchenko]

ci-test

[riuvshin]

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:11092
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.