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.

Sign up for GitHub

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

Jump to bottom

overrideAuthority does not impact authority for JWT in jwt_token_creds #2682

Closed
ejona86 opened this issue Feb 1, 2017 · 2 comments · Fixed by #2956
Closed

overrideAuthority does not impact authority for JWT in jwt_token_creds #2682

ejona86 opened this issue Feb 1, 2017 · 2 comments · Fixed by #2956
Assignees
@ejona86
Labels
bug
Milestone
1.4 Early

Comments

@ejona86
Copy link
Member

ejona86 commented Feb 1, 2017

65e4d9f#diff-850c920bd69ac031d7e98cab9459dec8 caused grpc/grpc#9497
@ejona86 ejona86 added the bug label Feb 1, 2017
@ejona86 ejona86 added this to the 1.2 milestone Feb 1, 2017
@ejona86 ejona86 self-assigned this Feb 1, 2017
ejona86 added a commit to ejona86/grpc-java that referenced this issue Feb 1, 2017
@ejona86
interop-testing: overrideAuthority breaks JWT
1d8d7d7 
Commit 65e4d9f caused grpc/grpc#9497 and grpc#2680 (dup). It is believed to
be because the JWT does not see the authority passed to
overrideAuthority. So the changes to interop-testing client are
temporarily reverted here. Note that this breaks GRPC_PROXY_EXP testing,
so the incompatibility needs to be resolved.

Solving grpc#2682 will allow reverting this change.
ejona86 added a commit to ejona86/grpc-java that referenced this issue Feb 1, 2017
@ejona86
interop-testing: overrideAuthority breaks JWT
3b9ac7a 
Commit 65e4d9f broke the jwt_token_creds. It is believed to be because
the JWT does not see the authority passed to overrideAuthority. So the
changes to interop-testing client are temporarily reverted here. Note
that this breaks GRPC_PROXY_EXP testing, so the incompatibility needs to
be resolved.

Solving grpc#2682 will allow reverting this change.

Fixes grpc/grpc#9497
Fixes grpc#2680
@ejona86 ejona86 mentioned this issue Feb 1, 2017
Merged
ejona86 added a commit that referenced this issue Feb 1, 2017
@ejona86
interop-testing: overrideAuthority breaks JWT
57b9105 
Commit 65e4d9f broke the jwt_token_creds. It is believed to be because
the JWT does not see the authority passed to overrideAuthority. So the
changes to interop-testing client are temporarily reverted here. Note
that this breaks GRPC_PROXY_EXP testing, so the incompatibility needs to
be resolved.

Solving #2682 will allow reverting this change.

Fixes grpc/grpc#9497
Fixes #2680
@ejona86
Copy link
Member Author

ejona86 commented Feb 1, 2017

Root problem:

  1. overrideAuthority is implemented as a TransportFactory wrapper (AuthorityOverridingTransportFactory)
  2. CallCredentials handling is implemented as a TransportFactory wrapper (CallCredentialsApplyingTransportFactory)
  3. 2 wraps 1, so it doesn't see the overridden authority

@ejona86
Copy link
Member Author

ejona86 commented Feb 1, 2017

I believe this is broken independent of whether using Channel.authority() or the CallCredentials. The test was using CallCredentials, but that indirectly uses Channel.authority().

@carl-mastrangelo carl-mastrangelo modified the milestones: Next, 1.2 Mar 15, 2017
@zpencer zpencer mentioned this issue Apr 27, 2017
Merged
zpencer added a commit that referenced this issue Apr 27, 2017
@zpencer
Channel.authority() should return the value of overrideAuthority (#2956)
a317912 
AbstractManagedChannelImplBuilder accepts an overrideAuthority parameter, but this value is not hooked up to the name resolver object. Ultimately, Channel.authority consults with the NameResolver, so the overrideAuthority should be hooked into the NameResolverFactory, while all other functionality should be preserved.

Also, add unit tests for all the variants of OkHttpChannelBuilder and NettyChannelBuilder constructors, namely to test the slightly different NettyChannelBuilder(SocketAddress) code path.

Fixes #2682
@ejona86 ejona86 modified the milestones: 1.4 Early, Next Jun 1, 2017
ejona86 added a commit to ejona86/grpc-java that referenced this issue Jul 19, 2017
@ejona86
Revert "interop-testing: overrideAuthority breaks JWT"
c71496f 
This reverts commit 57b9105.

Issue grpc#2682 is fixed, so we can revert the commit as planned. This
re-applies a previously-reverted modernization.
@ejona86 ejona86 mentioned this issue Jul 19, 2017
Merged
ejona86 added a commit that referenced this issue Jul 19, 2017
@ejona86
Revert "interop-testing: overrideAuthority breaks JWT"
924b0b2 
This reverts commit 57b9105.

Issue #2682 is fixed, so we can revert the commit as planned. This
re-applies a previously-reverted modernization.
@lock lock bot locked as resolved and limited conversation to collaborators Sep 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ejona86 ejona86

Labels
bug
Projects
None yet
Milestone
1.4 Early
Development

Successfully merging a pull request may close this issue.

Channel.authority() should return the value of overrideAuthority zpencer/grpc-java
2 participants
@ejona86 @carl-mastrangelo