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

[#1099] adjusting new logic and configuration for security #1104

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

chengyouling
Copy link
Collaborator

@chengyouling chengyouling commented Nov 17, 2023

1、策略uri配置/*时,客户端对应方法类型的请求都通过;
2、开启安全策略校验,但是没有设置策略或者安全策略白名单为空时,打印告警日志(发送告警信息),宽容模式请求通过,强制模式,请求拦截;
3、header中获取token的key调整为支持配置,默认为X-SM-Token。

@codecov-commenter
Copy link

codecov-commenter commented Nov 17, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (69f0f07) 1.90% compared to head (1f19c8a) 1.80%.
Report is 3 commits behind head on 2021.0.x.

❗ Current head 1f19c8a differs from pull request most recent head dd9a3bf. Consider uploading reports for the commit dd9a3bf to get more accurate results

Files Patch % Lines
...rnance/authentication/RSAProviderTokenManager.java 0.00% 4 Missing ⚠️
.../governance/authentication/RSATokenCheckUtils.java 0.00% 1 Missing ⚠️
...securityPolicy/SecurityPolicyAccessController.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             2021.0.x   #1104      +/-   ##
=============================================
- Coverage        1.90%   1.80%   -0.10%     
+ Complexity         29      28       -1     
=============================================
  Files             209     209              
  Lines            4094    4091       -3     
  Branches          360     358       -2     
=============================================
- Hits               78      74       -4     
- Misses           4006    4007       +1     
  Partials           10      10              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chengyouling chengyouling self-assigned this Nov 20, 2023
AuthenticationAdapter authenticationAdapter) throws Exception {
String token = request.getHeader(Const.AUTH_TOKEN);
AuthenticationAdapter authenticationAdapter, String headerTokenKey) throws Exception {
String token = request.getHeader(headerTokenKey);
if (StringUtils.isEmpty(token)) {
token = InvocationContextHolder.getOrCreateInvocationContext().getContext(Const.AUTH_TOKEN);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't fallback to X-Auth-Token, throws error here directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

microservice call microservice is X-Auth-Token, no affected with header token.

@@ -54,15 +54,12 @@ public boolean isAllowed(AuthRequestExtractor extractor) throws Exception {

private boolean checkDeny(String serviceName, AuthRequestExtractor extractor) {
if (securityPolicyProperties.matchDeny(serviceName, extractor.uri(), extractor.method())) {
// both permissive and enforcing model need print logs(send alarm info).
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

logging level should be WARN if denied in permissive

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -54,15 +54,12 @@ public boolean isAllowed(AuthRequestExtractor extractor) throws Exception {

private boolean checkDeny(String serviceName, AuthRequestExtractor extractor) {
if (securityPolicyProperties.matchDeny(serviceName, extractor.uri(), extractor.method())) {
// both permissive and enforcing model need print logs(send alarm info).
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change "unauthorized request" to "[autoauthz] request denied. consumer={} ....."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

use security team context

@@ -72,15 +69,12 @@ private boolean checkAllowAndDeny(String serviceName, AuthRequestExtractor extra
if (securityPolicyProperties.matchAllow(serviceName, extractor.uri(), extractor.method())) {
return !checkDeny(serviceName, extractor);
} else {
// both permissive and enforcing model need print logs(send alarm info).
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above. Logging level shoud be WARN and message should be "request denied ...."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@dantesun dantesun merged commit c0c5746 into 2021.0.x Nov 22, 2023
1 check passed
@chengyouling chengyouling deleted the 2021.0.x_secNew branch November 27, 2023 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants