Skip to content

Commit

Permalink
SelfSubjectAccessReview does not authorize with api groups
Browse files Browse the repository at this point in the history
Also, version/kind defaulting was not working for these resources. Add
tests
  • Loading branch information
smarterclayton committed Apr 11, 2017
1 parent e01d36c commit 11e1349
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
18 changes: 13 additions & 5 deletions pkg/authorization/authorizer/personal_subjectaccessreview.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"strings"

kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/apiserver/request"
"k8s.io/kubernetes/pkg/runtime"

authorizationapi "github.com/openshift/origin/pkg/authorization/api"
)
Expand All @@ -35,7 +35,7 @@ func (a *personalSARRequestInfoResolver) NewRequestInfo(req *http.Request) (*req
case !requestInfo.IsResourceRequest:
return requestInfo, nil

case len(requestInfo.APIGroup) != 0:
case len(requestInfo.APIGroup) != 0 && requestInfo.APIGroup != "authorization.openshift.io":
return requestInfo, nil

case len(requestInfo.Subresource) != 0:
Expand All @@ -49,7 +49,7 @@ func (a *personalSARRequestInfoResolver) NewRequestInfo(req *http.Request) (*req
}

// at this point we're probably running a SAR or LSAR. Decode the body and check. This is expensive.
isSelfSAR, err := isPersonalAccessReviewFromRequest(req)
isSelfSAR, err := isPersonalAccessReviewFromRequest(req, requestInfo)
if err != nil {
return nil, err
}
Expand All @@ -65,7 +65,7 @@ func (a *personalSARRequestInfoResolver) NewRequestInfo(req *http.Request) (*req
}

// isPersonalAccessReviewFromRequest this variant handles the case where we have an httpRequest
func isPersonalAccessReviewFromRequest(req *http.Request) (bool, error) {
func isPersonalAccessReviewFromRequest(req *http.Request, requestInfo *request.RequestInfo) (bool, error) {
// TODO once we're integrated with the api installer, we should have direct access to the deserialized content
// for now, this only happens on subjectaccessreviews with a personal check, pay the double retrieve and decode cost
body, err := ioutil.ReadAll(req.Body)
Expand All @@ -74,7 +74,15 @@ func isPersonalAccessReviewFromRequest(req *http.Request) (bool, error) {
}
req.Body = ioutil.NopCloser(bytes.NewBuffer(body))

obj, err := runtime.Decode(kapi.Codecs.UniversalDecoder(), body)
defaultGVK := unversioned.GroupVersionKind{Version: requestInfo.APIVersion, Group: requestInfo.APIGroup}
switch strings.ToLower(requestInfo.Resource) {
case "subjectaccessreviews":
defaultGVK.Kind = "SubjectAccessReview"
case "localsubjectaccessreviews":
defaultGVK.Kind = "LocalSubjectAccessReview"
}

obj, _, err := kapi.Codecs.UniversalDecoder().Decode(body, &defaultGVK, nil)
if err != nil {
return false, err
}
Expand Down
4 changes: 4 additions & 0 deletions test/cmd/authentication.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ os::cmd::expect_success_and_text "oc policy can-i --list --token='${allescalatin

accesstoken="$(oc process -f "${OS_ROOT}/test/testdata/authentication/scoped-token-template.yaml" TOKEN_PREFIX=access SCOPE=user:check-access USER_NAME="${username}" USER_UID="${useruid}" | oc create -f - -o name | awk -F/ '{print $2}')"
os::cmd::expect_success_and_text "curl -k -XPOST -H 'Content-Type: application/json' -H 'Authorization: Bearer ${accesstoken}' '${API_SCHEME}://${API_HOST}:${API_PORT}/oapi/v1/namespaces/${project}/localsubjectaccessreviews' -d @${OS_ROOT}/test/testdata/authentication/localsubjectaccessreview.json" '"kind": "SubjectAccessReviewResponse"'
os::cmd::expect_success_and_text "curl -k -XPOST -H 'Content-Type: application/json' -H 'Authorization: Bearer ${accesstoken}' '${API_SCHEME}://${API_HOST}:${API_PORT}/apis/authorization.openshift.io/v1/namespaces/${project}/localsubjectaccessreviews' -d '{\"kind\":\"LocalSubjectAccessReview\",\"apiVersion\":\"authorization.openshift.io/v1\",\"namespace\":\"${project}\",\"verb\":\"create\",\"resource\":\"pods\"}'" '"kind": "SubjectAccessReviewResponse"'
# verify group and kind defaulting works correctly
os::cmd::expect_success_and_text "curl -k -XPOST -H 'Content-Type: application/json' -H 'Authorization: Bearer ${accesstoken}' '${API_SCHEME}://${API_HOST}:${API_PORT}/oapi/v1/subjectaccessreviews' -d '{\"namespace\":\"${project}\",\"verb\":\"create\",\"resource\":\"pods\"}'" '"kind": "SubjectAccessReviewResponse"'
os::cmd::expect_success_and_text "curl -k -XPOST -H 'Content-Type: application/json' -H 'Authorization: Bearer ${accesstoken}' '${API_SCHEME}://${API_HOST}:${API_PORT}/apis/authorization.openshift.io/v1/subjectaccessreviews' -d '{\"namespace\":\"${project}\",\"verb\":\"create\",\"resource\":\"pods\"}'" '"kind": "SubjectAccessReviewResponse"'
os::cmd::expect_success_and_text "oc policy can-i create pods --token='${accesstoken}' -n '${project}' --ignore-scopes" 'yes'
os::cmd::expect_success_and_text "oc policy can-i create pods --token='${accesstoken}' -n '${project}'" 'no'
os::cmd::expect_success_and_text "oc policy can-i create subjectaccessreviews.v1. --token='${accesstoken}' -n '${project}'" 'no'
Expand Down

0 comments on commit 11e1349

Please sign in to comment.