Merge pull request #47384 from shiywang/api403

Automatic merge from submit-queue (batch tested with PRs 48383, 47384)

Fix 401/403 apiserver errors do not return 'Status' objects

fixes https://github.com/kubernetes/kubernetes/issues/45970
pull/6/head
Kubernetes Submit Queue 2017-07-05 02:18:21 -07:00 committed by GitHub
commit c746680143
11 changed files with 187 additions and 67 deletions

View File

@ -23,6 +23,8 @@ go_test(
deps = [
"//vendor/k8s.io/api/authentication/v1:go_default_library",
"//vendor/k8s.io/api/batch/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/k8s.io/apiserver/pkg/apis/audit:go_default_library",
@ -54,7 +56,10 @@ go_library(
"//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
"//vendor/k8s.io/api/authentication/v1:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//vendor/k8s.io/apiserver/pkg/apis/audit:go_default_library",

View File

@ -17,13 +17,19 @@ limitations under the License.
package filters
import (
"errors"
"net/http"
"strings"
"github.com/golang/glog"
"github.com/prometheus/client_golang/prometheus"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/authentication/authenticator"
"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
"k8s.io/apiserver/pkg/endpoints/request"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
)
@ -76,22 +82,25 @@ func WithAuthentication(handler http.Handler, mapper genericapirequest.RequestCo
)
}
func Unauthorized(supportsBasicAuth bool) http.HandlerFunc {
if supportsBasicAuth {
return unauthorizedBasicAuth
}
return unauthorized
}
func Unauthorized(requestContextMapper request.RequestContextMapper, s runtime.NegotiatedSerializer, supportsBasicAuth bool) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if supportsBasicAuth {
w.Header().Set("WWW-Authenticate", `Basic realm="kubernetes-master"`)
}
ctx, ok := requestContextMapper.Get(req)
if !ok {
responsewriters.InternalError(w, req, errors.New("no context found for request"))
return
}
requestInfo, found := request.RequestInfoFrom(ctx)
if !found {
responsewriters.InternalError(w, req, errors.New("no RequestInfo found in the context"))
return
}
// unauthorizedBasicAuth serves an unauthorized message to clients.
func unauthorizedBasicAuth(w http.ResponseWriter, req *http.Request) {
w.Header().Set("WWW-Authenticate", `Basic realm="kubernetes-master"`)
http.Error(w, "Unauthorized", http.StatusUnauthorized)
}
// unauthorized serves an unauthorized message to clients.
func unauthorized(w http.ResponseWriter, req *http.Request) {
http.Error(w, "Unauthorized", http.StatusUnauthorized)
gv := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
responsewriters.ErrorNegotiated(ctx, apierrors.NewUnauthorized("Unauthorized"), s, gv, w, req)
})
}
// compressUsername maps all possible usernames onto a small set of categories

View File

@ -22,13 +22,14 @@ import (
"github.com/golang/glog"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
"k8s.io/apiserver/pkg/endpoints/request"
)
// WithAuthorizationCheck passes all authorized requests on to handler, and returns a forbidden error otherwise.
func WithAuthorization(handler http.Handler, requestContextMapper request.RequestContextMapper, a authorizer.Authorizer) http.Handler {
func WithAuthorization(handler http.Handler, requestContextMapper request.RequestContextMapper, a authorizer.Authorizer, s runtime.NegotiatedSerializer) http.Handler {
if a == nil {
glog.Warningf("Authorization is disabled")
return handler
@ -56,7 +57,7 @@ func WithAuthorization(handler http.Handler, requestContextMapper request.Reques
}
glog.V(4).Infof("Forbidden: %#v, Reason: %q", req.RequestURI, reason)
responsewriters.Forbidden(attributes, w, req, reason)
responsewriters.Forbidden(ctx, attributes, w, req, reason, s)
})
}

View File

@ -26,6 +26,7 @@ import (
authenticationv1 "k8s.io/api/authentication/v1"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/authentication/serviceaccount"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
@ -35,7 +36,7 @@ import (
)
// WithImpersonation is a filter that will inspect and check requests that attempt to change the user.Info for their requests
func WithImpersonation(handler http.Handler, requestContextMapper request.RequestContextMapper, a authorizer.Authorizer) http.Handler {
func WithImpersonation(handler http.Handler, requestContextMapper request.RequestContextMapper, a authorizer.Authorizer, s runtime.NegotiatedSerializer) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
impersonationRequests, err := buildImpersonationRequests(req.Header)
if err != nil {
@ -104,14 +105,14 @@ func WithImpersonation(handler http.Handler, requestContextMapper request.Reques
default:
glog.V(4).Infof("unknown impersonation request type: %v", impersonationRequest)
responsewriters.Forbidden(actingAsAttributes, w, req, fmt.Sprintf("unknown impersonation request type: %v", impersonationRequest))
responsewriters.Forbidden(ctx, actingAsAttributes, w, req, fmt.Sprintf("unknown impersonation request type: %v", impersonationRequest), s)
return
}
allowed, reason, err := a.Authorize(actingAsAttributes)
if err != nil || !allowed {
glog.V(4).Infof("Forbidden: %#v, Reason: %s, Error: %v", req.RequestURI, reason, err)
responsewriters.Forbidden(actingAsAttributes, w, req, reason)
responsewriters.Forbidden(ctx, actingAsAttributes, w, req, reason, s)
return
}
}

View File

@ -26,6 +26,8 @@ import (
"testing"
authenticationapi "k8s.io/api/authentication/v1"
"k8s.io/apimachinery/pkg/runtime"
serializer "k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
@ -356,7 +358,7 @@ func TestImpersonationFilter(t *testing.T) {
delegate.ServeHTTP(w, req)
})
}(WithImpersonation(doNothingHandler, requestContextMapper, impersonateAuthorizer{}))
}(WithImpersonation(doNothingHandler, requestContextMapper, impersonateAuthorizer{}, serializer.NewCodecFactory(runtime.NewScheme())))
handler = request.WithRequestContext(handler, requestContextMapper)
server := httptest.NewServer(handler)

View File

@ -19,9 +19,12 @@ go_test(
deps = [
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
"//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
],
)
@ -35,6 +38,7 @@ go_library(
],
tags = ["automanaged"],
deps = [
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",

View File

@ -21,8 +21,12 @@ import (
"net/http"
"strings"
"k8s.io/apimachinery/pkg/util/runtime"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
)
// Avoid emitting errors that look like valid HTML. Quotes are okay.
@ -37,17 +41,20 @@ func BadGatewayError(w http.ResponseWriter, req *http.Request) {
}
// Forbidden renders a simple forbidden error
func Forbidden(attributes authorizer.Attributes, w http.ResponseWriter, req *http.Request, reason string) {
func Forbidden(ctx request.Context, attributes authorizer.Attributes, w http.ResponseWriter, req *http.Request, reason string, s runtime.NegotiatedSerializer) {
msg := sanitizer.Replace(forbiddenMessage(attributes))
w.Header().Set("Content-Type", "text/plain")
w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(http.StatusForbidden)
var errMsg string
if len(reason) == 0 {
fmt.Fprintf(w, "%s", msg)
errMsg = fmt.Sprintf("%s", msg)
} else {
fmt.Fprintf(w, "%s: %q", msg, reason)
errMsg = fmt.Sprintf("%s: %q", msg, reason)
}
gv := schema.GroupVersion{Group: attributes.GetAPIGroup(), Version: attributes.GetAPIVersion()}
gr := schema.GroupResource{Group: attributes.GetAPIGroup(), Resource: attributes.GetResource()}
ErrorNegotiated(ctx, apierrors.NewForbidden(gr, attributes.GetName(), fmt.Errorf(errMsg)), s, gv, w, req)
}
func forbiddenMessage(attributes authorizer.Attributes) string {
@ -81,7 +88,7 @@ func InternalError(w http.ResponseWriter, req *http.Request, err error) {
w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprintf(w, "Internal Server Error: %q: %v", sanitizer.Replace(req.RequestURI), err)
runtime.HandleError(err)
utilruntime.HandleError(err)
}
// NotFound renders a simple not found error.

View File

@ -22,8 +22,11 @@ import (
"net/http/httptest"
"testing"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
)
func TestErrors(t *testing.T) {
@ -64,18 +67,20 @@ func TestForbidden(t *testing.T) {
attributes authorizer.Attributes
reason string
}{
{`User "NAME" cannot GET path "/whatever".`,
authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/whatever"}, ""},
{`User "NAME" cannot GET path "/<script>".`,
authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/<script>"}, ""},
{`User "NAME" cannot GET pod at the cluster scope.`,
authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true}, ""},
{`User "NAME" cannot GET pod.v2/quota in the namespace "test".`,
authorizer.AttributesRecord{User: u, Verb: "GET", Namespace: "test", APIGroup: "v2", Resource: "pod", Subresource: "quota", ResourceRequest: true}, ""},
{`{"metadata":{},"status":"Failure","message":" \"\" is forbidden: User \"NAME\" cannot GET path \"/whatever\".","reason":"Forbidden","details":{},"code":403}
`, authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/whatever"}, ""},
{`{"metadata":{},"status":"Failure","message":" \"\" is forbidden: User \"NAME\" cannot GET path \"/\u0026lt;script\u0026gt;\".","reason":"Forbidden","details":{},"code":403}
`, authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/<script>"}, ""},
{`{"metadata":{},"status":"Failure","message":"pod \"\" is forbidden: User \"NAME\" cannot GET pod at the cluster scope.","reason":"Forbidden","details":{"kind":"pod"},"code":403}
`, authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true}, ""},
{`{"metadata":{},"status":"Failure","message":"pod.v2 \"\" is forbidden: User \"NAME\" cannot GET pod.v2/quota in the namespace \"test\".","reason":"Forbidden","details":{"group":"v2","kind":"pod"},"code":403}
`, authorizer.AttributesRecord{User: u, Verb: "GET", Namespace: "test", APIGroup: "v2", Resource: "pod", Subresource: "quota", ResourceRequest: true}, ""},
}
for _, test := range cases {
observer := httptest.NewRecorder()
Forbidden(test.attributes, observer, &http.Request{}, test.reason)
scheme := runtime.NewScheme()
negotiatedSerializer := serializer.DirectCodecFactory{CodecFactory: serializer.NewCodecFactory(scheme)}
Forbidden(request.NewDefaultContext(), test.attributes, observer, &http.Request{}, test.reason, negotiatedSerializer)
result := string(observer.Body.Bytes())
if result != test.expected {
t.Errorf("Forbidden(%#v...) != %#v, got %#v", test.attributes, test.expected, result)

View File

@ -467,14 +467,14 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G
}
func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
handler := genericapifilters.WithAuthorization(apiHandler, c.RequestContextMapper, c.Authorizer)
handler = genericapifilters.WithImpersonation(handler, c.RequestContextMapper, c.Authorizer)
handler := genericapifilters.WithAuthorization(apiHandler, c.RequestContextMapper, c.Authorizer, c.Serializer)
handler = genericapifilters.WithImpersonation(handler, c.RequestContextMapper, c.Authorizer, c.Serializer)
if utilfeature.DefaultFeatureGate.Enabled(features.AdvancedAuditing) {
handler = genericapifilters.WithAudit(handler, c.RequestContextMapper, c.AuditBackend, c.AuditPolicyChecker, c.LongRunningFunc)
} else {
handler = genericapifilters.WithLegacyAudit(handler, c.RequestContextMapper, c.LegacyAuditWriter)
}
handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.SupportsBasicAuth))
handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.RequestContextMapper, c.Serializer, c.SupportsBasicAuth))
handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
handler = genericfilters.WithPanicRecovery(handler)
handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc)

View File

@ -19,6 +19,7 @@ go_test(
"//pkg/api/testapi:go_default_library",
"//pkg/client/clientset_generated/clientset/typed/core/v1:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/master:go_default_library",
"//test/integration:go_default_library",
"//test/integration/framework:go_default_library",
"//vendor/github.com/ghodss/yaml:go_default_library",
@ -26,6 +27,12 @@ go_test(
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/group:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/request/bearertoken:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authorization/authorizerfactory:go_default_library",
"//vendor/k8s.io/apiserver/plugin/pkg/authenticator/token/tokentest:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
],
)

View File

@ -35,15 +35,36 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/authentication/group"
"k8s.io/apiserver/pkg/authentication/request/bearertoken"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/authorization/authorizerfactory"
"k8s.io/apiserver/plugin/pkg/authenticator/token/tokentest"
restclient "k8s.io/client-go/rest"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/testapi"
clienttypedv1 "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/core/v1"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/master"
"k8s.io/kubernetes/test/integration"
"k8s.io/kubernetes/test/integration/framework"
)
const (
AliceToken string = "abc123" // username: alice. Present in token file.
BobToken string = "xyz987" // username: bob. Present in token file.
)
type allowAliceAuthorizer struct{}
func (allowAliceAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
if a.GetUser() != nil && a.GetUser().GetName() == "alice" {
return true, "", nil
}
return false, "I can't allow that. Go ask alice.", nil
}
func testPrefix(t *testing.T, prefix string) {
_, s, closeFn := framework.RunAMaster(nil)
defer closeFn()
@ -101,38 +122,96 @@ func TestEmptyList(t *testing.T) {
}
}
func initStatusForbiddenMasterCongfig() *master.Config {
masterConfig := framework.NewIntegrationTestMasterConfig()
masterConfig.GenericConfig.Authorizer = authorizerfactory.NewAlwaysDenyAuthorizer()
return masterConfig
}
func initUnauthorizedMasterCongfig() *master.Config {
masterConfig := framework.NewIntegrationTestMasterConfig()
tokenAuthenticator := tokentest.New()
tokenAuthenticator.Tokens[AliceToken] = &user.DefaultInfo{Name: "alice", UID: "1"}
tokenAuthenticator.Tokens[BobToken] = &user.DefaultInfo{Name: "bob", UID: "2"}
masterConfig.GenericConfig.Authenticator = group.NewGroupAdder(bearertoken.New(tokenAuthenticator), []string{user.AllAuthenticated})
masterConfig.GenericConfig.Authorizer = allowAliceAuthorizer{}
return masterConfig
}
func TestStatus(t *testing.T) {
_, s, closeFn := framework.RunAMaster(nil)
defer closeFn()
testCases := []struct {
name string
masterConfig *master.Config
statusCode int
reqPath string
reason string
message string
}{
{
name: "404",
masterConfig: nil,
statusCode: http.StatusNotFound,
reqPath: "/apis/batch/v1/namespaces/default/jobs/foo",
reason: "NotFound",
message: `jobs.batch "foo" not found`,
},
{
name: "403",
masterConfig: initStatusForbiddenMasterCongfig(),
statusCode: http.StatusForbidden,
reqPath: "/apis",
reason: "Forbidden",
message: ` "" is forbidden: User "" cannot get path "/apis".: "Everything is forbidden."`,
},
{
name: "401",
masterConfig: initUnauthorizedMasterCongfig(),
statusCode: http.StatusUnauthorized,
reqPath: "/apis",
reason: "Unauthorized",
message: `Unauthorized`,
},
}
u := s.URL + "/apis/batch/v1/namespaces/default/jobs/foo"
resp, err := http.Get(u)
if err != nil {
t.Fatalf("unexpected error getting %s: %v", u, err)
}
if resp.StatusCode != http.StatusNotFound {
t.Fatalf("got status %v instead of 404", resp.StatusCode)
}
defer resp.Body.Close()
data, _ := ioutil.ReadAll(resp.Body)
decodedData := map[string]interface{}{}
if err := json.Unmarshal(data, &decodedData); err != nil {
for _, tc := range testCases {
_, s, closeFn := framework.RunAMaster(tc.masterConfig)
defer closeFn()
u := s.URL + tc.reqPath
resp, err := http.Get(u)
if err != nil {
t.Fatalf("unexpected error getting %s: %v", u, err)
}
if resp.StatusCode != tc.statusCode {
t.Fatalf("got status %v instead of %s", resp.StatusCode, tc.name)
}
defer resp.Body.Close()
data, _ := ioutil.ReadAll(resp.Body)
decodedData := map[string]interface{}{}
if err := json.Unmarshal(data, &decodedData); err != nil {
t.Logf("body: %s", string(data))
t.Fatalf("got error decoding data: %v", err)
}
t.Logf("body: %s", string(data))
t.Fatalf("got error decoding data: %v", err)
}
t.Logf("body: %s", string(data))
if got, expected := decodedData["apiVersion"], "v1"; got != expected {
t.Errorf("unexpected apiVersion %q, expected %q", got, expected)
}
if got, expected := decodedData["kind"], "Status"; got != expected {
t.Errorf("unexpected kind %q, expected %q", got, expected)
}
if got, expected := decodedData["status"], "Failure"; got != expected {
t.Errorf("unexpected status %q, expected %q", got, expected)
}
if got, expected := decodedData["code"], float64(404); got != expected {
t.Errorf("unexpected code %v, expected %v", got, expected)
if got, expected := decodedData["apiVersion"], "v1"; got != expected {
t.Errorf("unexpected apiVersion %q, expected %q", got, expected)
}
if got, expected := decodedData["kind"], "Status"; got != expected {
t.Errorf("unexpected kind %q, expected %q", got, expected)
}
if got, expected := decodedData["status"], "Failure"; got != expected {
t.Errorf("unexpected status %q, expected %q", got, expected)
}
if got, expected := decodedData["code"], float64(tc.statusCode); got != expected {
t.Errorf("unexpected code %v, expected %v", got, expected)
}
if got, expected := decodedData["reason"], tc.reason; got != expected {
t.Errorf("unexpected reason %v, expected %v", got, expected)
}
if got, expected := decodedData["message"], tc.message; got != expected {
t.Errorf("unexpected message %v, expected %v", got, expected)
}
}
}