Skip to content

Commit

Permalink
Fix impresonation regression for service accounts
Browse files Browse the repository at this point in the history
Signed-off-by: Radek Gruchalski <radek.gruchalski@klarrio.com>
  • Loading branch information
rgruchalski-klarrio committed Mar 2, 2024
1 parent 77b11ad commit 8ad49e4
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
13 changes: 12 additions & 1 deletion internal/request/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,18 @@ import (
authenticationv1 "k8s.io/api/authentication/v1"
authorizationv1 "k8s.io/api/authorization/v1"
"k8s.io/apiserver/pkg/authentication/serviceaccount"
"k8s.io/apiserver/pkg/authentication/user"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var defaultServiceAccountGroups = []string{
serviceaccount.AllServiceAccountsGroup,
user.AllAuthenticated}

func GetDefaultServiceAccountGroups() []string {
return defaultServiceAccountGroups
}

type http struct {
*h.Request
authTypes []AuthType
Expand All @@ -34,6 +43,7 @@ func (h http) GetHTTPRequest() *h.Request {

//nolint:funlen
func (h http) GetUserAndGroups() (username string, groups []string, err error) {

for _, fn := range h.authenticationFns() {
// User authentication data is extracted according to the preferred order:
// in case of first match blocking the iteration
Expand Down Expand Up @@ -106,9 +116,10 @@ func (h http) GetUserAndGroups() (username string, groups []string, err error) {
// by appending the expected service account groups:
// - system:serviceaccounts:<namespace>
// - system:serviceaccounts
// - system:authenticated
if namespace, _, err := serviceaccount.SplitUsername(username); err == nil {
groups = append(groups, serviceaccount.AllServiceAccountsGroup)
groups = append(groups, fmt.Sprintf("%s%s", serviceaccount.ServiceAccountGroupPrefix, namespace))
groups = append(groups, defaultServiceAccountGroups...)
}
}()
}
Expand Down
33 changes: 33 additions & 0 deletions internal/request/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
authorizationv1 "k8s.io/api/authorization/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

"k8s.io/apiserver/pkg/authentication/serviceaccount"

"github.com/projectcapsule/capsule-proxy/internal/request"
)

Expand Down Expand Up @@ -101,6 +103,37 @@ func Test_http_GetUserAndGroups(t *testing.T) {
wantGroups: []string{"ImpersonatedGroup"},
wantErr: false,
},
{
name: "Certificate-ServiceAccount",
fields: fields{
Request: &http.Request{
Header: map[string][]string{
authenticationv1.ImpersonateUserHeader: {serviceaccount.ServiceAccountUsernamePrefix + "ns:account"},
},
TLS: &tls.ConnectionState{
PeerCertificates: []*x509.Certificate{
{
Subject: pkix.Name{
CommonName: serviceaccount.ServiceAccountUsernamePrefix + "ns:account",
},
},
},
},
},
authTypes: []request.AuthType{
request.BearerToken,
request.TLSCertificate,
},
client: testClient(func(ctx context.Context, obj client.Object) error {
ac := obj.(*authorizationv1.SubjectAccessReview)
ac.Status.Allowed = true
return nil
}),
},
wantUsername: serviceaccount.ServiceAccountUsernamePrefix + "ns:account",
wantGroups: append([]string{fmt.Sprintf("%s%s", serviceaccount.ServiceAccountGroupPrefix, "ns")}, request.GetDefaultServiceAccountGroups()...),
wantErr: false,
},
{
name: "Bearer",
fields: fields{
Expand Down

0 comments on commit 8ad49e4

Please sign in to comment.