Skip to content

Commit

Permalink
fix: impresonation regression for service accounts (#405)
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 authored Mar 5, 2024
1 parent 0b04e1b commit 16deb06
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
5 changes: 4 additions & 1 deletion internal/request/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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"
)

Expand Down Expand Up @@ -106,9 +107,11 @@ 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, serviceaccount.AllServiceAccountsGroup)
groups = append(groups, user.AllAuthenticated)
}
}()
}
Expand Down
36 changes: 36 additions & 0 deletions internal/request/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ 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"

"github.com/projectcapsule/capsule-proxy/internal/request"
Expand Down Expand Up @@ -101,6 +103,38 @@ 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 + testServiceAccountSuffix},
},
TLS: &tls.ConnectionState{
PeerCertificates: []*x509.Certificate{
{
Subject: pkix.Name{
CommonName: serviceaccount.ServiceAccountUsernamePrefix + testServiceAccountSuffix,
},
},
},
},
},
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 + testServiceAccountSuffix,
wantGroups: []string{fmt.Sprintf("%s%s", serviceaccount.ServiceAccountGroupPrefix, "ns"), serviceaccount.AllServiceAccountsGroup, user.AllAuthenticated},
wantErr: false,
},
{
name: "Bearer",
fields: fields{
Expand Down Expand Up @@ -187,3 +221,5 @@ func Test_http_GetUserAndGroups(t *testing.T) {
})
}
}

const testServiceAccountSuffix = "ns:account"

0 comments on commit 16deb06

Please sign in to comment.