You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "kyooosukedn (via GitHub)" <gi...@apache.org> on 2023/04/06 22:13:10 UTC
[GitHub] [eventmesh] kyooosukedn opened a new pull request, #3644: Code optimization
kyooosukedn opened a new pull request, #3644:
URL: https://github.com/apache/eventmesh/pull/3644
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Closes #<XXX>`.)
-->
Fixes #3515 .
### Motivation
*Code Optimization on AuthTokenUtils.java*
*Remove redundancy*
### Modifications
*Remove duplicate code and redundancies*
### Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1452772065
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +74,44 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
+
if (!(topics instanceof Set)) {
Review Comment:
Yes, if I am not mistaken :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1451784848
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +74,44 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
+
if (!(topics instanceof Set)) {
Review Comment:
Is this PR ready for review now?😊
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1454779835
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -146,6 +81,50 @@ public static boolean authAccess(AclProperties aclProperties) {
Set<String> groupTopics = TypeUtils.castSet(topics, String.class);
return groupTopics.contains(topic);
+
+ }
Review Comment:
We generally won't leave a blank line at the end of a method.
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -146,6 +81,50 @@ public static boolean authAccess(AclProperties aclProperties) {
Set<String> groupTopics = TypeUtils.castSet(topics, String.class);
return groupTopics.contains(topic);
+
+ }
+
+ private static String getPublicKeyUrl() {
+ String publicKeyUrl = null;
+ for (String key : ConfigurationContextUtil.KEYS) {
+ CommonConfiguration commonConfiguration = ConfigurationContextUtil.get(key);
+ if (null == commonConfiguration) {
+ continue;
+ }
+ if (StringUtils.isBlank(commonConfiguration.getEventMeshSecurityPublickey())) {
+ throw new AclException("publicKeyUrl cannot be null");
+ }
+ publicKeyUrl = commonConfiguration.getEventMeshSecurityPublickey();
+ }
+ return publicKeyUrl;
+ }
+
+ private static void validateToken(String token, String publicKeyUrl, AclProperties aclProperties) {
+ String sub;
+ token = token.replace("Bearer ", "");
+ byte[] validationKeyBytes;
+ try {
+ validationKeyBytes = Files.readAllBytes(Paths.get(Objects.requireNonNull(publicKeyUrl)));
+ X509EncodedKeySpec spec = new X509EncodedKeySpec(validationKeyBytes);
+ KeyFactory kf = KeyFactory.getInstance("RSA");
+ Key validationKey = kf.generatePublic(spec);
+ JwtParser signedParser = Jwts.parserBuilder().setSigningKey(validationKey).build();
+ Jwt<?, Claims> signJwt = signedParser.parseClaimsJws(token);
+ sub = signJwt.getBody().get("sub", String.class);
+ if (!sub.contains(aclProperties.getExtendedField("group").toString()) && !sub.contains("pulsar-admin")) {
+ throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access eventMesh:"
+ + aclProperties.getTopic());
+ }
+ } catch (IOException e) {
+ throw new AclException("public key read error!", e);
+ } catch (NoSuchAlgorithmException e) {
+ throw new AclException("no such RSA algorithm!", e);
+ } catch (InvalidKeySpecException e) {
+ throw new AclException("invalid public key spec!", e);
+ } catch (JwtException e) {
+ throw new AclException("invalid token!", e);
+ }
+
}
}
Review Comment:
Redundant lines here too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] Alonexc commented on a diff in pull request #3644: [ISSUE #3515] Do some code optimization[AuthTokenUtils]
Posted by "Alonexc (via GitHub)" <gi...@apache.org>.
Alonexc commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1163784652
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/HTTPClientHandler.java:
##########
@@ -139,7 +139,7 @@ void list(HttpExchange httpExchange) throws IOException {
});
String result = JsonUtils.toJSONString(getClientResponseList);
- httpExchange.sendResponseHeaders(200, result.getBytes().length);
+ httpExchange.sendResponseHeaders(200, result.getBytes(Constants.DEFAULT_CHARSET).length);
Review Comment:
Here `Constants.DEFAULT_CHARSET` needs to be imported:
org.apache.eventmesh.common.Constants;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] Alonexc commented on pull request #3644: [ISSUE #3515] Do some code optimization[AuthTokenUtils]
Posted by "Alonexc (via GitHub)" <gi...@apache.org>.
Alonexc commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1499862769
Please check the checkstyle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] kyooosukedn commented on pull request #3644: [ISSUE #3515] Do some code optimization[AuthTokenUtils]
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1500311865
>  Please check the checkstyle.
Hi, how do I get those warnings? I ran ./gradlew check and did code inspection on file AuthTokenUtils.java and found nothing, I have imported checkstyle.xml though
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1899943680
cc @Alonexc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1457064007
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -148,4 +83,46 @@ public static boolean authAccess(AclProperties aclProperties) {
return groupTopics.contains(topic);
}
+ private static String getPublicKeyUrl() {
+ String publicKeyUrl = null;
+ for (String key : ConfigurationContextUtil.KEYS) {
+ CommonConfiguration commonConfiguration = ConfigurationContextUtil.get(key);
+ if (null == commonConfiguration) {
+ continue;
+ }
+ if (StringUtils.isBlank(commonConfiguration.getEventMeshSecurityPublickey())) {
+ throw new AclException("publicKeyUrl cannot be null");
+ }
+ publicKeyUrl = commonConfiguration.getEventMeshSecurityPublickey();
+ }
+ return publicKeyUrl;
+ }
+
+ private static void validateToken(String token, String publicKeyUrl, AclProperties aclProperties) {
+ String sub;
+ token = token.replace("Bearer ", "");
+ byte[] validationKeyBytes;
+ try {
+ validationKeyBytes = Files.readAllBytes(Paths.get(Objects.requireNonNull(publicKeyUrl)));
+ X509EncodedKeySpec spec = new X509EncodedKeySpec(validationKeyBytes);
+ KeyFactory kf = KeyFactory.getInstance("RSA");
+ Key validationKey = kf.generatePublic(spec);
+ JwtParser signedParser = Jwts.parserBuilder().setSigningKey(validationKey).build();
+ Jwt<?, Claims> signJwt = signedParser.parseClaimsJws(token);
+ sub = signJwt.getBody().get("sub", String.class);
+ if (!sub.contains(aclProperties.getExtendedField("group").toString()) && !sub.contains("pulsar-admin")) {
+ throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access eventMesh:"
+ + aclProperties.getTopic());
+ }
+ } catch (IOException e) {
+ throw new AclException("public key read error!", e);
+ } catch (NoSuchAlgorithmException e) {
+ throw new AclException("no such RSA algorithm!", e);
+ } catch (InvalidKeySpecException e) {
+ throw new AclException("invalid public key spec!", e);
+ } catch (JwtException e) {
+ throw new AclException("invalid token!", e);
+ }
+ }
+
}
Review Comment:
L127 still has an extra blank line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1894611910
Hi @Pil0tXia, I have corrected as you suggested. Thank u for the review :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1883117589
Please resolve conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-2070570806
## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/3644?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 17.59%. Comparing base [(`5850d54`)](https://app.codecov.io/gh/apache/eventmesh/commit/5850d5414b6ae21646cd9194dc4e93fb5d7dedf1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`12b521c`)](https://app.codecov.io/gh/apache/eventmesh/pull/3644?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 49 commits behind head on master.
> :exclamation: Current head 12b521c differs from pull request most recent head 13b2bee. Consider uploading reports for the commit 13b2bee to get more accurate results
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3644 +/- ##
============================================
+ Coverage 17.55% 17.59% +0.04%
+ Complexity 1778 1774 -4
============================================
Files 797 797
Lines 29871 29786 -85
Branches 2581 2573 -8
============================================
- Hits 5243 5242 -1
+ Misses 24145 24063 -82
+ Partials 483 481 -2
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/eventmesh/pull/3644?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-2070569856
It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback.
If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1453304782
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +75,58 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
if (!(topics instanceof Set)) {
return false;
}
Set<String> groupTopics = TypeUtils.castSet(topics, String.class);
return groupTopics.contains(topic);
+
+ }
+
+ public static void validateToken(String token, String publicKeyUrl, AclProperties aclProperties) {
+ String sub;
+ token = token.replace("Bearer ", "");
+ byte[] validationKeyBytes;
+ try {
+ validationKeyBytes = Files.readAllBytes(Paths.get(Objects.requireNonNull(publicKeyUrl)));
+ X509EncodedKeySpec spec = new X509EncodedKeySpec(validationKeyBytes);
+ KeyFactory kf = KeyFactory.getInstance("RSA");
+ Key validationKey = kf.generatePublic(spec);
+ JwtParser signedParser = Jwts.parserBuilder().setSigningKey(validationKey).build();
+ Jwt<?, Claims> signJwt = signedParser.parseClaimsJws(token);
+ sub = signJwt.getBody().get("sub", String.class);
+ if (!sub.contains(aclProperties.getExtendedField("group").toString()) && !sub.contains("pulsar-admin")) {
+ throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access eventMesh:"
+ + aclProperties.getTopic());
+ }
+ } catch (IOException e) {
+ throw new AclException("public key read error!", e);
+ } catch (NoSuchAlgorithmException e) {
+ throw new AclException("no such RSA algorithm!", e);
+ } catch (InvalidKeySpecException e) {
+ throw new AclException("invalid public key spec!", e);
+ } catch (JwtException e) {
+ throw new AclException("invalid token!", e);
+ }
+
+ }
+
+ public static String getPublicKeyUrl() {
+ String publicKeyUrl = null;
+ for (String key : ConfigurationContextUtil.KEYS) {
+ CommonConfiguration commonConfiguration = ConfigurationContextUtil.get(key);
+ if (null == commonConfiguration) {
+ continue;
+ }
+ if (StringUtils.isBlank(commonConfiguration.getEventMeshSecurityPublickey())) {
+ throw new AclException("publicKeyUrl cannot be null");
+ }
+ publicKeyUrl = commonConfiguration.getEventMeshSecurityPublickey();
+ }
+ return publicKeyUrl;
+
}
Review Comment:
Redundant blank line at the end of the method.
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +75,58 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
if (!(topics instanceof Set)) {
Review Comment:
I think this blank line is better not removed to maintain the formatting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1453300440
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -51,41 +52,9 @@ public static void authTokenByPublicKey(AclProperties aclProperties) {
throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access the topic:"
+ aclProperties.getTopic());
}
- String publicKeyUrl = null;
token = token.replace("Bearer ", "");
Review Comment:
Redundant `replace()` here.
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +75,58 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
if (!(topics instanceof Set)) {
return false;
}
Set<String> groupTopics = TypeUtils.castSet(topics, String.class);
return groupTopics.contains(topic);
+
+ }
+
+ public static void validateToken(String token, String publicKeyUrl, AclProperties aclProperties) {
+ String sub;
+ token = token.replace("Bearer ", "");
+ byte[] validationKeyBytes;
+ try {
+ validationKeyBytes = Files.readAllBytes(Paths.get(Objects.requireNonNull(publicKeyUrl)));
+ X509EncodedKeySpec spec = new X509EncodedKeySpec(validationKeyBytes);
+ KeyFactory kf = KeyFactory.getInstance("RSA");
+ Key validationKey = kf.generatePublic(spec);
+ JwtParser signedParser = Jwts.parserBuilder().setSigningKey(validationKey).build();
+ Jwt<?, Claims> signJwt = signedParser.parseClaimsJws(token);
+ sub = signJwt.getBody().get("sub", String.class);
+ if (!sub.contains(aclProperties.getExtendedField("group").toString()) && !sub.contains("pulsar-admin")) {
+ throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access eventMesh:"
+ + aclProperties.getTopic());
+ }
+ } catch (IOException e) {
+ throw new AclException("public key read error!", e);
+ } catch (NoSuchAlgorithmException e) {
+ throw new AclException("no such RSA algorithm!", e);
+ } catch (InvalidKeySpecException e) {
+ throw new AclException("invalid public key spec!", e);
+ } catch (JwtException e) {
+ throw new AclException("invalid token!", e);
+ }
+
+ }
+
+ public static String getPublicKeyUrl() {
+ String publicKeyUrl = null;
+ for (String key : ConfigurationContextUtil.KEYS) {
+ CommonConfiguration commonConfiguration = ConfigurationContextUtil.get(key);
+ if (null == commonConfiguration) {
+ continue;
+ }
+ if (StringUtils.isBlank(commonConfiguration.getEventMeshSecurityPublickey())) {
+ throw new AclException("publicKeyUrl cannot be null");
+ }
+ publicKeyUrl = commonConfiguration.getEventMeshSecurityPublickey();
+ }
+ return publicKeyUrl;
+
}
Review Comment:
Redundant line at the end of the method.
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -94,40 +63,8 @@ public static void authTokenByPublicKey(AclProperties aclProperties) {
public static void helloTaskAuthTokenByPublicKey(AclProperties aclProperties) {
String token = aclProperties.getToken();
if (StringUtils.isNotBlank(token)) {
- String publicKeyUrl = null;
- token = token.replace("Bearer ", "");
- for (String key : ConfigurationContextUtil.KEYS) {
- CommonConfiguration commonConfiguration = ConfigurationContextUtil.get(key);
- if (null == commonConfiguration) {
- continue;
- }
- if (StringUtils.isBlank(commonConfiguration.getEventMeshSecurityPublickey())) {
- throw new AclException("publicKeyUrl cannot be null");
- }
- publicKeyUrl = commonConfiguration.getEventMeshSecurityPublickey();
- }
- byte[] validationKeyBytes = new byte[0];
- try {
- validationKeyBytes = Files.readAllBytes(Paths.get(publicKeyUrl));
- X509EncodedKeySpec spec = new X509EncodedKeySpec(validationKeyBytes);
- KeyFactory kf = KeyFactory.getInstance("RSA");
- Key validationKey = kf.generatePublic(spec);
- JwtParser signedParser = Jwts.parserBuilder().setSigningKey(validationKey).build();
- Jwt<?, Claims> signJwt = signedParser.parseClaimsJws(token);
- String sub = signJwt.getBody().get("sub", String.class);
- if (!sub.contains(aclProperties.getExtendedField("group").toString()) && !sub.contains("pulsar-admin")) {
- throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access eventMesh:"
- + aclProperties.getTopic());
- }
- } catch (IOException e) {
- throw new AclException("public key read error!", e);
- } catch (NoSuchAlgorithmException e) {
- throw new AclException("no such RSA algorithm!", e);
- } catch (InvalidKeySpecException e) {
- throw new AclException("invalid public key spec!", e);
- } catch (JwtException e) {
- throw new AclException("invalid token!", e);
- }
+ String publicKeyUrl = getPublicKeyUrl();
+ validateToken(token, publicKeyUrl, aclProperties);
Review Comment:
You can pass `getPublicKeyUrl()` as a parameter to `validateToken()`.
```suggestion
validateToken(token, getPublicKeyUrl(), aclProperties);
```
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +75,58 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
if (!(topics instanceof Set)) {
Review Comment:
I think this line is better not removed to maintain the formatting.
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +75,58 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
if (!(topics instanceof Set)) {
return false;
}
Set<String> groupTopics = TypeUtils.castSet(topics, String.class);
return groupTopics.contains(topic);
+
+ }
+
+ public static void validateToken(String token, String publicKeyUrl, AclProperties aclProperties) {
+ String sub;
+ token = token.replace("Bearer ", "");
+ byte[] validationKeyBytes;
+ try {
+ validationKeyBytes = Files.readAllBytes(Paths.get(Objects.requireNonNull(publicKeyUrl)));
+ X509EncodedKeySpec spec = new X509EncodedKeySpec(validationKeyBytes);
+ KeyFactory kf = KeyFactory.getInstance("RSA");
+ Key validationKey = kf.generatePublic(spec);
+ JwtParser signedParser = Jwts.parserBuilder().setSigningKey(validationKey).build();
+ Jwt<?, Claims> signJwt = signedParser.parseClaimsJws(token);
+ sub = signJwt.getBody().get("sub", String.class);
+ if (!sub.contains(aclProperties.getExtendedField("group").toString()) && !sub.contains("pulsar-admin")) {
+ throw new AclException("group:" + aclProperties.getExtendedField("group ") + " has no auth to access eventMesh:"
+ + aclProperties.getTopic());
+ }
+ } catch (IOException e) {
+ throw new AclException("public key read error!", e);
+ } catch (NoSuchAlgorithmException e) {
+ throw new AclException("no such RSA algorithm!", e);
+ } catch (InvalidKeySpecException e) {
+ throw new AclException("invalid public key spec!", e);
+ } catch (JwtException e) {
+ throw new AclException("invalid token!", e);
+ }
+
+ }
+
+ public static String getPublicKeyUrl() {
+ String publicKeyUrl = null;
+ for (String key : ConfigurationContextUtil.KEYS) {
Review Comment:
`getPublicKeyUrl()` and `validateToken()` are private methods. Besides, `getPublicKeyUrl()` is called in front of `validateToken()` and its method body should be placed in front of `validateToken()` too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1456576900
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -146,6 +81,50 @@ public static boolean authAccess(AclProperties aclProperties) {
Set<String> groupTopics = TypeUtils.castSet(topics, String.class);
return groupTopics.contains(topic);
+
+ }
Review Comment:
gotcha
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1448377797
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +74,44 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
+
if (!(topics instanceof Set)) {
Review Comment:
Please remove these redundant whitespaces here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1891039678
## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/3644?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`5850d54`)](https://app.codecov.io/gh/apache/eventmesh/commit/5850d5414b6ae21646cd9194dc4e93fb5d7dedf1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 17.55% compared to head [(`12b521c`)](https://app.codecov.io/gh/apache/eventmesh/pull/3644?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 17.59%.
> Report is 5 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3644 +/- ##
============================================
+ Coverage 17.55% 17.59% +0.04%
+ Complexity 1778 1774 -4
============================================
Files 797 797
Lines 29871 29786 -85
Branches 2581 2573 -8
============================================
- Hits 5243 5242 -1
+ Misses 24145 24063 -82
+ Partials 483 481 -2
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/eventmesh/pull/3644?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1450971400
##########
eventmesh-security-plugin/eventmesh-security-auth-token/src/main/java/org/apache/eventmesh/auth/token/impl/auth/AuthTokenUtils.java:
##########
@@ -138,14 +74,44 @@ public static boolean authAccess(AclProperties aclProperties) {
String topic = aclProperties.getTopic();
Object topics = aclProperties.getExtendedField("topics");
-
+
if (!(topics instanceof Set)) {
Review Comment:
Thanks for your review, will do :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-2071352057
@kyooosukedn Sorry for the delay. May you please resolve conflicts? 😊
@Alonexc May you please help merge this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] kyooosukedn commented on pull request #3644: [ISSUE #3515] Do some code optimization[AuthTokenUtils]
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1522422599
Hi @Alonexc Is there something else that needs to be done?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] kyooosukedn commented on a diff in pull request #3644: [ISSUE #3515] Do some code optimization[AuthTokenUtils]
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on code in PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#discussion_r1164588459
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/HTTPClientHandler.java:
##########
@@ -139,7 +139,7 @@ void list(HttpExchange httpExchange) throws IOException {
});
String result = JsonUtils.toJSONString(getClientResponseList);
- httpExchange.sendResponseHeaders(200, result.getBytes().length);
+ httpExchange.sendResponseHeaders(200, result.getBytes(Constants.DEFAULT_CHARSET).length);
Review Comment:
Thanks :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] Alonexc commented on pull request #3644: [ISSUE #3515] Do some code optimization[AuthTokenUtils]
Posted by "Alonexc (via GitHub)" <gi...@apache.org>.
Alonexc commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-1504849470
@kyooosukedn
Download this plugin：
Import the checkstyle of the local project and check the box.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3515] Do some code optimization[AuthTokenUtils] (eventmesh)
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on PR #3644:
URL: https://github.com/apache/eventmesh/pull/3644#issuecomment-2081450846
@Alonexc @Pil0tXia kindly review this, thanks :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org