Skip to content

[dubbo-8601] Support Dynamic Token And add BroadCastTest#9520

Closed
kaori-seasons wants to merge 2 commits intoapache:3.0from
kaori-seasons:dubbo-8601
Closed

[dubbo-8601] Support Dynamic Token And add BroadCastTest#9520
kaori-seasons wants to merge 2 commits intoapache:3.0from
kaori-seasons:dubbo-8601

Conversation

@kaori-seasons
Copy link

What is the purpose of the change

When the user uses the broadcast address, an exception is thrown after the dynamic token is updated. And add related unit tests

Related to dubbo-8601

Brief changelog

Verifying this change

BroacastCluster2Test
TokenFilterTest#testInvokeWithoutDynamicToken

Checklist

  • Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Add some description to dubbo-website project if you are requesting to add a feature.
  • GitHub Actions works fine on your own branch.
  • If this contribution is large, please follow the Software Donation Guide.

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2022

Codecov Report

Merging #9520 (6f6700f) into 3.0 (d44ca1f) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.0    #9520      +/-   ##
============================================
- Coverage     65.67%   65.64%   -0.03%     
  Complexity      297      297              
============================================
  Files          1180     1180              
  Lines         51584    51585       +1     
  Branches       7796     7797       +1     
============================================
- Hits          33877    33865      -12     
- Misses        14046    14054       +8     
- Partials       3661     3666       +5     
Impacted Files Coverage Δ
...he/dubbo/rpc/cluster/support/BroadcastCluster.java 0.00% <ø> (ø)
.../java/org/apache/dubbo/rpc/filter/TokenFilter.java 83.33% <100.00%> (+1.51%) ⬆️
...nt/metadata/ServiceInstanceHostPortCustomizer.java 65.78% <0.00%> (-21.06%) ⬇️
.../common/threadpool/serial/SerializingExecutor.java 70.37% <0.00%> (-7.41%) ⬇️
.../apache/dubbo/remoting/transport/AbstractPeer.java 67.39% <0.00%> (-4.35%) ⬇️
...apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java 61.51% <0.00%> (-3.29%) ⬇️
...bo/rpc/protocol/dubbo/DecodeableRpcInvocation.java 71.42% <0.00%> (-2.68%) ⬇️
...bbo/common/resource/GlobalResourcesRepository.java 70.90% <0.00%> (-1.82%) ⬇️
...ubbo/registry/client/AbstractServiceDiscovery.java 83.47% <0.00%> (-1.74%) ⬇️
...ubbo/config/deploy/DefaultApplicationDeployer.java 72.69% <0.00%> (-0.41%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d44ca1f...6f6700f. Read the comment docs.

@Test
public void testFailoverInvokerSelect(){
given(dic.list(invocation)).willReturn(Arrays.asList(invoker1, invoker2, invoker3, invoker4));
//取得当前调用链的所有invoker,逐个判断调用是否成功
Copy link
Member

Choose a reason for hiding this comment

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

Please describe in English.

}


//设置一个注册中心地址,便于consumer本地进行远程调用测试
Copy link
Member

Choose a reason for hiding this comment

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

Please describe in English.

throw new RpcException("Invalid token! Forbid invoke remote service " + serviceType + " method " + inv.getMethodName() +
"() from consumer " + RpcContext.getServiceContext().getRemoteHost() + " to provider " +
RpcContext.getServiceContext().getLocalHost()+ ", consumer incorrect token is " + remoteToken);
if (ConfigUtils.isDefault(token)){
Copy link
Member

Choose a reason for hiding this comment

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

This means that as long as the token=default is configured, the request can be executed arbitrarily, right?

Copy link
Author

Choose a reason for hiding this comment

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

yes as described in the question

if (ConfigUtils.isDefault(token)){
inv.setAttachment(TOKEN_KEY,token);
}
if (!token.equals(remoteToken) && !ConfigUtils.isDefault(token)) {
Copy link
Member

Choose a reason for hiding this comment

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

This may cause token downgrade attack. If provider use default token, TokenFilter in provider side may not work.

Copy link
Author

Choose a reason for hiding this comment

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

This may cause token downgrade attack. If provider use default token, TokenFilter in provider side may not work.

@AlbumenJ You mean that when the token is passed in the thread context it should fake a value, or expose a configuration. Is it used to guarantee idempotency when downgrading?

@AlbumenJ AlbumenJ closed this Oct 22, 2022
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