Skip to content
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

Extend permission checks in MessageTasks and add a test coverage [HZ-2090] [5.3.z] #25539

Merged

Conversation

kwart
Copy link
Member

@kwart kwart commented Sep 26, 2023

Backport of #25509

This PR extends permission checks in client messages and adds basic test coverage.

…2090] (hazelcast#25509)

This change extends permission checks in client messages and adds a basic test
coverage.
@kwart kwart added this to the 5.3.z milestone Sep 26, 2023
@kwart kwart requested a review from a team as a code owner September 26, 2023 20:18
@kwart kwart self-assigned this Sep 26, 2023
@hz-devops-test
Copy link

The job Hazelcast-pr-EE-compiler of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
---------ERRORS-----------
--------------------------
[ERROR] 'dependencies.dependency.groupId' for $com.google.protobuf:protobuf-java:jar with value '$com.google.protobuf' does not match a valid id pattern. @ line 17, column 16
--------------------------
[ERROR] 'dependencies.dependency.version' for $com.google.protobuf:protobuf-java:jar is missing. @ line 16, column 17
--------------------------
[ERROR] 'dependencies.dependency.groupId' for $com.google.protobuf:protobuf-java:jar with value '$com.google.protobuf' does not match a valid id pattern. @ line 17, column 16
--------------------------
[ERROR] 'dependencies.dependency.version' for $com.google.protobuf:protobuf-java:jar is missing. @ line 16, column 17
--------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:license-maven-plugin:2.0.1:add-third-party (add-third-party) on project hazelcast-jet-files-s3: could not init goal AddThirdPartyMojo for reason : null: ConcurrentModificationException -> [Help 1]
--------------------------
[ERROR] 
--------------------------
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
--------------------------
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
--------------------------
[ERROR] 
--------------------------
[ERROR] For more information about the errors and possible solutions, please read the following articles:
--------------------------
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
--------------------------
[ERROR] 
--------------------------
[ERROR] After correcting the problems, you can resume the build with the command
--------------------------
[ERROR]   mvn  -rf :hazelcast-jet-files-s3
--------------------------

Copy link
Contributor

@ihsandemir ihsandemir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a 1:1 cherry pick from the master PR. Hence, it looks good to me.

Copy link
Contributor

@ihsandemir ihsandemir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwart This may break some existing customer code, right? Are we OK to merge in the patch since it is a security issue?

@kwart
Copy link
Member Author

kwart commented Oct 1, 2023

@kwart This may break some existing customer code, right? Are we OK to merge in the patch since it is a security issue?

The change is not expected to break existing code. Only if there is a client permission misconfiguration on members (i.e. missing permission grants), then after applying the fix it might require a configuration fix on members.

@kwart kwart requested a review from olukas October 2, 2023 13:42
@kwart
Copy link
Member Author

kwart commented Oct 2, 2023

Adding Ondrej as a gatekeeper for the PR merge 😉

@kwart
Copy link
Member Author

kwart commented Oct 4, 2023

Merging after a discussion with Ondrej.

@kwart kwart merged commit e9d9b6d into hazelcast:5.3.z Oct 4, 2023
7 of 8 checks passed
@kwart kwart deleted the fix/5.3.z/HZ-2090-getRequiredPermission branch October 4, 2023 11:02
@ihsandemir
Copy link
Contributor

@kwart This may break some existing customer code, right? Are we OK to merge in the patch since it is a security issue?

The change is not expected to break existing code. Only if there is a client permission misconfiguration on members (i.e. missing permission grants), then after applying the fix it might require a configuration fix on members.

@kwart Yes, but it will make working user code not work but require some changes in the config to make it work. That is why I raised this concern. It may be OK since it is security related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants