improve the forbidden message

This commit is contained in:
deads2k 2016-12-13 09:17:46 -05:00
parent 9705bb728e
commit f6829bbde7
5 changed files with 37 additions and 15 deletions

View File

@ -308,7 +308,8 @@ func NewGenericServerResponse(code int, verb string, qualifiedResource schema.Gr
message = "the server has asked for the client to provide credentials" message = "the server has asked for the client to provide credentials"
case http.StatusForbidden: case http.StatusForbidden:
reason = metav1.StatusReasonForbidden reason = metav1.StatusReasonForbidden
message = "the server does not allow access to the requested resource" // the server message has details about who is trying to perform what action. Keep its message.
message = serverMessage
case http.StatusMethodNotAllowed: case http.StatusMethodNotAllowed:
reason = metav1.StatusReasonMethodNotAllowed reason = metav1.StatusReasonMethodNotAllowed
message = "the server does not allow this method on the requested resource" message = "the server does not allow this method on the requested resource"

View File

@ -40,12 +40,12 @@ func WithAuthorization(handler http.Handler, requestContextMapper api.RequestCon
return return
} }
attrs, err := GetAuthorizerAttributes(ctx) attributes, err := GetAuthorizerAttributes(ctx)
if err != nil { if err != nil {
internalError(w, req, err) internalError(w, req, err)
return return
} }
authorized, reason, err := a.Authorize(attrs) authorized, reason, err := a.Authorize(attributes)
if authorized { if authorized {
handler.ServeHTTP(w, req) handler.ServeHTTP(w, req)
return return
@ -55,8 +55,8 @@ func WithAuthorization(handler http.Handler, requestContextMapper api.RequestCon
return return
} }
glog.V(4).Infof("Forbidden: %#v, Reason: %s", req.RequestURI, reason) glog.V(4).Infof("Forbidden: %#v, Reason: %q", req.RequestURI, reason)
forbidden(w, req) forbidden(attributes, w, req, reason)
}) })
} }

View File

@ -20,6 +20,7 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"k8s.io/kubernetes/pkg/auth/authorizer"
"k8s.io/kubernetes/pkg/util/runtime" "k8s.io/kubernetes/pkg/util/runtime"
) )
@ -30,9 +31,28 @@ func badGatewayError(w http.ResponseWriter, req *http.Request) {
} }
// forbidden renders a simple forbidden error // forbidden renders a simple forbidden error
func forbidden(w http.ResponseWriter, req *http.Request) { func forbidden(attributes authorizer.Attributes, w http.ResponseWriter, req *http.Request, reason string) {
msg := forbiddenMessage(attributes)
w.WriteHeader(http.StatusForbidden) w.WriteHeader(http.StatusForbidden)
fmt.Fprintf(w, "Forbidden: %#v", req.RequestURI) fmt.Fprintf(w, "%s: %q", msg, reason)
}
func forbiddenMessage(attributes authorizer.Attributes) string {
username := ""
if user := attributes.GetUser(); user != nil {
username = user.GetName()
}
resource := attributes.GetResource()
if group := attributes.GetAPIGroup(); len(group) > 0 {
resource = resource + "." + group
}
if ns := attributes.GetNamespace(); len(ns) > 0 {
return fmt.Sprintf("User %q cannot %s %s in the namespace %q.", username, attributes.GetVerb(), resource, ns)
}
return fmt.Sprintf("User %q cannot %s %s at the cluster scope.", username, attributes.GetVerb(), resource)
} }
// internalError renders a simple internal error // internalError renders a simple internal error

View File

@ -17,6 +17,7 @@ limitations under the License.
package filters package filters
import ( import (
"errors"
"fmt" "fmt"
"net/http" "net/http"
"strings" "strings"
@ -37,7 +38,7 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon
impersonationRequests, err := buildImpersonationRequests(req.Header) impersonationRequests, err := buildImpersonationRequests(req.Header)
if err != nil { if err != nil {
glog.V(4).Infof("%v", err) glog.V(4).Infof("%v", err)
forbidden(w, req) internalError(w, req, err)
return return
} }
if len(impersonationRequests) == 0 { if len(impersonationRequests) == 0 {
@ -47,12 +48,12 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon
ctx, exists := requestContextMapper.Get(req) ctx, exists := requestContextMapper.Get(req)
if !exists { if !exists {
forbidden(w, req) internalError(w, req, errors.New("no context found for request"))
return return
} }
requestor, exists := api.UserFrom(ctx) requestor, exists := api.UserFrom(ctx)
if !exists { if !exists {
forbidden(w, req) internalError(w, req, errors.New("no user found for request"))
return return
} }
@ -100,15 +101,15 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon
userExtra[extraKey] = append(userExtra[extraKey], extraValue) userExtra[extraKey] = append(userExtra[extraKey], extraValue)
default: default:
glog.V(4).Infof("unknown impersonation request type: %v\n", impersonationRequest) glog.V(4).Infof("unknown impersonation request type: %v", impersonationRequest)
forbidden(w, req) forbidden(actingAsAttributes, w, req, fmt.Sprintf("unknown impersonation request type: %v", impersonationRequest))
return return
} }
allowed, reason, err := a.Authorize(actingAsAttributes) allowed, reason, err := a.Authorize(actingAsAttributes)
if err != nil || !allowed { if err != nil || !allowed {
glog.V(4).Infof("Forbidden: %#v, Reason: %s, Error: %v", req.RequestURI, reason, err) glog.V(4).Infof("Forbidden: %#v, Reason: %s, Error: %v", req.RequestURI, reason, err)
forbidden(w, req) forbidden(actingAsAttributes, w, req, reason)
return return
} }
} }

View File

@ -118,7 +118,7 @@ func TestImpersonationFilter(t *testing.T) {
expectedUser: &user.DefaultInfo{ expectedUser: &user.DefaultInfo{
Name: "tester", Name: "tester",
}, },
expectedCode: http.StatusForbidden, expectedCode: http.StatusInternalServerError,
}, },
{ {
name: "impersonating-extra-without-user", name: "impersonating-extra-without-user",
@ -129,7 +129,7 @@ func TestImpersonationFilter(t *testing.T) {
expectedUser: &user.DefaultInfo{ expectedUser: &user.DefaultInfo{
Name: "tester", Name: "tester",
}, },
expectedCode: http.StatusForbidden, expectedCode: http.StatusInternalServerError,
}, },
{ {
name: "disallowed-group", name: "disallowed-group",