alts: add client authorization util library#6529
Conversation
| if (altsCtx == null) { | ||
| return Status.NOT_FOUND.withDescription("Peer ALTS AuthContext not found"); | ||
| } | ||
| for (String serviceAccount : expectedServiceAccounts) { |
There was a problem hiding this comment.
if (expectedServiceAccounts.contains(altsCtx.getPeerServiceAccount())) {
return Status.OK;
}| public static Status clientAuthorizationCheck( | ||
| ServerCall<?, ?> call, ImmutableList<String> expectedServiceAccounts) { | ||
| AltsAuthContext altsCtx = | ||
| (AltsAuthContext) call.getAttributes().get(AltsProtocolNegotiator.AUTH_CONTEXT_KEY); |
There was a problem hiding this comment.
Why isn't AUTH_CONTEXT_KEY a Attributes.Key<AltsAuthContext>?
There was a problem hiding this comment.
This because extractPeerObject() returns an Object, rather than AltAuthContext. Internally, we have a different AuthContext class. We need to use share the same API for returning the peer object. Let me know if you have better ideas.
|
|
||
| @Override | ||
| public void request(int numMessages) { | ||
| throw new AssertionError("Should not be called"); |
There was a problem hiding this comment.
A trick you can do for these is to extend ForwardingServerCall and then implement delegate() to throw like you're doing here. This is fine, though.
There was a problem hiding this comment.
Ack on the trick. Let's keep the current FakeServerCall if you don't mind.
jiangtaoli2016
force-pushed
the
authz
branch
from
3288402 to
044a53d
Compare
ejona86
left a comment
ejona86
left a comment
There was a problem hiding this comment.
It's a bit weird to require a List when a Set would work (which Collection would allow), but this seems fine.
|
Changed to Collection instead of List. |
3 participants
Add a simple authorization utility library.
@ejona86