Skip to content

Commit d8a6ca7

Browse files
dnrrodrigozhou
authored andcommitted
Run no-op claim mapper even without auth info (#4197)
1 parent 3ce6308 commit d8a6ca7

File tree

4 files changed

+76
-1
lines changed

4 files changed

+76
-1
lines changed

common/authorization/claim_mapper.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,18 @@ type ClaimMapper interface {
5757

5858
// @@@SNIPEND
5959

60+
// Normally, GetClaims will never be called without either an auth token or TLS metadata set in
61+
// AuthInfo. However, if you want your ClaimMapper to be called in all cases, you can implement
62+
// this additional interface and return false.
63+
type ClaimMapperWithAuthInfoRequired interface {
64+
AuthInfoRequired() bool
65+
}
66+
6067
// No-op claim mapper that gives system level admin permission to everybody
6168
type noopClaimMapper struct{}
6269

6370
var _ ClaimMapper = (*noopClaimMapper)(nil)
71+
var _ ClaimMapperWithAuthInfoRequired = (*noopClaimMapper)(nil)
6472

6573
func NewNoopClaimMapper() ClaimMapper {
6674
return &noopClaimMapper{}
@@ -70,6 +78,11 @@ func (*noopClaimMapper) GetClaims(_ *AuthInfo) (*Claims, error) {
7078
return &Claims{System: RoleAdmin}, nil
7179
}
7280

81+
// This implementation can run even without auth info.
82+
func (*noopClaimMapper) AuthInfoRequired() bool {
83+
return false
84+
}
85+
7386
func GetClaimMapperFromConfig(config *config.Authorization, logger log.Logger) (ClaimMapper, error) {
7487

7588
switch strings.ToLower(config.ClaimMapper) {

common/authorization/claim_mapper_mock.go

Lines changed: 37 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

common/authorization/interceptor.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,13 @@ func (a *interceptor) Interceptor(
8989
tlsSubject = &clientCert.Subject
9090
}
9191

92+
authInfoRequired := true
93+
if cm, ok := a.claimMapper.(ClaimMapperWithAuthInfoRequired); ok {
94+
authInfoRequired = cm.AuthInfoRequired()
95+
}
96+
9297
// Add auth info to context only if there's some auth info
93-
if tlsSubject != nil || len(authHeaders) > 0 {
98+
if tlsSubject != nil || len(authHeaders) > 0 || !authInfoRequired {
9499
var authHeader string
95100
var authExtraHeader string
96101
var audience string

common/authorization/interceptor_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,23 @@ func (s *authorizerInterceptorSuite) TestAuthorizationFailed() {
133133
s.Nil(res)
134134
s.Error(err)
135135
}
136+
137+
func (s *authorizerInterceptorSuite) TestNoopClaimMapperWithoutTLS() {
138+
admin := &Claims{System: RoleAdmin}
139+
s.mockAuthorizer.EXPECT().Authorize(gomock.Any(), admin, describeNamespaceTarget).
140+
DoAndReturn(func(ctx context.Context, caller *Claims, target *CallTarget) (Result, error) {
141+
// check that claims are present in ctx also
142+
s.Equal(admin, ctx.Value(MappedClaims))
143+
144+
return Result{Decision: DecisionAllow}, nil
145+
})
146+
147+
interceptor := NewAuthorizationInterceptor(
148+
NewNoopClaimMapper(),
149+
s.mockAuthorizer,
150+
s.mockMetricsHandler,
151+
log.NewNoopLogger(),
152+
nil)
153+
_, err := interceptor(ctx, describeNamespaceRequest, describeNamespaceInfo, s.handler)
154+
s.NoError(err)
155+
}

0 commit comments

Comments
 (0)