-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[Fix] add resource full name check #15757
[Fix] add resource full name check #15757
Conversation
c002e20
to
8aaafde
Compare
a5a8c2d
to
b87241a
Compare
if (StringUtils.isEmpty(resTenantCode)) { | ||
return true; | ||
} | ||
return resTenantCode.equals(userTenantCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the resTenantCode be empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when resource create or upload, the resource tenant code is empty. But the better way is to remove checking if not need. I will update it.
Line 130 in dcc9d64
if (!isUserTenantValid(isAdmin(loginUser), tenantCode, "")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource tenant code will be empty when query resources list, so I keep the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #15757 +/- ##
============================================
- Coverage 39.10% 39.09% -0.02%
+ Complexity 4855 4850 -5
============================================
Files 1316 1316
Lines 44962 44936 -26
Branches 4808 4797 -11
============================================
- Hits 17583 17567 -16
Misses 25478 25478
+ Partials 1901 1891 -10 ☔ View full report in Codecov by Sentry. |
…/dolphinscheduler into fix/resource_full_name_check
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Purpose of the pull request
Add the resource full name check during resource operations to avoid illegal paths.
Brief change log
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java
Verify this pull request