From 4cd5a1f5b74fbcb5bc8d02ab93b8ec82cbadd3b8 Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Mon, 27 Apr 2020 06:34:47 +0000 Subject: [PATCH 01/17] Authorization service proto --- backend/api/BUILD.bazel | 1 + backend/api/auth.proto | 86 +++++++++ backend/api/go_client/BUILD.bazel | 2 + backend/api/go_client/auth.pb.go | 260 ++++++++++++++++++++++++++ backend/api/go_client/auth.pb.gw.go | 129 +++++++++++++ backend/api/swagger/auth.swagger.json | 140 ++++++++++++++ 6 files changed, 618 insertions(+) create mode 100644 backend/api/auth.proto create mode 100755 backend/api/go_client/auth.pb.go create mode 100755 backend/api/go_client/auth.pb.gw.go create mode 100755 backend/api/swagger/auth.swagger.json diff --git a/backend/api/BUILD.bazel b/backend/api/BUILD.bazel index 053903f449e..73af36c2245 100644 --- a/backend/api/BUILD.bazel +++ b/backend/api/BUILD.bazel @@ -5,6 +5,7 @@ load("@com_github_grpc_ecosystem_grpc_gateway//protoc-gen-swagger:defs.bzl", "pr proto_library( name = "go_client_proto", srcs = [ + "auth.proto", "error.proto", "experiment.proto", "filter.proto", diff --git a/backend/api/auth.proto b/backend/api/auth.proto new file mode 100644 index 00000000000..28a27b3aa43 --- /dev/null +++ b/backend/api/auth.proto @@ -0,0 +1,86 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +option go_package = "github.com/kubeflow/pipelines/backend/api/go_client"; +package api; + +import "google/api/annotations.proto"; +import "google/protobuf/empty.proto"; +import "backend/api/error.proto"; +import "protoc-gen-swagger/options/annotations.proto"; + +option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = { + responses: { + key: "default"; + value: { + schema: { + json_schema: { + ref: ".api.Status"; + } + } + } + } + // Use bearer token for authorizing access to job service. + // Kubernetes client library(https://kubernetes.io/docs/reference/using-api/client-libraries/) + // uses bearer token as default for authorization. The section below + // ensures security definition object is generated in the swagger definition. + // For more details see https://github.com/OAI/OpenAPI-Specification/blob/3.0.0/versions/2.0.md#securityDefinitionsObject + security_definitions: { + security: { + key: "Bearer"; + value: { + type: TYPE_API_KEY; + in: IN_HEADER; + name: "authorization"; + } + } + } + security: { + security_requirement: { + key: "Bearer"; + value: {}; + } + } +}; + +service AuthService { + rpc Authorize(AuthorizeRequest) returns (google.protobuf.Empty) { + option (google.api.http) = { + get: "/apis/v1beta1/auth" + }; + } +} + +// Ask for authorization of an access by providing resource's namespace, type +// and verb. User identity is not part of the message, because it is expected +// to be parsed from request headers. Caller should proxy user request's headers. +message AuthorizeRequest { + // Type of resources in pipelines system. + enum Resources { + UNASSIGNED_RESOURCES = 0; + VIEWERS = 1; + } + // Type of verbs that act on the resources. + enum Verb { + UNASSIGNED_VERB = 0; + CREATE = 1; + GET = 2; + DELETE = 3; + } + string namespace = 1; // Namespace the resource belongs to. + Resources resources = 2; // Resource type asking for authorization. + Verb verb = 3; // Verb on the resource asking for authorization. +} diff --git a/backend/api/go_client/BUILD.bazel b/backend/api/go_client/BUILD.bazel index 07c7677beb2..58da7ef4764 100644 --- a/backend/api/go_client/BUILD.bazel +++ b/backend/api/go_client/BUILD.bazel @@ -3,6 +3,8 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", srcs = [ + "auth.pb.go", + "auth.pb.gw.go", "error.pb.go", "experiment.pb.go", "experiment.pb.gw.go", diff --git a/backend/api/go_client/auth.pb.go b/backend/api/go_client/auth.pb.go new file mode 100755 index 00000000000..c434d1bf924 --- /dev/null +++ b/backend/api/go_client/auth.pb.go @@ -0,0 +1,260 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Code generated by protoc-gen-go. DO NOT EDIT. +// source: backend/api/auth.proto + +package go_client // import "github.com/kubeflow/pipelines/backend/api/go_client" + +import proto "github.com/golang/protobuf/proto" +import fmt "fmt" +import math "math" +import empty "github.com/golang/protobuf/ptypes/empty" +import _ "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options" +import _ "google.golang.org/genproto/googleapis/api/annotations" + +import ( + context "golang.org/x/net/context" + grpc "google.golang.org/grpc" +) + +// Reference imports to suppress errors if they are not otherwise used. +var _ = proto.Marshal +var _ = fmt.Errorf +var _ = math.Inf + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the proto package it is being compiled against. +// A compilation error at this line likely means your copy of the +// proto package needs to be updated. +const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package + +type AuthorizeRequest_Resources int32 + +const ( + AuthorizeRequest_UNASSIGNED_RESOURCES AuthorizeRequest_Resources = 0 + AuthorizeRequest_VIEWERS AuthorizeRequest_Resources = 1 +) + +var AuthorizeRequest_Resources_name = map[int32]string{ + 0: "UNASSIGNED_RESOURCES", + 1: "VIEWERS", +} +var AuthorizeRequest_Resources_value = map[string]int32{ + "UNASSIGNED_RESOURCES": 0, + "VIEWERS": 1, +} + +func (x AuthorizeRequest_Resources) String() string { + return proto.EnumName(AuthorizeRequest_Resources_name, int32(x)) +} +func (AuthorizeRequest_Resources) EnumDescriptor() ([]byte, []int) { + return fileDescriptor_auth_b463ef3269931e86, []int{0, 0} +} + +type AuthorizeRequest_Verb int32 + +const ( + AuthorizeRequest_UNASSIGNED_VERB AuthorizeRequest_Verb = 0 + AuthorizeRequest_CREATE AuthorizeRequest_Verb = 1 + AuthorizeRequest_GET AuthorizeRequest_Verb = 2 + AuthorizeRequest_DELETE AuthorizeRequest_Verb = 3 +) + +var AuthorizeRequest_Verb_name = map[int32]string{ + 0: "UNASSIGNED_VERB", + 1: "CREATE", + 2: "GET", + 3: "DELETE", +} +var AuthorizeRequest_Verb_value = map[string]int32{ + "UNASSIGNED_VERB": 0, + "CREATE": 1, + "GET": 2, + "DELETE": 3, +} + +func (x AuthorizeRequest_Verb) String() string { + return proto.EnumName(AuthorizeRequest_Verb_name, int32(x)) +} +func (AuthorizeRequest_Verb) EnumDescriptor() ([]byte, []int) { + return fileDescriptor_auth_b463ef3269931e86, []int{0, 1} +} + +type AuthorizeRequest struct { + Namespace string `protobuf:"bytes,1,opt,name=namespace,proto3" json:"namespace,omitempty"` + Resources AuthorizeRequest_Resources `protobuf:"varint,2,opt,name=resources,proto3,enum=api.AuthorizeRequest_Resources" json:"resources,omitempty"` + Verb AuthorizeRequest_Verb `protobuf:"varint,3,opt,name=verb,proto3,enum=api.AuthorizeRequest_Verb" json:"verb,omitempty"` + XXX_NoUnkeyedLiteral struct{} `json:"-"` + XXX_unrecognized []byte `json:"-"` + XXX_sizecache int32 `json:"-"` +} + +func (m *AuthorizeRequest) Reset() { *m = AuthorizeRequest{} } +func (m *AuthorizeRequest) String() string { return proto.CompactTextString(m) } +func (*AuthorizeRequest) ProtoMessage() {} +func (*AuthorizeRequest) Descriptor() ([]byte, []int) { + return fileDescriptor_auth_b463ef3269931e86, []int{0} +} +func (m *AuthorizeRequest) XXX_Unmarshal(b []byte) error { + return xxx_messageInfo_AuthorizeRequest.Unmarshal(m, b) +} +func (m *AuthorizeRequest) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + return xxx_messageInfo_AuthorizeRequest.Marshal(b, m, deterministic) +} +func (dst *AuthorizeRequest) XXX_Merge(src proto.Message) { + xxx_messageInfo_AuthorizeRequest.Merge(dst, src) +} +func (m *AuthorizeRequest) XXX_Size() int { + return xxx_messageInfo_AuthorizeRequest.Size(m) +} +func (m *AuthorizeRequest) XXX_DiscardUnknown() { + xxx_messageInfo_AuthorizeRequest.DiscardUnknown(m) +} + +var xxx_messageInfo_AuthorizeRequest proto.InternalMessageInfo + +func (m *AuthorizeRequest) GetNamespace() string { + if m != nil { + return m.Namespace + } + return "" +} + +func (m *AuthorizeRequest) GetResources() AuthorizeRequest_Resources { + if m != nil { + return m.Resources + } + return AuthorizeRequest_UNASSIGNED_RESOURCES +} + +func (m *AuthorizeRequest) GetVerb() AuthorizeRequest_Verb { + if m != nil { + return m.Verb + } + return AuthorizeRequest_UNASSIGNED_VERB +} + +func init() { + proto.RegisterType((*AuthorizeRequest)(nil), "api.AuthorizeRequest") + proto.RegisterEnum("api.AuthorizeRequest_Resources", AuthorizeRequest_Resources_name, AuthorizeRequest_Resources_value) + proto.RegisterEnum("api.AuthorizeRequest_Verb", AuthorizeRequest_Verb_name, AuthorizeRequest_Verb_value) +} + +// Reference imports to suppress errors if they are not otherwise used. +var _ context.Context +var _ grpc.ClientConn + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the grpc package it is being compiled against. +const _ = grpc.SupportPackageIsVersion4 + +// AuthServiceClient is the client API for AuthService service. +// +// For semantics around ctx use and closing/ending streaming RPCs, please refer to https://godoc.org/google.golang.org/grpc#ClientConn.NewStream. +type AuthServiceClient interface { + Authorize(ctx context.Context, in *AuthorizeRequest, opts ...grpc.CallOption) (*empty.Empty, error) +} + +type authServiceClient struct { + cc *grpc.ClientConn +} + +func NewAuthServiceClient(cc *grpc.ClientConn) AuthServiceClient { + return &authServiceClient{cc} +} + +func (c *authServiceClient) Authorize(ctx context.Context, in *AuthorizeRequest, opts ...grpc.CallOption) (*empty.Empty, error) { + out := new(empty.Empty) + err := c.cc.Invoke(ctx, "/api.AuthService/Authorize", in, out, opts...) + if err != nil { + return nil, err + } + return out, nil +} + +// AuthServiceServer is the server API for AuthService service. +type AuthServiceServer interface { + Authorize(context.Context, *AuthorizeRequest) (*empty.Empty, error) +} + +func RegisterAuthServiceServer(s *grpc.Server, srv AuthServiceServer) { + s.RegisterService(&_AuthService_serviceDesc, srv) +} + +func _AuthService_Authorize_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(AuthorizeRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(AuthServiceServer).Authorize(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: "/api.AuthService/Authorize", + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(AuthServiceServer).Authorize(ctx, req.(*AuthorizeRequest)) + } + return interceptor(ctx, in, info, handler) +} + +var _AuthService_serviceDesc = grpc.ServiceDesc{ + ServiceName: "api.AuthService", + HandlerType: (*AuthServiceServer)(nil), + Methods: []grpc.MethodDesc{ + { + MethodName: "Authorize", + Handler: _AuthService_Authorize_Handler, + }, + }, + Streams: []grpc.StreamDesc{}, + Metadata: "backend/api/auth.proto", +} + +func init() { proto.RegisterFile("backend/api/auth.proto", fileDescriptor_auth_b463ef3269931e86) } + +var fileDescriptor_auth_b463ef3269931e86 = []byte{ + // 460 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x52, 0xc1, 0x6e, 0xd3, 0x40, + 0x14, 0x8c, 0x9d, 0x2a, 0xc1, 0x2f, 0x94, 0x9a, 0x6d, 0x29, 0x91, 0x09, 0x6a, 0x94, 0x53, 0x0f, + 0xd4, 0x56, 0xd3, 0x2b, 0x1c, 0x92, 0x76, 0x55, 0x55, 0x82, 0x22, 0xad, 0xd3, 0x20, 0xf5, 0x52, + 0xad, 0xdd, 0x17, 0x67, 0x55, 0xc7, 0x6b, 0xd6, 0xeb, 0x54, 0x70, 0x44, 0xe2, 0x03, 0x80, 0x4f, + 0xe3, 0x17, 0xf8, 0x10, 0xe4, 0x4d, 0x9a, 0x46, 0x90, 0xd3, 0x6a, 0xdf, 0xcc, 0x9b, 0x19, 0x69, + 0x1e, 0xec, 0x47, 0x3c, 0xbe, 0xc3, 0xec, 0x36, 0xe0, 0xb9, 0x08, 0x78, 0xa9, 0xa7, 0x7e, 0xae, + 0xa4, 0x96, 0xa4, 0xce, 0x73, 0xe1, 0x75, 0x12, 0x29, 0x93, 0x14, 0x17, 0x58, 0x96, 0x49, 0xcd, + 0xb5, 0x90, 0x59, 0xb1, 0xa0, 0x78, 0xaf, 0x96, 0xa8, 0xf9, 0x45, 0xe5, 0x24, 0xc0, 0x59, 0xae, + 0xbf, 0x2c, 0xc1, 0x97, 0xeb, 0xba, 0xa8, 0x94, 0x54, 0x4b, 0xe0, 0x8d, 0x79, 0xe2, 0xa3, 0x04, + 0xb3, 0xa3, 0xe2, 0x9e, 0x27, 0x09, 0xaa, 0x40, 0xe6, 0x46, 0xf7, 0x7f, 0x8f, 0xde, 0x0f, 0x1b, + 0xdc, 0x41, 0xa9, 0xa7, 0x52, 0x89, 0xaf, 0xc8, 0xf0, 0x73, 0x89, 0x85, 0x26, 0x1d, 0x70, 0x32, + 0x3e, 0xc3, 0x22, 0xe7, 0x31, 0xb6, 0xad, 0xae, 0x75, 0xe8, 0xb0, 0xc7, 0x01, 0x79, 0x07, 0x8e, + 0xc2, 0x42, 0x96, 0x2a, 0xc6, 0xa2, 0x6d, 0x77, 0xad, 0xc3, 0x67, 0xfd, 0x03, 0x9f, 0xe7, 0xc2, + 0xff, 0x57, 0xc7, 0x67, 0x0f, 0x34, 0xf6, 0xb8, 0x41, 0x7c, 0xd8, 0x9a, 0xa3, 0x8a, 0xda, 0x75, + 0xb3, 0xe9, 0x6d, 0xde, 0x1c, 0xa3, 0x8a, 0x98, 0xe1, 0xf5, 0xfa, 0xe0, 0xac, 0x74, 0x48, 0x1b, + 0xf6, 0xae, 0x2e, 0x07, 0x61, 0x78, 0x71, 0x7e, 0x49, 0xcf, 0x6e, 0x18, 0x0d, 0x3f, 0x5e, 0xb1, + 0x53, 0x1a, 0xba, 0x35, 0xd2, 0x82, 0xe6, 0xf8, 0x82, 0x7e, 0xa2, 0x2c, 0x74, 0xad, 0xde, 0x5b, + 0xd8, 0xaa, 0x14, 0xc8, 0x2e, 0xec, 0xac, 0xd1, 0xc7, 0x94, 0x0d, 0xdd, 0x1a, 0x01, 0x68, 0x9c, + 0x32, 0x3a, 0x18, 0x51, 0xd7, 0x22, 0x4d, 0xa8, 0x9f, 0xd3, 0x91, 0x6b, 0x57, 0xc3, 0x33, 0xfa, + 0x9e, 0x8e, 0xa8, 0x5b, 0xef, 0x23, 0xb4, 0xaa, 0x40, 0x21, 0xaa, 0xb9, 0x88, 0x91, 0x8c, 0xc1, + 0x59, 0xe5, 0x23, 0x2f, 0x36, 0xe6, 0xf5, 0xf6, 0xfd, 0x45, 0x57, 0xfe, 0x43, 0x57, 0x3e, 0xad, + 0xba, 0xea, 0x79, 0xdf, 0x7e, 0xff, 0xf9, 0x65, 0xef, 0x11, 0x52, 0xd5, 0x54, 0x04, 0xf3, 0xe3, + 0x08, 0x35, 0x3f, 0x36, 0x77, 0x30, 0xfc, 0x6e, 0xfd, 0x1c, 0x7c, 0x60, 0x1d, 0x68, 0xde, 0xe2, + 0x84, 0x97, 0xa9, 0x26, 0xcf, 0xc9, 0x0e, 0x6c, 0x7b, 0x2d, 0xe3, 0x10, 0x6a, 0xae, 0xcb, 0xe2, + 0xfa, 0x00, 0x5e, 0x43, 0x63, 0x88, 0x5c, 0xa1, 0x22, 0xbb, 0x4f, 0x6c, 0x6f, 0x9b, 0x2f, 0x9d, + 0x4d, 0x89, 0x5d, 0x3b, 0x7a, 0x0a, 0xb0, 0x22, 0xd4, 0xae, 0x4f, 0x12, 0xa1, 0xa7, 0x65, 0xe4, + 0xc7, 0x72, 0x16, 0xdc, 0x95, 0x11, 0x4e, 0x52, 0x79, 0x1f, 0xe4, 0x22, 0xc7, 0x54, 0x64, 0x58, + 0x04, 0xeb, 0x27, 0x93, 0xc8, 0x9b, 0x38, 0x15, 0x98, 0xe9, 0xa8, 0x61, 0x32, 0x9f, 0xfc, 0x0d, + 0x00, 0x00, 0xff, 0xff, 0x8e, 0x47, 0x2d, 0x41, 0xaa, 0x02, 0x00, 0x00, +} diff --git a/backend/api/go_client/auth.pb.gw.go b/backend/api/go_client/auth.pb.gw.go new file mode 100755 index 00000000000..41119c97f80 --- /dev/null +++ b/backend/api/go_client/auth.pb.gw.go @@ -0,0 +1,129 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Code generated by protoc-gen-grpc-gateway. DO NOT EDIT. +// source: backend/api/auth.proto + +/* +Package go_client is a reverse proxy. + +It translates gRPC into RESTful JSON APIs. +*/ +package go_client + +import ( + "context" + "io" + "net/http" + + "github.com/golang/protobuf/proto" + "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/grpc-ecosystem/grpc-gateway/utilities" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/grpclog" + "google.golang.org/grpc/status" +) + +var _ codes.Code +var _ io.Reader +var _ status.Status +var _ = runtime.String +var _ = utilities.NewDoubleArray + +var ( + filter_AuthService_Authorize_0 = &utilities.DoubleArray{Encoding: map[string]int{}, Base: []int(nil), Check: []int(nil)} +) + +func request_AuthService_Authorize_0(ctx context.Context, marshaler runtime.Marshaler, client AuthServiceClient, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) { + var protoReq AuthorizeRequest + var metadata runtime.ServerMetadata + + if err := runtime.PopulateQueryParameters(&protoReq, req.URL.Query(), filter_AuthService_Authorize_0); err != nil { + return nil, metadata, status.Errorf(codes.InvalidArgument, "%v", err) + } + + msg, err := client.Authorize(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD)) + return msg, metadata, err + +} + +// RegisterAuthServiceHandlerFromEndpoint is same as RegisterAuthServiceHandler but +// automatically dials to "endpoint" and closes the connection when "ctx" gets done. +func RegisterAuthServiceHandlerFromEndpoint(ctx context.Context, mux *runtime.ServeMux, endpoint string, opts []grpc.DialOption) (err error) { + conn, err := grpc.Dial(endpoint, opts...) + if err != nil { + return err + } + defer func() { + if err != nil { + if cerr := conn.Close(); cerr != nil { + grpclog.Infof("Failed to close conn to %s: %v", endpoint, cerr) + } + return + } + go func() { + <-ctx.Done() + if cerr := conn.Close(); cerr != nil { + grpclog.Infof("Failed to close conn to %s: %v", endpoint, cerr) + } + }() + }() + + return RegisterAuthServiceHandler(ctx, mux, conn) +} + +// RegisterAuthServiceHandler registers the http handlers for service AuthService to "mux". +// The handlers forward requests to the grpc endpoint over "conn". +func RegisterAuthServiceHandler(ctx context.Context, mux *runtime.ServeMux, conn *grpc.ClientConn) error { + return RegisterAuthServiceHandlerClient(ctx, mux, NewAuthServiceClient(conn)) +} + +// RegisterAuthServiceHandlerClient registers the http handlers for service AuthService +// to "mux". The handlers forward requests to the grpc endpoint over the given implementation of "AuthServiceClient". +// Note: the gRPC framework executes interceptors within the gRPC handler. If the passed in "AuthServiceClient" +// doesn't go through the normal gRPC flow (creating a gRPC client etc.) then it will be up to the passed in +// "AuthServiceClient" to call the correct interceptors. +func RegisterAuthServiceHandlerClient(ctx context.Context, mux *runtime.ServeMux, client AuthServiceClient) error { + + mux.Handle("GET", pattern_AuthService_Authorize_0, func(w http.ResponseWriter, req *http.Request, pathParams map[string]string) { + ctx, cancel := context.WithCancel(req.Context()) + defer cancel() + inboundMarshaler, outboundMarshaler := runtime.MarshalerForRequest(mux, req) + rctx, err := runtime.AnnotateContext(ctx, mux, req) + if err != nil { + runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err) + return + } + resp, md, err := request_AuthService_Authorize_0(rctx, inboundMarshaler, client, req, pathParams) + ctx = runtime.NewServerMetadataContext(ctx, md) + if err != nil { + runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err) + return + } + + forward_AuthService_Authorize_0(ctx, mux, outboundMarshaler, w, req, resp, mux.GetForwardResponseOptions()...) + + }) + + return nil +} + +var ( + pattern_AuthService_Authorize_0 = runtime.MustPattern(runtime.NewPattern(1, []int{2, 0, 2, 1, 2, 2}, []string{"apis", "v1beta1", "auth"}, "")) +) + +var ( + forward_AuthService_Authorize_0 = runtime.ForwardResponseMessage +) diff --git a/backend/api/swagger/auth.swagger.json b/backend/api/swagger/auth.swagger.json new file mode 100755 index 00000000000..5b8e5beb0e6 --- /dev/null +++ b/backend/api/swagger/auth.swagger.json @@ -0,0 +1,140 @@ +{ + "swagger": "2.0", + "info": { + "title": "backend/api/auth.proto", + "version": "version not set" + }, + "schemes": [ + "http", + "https" + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "paths": { + "/apis/v1beta1/auth": { + "get": { + "operationId": "Authorize", + "responses": { + "200": { + "description": "A successful response.", + "schema": { + "properties": {} + } + }, + "default": { + "description": "", + "schema": { + "$ref": "#/definitions/apiStatus" + } + } + }, + "parameters": [ + { + "name": "namespace", + "in": "query", + "required": false, + "type": "string" + }, + { + "name": "resources", + "in": "query", + "required": false, + "type": "string", + "enum": [ + "UNASSIGNED_RESOURCES", + "VIEWERS" + ], + "default": "UNASSIGNED_RESOURCES" + }, + { + "name": "verb", + "in": "query", + "required": false, + "type": "string", + "enum": [ + "UNASSIGNED_VERB", + "CREATE", + "GET", + "DELETE" + ], + "default": "UNASSIGNED_VERB" + } + ], + "tags": [ + "AuthService" + ] + } + } + }, + "definitions": { + "AuthorizeRequestResources": { + "type": "string", + "enum": [ + "UNASSIGNED_RESOURCES", + "VIEWERS" + ], + "default": "UNASSIGNED_RESOURCES", + "description": "Type of resources in pipelines system." + }, + "AuthorizeRequestVerb": { + "type": "string", + "enum": [ + "UNASSIGNED_VERB", + "CREATE", + "GET", + "DELETE" + ], + "default": "UNASSIGNED_VERB", + "description": "Type of verbs that act on the resources." + }, + "apiStatus": { + "type": "object", + "properties": { + "error": { + "type": "string" + }, + "code": { + "type": "integer", + "format": "int32" + }, + "details": { + "type": "array", + "items": { + "$ref": "#/definitions/protobufAny" + } + } + } + }, + "protobufAny": { + "type": "object", + "properties": { + "type_url": { + "type": "string", + "description": "A URL/resource name that uniquely identifies the type of the serialized\nprotocol buffer message. The last segment of the URL's path must represent\nthe fully qualified name of the type (as in\n`path/google.protobuf.Duration`). The name should be in a canonical form\n(e.g., leading \".\" is not accepted).\n\nIn practice, teams usually precompile into the binary all types that they\nexpect it to use in the context of Any. However, for URLs which use the\nscheme `http`, `https`, or no scheme, one can optionally set up a type\nserver that maps type URLs to message definitions as follows:\n\n* If no scheme is provided, `https` is assumed.\n* An HTTP GET on the URL must yield a [google.protobuf.Type][]\n value in binary format, or produce an error.\n* Applications are allowed to cache lookup results based on the\n URL, or have them precompiled into a binary to avoid any\n lookup. Therefore, binary compatibility needs to be preserved\n on changes to types. (Use versioned type names to manage\n breaking changes.)\n\nNote: this functionality is not currently available in the official\nprotobuf release, and it is not used for type URLs beginning with\ntype.googleapis.com.\n\nSchemes other than `http`, `https` (or the empty scheme) might be\nused with implementation specific semantics." + }, + "value": { + "type": "string", + "format": "byte", + "description": "Must be a valid serialized protocol buffer of the above specified type." + } + }, + "description": "`Any` contains an arbitrary serialized protocol buffer message along with a\nURL that describes the type of the serialized message.\n\nProtobuf library provides support to pack/unpack Any values in the form\nof utility functions or additional generated methods of the Any type.\n\nExample 1: Pack and unpack a message in C++.\n\n Foo foo = ...;\n Any any;\n any.PackFrom(foo);\n ...\n if (any.UnpackTo(\u0026foo)) {\n ...\n }\n\nExample 2: Pack and unpack a message in Java.\n\n Foo foo = ...;\n Any any = Any.pack(foo);\n ...\n if (any.is(Foo.class)) {\n foo = any.unpack(Foo.class);\n }\n\n Example 3: Pack and unpack a message in Python.\n\n foo = Foo(...)\n any = Any()\n any.Pack(foo)\n ...\n if any.Is(Foo.DESCRIPTOR):\n any.Unpack(foo)\n ...\n\n Example 4: Pack and unpack a message in Go\n\n foo := \u0026pb.Foo{...}\n any, err := ptypes.MarshalAny(foo)\n ...\n foo := \u0026pb.Foo{}\n if err := ptypes.UnmarshalAny(any, foo); err != nil {\n ...\n }\n\nThe pack methods provided by protobuf library will by default use\n'type.googleapis.com/full.type.name' as the type URL and the unpack\nmethods only use the fully qualified type name after the last '/'\nin the type URL, for example \"foo.bar.com/x/y.z\" will yield type\nname \"y.z\".\n\n\nJSON\n====\nThe JSON representation of an `Any` value uses the regular\nrepresentation of the deserialized, embedded message, with an\nadditional field `@type` which contains the type URL. Example:\n\n package google.profile;\n message Person {\n string first_name = 1;\n string last_name = 2;\n }\n\n {\n \"@type\": \"type.googleapis.com/google.profile.Person\",\n \"firstName\": \u003cstring\u003e,\n \"lastName\": \u003cstring\u003e\n }\n\nIf the embedded message type is well-known and has a custom JSON\nrepresentation, that representation will be embedded adding a field\n`value` which holds the custom JSON in addition to the `@type`\nfield. Example (for message [google.protobuf.Duration][]):\n\n {\n \"@type\": \"type.googleapis.com/google.protobuf.Duration\",\n \"value\": \"1.212s\"\n }" + } + }, + "securityDefinitions": { + "Bearer": { + "type": "apiKey", + "name": "authorization", + "in": "header" + } + }, + "security": [ + { + "Bearer": [] + } + ] +} From 07ace34a33780ba462bd349f3c7c90357d0d3dcd Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Mon, 27 Apr 2020 07:07:51 +0000 Subject: [PATCH 02/17] implement auth service --- backend/src/apiserver/main.go | 2 + backend/src/apiserver/server/auth_server.go | 52 +++++++++++++++++++++ backend/src/apiserver/server/util.go | 16 +++++++ 3 files changed, 70 insertions(+) create mode 100644 backend/src/apiserver/server/auth_server.go diff --git a/backend/src/apiserver/main.go b/backend/src/apiserver/main.go index b072943a6b4..12e3ba0ddf1 100644 --- a/backend/src/apiserver/main.go +++ b/backend/src/apiserver/main.go @@ -101,6 +101,7 @@ func startRpcServer(resourceManager *resource.ResourceManager) { common.GetStringConfig(visualizationServiceHost), common.GetStringConfig(visualizationServicePort), )) + api.RegisterAuthServiceServer(s, server.NewAuthServer(resourceManager)) // Register reflection service on gRPC server. reflection.Register(s) @@ -125,6 +126,7 @@ func startHttpProxy(resourceManager *resource.ResourceManager) { registerHttpHandlerFromEndpoint(api.RegisterRunServiceHandlerFromEndpoint, "RunService", ctx, mux) registerHttpHandlerFromEndpoint(api.RegisterReportServiceHandlerFromEndpoint, "ReportService", ctx, mux) registerHttpHandlerFromEndpoint(api.RegisterVisualizationServiceHandlerFromEndpoint, "Visualization", ctx, mux) + registerHttpHandlerFromEndpoint(api.RegisterAuthServiceHandlerFromEndpoint, "AuthService", ctx, mux) // Create a top level mux to include both pipeline upload server and gRPC servers. topMux := http.NewServeMux() diff --git a/backend/src/apiserver/server/auth_server.go b/backend/src/apiserver/server/auth_server.go new file mode 100644 index 00000000000..dcc5c22bc85 --- /dev/null +++ b/backend/src/apiserver/server/auth_server.go @@ -0,0 +1,52 @@ +package server + +import ( + "context" + + "github.com/golang/protobuf/ptypes/empty" + api "github.com/kubeflow/pipelines/backend/api/go_client" + "github.com/kubeflow/pipelines/backend/src/apiserver/resource" + "github.com/kubeflow/pipelines/backend/src/common/util" +) + +type AuthServer struct { + resourceManager *resource.ResourceManager +} + +func (s *AuthServer) Authorize(ctx context.Context, request *api.AuthorizeRequest) ( + *empty.Empty, error) { + err := ValidateAuthorizeRequest(request) + if err != nil { + return nil, util.Wrap(err, "Validate authorize request failed.") + } + + // TODO: when KFP changes authorization implementation to have more + // granularity, we need to start using resources and verb info in the + // request. + err = CanAccessNamespace(s.resourceManager, ctx, request.Namespace) + if err != nil { + return nil, util.Wrap(err, "Failed to authorize the request.") + } + + return &empty.Empty{}, nil +} + +func ValidateAuthorizeRequest(request *api.AuthorizeRequest) error { + if request == nil { + return util.NewInvalidInputError("request object is empty.") + } + if len(request.Namespace) == 0 { + return util.NewInvalidInputError("Namespace is empty. Please specify a valid namespace.") + } + if request.Resources == api.AuthorizeRequest_UNASSIGNED_RESOURCES { + return util.NewInvalidInputError("Resources not specified. Please specify a valid resources.") + } + if request.Verb == api.AuthorizeRequest_UNASSIGNED_VERB { + return util.NewInvalidInputError("Verb not specified. Please specify a valid verb.") + } + return nil +} + +func NewAuthServer(resourceManager *resource.ResourceManager) *AuthServer { + return &AuthServer{resourceManager: resourceManager} +} diff --git a/backend/src/apiserver/server/util.go b/backend/src/apiserver/server/util.go index 8029e0210bd..c97405fa7b3 100644 --- a/backend/src/apiserver/server/util.go +++ b/backend/src/apiserver/server/util.go @@ -347,6 +347,22 @@ func CanAccessNamespaceInResourceReferences(resourceManager *resource.ResourceMa return nil } +func CanAccessNamespace(resourceManager *resource.ResourceManager, ctx context.Context, namespace string) error { + if common.IsMultiUserMode() == false { + // Skip authz if not multi-user mode. + return nil + } + + if len(namespace) == 0 { + return util.NewBadRequestError(errors.New("Namespace required for authorization."), "Namespace required for authorization.") + } + err := isAuthorized(resourceManager, ctx, namespace) + if err != nil { + return util.Wrap(err, "Failed to authorize namespace") + } + return nil +} + // isAuthorized verified whether the user identity, which is contains in the context object, // can access the target namespace. If the returned error is nil, the authorization passes. // Otherwise, Authorization fails with a non-nil error. From 43d17e89346c5b5bcc48545564ff9d49038bec7a Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Mon, 27 Apr 2020 07:52:16 +0000 Subject: [PATCH 03/17] Add unit tests --- backend/src/apiserver/server/BUILD.bazel | 2 + backend/src/apiserver/server/auth_server.go | 4 +- .../src/apiserver/server/auth_server_test.go | 98 +++++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 backend/src/apiserver/server/auth_server_test.go diff --git a/backend/src/apiserver/server/BUILD.bazel b/backend/src/apiserver/server/BUILD.bazel index 1b076e831b6..b26a2794b30 100644 --- a/backend/src/apiserver/server/BUILD.bazel +++ b/backend/src/apiserver/server/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "go_default_library", srcs = [ "api_converter.go", + "auth_server.go", "experiment_server.go", "job_server.go", "list_request_util.go", @@ -46,6 +47,7 @@ go_test( name = "go_default_test", srcs = [ "api_converter_test.go", + "auth_server_test.go", "experiment_server_test.go", "job_server_test.go", "list_request_util_test.go", diff --git a/backend/src/apiserver/server/auth_server.go b/backend/src/apiserver/server/auth_server.go index dcc5c22bc85..0cc6750c6f1 100644 --- a/backend/src/apiserver/server/auth_server.go +++ b/backend/src/apiserver/server/auth_server.go @@ -17,7 +17,7 @@ func (s *AuthServer) Authorize(ctx context.Context, request *api.AuthorizeReques *empty.Empty, error) { err := ValidateAuthorizeRequest(request) if err != nil { - return nil, util.Wrap(err, "Validate authorize request failed.") + return nil, util.Wrap(err, "Authorize request is not valid") } // TODO: when KFP changes authorization implementation to have more @@ -25,7 +25,7 @@ func (s *AuthServer) Authorize(ctx context.Context, request *api.AuthorizeReques // request. err = CanAccessNamespace(s.resourceManager, ctx, request.Namespace) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the request.") + return nil, util.Wrap(err, "Failed to authorize the request") } return &empty.Empty{}, nil diff --git a/backend/src/apiserver/server/auth_server_test.go b/backend/src/apiserver/server/auth_server_test.go new file mode 100644 index 00000000000..b6797d28b95 --- /dev/null +++ b/backend/src/apiserver/server/auth_server_test.go @@ -0,0 +1,98 @@ +package server + +import ( + "context" + "testing" + + api "github.com/kubeflow/pipelines/backend/api/go_client" + "github.com/kubeflow/pipelines/backend/src/apiserver/client" + "github.com/kubeflow/pipelines/backend/src/apiserver/common" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "google.golang.org/grpc/metadata" +) + +func TestAuthorizeRequest_SingleUserMode(t *testing.T) { + clients, manager, _ := initWithExperiment(t) + defer clients.Close() + authServer := AuthServer{resourceManager: manager} + clients.KfamClientFake = client.NewFakeKFAMClientUnauthorized() + + md := metadata.New(map[string]string{}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + request := &api.AuthorizeRequest{ + Namespace: "ns1", + Resources: api.AuthorizeRequest_VIEWERS, + Verb: api.AuthorizeRequest_GET, + } + + _, err := authServer.Authorize(ctx, request) + // Authz is completely skipped without checking anything. + assert.Nil(t, err) +} + +func TestAuthorizeRequest_InvalidRequest(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + clients, manager, _ := initWithExperiment(t) + defer clients.Close() + authServer := AuthServer{resourceManager: manager} + + md := metadata.New(map[string]string{}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + request := &api.AuthorizeRequest{ + Namespace: "", + Resources: api.AuthorizeRequest_UNASSIGNED_RESOURCES, + Verb: api.AuthorizeRequest_UNASSIGNED_VERB, + } + + _, err := authServer.Authorize(ctx, request) + assert.Error(t, err) + assert.EqualError(t, err, "Authorize request is not valid: Invalid input error: Namespace is empty. Please specify a valid namespace.") +} + +func TestAuthorizeRequest_Authorized(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + clients, manager, _ := initWithExperiment(t) + defer clients.Close() + authServer := AuthServer{resourceManager: manager} + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + request := &api.AuthorizeRequest{ + Namespace: "ns1", + Resources: api.AuthorizeRequest_VIEWERS, + Verb: api.AuthorizeRequest_GET, + } + + _, err := authServer.Authorize(ctx, request) + assert.Nil(t, err) +} + +func TestAuthorizeRequest_Unauthorized(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + clients, manager, _ := initWithExperiment_KFAM_Unauthorized(t) + defer clients.Close() + authServer := AuthServer{resourceManager: manager} + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + request := &api.AuthorizeRequest{ + Namespace: "ns1", + Resources: api.AuthorizeRequest_VIEWERS, + Verb: api.AuthorizeRequest_GET, + } + + _, err := authServer.Authorize(ctx, request) + assert.Error(t, err) + assert.EqualError(t, err, "Failed to authorize the request: Failed to authorize namespace: BadRequestError: Unauthorized access for user@google.com to namespace ns1: Unauthorized access for user@google.com to namespace ns1") +} From 844a7408ac8fbca9cff57ed7472cfcbb5a0d53be Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Mon, 27 Apr 2020 16:09:30 +0800 Subject: [PATCH 04/17] Generate auth api client --- frontend/package.json | 1 + frontend/server/apis/auth/.gitignore | 3 + .../server/apis/auth/.swagger-codegen-ignore | 23 ++ .../server/apis/auth/.swagger-codegen/VERSION | 1 + frontend/server/apis/auth/api.ts | 319 ++++++++++++++++++ frontend/server/apis/auth/configuration.ts | 65 ++++ frontend/server/apis/auth/custom.d.ts | 2 + frontend/server/apis/auth/git_push.sh | 51 +++ frontend/server/apis/auth/index.ts | 15 + frontend/server/package-lock.json | 6 + frontend/server/package.json | 1 + 11 files changed, 487 insertions(+) create mode 100644 frontend/server/apis/auth/.gitignore create mode 100644 frontend/server/apis/auth/.swagger-codegen-ignore create mode 100644 frontend/server/apis/auth/.swagger-codegen/VERSION create mode 100644 frontend/server/apis/auth/api.ts create mode 100644 frontend/server/apis/auth/configuration.ts create mode 100644 frontend/server/apis/auth/custom.d.ts create mode 100644 frontend/server/apis/auth/git_push.sh create mode 100644 frontend/server/apis/auth/index.ts diff --git a/frontend/package.json b/frontend/package.json index f2cae0849b7..968cabd33d8 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -36,6 +36,7 @@ "apis:run": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/run.swagger.json -l typescript-fetch -o ./src/apis/run -c ./swagger-config.json", "apis:filter": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/filter.swagger.json -l typescript-fetch -o ./src/apis/filter -c ./swagger-config.json", "apis:visualization": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/visualization.swagger.json -l typescript-fetch -o ./src/apis/visualization -c ./swagger-config.json", + "apis:auth": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/auth.swagger.json -l typescript-fetch -o ./server/apis/auth -c ./swagger-config.json", "build": "npm run lint && EXTEND_ESLINT=true react-scripts build", "docker": "COMMIT_HASH=`git rev-parse HEAD`; docker build -q -t ml-pipelines-frontend:${COMMIT_HASH} --build-arg COMMIT_HASH=${COMMIT_HASH} --build-arg DATE=\"`date -u`\" -f Dockerfile ..", "eject": "react-scripts eject", diff --git a/frontend/server/apis/auth/.gitignore b/frontend/server/apis/auth/.gitignore new file mode 100644 index 00000000000..35e2fb2b02e --- /dev/null +++ b/frontend/server/apis/auth/.gitignore @@ -0,0 +1,3 @@ +wwwroot/*.js +node_modules +typings diff --git a/frontend/server/apis/auth/.swagger-codegen-ignore b/frontend/server/apis/auth/.swagger-codegen-ignore new file mode 100644 index 00000000000..c5fa491b4c5 --- /dev/null +++ b/frontend/server/apis/auth/.swagger-codegen-ignore @@ -0,0 +1,23 @@ +# Swagger Codegen Ignore +# Generated by swagger-codegen https://github.com/swagger-api/swagger-codegen + +# Use this file to prevent files from being overwritten by the generator. +# The patterns follow closely to .gitignore or .dockerignore. + +# As an example, the C# client generator defines ApiClient.cs. +# You can make changes and tell Swagger Codgen to ignore just this file by uncommenting the following line: +#ApiClient.cs + +# You can match any string of characters against a directory, file or extension with a single asterisk (*): +#foo/*/qux +# The above matches foo/bar/qux and foo/baz/qux, but not foo/bar/baz/qux + +# You can recursively match patterns against a directory, file or extension with a double asterisk (**): +#foo/**/qux +# This matches foo/bar/qux, foo/baz/qux, and foo/bar/baz/qux + +# You can also negate patterns with an exclamation (!). +# For example, you can ignore all files in a docs folder with the file extension .md: +#docs/*.md +# Then explicitly reverse the ignore rule for a single file: +#!docs/README.md diff --git a/frontend/server/apis/auth/.swagger-codegen/VERSION b/frontend/server/apis/auth/.swagger-codegen/VERSION new file mode 100644 index 00000000000..48a6b508dc9 --- /dev/null +++ b/frontend/server/apis/auth/.swagger-codegen/VERSION @@ -0,0 +1 @@ +2.4.7 \ No newline at end of file diff --git a/frontend/server/apis/auth/api.ts b/frontend/server/apis/auth/api.ts new file mode 100644 index 00000000000..06f7c76425b --- /dev/null +++ b/frontend/server/apis/auth/api.ts @@ -0,0 +1,319 @@ +/// +// tslint:disable +/** + * backend/api/auth.proto + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * OpenAPI spec version: version not set + * + * + * NOTE: This class is auto generated by the swagger code generator program. + * https://github.com/swagger-api/swagger-codegen.git + * Do not edit the class manually. + */ + +import * as url from 'url'; +import * as portableFetch from 'portable-fetch'; +import { Configuration } from './configuration'; + +const BASE_PATH = 'http://localhost'.replace(/\/+$/, ''); + +/** + * + * @export + */ +export const COLLECTION_FORMATS = { + csv: ',', + ssv: ' ', + tsv: '\t', + pipes: '|', +}; + +/** + * + * @export + * @interface FetchAPI + */ +export interface FetchAPI { + (url: string, init?: any): Promise; +} + +/** + * + * @export + * @interface FetchArgs + */ +export interface FetchArgs { + url: string; + options: any; +} + +/** + * + * @export + * @class BaseAPI + */ +export class BaseAPI { + protected configuration: Configuration; + + constructor( + configuration?: Configuration, + protected basePath: string = BASE_PATH, + protected fetch: FetchAPI = portableFetch, + ) { + if (configuration) { + this.configuration = configuration; + this.basePath = configuration.basePath || this.basePath; + } + } +} + +/** + * + * @export + * @class RequiredError + * @extends {Error} + */ +export class RequiredError extends Error { + name: 'RequiredError'; + constructor(public field: string, msg?: string) { + super(msg); + } +} + +/** + * + * @export + * @interface ApiStatus + */ +export interface ApiStatus { + /** + * + * @type {string} + * @memberof ApiStatus + */ + error?: string; + /** + * + * @type {number} + * @memberof ApiStatus + */ + code?: number; + /** + * + * @type {Array} + * @memberof ApiStatus + */ + details?: Array; +} + +/** + * Type of resources in pipelines system. + * @export + * @enum {string} + */ +export enum AuthorizeRequestResources { + UNASSIGNEDRESOURCES = 'UNASSIGNED_RESOURCES', + VIEWERS = 'VIEWERS', +} + +/** + * Type of verbs that act on the resources. + * @export + * @enum {string} + */ +export enum AuthorizeRequestVerb { + UNASSIGNEDVERB = 'UNASSIGNED_VERB', + CREATE = 'CREATE', + GET = 'GET', + DELETE = 'DELETE', +} + +/** + * `Any` contains an arbitrary serialized protocol buffer message along with a URL that describes the type of the serialized message. Protobuf library provides support to pack/unpack Any values in the form of utility functions or additional generated methods of the Any type. Example 1: Pack and unpack a message in C++. Foo foo = ...; Any any; any.PackFrom(foo); ... if (any.UnpackTo(&foo)) { ... } Example 2: Pack and unpack a message in Java. Foo foo = ...; Any any = Any.pack(foo); ... if (any.is(Foo.class)) { foo = any.unpack(Foo.class); } Example 3: Pack and unpack a message in Python. foo = Foo(...) any = Any() any.Pack(foo) ... if any.Is(Foo.DESCRIPTOR): any.Unpack(foo) ... Example 4: Pack and unpack a message in Go foo := &pb.Foo{...} any, err := ptypes.MarshalAny(foo) ... foo := &pb.Foo{} if err := ptypes.UnmarshalAny(any, foo); err != nil { ... } The pack methods provided by protobuf library will by default use 'type.googleapis.com/full.type.name' as the type URL and the unpack methods only use the fully qualified type name after the last '/' in the type URL, for example \"foo.bar.com/x/y.z\" will yield type name \"y.z\". JSON ==== The JSON representation of an `Any` value uses the regular representation of the deserialized, embedded message, with an additional field `@type` which contains the type URL. Example: package google.profile; message Person { string first_name = 1; string last_name = 2; } { \"@type\": \"type.googleapis.com/google.profile.Person\", \"firstName\": , \"lastName\": } If the embedded message type is well-known and has a custom JSON representation, that representation will be embedded adding a field `value` which holds the custom JSON in addition to the `@type` field. Example (for message [google.protobuf.Duration][]): { \"@type\": \"type.googleapis.com/google.protobuf.Duration\", \"value\": \"1.212s\" } + * @export + * @interface ProtobufAny + */ +export interface ProtobufAny { + /** + * A URL/resource name that uniquely identifies the type of the serialized protocol buffer message. The last segment of the URL's path must represent the fully qualified name of the type (as in `path/google.protobuf.Duration`). The name should be in a canonical form (e.g., leading \".\" is not accepted). In practice, teams usually precompile into the binary all types that they expect it to use in the context of Any. However, for URLs which use the scheme `http`, `https`, or no scheme, one can optionally set up a type server that maps type URLs to message definitions as follows: * If no scheme is provided, `https` is assumed. * An HTTP GET on the URL must yield a [google.protobuf.Type][] value in binary format, or produce an error. * Applications are allowed to cache lookup results based on the URL, or have them precompiled into a binary to avoid any lookup. Therefore, binary compatibility needs to be preserved on changes to types. (Use versioned type names to manage breaking changes.) Note: this functionality is not currently available in the official protobuf release, and it is not used for type URLs beginning with type.googleapis.com. Schemes other than `http`, `https` (or the empty scheme) might be used with implementation specific semantics. + * @type {string} + * @memberof ProtobufAny + */ + type_url?: string; + /** + * Must be a valid serialized protocol buffer of the above specified type. + * @type {string} + * @memberof ProtobufAny + */ + value?: string; +} + +/** + * AuthServiceApi - fetch parameter creator + * @export + */ +export const AuthServiceApiFetchParamCreator = function(configuration?: Configuration) { + return { + /** + * + * @param {string} [namespace] + * @param {'UNASSIGNED_RESOURCES' | 'VIEWERS'} [resources] + * @param {'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE'} [verb] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + authorize( + namespace?: string, + resources?: 'UNASSIGNED_RESOURCES' | 'VIEWERS', + verb?: 'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE', + options: any = {}, + ): FetchArgs { + const localVarPath = `/apis/v1beta1/auth`; + const localVarUrlObj = url.parse(localVarPath, true); + const localVarRequestOptions = Object.assign({ method: 'GET' }, options); + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + // authentication Bearer required + if (configuration && configuration.apiKey) { + const localVarApiKeyValue = + typeof configuration.apiKey === 'function' + ? configuration.apiKey('authorization') + : configuration.apiKey; + localVarHeaderParameter['authorization'] = localVarApiKeyValue; + } + + if (namespace !== undefined) { + localVarQueryParameter['namespace'] = namespace; + } + + if (resources !== undefined) { + localVarQueryParameter['resources'] = resources; + } + + if (verb !== undefined) { + localVarQueryParameter['verb'] = verb; + } + + localVarUrlObj.query = Object.assign( + {}, + localVarUrlObj.query, + localVarQueryParameter, + options.query, + ); + // fix override query string Detail: https://stackoverflow.com/a/7517673/1077943 + delete localVarUrlObj.search; + localVarRequestOptions.headers = Object.assign({}, localVarHeaderParameter, options.headers); + + return { + url: url.format(localVarUrlObj), + options: localVarRequestOptions, + }; + }, + }; +}; + +/** + * AuthServiceApi - functional programming interface + * @export + */ +export const AuthServiceApiFp = function(configuration?: Configuration) { + return { + /** + * + * @param {string} [namespace] + * @param {'UNASSIGNED_RESOURCES' | 'VIEWERS'} [resources] + * @param {'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE'} [verb] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + authorize( + namespace?: string, + resources?: 'UNASSIGNED_RESOURCES' | 'VIEWERS', + verb?: 'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE', + options?: any, + ): (fetch?: FetchAPI, basePath?: string) => Promise { + const localVarFetchArgs = AuthServiceApiFetchParamCreator(configuration).authorize( + namespace, + resources, + verb, + options, + ); + return (fetch: FetchAPI = portableFetch, basePath: string = BASE_PATH) => { + return fetch(basePath + localVarFetchArgs.url, localVarFetchArgs.options).then(response => { + if (response.status >= 200 && response.status < 300) { + return response.json(); + } else { + throw response; + } + }); + }; + }, + }; +}; + +/** + * AuthServiceApi - factory interface + * @export + */ +export const AuthServiceApiFactory = function( + configuration?: Configuration, + fetch?: FetchAPI, + basePath?: string, +) { + return { + /** + * + * @param {string} [namespace] + * @param {'UNASSIGNED_RESOURCES' | 'VIEWERS'} [resources] + * @param {'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE'} [verb] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + authorize( + namespace?: string, + resources?: 'UNASSIGNED_RESOURCES' | 'VIEWERS', + verb?: 'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE', + options?: any, + ) { + return AuthServiceApiFp(configuration).authorize( + namespace, + resources, + verb, + options, + )(fetch, basePath); + }, + }; +}; + +/** + * AuthServiceApi - object-oriented interface + * @export + * @class AuthServiceApi + * @extends {BaseAPI} + */ +export class AuthServiceApi extends BaseAPI { + /** + * + * @param {string} [namespace] + * @param {'UNASSIGNED_RESOURCES' | 'VIEWERS'} [resources] + * @param {'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE'} [verb] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof AuthServiceApi + */ + public authorize( + namespace?: string, + resources?: 'UNASSIGNED_RESOURCES' | 'VIEWERS', + verb?: 'UNASSIGNED_VERB' | 'CREATE' | 'GET' | 'DELETE', + options?: any, + ) { + return AuthServiceApiFp(this.configuration).authorize( + namespace, + resources, + verb, + options, + )(this.fetch, this.basePath); + } +} diff --git a/frontend/server/apis/auth/configuration.ts b/frontend/server/apis/auth/configuration.ts new file mode 100644 index 00000000000..4bca8b9d672 --- /dev/null +++ b/frontend/server/apis/auth/configuration.ts @@ -0,0 +1,65 @@ +// tslint:disable +/** + * backend/api/auth.proto + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * OpenAPI spec version: version not set + * + * + * NOTE: This class is auto generated by the swagger code generator program. + * https://github.com/swagger-api/swagger-codegen.git + * Do not edit the class manually. + */ + +export interface ConfigurationParameters { + apiKey?: string | ((name: string) => string); + username?: string; + password?: string; + accessToken?: string | ((name: string, scopes?: string[]) => string); + basePath?: string; +} + +export class Configuration { + /** + * parameter for apiKey security + * @param name security name + * @memberof Configuration + */ + apiKey?: string | ((name: string) => string); + /** + * parameter for basic security + * + * @type {string} + * @memberof Configuration + */ + username?: string; + /** + * parameter for basic security + * + * @type {string} + * @memberof Configuration + */ + password?: string; + /** + * parameter for oauth2 security + * @param name security name + * @param scopes oauth2 scope + * @memberof Configuration + */ + accessToken?: string | ((name: string, scopes?: string[]) => string); + /** + * override base path + * + * @type {string} + * @memberof Configuration + */ + basePath?: string; + + constructor(param: ConfigurationParameters = {}) { + this.apiKey = param.apiKey; + this.username = param.username; + this.password = param.password; + this.accessToken = param.accessToken; + this.basePath = param.basePath; + } +} diff --git a/frontend/server/apis/auth/custom.d.ts b/frontend/server/apis/auth/custom.d.ts new file mode 100644 index 00000000000..4c611cc3216 --- /dev/null +++ b/frontend/server/apis/auth/custom.d.ts @@ -0,0 +1,2 @@ +declare module 'portable-fetch'; +declare module 'url'; diff --git a/frontend/server/apis/auth/git_push.sh b/frontend/server/apis/auth/git_push.sh new file mode 100644 index 00000000000..a1ff4c9bcba --- /dev/null +++ b/frontend/server/apis/auth/git_push.sh @@ -0,0 +1,51 @@ +#!/bin/sh +# ref: https://help.github.com/articles/adding-an-existing-project-to-github-using-the-command-line/ +# +# Usage example: /bin/sh ./git_push.sh wing328 swagger-petstore-perl "minor update" + +git_user_id=$1 +git_repo_id=$2 +release_note=$3 + +if [ "$git_user_id" = "" ]; then + git_user_id="GIT_USER_ID" + echo "[INFO] No command line input provided. Set \$git_user_id to $git_user_id" +fi + +if [ "$git_repo_id" = "" ]; then + git_repo_id="GIT_REPO_ID" + echo "[INFO] No command line input provided. Set \$git_repo_id to $git_repo_id" +fi + +if [ "$release_note" = "" ]; then + release_note="Minor update" + echo "[INFO] No command line input provided. Set \$release_note to $release_note" +fi + +# Initialize the local directory as a Git repository +git init + +# Adds the files in the local repository and stages them for commit. +git add . + +# Commits the tracked changes and prepares them to be pushed to a remote repository. +git commit -m "$release_note" + +# Sets the new remote +git_remote=`git remote` +if [ "$git_remote" = "" ]; then # git remote not defined + + if [ "$GIT_TOKEN" = "" ]; then + echo "[INFO] \$GIT_TOKEN (environment variable) is not set. Using the git credential in your environment." + git remote add origin https://github.com/${git_user_id}/${git_repo_id}.git + else + git remote add origin https://${git_user_id}:${GIT_TOKEN}@github.com/${git_user_id}/${git_repo_id}.git + fi + +fi + +git pull origin master + +# Pushes (Forces) the changes in the local repository up to the remote repository +echo "Git pushing to https://github.com/${git_user_id}/${git_repo_id}.git" +git push origin master 2>&1 | grep -v 'To https' diff --git a/frontend/server/apis/auth/index.ts b/frontend/server/apis/auth/index.ts new file mode 100644 index 00000000000..4f6fe295dd6 --- /dev/null +++ b/frontend/server/apis/auth/index.ts @@ -0,0 +1,15 @@ +// tslint:disable +/** + * backend/api/auth.proto + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * OpenAPI spec version: version not set + * + * + * NOTE: This class is auto generated by the swagger code generator program. + * https://github.com/swagger-api/swagger-codegen.git + * Do not edit the class manually. + */ + +export * from './api'; +export * from './configuration'; diff --git a/frontend/server/package-lock.json b/frontend/server/package-lock.json index 5998b4a5786..e1f76415786 100644 --- a/frontend/server/package-lock.json +++ b/frontend/server/package-lock.json @@ -7046,6 +7046,12 @@ "resolved": "https://registry.npmjs.org/prelude-ls/-/prelude-ls-1.1.2.tgz", "integrity": "sha1-IZMqVJ9eUv/ZqCf1cOBL5iqX2lQ=" }, + "prettier": { + "version": "1.19.1", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-1.19.1.tgz", + "integrity": "sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew==", + "dev": true + }, "pretty-format": { "version": "24.9.0", "resolved": "https://registry.npmjs.org/pretty-format/-/pretty-format-24.9.0.tgz", diff --git a/frontend/server/package.json b/frontend/server/package.json index 61ed9c6a562..e7da6887d1e 100644 --- a/frontend/server/package.json +++ b/frontend/server/package.json @@ -28,6 +28,7 @@ "@types/tar": "^4.0.3", "@types/tar-stream": "^1.6.1", "jest": "^25.3.0", + "prettier": "1.19.1", "supertest": "^4.0.2", "ts-jest": "^25.2.1", "tslint": "^5.20.1", From dd0d66f24b2b7ae763f1930417c19468d92fcb7b Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Mon, 27 Apr 2020 16:28:38 +0800 Subject: [PATCH 05/17] Authorization checks for tensorboard apis --- frontend/package.json | 4 +- frontend/server/app.ts | 24 ++-- frontend/server/configs.ts | 9 ++ frontend/server/handlers/tensorboard.ts | 126 ++++++++++-------- .../{ => src/generated}/apis/auth/.gitignore | 0 .../apis/auth/.swagger-codegen-ignore | 0 .../apis/auth/.swagger-codegen/VERSION | 0 .../{ => src/generated}/apis/auth/api.ts | 0 .../generated}/apis/auth/configuration.ts | 0 .../{ => src/generated}/apis/auth/custom.d.ts | 0 .../{ => src/generated}/apis/auth/git_push.sh | 0 .../{ => src/generated}/apis/auth/index.ts | 0 frontend/server/tsconfig.json | 3 +- 13 files changed, 98 insertions(+), 68 deletions(-) rename frontend/server/{ => src/generated}/apis/auth/.gitignore (100%) rename frontend/server/{ => src/generated}/apis/auth/.swagger-codegen-ignore (100%) rename frontend/server/{ => src/generated}/apis/auth/.swagger-codegen/VERSION (100%) rename frontend/server/{ => src/generated}/apis/auth/api.ts (100%) rename frontend/server/{ => src/generated}/apis/auth/configuration.ts (100%) rename frontend/server/{ => src/generated}/apis/auth/custom.d.ts (100%) rename frontend/server/{ => src/generated}/apis/auth/git_push.sh (100%) rename frontend/server/{ => src/generated}/apis/auth/index.ts (100%) diff --git a/frontend/package.json b/frontend/package.json index 968cabd33d8..122fa637e66 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -29,14 +29,14 @@ }, "scripts": { "analyze-bundle": "node analyze_bundle.js", - "apis": "npm run apis:experiment && npm run apis:job && npm run apis:pipeline && npm run apis:run && npm run apis:filter && npm run apis:visualization", + "apis": "npm run apis:experiment && npm run apis:job && npm run apis:pipeline && npm run apis:run && npm run apis:filter && npm run apis:visualization && npm run apis:auth", "apis:experiment": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/experiment.swagger.json -l typescript-fetch -o ./src/apis/experiment -c ./swagger-config.json", "apis:job": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/job.swagger.json -l typescript-fetch -o ./src/apis/job -c ./swagger-config.json", "apis:pipeline": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/pipeline.swagger.json -l typescript-fetch -o ./src/apis/pipeline -c ./swagger-config.json", "apis:run": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/run.swagger.json -l typescript-fetch -o ./src/apis/run -c ./swagger-config.json", "apis:filter": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/filter.swagger.json -l typescript-fetch -o ./src/apis/filter -c ./swagger-config.json", "apis:visualization": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/visualization.swagger.json -l typescript-fetch -o ./src/apis/visualization -c ./swagger-config.json", - "apis:auth": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/auth.swagger.json -l typescript-fetch -o ./server/apis/auth -c ./swagger-config.json", + "apis:auth": "java -jar swagger-codegen-cli.jar generate -i ../backend/api/swagger/auth.swagger.json -l typescript-fetch -o ./server/src/generated/apis/auth -c ./swagger-config.json", "build": "npm run lint && EXTEND_ESLINT=true react-scripts build", "docker": "COMMIT_HASH=`git rev-parse HEAD`; docker build -q -t ml-pipelines-frontend:${COMMIT_HASH} --build-arg COMMIT_HASH=${COMMIT_HASH} --build-arg DATE=\"`date -u`\" -f Dockerfile ..", "eject": "react-scripts eject", diff --git a/frontend/server/app.ts b/frontend/server/app.ts index 0de7b614781..9a9e57c30b7 100644 --- a/frontend/server/app.ts +++ b/frontend/server/app.ts @@ -24,11 +24,7 @@ import { getArtifactsProxyHandler, getArtifactServiceGetter, } from './handlers/artifacts'; -import { - getCreateTensorboardHandler, - getTensorboardHandler, - deleteTensorboardHandler, -} from './handlers/tensorboard'; +import { getTensorboardHandlers } from './handlers/tensorboard'; import { getPodLogsHandler } from './handlers/pod-logs'; import { podInfoHandler, podEventsHandler } from './handlers/pod-info'; import { getClusterNameHandler, getProjectIdHandler } from './handlers/gke-metadata'; @@ -129,13 +125,17 @@ function createUIServer(options: UIConfigs) { registerHandler(app.get, '/artifacts/get', getArtifactsHandler(options.artifacts)); /** Tensorboard viewer */ - registerHandler(app.get, '/apps/tensorboard', getTensorboardHandler); - registerHandler(app.delete, '/apps/tensorboard', deleteTensorboardHandler); - registerHandler( - app.post, - '/apps/tensorboard', - getCreateTensorboardHandler(options.viewer.tensorboard), - ); + const { + get: tensorboardGetHandler, + create: tensorboardCreateHandler, + delete: tensorboardDeleteHandler, + } = getTensorboardHandlers(options.viewer.tensorboard, { + apiServerAddress, + authzEnabled: options.auth.enabled, + }); + registerHandler(app.get, '/apps/tensorboard', tensorboardGetHandler); + registerHandler(app.delete, '/apps/tensorboard', tensorboardDeleteHandler); + registerHandler(app.post, '/apps/tensorboard', tensorboardCreateHandler); /** Pod logs */ registerHandler(app.get, '/k8s/pod/logs', getPodLogsHandler(options.argo, options.artifacts)); diff --git a/frontend/server/configs.ts b/frontend/server/configs.ts index 4ee4767e737..e7bc7fe8bea 100644 --- a/frontend/server/configs.ts +++ b/frontend/server/configs.ts @@ -88,6 +88,8 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs { ARGO_ARCHIVE_PREFIX = 'logs', /** Disables GKE metadata endpoint. */ DISABLE_GKE_METADATA = 'false', + /** Enable authorization checks for multi user mode. */ + ENABLE_AUTHZ = 'false', /** Deployment type. */ DEPLOYMENT: DEPLOYMENT_STR = '', } = env; @@ -158,6 +160,9 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs { gkeMetadata: { disabled: asBool(DISABLE_GKE_METADATA), }, + auth: { + enabled: asBool(ENABLE_AUTHZ), + }, }; } @@ -216,6 +221,9 @@ export interface ServerConfigs { export interface GkeMetadataConfigs { disabled: boolean; } +export interface AuthorizationConfigs { + enabled: boolean; +} export interface UIConfigs { server: ServerConfigs; artifacts: { @@ -230,4 +238,5 @@ export interface UIConfigs { viewer: ViewerConfigs; pipeline: PipelineConfigs; gkeMetadata: GkeMetadataConfigs; + auth: AuthorizationConfigs; } diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index 9721bbe8749..a3a32ba83e6 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -14,40 +14,48 @@ import { Handler } from 'express'; import * as k8sHelper from '../k8s-helper'; import { ViewerTensorboardConfig } from '../configs'; +import { AuthServiceApi } from '../src/generated/apis/auth'; -/** - * A handler which retrieve the endpoint for a tensorboard instance. The - * handler expects a query string `logdir`. - */ -export const getTensorboardHandler: Handler = async (req, res) => { - const { logdir, namespace } = req.query; - if (!logdir) { - res.status(404).send('logdir argument is required'); - return; - } - if (!namespace) { - res.status(404).send('namespace argument is required'); - return; - } +export const getTensorboardHandlers = ( + tensorboardConfig: ViewerTensorboardConfig, + otherConfig: { apiServerAddress: string; authzEnabled: boolean }, +): { get: Handler; create: Handler; delete: Handler } => { + const { apiServerAddress, authzEnabled } = otherConfig; + const authService = new AuthServiceApi({ basePath: apiServerAddress }); + /** + * A handler which retrieve the endpoint for a tensorboard instance. The + * handler expects a query string `logdir`. + */ + const get: Handler = async (req, res) => { + const { logdir, namespace } = req.query; + if (!logdir) { + res.status(404).send('logdir argument is required'); + return; + } + if (!namespace) { + res.status(404).send('namespace argument is required'); + return; + } - try { - res.send(await k8sHelper.getTensorboardInstance(logdir, namespace)); - } catch (err) { - console.error('Failed to list Tensorboard pods: ', err?.body || err); - res.status(500).send(`Failed to list Tensorboard pods: ${err}`); - } -}; + try { + if (authzEnabled) { + await authService.authorize(namespace, 'VIEWERS', 'GET'); + } + res.send(await k8sHelper.getTensorboardInstance(logdir, namespace)); + } catch (err) { + console.error('Failed to list Tensorboard pods: ', err?.body || err); + res.status(500).send(`Failed to list Tensorboard pods: ${err}`); + } + }; -/** - * Returns a handler which will create a tensorboard viewer CRD, waits for the - * tensorboard instance to be ready, and return the endpoint to the instance. - * The handler expects the following query strings in the request: - * - `logdir` - * - `tfversion` - * @param tensorboardConfig The configuration for Tensorboard. - */ -export function getCreateTensorboardHandler(tensorboardConfig: ViewerTensorboardConfig): Handler { - return async (req, res) => { + /** + * A handler which will create a tensorboard viewer CRD, waits for the + * tensorboard instance to be ready, and return the endpoint to the instance. + * The handler expects the following query strings in the request: + * - `logdir` + * - `tfversion` + */ + const create: Handler = async (req, res) => { const { logdir, namespace, tfversion } = req.query; if (!logdir) { res.status(404).send('logdir argument is required'); @@ -63,6 +71,9 @@ export function getCreateTensorboardHandler(tensorboardConfig: ViewerTensorboard } try { + if (authzEnabled) { + await authService.authorize(namespace, 'VIEWERS', 'CREATE'); + } await k8sHelper.newTensorboardInstance( logdir, namespace, @@ -81,28 +92,37 @@ export function getCreateTensorboardHandler(tensorboardConfig: ViewerTensorboard res.status(500).send(`Failed to start Tensorboard app: ${err}`); } }; -} -/** - * A handler that deletes a tensorboard viewer. The handler expects query string - * `logdir` in the request. - */ -export const deleteTensorboardHandler: Handler = async (req, res) => { - const { logdir, namespace } = req.query; - if (!logdir) { - res.status(404).send('logdir argument is required'); - return; - } - if (!namespace) { - res.status(404).send('namespace argument is required'); - return; - } + /** + * A handler that deletes a tensorboard viewer. The handler expects query string + * `logdir` in the request. + */ + const deleteHandler: Handler = async (req, res) => { + const { logdir, namespace } = req.query; + if (!logdir) { + res.status(404).send('logdir argument is required'); + return; + } + if (!namespace) { + res.status(404).send('namespace argument is required'); + return; + } - try { - await k8sHelper.deleteTensorboardInstance(logdir, namespace); - res.send('Tensorboard deleted.'); - } catch (err) { - console.error('Failed to delete Tensorboard app: ', err?.body || err); - res.status(500).send(`Failed to delete Tensorboard app: ${err}`); - } + try { + if (authzEnabled) { + await authService.authorize(namespace, 'VIEWERS', 'DELETE'); + } + await k8sHelper.deleteTensorboardInstance(logdir, namespace); + res.send('Tensorboard deleted.'); + } catch (err) { + console.error('Failed to delete Tensorboard app: ', err?.body || err); + res.status(500).send(`Failed to delete Tensorboard app: ${err}`); + } + }; + + return { + get, + create, + delete: deleteHandler, + }; }; diff --git a/frontend/server/apis/auth/.gitignore b/frontend/server/src/generated/apis/auth/.gitignore similarity index 100% rename from frontend/server/apis/auth/.gitignore rename to frontend/server/src/generated/apis/auth/.gitignore diff --git a/frontend/server/apis/auth/.swagger-codegen-ignore b/frontend/server/src/generated/apis/auth/.swagger-codegen-ignore similarity index 100% rename from frontend/server/apis/auth/.swagger-codegen-ignore rename to frontend/server/src/generated/apis/auth/.swagger-codegen-ignore diff --git a/frontend/server/apis/auth/.swagger-codegen/VERSION b/frontend/server/src/generated/apis/auth/.swagger-codegen/VERSION similarity index 100% rename from frontend/server/apis/auth/.swagger-codegen/VERSION rename to frontend/server/src/generated/apis/auth/.swagger-codegen/VERSION diff --git a/frontend/server/apis/auth/api.ts b/frontend/server/src/generated/apis/auth/api.ts similarity index 100% rename from frontend/server/apis/auth/api.ts rename to frontend/server/src/generated/apis/auth/api.ts diff --git a/frontend/server/apis/auth/configuration.ts b/frontend/server/src/generated/apis/auth/configuration.ts similarity index 100% rename from frontend/server/apis/auth/configuration.ts rename to frontend/server/src/generated/apis/auth/configuration.ts diff --git a/frontend/server/apis/auth/custom.d.ts b/frontend/server/src/generated/apis/auth/custom.d.ts similarity index 100% rename from frontend/server/apis/auth/custom.d.ts rename to frontend/server/src/generated/apis/auth/custom.d.ts diff --git a/frontend/server/apis/auth/git_push.sh b/frontend/server/src/generated/apis/auth/git_push.sh similarity index 100% rename from frontend/server/apis/auth/git_push.sh rename to frontend/server/src/generated/apis/auth/git_push.sh diff --git a/frontend/server/apis/auth/index.ts b/frontend/server/src/generated/apis/auth/index.ts similarity index 100% rename from frontend/server/apis/auth/index.ts rename to frontend/server/src/generated/apis/auth/index.ts diff --git a/frontend/server/tsconfig.json b/frontend/server/tsconfig.json index 25e4d86db29..2b47c6aad8a 100644 --- a/frontend/server/tsconfig.json +++ b/frontend/server/tsconfig.json @@ -21,7 +21,8 @@ "strictNullChecks": true, "suppressImplicitAnyIndexErrors": true, "skipLibCheck": true, + "strictPropertyInitialization": false, // Workaround: swagger generated client breaks this. "strict": true }, - "exclude": ["dist", "coverage", "node_modules"] + "exclude": ["dist", "coverage", "node_modules", "src/generated"] } From f94f9380965c83878385f807df9df54df377e97f Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Mon, 27 Apr 2020 22:53:41 +0800 Subject: [PATCH 06/17] UI Server authorization checks --- frontend/package.json | 2 + frontend/scripts/start-proxy-and-server.sh | 6 +- frontend/server/handlers/tensorboard.ts | 29 ++++--- frontend/server/utils.ts | 77 ++++++++++++++++++ .../src/components/viewers/Tensorboard.tsx | 81 ++++++++++++------- frontend/src/index.tsx | 2 +- 6 files changed, 156 insertions(+), 41 deletions(-) diff --git a/frontend/package.json b/frontend/package.json index 122fa637e66..4329dd228cc 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -46,9 +46,11 @@ "lint": "eslint --ext js,ts,tsx src", "mock:api": "ts-node-dev -O '{\"module\": \"commonjs\"}' --project mock-backend/tsconfig.json mock-backend/mock-api-server.ts 3001", "mock:server": "node server/dist/server.js build", + "mock:server:inspect": "node inspect server/dist/server.js build", "postinstall": "cd ./server && npm ci && cd ../mock-backend && npm ci", "start:proxy": "./scripts/start-proxy.sh", "start:proxy-and-server": "./scripts/start-proxy-and-server.sh", + "start:proxy-and-server-inspect": "./scripts/start-proxy-and-server.sh --inspect", "start": "EXTEND_ESLINT=true react-scripts start", "sync-backend-sample-config": "node scripts/sync-backend-sample-config.js", "test": "react-scripts test", diff --git a/frontend/scripts/start-proxy-and-server.sh b/frontend/scripts/start-proxy-and-server.sh index 3ab86774f0e..960db90111f 100755 --- a/frontend/scripts/start-proxy-and-server.sh +++ b/frontend/scripts/start-proxy-and-server.sh @@ -37,4 +37,8 @@ kubectl port-forward -n $NAMESPACE svc/ml-pipeline 3002:8888 & kubectl port-forward -n $NAMESPACE svc/minio-service 9000:9000 & export MINIO_HOST=localhost export MINIO_NAMESPACE= -ML_PIPELINE_SERVICE_PORT=3002 npm run mock:server 3001 +if [ "$1" == "--inspect" ]; then + ML_PIPELINE_SERVICE_PORT=3002 npm run mock:server:inspect 3001 +else + ML_PIPELINE_SERVICE_PORT=3002 npm run mock:server 3001 +fi diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index a3a32ba83e6..3e8ab0f6fd3 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -15,13 +15,16 @@ import { Handler } from 'express'; import * as k8sHelper from '../k8s-helper'; import { ViewerTensorboardConfig } from '../configs'; import { AuthServiceApi } from '../src/generated/apis/auth'; +import 'portable-fetch'; +import { parseError } from '../utils'; export const getTensorboardHandlers = ( tensorboardConfig: ViewerTensorboardConfig, otherConfig: { apiServerAddress: string; authzEnabled: boolean }, ): { get: Handler; create: Handler; delete: Handler } => { const { apiServerAddress, authzEnabled } = otherConfig; - const authService = new AuthServiceApi({ basePath: apiServerAddress }); + console.log('api server address ' + apiServerAddress); + const authService = new AuthServiceApi({ basePath: apiServerAddress }, undefined, fetch); /** * A handler which retrieve the endpoint for a tensorboard instance. The * handler expects a query string `logdir`. @@ -39,12 +42,14 @@ export const getTensorboardHandlers = ( try { if (authzEnabled) { - await authService.authorize(namespace, 'VIEWERS', 'GET'); + await authService.authorize(namespace, 'VIEWERS', 'GET', { headers: req.headers }); + console.debug(`Authorized to ${'GET'} ${'VIEWERS'} in namespace ${namespace}.`); } res.send(await k8sHelper.getTensorboardInstance(logdir, namespace)); } catch (err) { - console.error('Failed to list Tensorboard pods: ', err?.body || err); - res.status(500).send(`Failed to list Tensorboard pods: ${err}`); + const details = await parseError(err); + console.error(`Failed to list Tensorboard pods: ${details.message}`, details.additionalInfo); + res.status(500).send(`Failed to list Tensorboard pods: ${details.message}`); } }; @@ -72,7 +77,8 @@ export const getTensorboardHandlers = ( try { if (authzEnabled) { - await authService.authorize(namespace, 'VIEWERS', 'CREATE'); + await authService.authorize(namespace, 'VIEWERS', 'CREATE', { headers: req.headers }); + console.debug(`Authorized to ${'CREATE'} ${'VIEWERS'} in namespace ${namespace}.`); } await k8sHelper.newTensorboardInstance( logdir, @@ -88,8 +94,9 @@ export const getTensorboardHandlers = ( ); res.send(tensorboardAddress); } catch (err) { - console.error('Failed to start Tensorboard app: ', err?.body || err); - res.status(500).send(`Failed to start Tensorboard app: ${err}`); + const details = await parseError(err); + console.error(`Failed to start Tensorboard app: ${details.message}`, details.additionalInfo); + res.status(500).send(`Failed to start Tensorboard app: ${details.message}`); } }; @@ -110,13 +117,15 @@ export const getTensorboardHandlers = ( try { if (authzEnabled) { - await authService.authorize(namespace, 'VIEWERS', 'DELETE'); + await authService.authorize(namespace, 'VIEWERS', 'DELETE', { headers: req.headers }); + console.debug(`Authorized to ${'DELETE'} ${'VIEWERS'} in namespace ${namespace}.`); } await k8sHelper.deleteTensorboardInstance(logdir, namespace); res.send('Tensorboard deleted.'); } catch (err) { - console.error('Failed to delete Tensorboard app: ', err?.body || err); - res.status(500).send(`Failed to delete Tensorboard app: ${err}`); + const details = await parseError(err); + console.error(`Failed to delete Tensorboard app: ${details.message}`, details.additionalInfo); + res.status(500).send(`Failed to delete Tensorboard app: ${details.message}`); } }; diff --git a/frontend/server/utils.ts b/frontend/server/utils.ts index 746f35e9fa4..c1b630b114a 100644 --- a/frontend/server/utils.ts +++ b/frontend/server/utils.ts @@ -94,3 +94,80 @@ export class PreviewStream extends Transform { super({ ...opts, transform }); } } + +export interface ErrorDetails { + message: string; + additionalInfo: any; +} +export const UNKOWN_ERROR: ErrorDetails = { + message: 'Unknown error', + additionalInfo: 'Unknown error', +}; +export async function parseError(error: any): Promise { + return ( + parseGenericError(error) || + (await parseKfpApiError(error)) || + parseK8sError(error) || + UNKOWN_ERROR + ); +} + +function parseGenericError(error: any): ErrorDetails | undefined { + if (!error) { + return undefined; + } else if (typeof error === 'string') { + return { + message: error, + additionalInfo: error, + }; + } else if (error instanceof Error) { + return { message: error.message, additionalInfo: error }; + } + // Cannot understand error type + return undefined; +} +async function parseKfpApiError(error: any): Promise { + if (!error || !error.json || typeof error.json !== 'function') { + return undefined; + } + try { + const json = await error.json(); + const { error: message, details } = json; + if (message && details && typeof message === 'string' && typeof details === 'object') { + return { + message, + additionalInfo: details, + }; + } else { + return undefined; + } + } catch (err) { + return undefined; + } +} +function parseK8sError(error: any): ErrorDetails | undefined { + if (!error || !error.body || typeof error.body !== 'object') { + return undefined; + } + + if (typeof error.body.message !== 'string') { + return undefined; + } + + // Kubernetes client http error has body with all the info. + // Example error.body + // { + // kind: 'Status', + // apiVersion: 'v1', + // metadata: {}, + // status: 'Failure', + // message: 'pods "test-pod" not found', + // reason: 'NotFound', + // details: { name: 'test-pod', kind: 'pods' }, + // code: 404 + // } + return { + message: error.body.message, + additionalInfo: error.body, + }; +} diff --git a/frontend/src/components/viewers/Tensorboard.tsx b/frontend/src/components/viewers/Tensorboard.tsx index a1739f83da4..c5774aae4f9 100644 --- a/frontend/src/components/viewers/Tensorboard.tsx +++ b/frontend/src/components/viewers/Tensorboard.tsx @@ -19,7 +19,7 @@ import BusyButton from '../../atoms/BusyButton'; import Button from '@material-ui/core/Button'; import Viewer, { ViewerConfig } from './Viewer'; import { Apis } from '../../lib/Apis'; -import { commonCss, padding } from '../../Css'; +import { commonCss, padding, color } from '../../Css'; import InputLabel from '@material-ui/core/InputLabel'; import Input from '@material-ui/core/Input'; import MenuItem from '@material-ui/core/MenuItem'; @@ -47,6 +47,12 @@ export const css = stylesheet({ shortButton: { width: 50, }, + warningText: { + color: color.warningText, + }, + errorText: { + color: color.errorText, + }, }); export interface TensorboardViewerConfig extends ViewerConfig { @@ -67,6 +73,7 @@ interface TensorboardViewerState { tensorflowVersion: string; // When podAddress is not null, we need to further tell whether the TensorBoard pod is accessible or not tensorboardReady: boolean; + errorMessage?: string; } // TODO(jingzhang36): we'll later parse Tensorboard version from mlpipeline-ui-metadata.json file. @@ -84,6 +91,7 @@ class TensorboardViewer extends Viewer + {this.state.errorMessage &&
{this.state.errorMessage}
} {this.state.podAddress && (
Tensorboard is starting, and you may need to wait for a few minutes.
+
+ Tensorboard is starting, and you may need to wait for a few minutes. +
)} @@ -264,44 +275,56 @@ class TensorboardViewer extends Viewer { this.setState({ busy: true }, async () => { - const { podAddress, tfVersion } = await Apis.getTensorboardApp( - this._buildUrl(), - this._getNamespace(), - ); - if (podAddress) { - this.setState({ busy: false, podAddress, tensorflowVersion: tfVersion }); - } else { - // No existing pod - this.setState({ busy: false }); + try { + const { podAddress, tfVersion } = await Apis.getTensorboardApp( + this._buildUrl(), + this._getNamespace(), + ); + if (podAddress) { + this.setState({ busy: false, podAddress, tensorflowVersion: tfVersion }); + } else { + // No existing pod + this.setState({ busy: false }); + } + } catch (err) { + this.setState({ busy: false, errorMessage: err?.message || 'Unknown error' }); } }); } private _startTensorboard = async () => { - this.setState({ busy: true }, async () => { - await Apis.startTensorboardApp( - this._buildUrl(), - this.state.tensorflowVersion, - this._getNamespace(), - ); - this.setState({ busy: false, tensorboardReady: false }, () => { - this._checkTensorboardApp(); - }); + this.setState({ busy: true, errorMessage: undefined }, async () => { + try { + await Apis.startTensorboardApp( + this._buildUrl(), + this.state.tensorflowVersion, + this._getNamespace(), + ); + this.setState({ busy: false, tensorboardReady: false }, () => { + this._checkTensorboardApp(); + }); + } catch (err) { + this.setState({ busy: false, errorMessage: err?.message || 'Unknown error' }); + } }); }; private _deleteTensorboard = async () => { // delete the already opened Tensorboard, clear the podAddress recorded in frontend, // and return to the select & start tensorboard page - this.setState({ busy: true }, async () => { - await Apis.deleteTensorboardApp(this._buildUrl(), this._getNamespace()); - this.setState({ - busy: false, - deleteDialogOpen: false, - podAddress: '', - tensorflowVersion: DEFAULT_TENSORBOARD_VERSION, - tensorboardReady: false, - }); + this.setState({ busy: true, errorMessage: undefined }, async () => { + try { + await Apis.deleteTensorboardApp(this._buildUrl(), this._getNamespace()); + this.setState({ + busy: false, + deleteDialogOpen: false, + podAddress: '', + tensorflowVersion: DEFAULT_TENSORBOARD_VERSION, + tensorboardReady: false, + }); + } catch (err) { + this.setState({ busy: false, errorMessage: err?.message || 'Unknown error' }); + } }); }; } diff --git a/frontend/src/index.tsx b/frontend/src/index.tsx index 9236a930a2d..694c4dc5f2f 100644 --- a/frontend/src/index.tsx +++ b/frontend/src/index.tsx @@ -60,7 +60,7 @@ ReactDOM.render( {app} ) : ( // Uncomment the following for namespace switch during development. - // {app} + // {app} {app} ), document.getElementById('root'), From 01a02a2a7e5cad1fd33ef84d726aacdea1c84f4f Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 11:08:46 +0800 Subject: [PATCH 07/17] Clean up error parsing --- frontend/server/k8s-helper.ts | 47 +++-------------------------------- frontend/server/utils.ts | 6 +++-- 2 files changed, 7 insertions(+), 46 deletions(-) diff --git a/frontend/server/k8s-helper.ts b/frontend/server/k8s-helper.ts index 3d5abb4ab7e..156197a20f8 100644 --- a/frontend/server/k8s-helper.ts +++ b/frontend/server/k8s-helper.ts @@ -23,6 +23,7 @@ import { import * as crypto from 'crypto-js'; import * as fs from 'fs'; import { PartialArgoWorkflow } from './workflow-helper'; +import { parseError } from './utils'; // If this is running inside a k8s Pod, its namespace should be written at this // path, this is also how we can tell whether we're running in the cluster. @@ -233,7 +234,7 @@ export async function getPod( const { body } = await k8sV1Client.readNamespacedPod(podName, podNamespace); return [body, undefined]; } catch (error) { - const { message, additionalInfo } = parseK8sError(error); + const { message, additionalInfo } = await parseError(error); const userMessage = `Could not get pod ${podName} in namespace ${podNamespace}: ${message}`; return [undefined, { message: userMessage, additionalInfo }]; } @@ -258,7 +259,7 @@ export async function listPodEvents( ); return [body, undefined]; } catch (error) { - const { message, additionalInfo } = parseK8sError(error); + const { message, additionalInfo } = await parseError(error); const userMessage = `Error when listing pod events for pod "${podName}" in "${podNamespace}" namespace: ${message}`; return [undefined, { message: userMessage, additionalInfo }]; } @@ -307,48 +308,6 @@ export async function getK8sSecret(name: string, key: string) { return buff.toString('ascii'); } -const UNKOWN_ERROR = { - message: 'Unknown error', - additionalInfo: 'Unknown error', -}; -function parseK8sError(error: any): { message: string; additionalInfo: any } { - try { - if (!error) { - return UNKOWN_ERROR; - } else if (typeof error === 'string') { - return { - message: error, - additionalInfo: error, - }; - } else if (error.body) { - // Kubernetes client http error has body with all the info. - // Example error.body - // { - // kind: 'Status', - // apiVersion: 'v1', - // metadata: {}, - // status: 'Failure', - // message: 'pods "test-pod" not found', - // reason: 'NotFound', - // details: { name: 'test-pod', kind: 'pods' }, - // code: 404 - // } - return { - message: error.body.message || UNKOWN_ERROR.message, - additionalInfo: error.body, - }; - } else { - return { - message: error.message || UNKOWN_ERROR.message, - additionalInfo: error, - }; - } - } catch (parsingError) { - console.error('There was a parsing error: ', parsingError); - return UNKOWN_ERROR; - } -} - export const TEST_ONLY = { k8sV1Client, k8sV1CustomObjectClient, diff --git a/frontend/server/utils.ts b/frontend/server/utils.ts index c1b630b114a..43f228ca004 100644 --- a/frontend/server/utils.ts +++ b/frontend/server/utils.ts @@ -105,9 +105,9 @@ export const UNKOWN_ERROR: ErrorDetails = { }; export async function parseError(error: any): Promise { return ( - parseGenericError(error) || - (await parseKfpApiError(error)) || parseK8sError(error) || + (await parseKfpApiError(error)) || + parseGenericError(error) || UNKOWN_ERROR ); } @@ -122,6 +122,8 @@ function parseGenericError(error: any): ErrorDetails | undefined { }; } else if (error instanceof Error) { return { message: error.message, additionalInfo: error }; + } else if (error.message && typeof error.message === 'string') { + return { message: error.message, additionalInfo: error }; } // Cannot understand error type return undefined; From 2c30e154bf0b393c5ed77a25244ef981afda6e23 Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 11:19:57 +0800 Subject: [PATCH 08/17] Revert changes --- frontend/src/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/index.tsx b/frontend/src/index.tsx index 694c4dc5f2f..9236a930a2d 100644 --- a/frontend/src/index.tsx +++ b/frontend/src/index.tsx @@ -60,7 +60,7 @@ ReactDOM.render( {app} ) : ( // Uncomment the following for namespace switch during development. - // {app} + // {app} {app} ), document.getElementById('root'), From 223e4c2464d289308434d32eab2ace803089a4cf Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 13:08:39 +0800 Subject: [PATCH 09/17] Fix portable-fetch not found bug --- frontend/server/handlers/tensorboard.ts | 4 ++-- frontend/server/tsconfig.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index 3e8ab0f6fd3..142abd2b309 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -15,7 +15,7 @@ import { Handler } from 'express'; import * as k8sHelper from '../k8s-helper'; import { ViewerTensorboardConfig } from '../configs'; import { AuthServiceApi } from '../src/generated/apis/auth'; -import 'portable-fetch'; +import fetch from 'node-fetch'; import { parseError } from '../utils'; export const getTensorboardHandlers = ( @@ -24,7 +24,7 @@ export const getTensorboardHandlers = ( ): { get: Handler; create: Handler; delete: Handler } => { const { apiServerAddress, authzEnabled } = otherConfig; console.log('api server address ' + apiServerAddress); - const authService = new AuthServiceApi({ basePath: apiServerAddress }, undefined, fetch); + const authService = new AuthServiceApi({ basePath: apiServerAddress }, undefined, fetch as any); /** * A handler which retrieve the endpoint for a tensorboard instance. The * handler expects a query string `logdir`. diff --git a/frontend/server/tsconfig.json b/frontend/server/tsconfig.json index 2b47c6aad8a..55c9190fab4 100644 --- a/frontend/server/tsconfig.json +++ b/frontend/server/tsconfig.json @@ -24,5 +24,5 @@ "strictPropertyInitialization": false, // Workaround: swagger generated client breaks this. "strict": true }, - "exclude": ["dist", "coverage", "node_modules", "src/generated"] + "exclude": ["dist", "coverage", "node_modules"] } From e9c71587f1a509bc198d7fe75794403d0fef171b Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 13:10:41 +0800 Subject: [PATCH 10/17] Fix unit test --- frontend/server/app.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts index c744597db59..4152967f835 100644 --- a/frontend/server/app.test.ts +++ b/frontend/server/app.test.ts @@ -853,7 +853,7 @@ describe('UIServer apis', () => { ) .expect( 500, - `Failed to start Tensorboard app: Error: There's already an existing tensorboard instance with a different version 2.1.0`, + `Failed to start Tensorboard app: There's already an existing tensorboard instance with a different version 2.1.0`, err => { expect(errorSpy).toHaveBeenCalledTimes(1); done(err); From e97bff2bd6ac0ba14a15fe6c087423304f78def1 Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 13:31:45 +0800 Subject: [PATCH 11/17] Include portable-fetch required by api client --- frontend/server/package-lock.json | 36 +++++++++++++++++++++++++++++-- frontend/server/package.json | 1 + 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/frontend/server/package-lock.json b/frontend/server/package-lock.json index e1f76415786..b90e31853db 100644 --- a/frontend/server/package-lock.json +++ b/frontend/server/package-lock.json @@ -2694,6 +2694,14 @@ "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-1.0.2.tgz", "integrity": "sha1-rT/0yG7C0CkyL1oCw6mmBslbP1k=" }, + "encoding": { + "version": "0.1.12", + "resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.12.tgz", + "integrity": "sha1-U4tm8+5izRq1HsMjgp0flIDHS+s=", + "requires": { + "iconv-lite": "~0.4.13" + } + }, "end-of-stream": { "version": "1.4.4", "resolved": "https://registry.npmjs.org/end-of-stream/-/end-of-stream-1.4.4.tgz", @@ -3784,8 +3792,7 @@ "is-stream": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-1.1.0.tgz", - "integrity": "sha1-EtSj3U5o4Lec6428hBc66A2RykQ=", - "dev": true + "integrity": "sha1-EtSj3U5o4Lec6428hBc66A2RykQ=" }, "is-stream-ended": { "version": "0.1.4", @@ -7036,6 +7043,26 @@ "integrity": "sha512-2qHaIQr2VLRFoxe2nASzsV6ef4yOOH+Fi9FBOVH6cqeSgUnoyySPZkxzLuzd+RYOQTRpROA0ztTMqxROKSb/nA==", "dev": true }, + "portable-fetch": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/portable-fetch/-/portable-fetch-3.0.0.tgz", + "integrity": "sha1-PL9KptvFpXNLQcBBnJJzMTv9mtg=", + "requires": { + "node-fetch": "^1.0.1", + "whatwg-fetch": ">=0.10.0" + }, + "dependencies": { + "node-fetch": { + "version": "1.7.3", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-1.7.3.tgz", + "integrity": "sha512-NhZ4CsKx7cYm2vSrBAr2PvFOe6sWDf0UYLRqA6svUYg7+/TSfVAu49jYC4BvQ4Sms9SZgdqGBgroqfDhJdTyKQ==", + "requires": { + "encoding": "^0.1.11", + "is-stream": "^1.0.1" + } + } + } + }, "posix-character-classes": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/posix-character-classes/-/posix-character-classes-0.1.1.tgz", @@ -8396,6 +8423,11 @@ "iconv-lite": "0.4.24" } }, + "whatwg-fetch": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/whatwg-fetch/-/whatwg-fetch-3.0.0.tgz", + "integrity": "sha512-9GSJUgz1D4MfyKU7KRqwOjXCXTqWdFNvEr7eUBYchQiVc744mqK/MzXPNR2WsPkmkOa4ywfg8C2n8h+13Bey1Q==" + }, "whatwg-mimetype": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/whatwg-mimetype/-/whatwg-mimetype-2.3.0.tgz", diff --git a/frontend/server/package.json b/frontend/server/package.json index e7da6887d1e..c5b4edd7cd8 100644 --- a/frontend/server/package.json +++ b/frontend/server/package.json @@ -13,6 +13,7 @@ "minio": "^7.0.0", "node-fetch": "^2.1.2", "peek-stream": "^1.1.3", + "portable-fetch": "^3.0.0", "tar-stream": "^2.1.0" }, "devDependencies": { From 2ac37c7398c0e0ba163c3501df4c633e11741c2b Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 13:33:17 +0800 Subject: [PATCH 12/17] Fix portable-fetch module import error --- frontend/server/handlers/tensorboard.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index 142abd2b309..df68443370a 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -15,7 +15,6 @@ import { Handler } from 'express'; import * as k8sHelper from '../k8s-helper'; import { ViewerTensorboardConfig } from '../configs'; import { AuthServiceApi } from '../src/generated/apis/auth'; -import fetch from 'node-fetch'; import { parseError } from '../utils'; export const getTensorboardHandlers = ( @@ -24,7 +23,8 @@ export const getTensorboardHandlers = ( ): { get: Handler; create: Handler; delete: Handler } => { const { apiServerAddress, authzEnabled } = otherConfig; console.log('api server address ' + apiServerAddress); - const authService = new AuthServiceApi({ basePath: apiServerAddress }, undefined, fetch as any); + // TODO: provide fetch to the constructor and stop importing portable-fetch, or use portable-fetch instead of node-fetch + const authService = new AuthServiceApi({ basePath: apiServerAddress }); /** * A handler which retrieve the endpoint for a tensorboard instance. The * handler expects a query string `logdir`. From 883a750fb640bcd896dd28a629cc69c1b08eff16 Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 13:53:53 +0800 Subject: [PATCH 13/17] Fix portable-fetch again --- frontend/server/handlers/tensorboard.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index df68443370a..000d7b2c5dc 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -15,6 +15,7 @@ import { Handler } from 'express'; import * as k8sHelper from '../k8s-helper'; import { ViewerTensorboardConfig } from '../configs'; import { AuthServiceApi } from '../src/generated/apis/auth'; +import fetch from 'node-fetch'; import { parseError } from '../utils'; export const getTensorboardHandlers = ( @@ -23,8 +24,8 @@ export const getTensorboardHandlers = ( ): { get: Handler; create: Handler; delete: Handler } => { const { apiServerAddress, authzEnabled } = otherConfig; console.log('api server address ' + apiServerAddress); - // TODO: provide fetch to the constructor and stop importing portable-fetch, or use portable-fetch instead of node-fetch - const authService = new AuthServiceApi({ basePath: apiServerAddress }); + // TODO: stop importing portable-fetch, or use portable-fetch instead of node-fetch + const authService = new AuthServiceApi({ basePath: apiServerAddress }, undefined, fetch as any); /** * A handler which retrieve the endpoint for a tensorboard instance. The * handler expects a query string `logdir`. From a01a8463985bd95e6f97fbec557a2b99c8f6874f Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 15:00:30 +0800 Subject: [PATCH 14/17] Add unit tests --- frontend/server/app.test.ts | 112 ++++++++++++++++++++---- frontend/server/handlers/tensorboard.ts | 69 ++++++++++++--- frontend/server/utils.ts | 21 +++-- 3 files changed, 170 insertions(+), 32 deletions(-) diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts index 4152967f835..b59b67a1d2b 100644 --- a/frontend/server/app.test.ts +++ b/frontend/server/app.test.ts @@ -33,6 +33,9 @@ jest.mock('node-fetch'); jest.mock('@google-cloud/storage'); jest.mock('./minio-helper'); +// TODO: move sections of tests here to individual files in `frontend/server/integration-tests/` +// for better organization and shorter/more focused tests. + const mockedFetch: jest.Mock = fetch as any; beforeEach(() => { @@ -627,10 +630,11 @@ describe('UIServer apis', () => { }); describe('/apps/tensorboard', () => { - let request: requests.SuperTest; let k8sGetCustomObjectSpy: jest.SpyInstance; let k8sDeleteCustomObjectSpy: jest.SpyInstance; let k8sCreateCustomObjectSpy: jest.SpyInstance; + let kfpApiServer: Server; + function newGetTensorboardResponse({ name = 'viewer-example', logDir = 'log-dir-example', @@ -655,9 +659,8 @@ describe('UIServer apis', () => { }, }; } + beforeEach(() => { - app = new UIServer(loadConfigs(argv, {})); - request = requests(app.start()); k8sGetCustomObjectSpy = jest.spyOn( K8S_TEST_EXPORT.k8sV1CustomObjectClient, 'getNamespacedCustomObject', @@ -672,18 +675,29 @@ describe('UIServer apis', () => { ); }); + afterEach(() => { + if (kfpApiServer) { + kfpApiServer.close(); + } + }); + describe('get', () => { it('requires logdir for get tensorboard', done => { - request.get('/apps/tensorboard').expect(404, 'logdir argument is required', done); + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) + .get('/apps/tensorboard') + .expect(404, 'logdir argument is required', done); }); it('requires namespace for get tensorboard', done => { - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .get('/apps/tensorboard?logdir=some-log-dir') .expect(404, 'namespace argument is required', done); }); it('does not crash with a weird query', done => { + app = new UIServer(loadConfigs(argv, {})); k8sGetCustomObjectSpy.mockImplementation(() => Promise.resolve(newGetTensorboardResponse()), ); @@ -691,10 +705,63 @@ describe('UIServer apis', () => { // exception, so this can verify handler doesn't do extra // decodeURIComponent on queries. const weirdLogDir = encodeURIComponent('%2'); - request.get(`/apps/tensorboard?logdir=${weirdLogDir}&namespace=test-ns`).expect(200, done); + requests(app.start()) + .get(`/apps/tensorboard?logdir=${weirdLogDir}&namespace=test-ns`) + .expect(200, done); + }); + + it('authorizes user requests from KFP auth api', done => { + app = new UIServer(loadConfigs(argv, { ENABLE_AUTHZ: 'true' })); + const appKfpApi = express(); + appKfpApi.get('/apis/v1beta1/auth', (_, res) => { + res.status(200).send('{}'); // Authorized + }); + kfpApiServer = appKfpApi.listen(3001); + k8sGetCustomObjectSpy.mockImplementation(() => + Promise.resolve(newGetTensorboardResponse()), + ); + requests(app.start()) + .get(`/apps/tensorboard?logdir=some-log-dir&namespace=test-ns`) + .expect(200, done); + }); + + it('rejects user requests when KFP auth api rejected', done => { + const errorSpy = jest.spyOn(console, 'error'); + errorSpy.mockImplementation(); + app = new UIServer(loadConfigs(argv, { ENABLE_AUTHZ: 'true' })); + const appKfpApi = express(); + appKfpApi.get('/apis/v1beta1/auth', (_, res) => { + res.status(400).send( + JSON.stringify({ + error: 'User xxx is not unauthorized to list viewers', + details: ['unauthorized', 'callstack'], + }), + ); + }); + kfpApiServer = appKfpApi.listen(3001); + k8sGetCustomObjectSpy.mockImplementation(() => + Promise.resolve(newGetTensorboardResponse()), + ); + requests(app.start()) + .get(`/apps/tensorboard?logdir=some-log-dir&namespace=test-ns`) + .expect( + 401, + 'User is not authorized to GET VIEWERS in namespace test-ns: User xxx is not unauthorized to list viewers', + err => { + expect(errorSpy).toHaveBeenCalledTimes(1); + expect( + errorSpy, + ).toHaveBeenCalledWith( + 'User is not authorized to GET VIEWERS in namespace test-ns: User xxx is not unauthorized to list viewers', + ['unauthorized', 'callstack'], + ); + done(err); + }, + ); }); it('gets tensorboard url and version', done => { + app = new UIServer(loadConfigs(argv, {})); k8sGetCustomObjectSpy.mockImplementation(() => Promise.resolve( newGetTensorboardResponse({ @@ -705,7 +772,7 @@ describe('UIServer apis', () => { ), ); - request + requests(app.start()) .get(`/apps/tensorboard?logdir=${encodeURIComponent('log-dir-1')}&namespace=test-ns`) .expect( 200, @@ -732,17 +799,22 @@ describe('UIServer apis', () => { describe('post (create)', () => { it('requires logdir', done => { - request.post('/apps/tensorboard').expect(404, 'logdir argument is required', done); + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) + .post('/apps/tensorboard') + .expect(404, 'logdir argument is required', done); }); it('requires namespace', done => { - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .post('/apps/tensorboard?logdir=some-log-dir') .expect(404, 'namespace argument is required', done); }); it('requires tfversion', done => { - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .post('/apps/tensorboard?logdir=some-log-dir&namespace=test-ns') .expect(404, 'tfversion (tensorflow version) argument is required', done); }); @@ -768,7 +840,8 @@ describe('UIServer apis', () => { }); k8sCreateCustomObjectSpy.mockImplementation(() => Promise.resolve()); - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .post( `/apps/tensorboard?logdir=${encodeURIComponent( 'log-dir-1', @@ -845,7 +918,8 @@ describe('UIServer apis', () => { ); k8sCreateCustomObjectSpy.mockImplementation(() => Promise.resolve()); - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .post( `/apps/tensorboard?logdir=${encodeURIComponent( 'log-dir-1', @@ -873,7 +947,8 @@ describe('UIServer apis', () => { ); k8sCreateCustomObjectSpy.mockImplementation(() => Promise.resolve()); - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .post( `/apps/tensorboard?logdir=${encodeURIComponent( 'log-dir-1', @@ -889,11 +964,15 @@ describe('UIServer apis', () => { describe('delete', () => { it('requires logdir', done => { - request.delete('/apps/tensorboard').expect(404, 'logdir argument is required', done); + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) + .delete('/apps/tensorboard') + .expect(404, 'logdir argument is required', done); }); it('requires namespace', done => { - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .delete('/apps/tensorboard?logdir=some-log-dir') .expect(404, 'namespace argument is required', done); }); @@ -910,7 +989,8 @@ describe('UIServer apis', () => { ); k8sDeleteCustomObjectSpy.mockImplementation(() => Promise.resolve()); - request + app = new UIServer(loadConfigs(argv, {})); + requests(app.start()) .delete(`/apps/tensorboard?logdir=${encodeURIComponent('log-dir-1')}&namespace=test-ns`) .expect(200, 'Tensorboard deleted.', err => { expect(k8sDeleteCustomObjectSpy.mock.calls[0]).toMatchInlineSnapshot(` diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index 000d7b2c5dc..c153ade0535 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -11,13 +11,40 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -import { Handler } from 'express'; +import { Handler, Request, Response } from 'express'; import * as k8sHelper from '../k8s-helper'; import { ViewerTensorboardConfig } from '../configs'; -import { AuthServiceApi } from '../src/generated/apis/auth'; -import fetch from 'node-fetch'; +import { + AuthServiceApi, + AuthorizeRequestResources, + AuthorizeRequestVerb, +} from '../src/generated/apis/auth'; +import portableFetch from 'portable-fetch'; import { parseError } from '../utils'; +async function authorize( + req: Request, + res: Response, + authService: AuthServiceApi, + { + resources, + verb, + namespace, + }: { resources: AuthorizeRequestResources; verb: AuthorizeRequestVerb; namespace: string }, +): Promise { + try { + await authService.authorize(namespace, resources as any, verb as any, { headers: req.headers }); + console.debug(`Authorized to ${verb} ${resources} in namespace ${namespace}.`); + return true; + } catch (err) { + const details = await parseError(err); + const message = `User is not authorized to ${verb} ${resources} in namespace ${namespace}: ${details.message}`; + console.error(message, details.additionalInfo); + res.status(401).send(message); + } + return false; +} + export const getTensorboardHandlers = ( tensorboardConfig: ViewerTensorboardConfig, otherConfig: { apiServerAddress: string; authzEnabled: boolean }, @@ -25,7 +52,11 @@ export const getTensorboardHandlers = ( const { apiServerAddress, authzEnabled } = otherConfig; console.log('api server address ' + apiServerAddress); // TODO: stop importing portable-fetch, or use portable-fetch instead of node-fetch - const authService = new AuthServiceApi({ basePath: apiServerAddress }, undefined, fetch as any); + const authService = new AuthServiceApi( + { basePath: apiServerAddress }, + undefined, + portableFetch as any, + ); /** * A handler which retrieve the endpoint for a tensorboard instance. The * handler expects a query string `logdir`. @@ -43,8 +74,14 @@ export const getTensorboardHandlers = ( try { if (authzEnabled) { - await authService.authorize(namespace, 'VIEWERS', 'GET', { headers: req.headers }); - console.debug(`Authorized to ${'GET'} ${'VIEWERS'} in namespace ${namespace}.`); + const authorized = await authorize(req, res, authService, { + verb: AuthorizeRequestVerb.GET, + resources: AuthorizeRequestResources.VIEWERS, + namespace, + }); + if (!authorized) { + return; + } } res.send(await k8sHelper.getTensorboardInstance(logdir, namespace)); } catch (err) { @@ -78,8 +115,14 @@ export const getTensorboardHandlers = ( try { if (authzEnabled) { - await authService.authorize(namespace, 'VIEWERS', 'CREATE', { headers: req.headers }); - console.debug(`Authorized to ${'CREATE'} ${'VIEWERS'} in namespace ${namespace}.`); + const authorized = await authorize(req, res, authService, { + verb: AuthorizeRequestVerb.CREATE, + resources: AuthorizeRequestResources.VIEWERS, + namespace, + }); + if (!authorized) { + return; + } } await k8sHelper.newTensorboardInstance( logdir, @@ -118,8 +161,14 @@ export const getTensorboardHandlers = ( try { if (authzEnabled) { - await authService.authorize(namespace, 'VIEWERS', 'DELETE', { headers: req.headers }); - console.debug(`Authorized to ${'DELETE'} ${'VIEWERS'} in namespace ${namespace}.`); + const authorized = await authorize(req, res, authService, { + verb: AuthorizeRequestVerb.DELETE, + resources: AuthorizeRequestResources.VIEWERS, + namespace, + }); + if (!authorized) { + return; + } } await k8sHelper.deleteTensorboardInstance(logdir, namespace); res.send('Tensorboard deleted.'); diff --git a/frontend/server/utils.ts b/frontend/server/utils.ts index 43f228ca004..f4e1ef18b78 100644 --- a/frontend/server/utils.ts +++ b/frontend/server/utils.ts @@ -99,16 +99,12 @@ export interface ErrorDetails { message: string; additionalInfo: any; } -export const UNKOWN_ERROR: ErrorDetails = { - message: 'Unknown error', - additionalInfo: 'Unknown error', -}; +const UNKOWN_ERROR = 'Unknown error'; export async function parseError(error: any): Promise { return ( parseK8sError(error) || (await parseKfpApiError(error)) || - parseGenericError(error) || - UNKOWN_ERROR + parseGenericError(error) || { message: UNKOWN_ERROR, additionalInfo: error } ); } @@ -124,6 +120,19 @@ function parseGenericError(error: any): ErrorDetails | undefined { return { message: error.message, additionalInfo: error }; } else if (error.message && typeof error.message === 'string') { return { message: error.message, additionalInfo: error }; + } else if ( + error.url && + typeof error.url === 'string' && + error.status && + typeof error.status === 'number' && + error.statusText && + typeof error.statusText === 'string' + ) { + const { url, status, statusText } = error; + return { + message: `Fetching ${url} failed with status code ${status} and message: ${statusText}`, + additionalInfo: { url, status, statusText }, + }; } // Cannot understand error type return undefined; From ce0e180da437e9315f927cdd3b4f045813122789 Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 15:13:43 +0800 Subject: [PATCH 15/17] Address CR comments --- frontend/server/app.test.ts | 14 +++++++------- frontend/server/handlers/tensorboard.ts | 17 +++++++++-------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts index b59b67a1d2b..6f905dcab08 100644 --- a/frontend/server/app.test.ts +++ b/frontend/server/app.test.ts @@ -686,14 +686,14 @@ describe('UIServer apis', () => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .get('/apps/tensorboard') - .expect(404, 'logdir argument is required', done); + .expect(400, 'logdir argument is required', done); }); it('requires namespace for get tensorboard', done => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .get('/apps/tensorboard?logdir=some-log-dir') - .expect(404, 'namespace argument is required', done); + .expect(400, 'namespace argument is required', done); }); it('does not crash with a weird query', done => { @@ -802,21 +802,21 @@ describe('UIServer apis', () => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .post('/apps/tensorboard') - .expect(404, 'logdir argument is required', done); + .expect(400, 'logdir argument is required', done); }); it('requires namespace', done => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .post('/apps/tensorboard?logdir=some-log-dir') - .expect(404, 'namespace argument is required', done); + .expect(400, 'namespace argument is required', done); }); it('requires tfversion', done => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .post('/apps/tensorboard?logdir=some-log-dir&namespace=test-ns') - .expect(404, 'tfversion (tensorflow version) argument is required', done); + .expect(400, 'tfversion (tensorflow version) argument is required', done); }); it('creates tensorboard viewer custom object and waits for it', done => { @@ -967,14 +967,14 @@ describe('UIServer apis', () => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .delete('/apps/tensorboard') - .expect(404, 'logdir argument is required', done); + .expect(400, 'logdir argument is required', done); }); it('requires namespace', done => { app = new UIServer(loadConfigs(argv, {})); requests(app.start()) .delete('/apps/tensorboard?logdir=some-log-dir') - .expect(404, 'namespace argument is required', done); + .expect(400, 'namespace argument is required', done); }); it('deletes tensorboard viewer custom object', done => { diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index c153ade0535..136c588a9d1 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -51,7 +51,8 @@ export const getTensorboardHandlers = ( ): { get: Handler; create: Handler; delete: Handler } => { const { apiServerAddress, authzEnabled } = otherConfig; console.log('api server address ' + apiServerAddress); - // TODO: stop importing portable-fetch, or use portable-fetch instead of node-fetch + // TODO: Use portable-fetch instead of node-fetch in other parts too. The generated api here only + // supports portable-fetch. const authService = new AuthServiceApi( { basePath: apiServerAddress }, undefined, @@ -64,11 +65,11 @@ export const getTensorboardHandlers = ( const get: Handler = async (req, res) => { const { logdir, namespace } = req.query; if (!logdir) { - res.status(404).send('logdir argument is required'); + res.status(400).send('logdir argument is required'); return; } if (!namespace) { - res.status(404).send('namespace argument is required'); + res.status(400).send('namespace argument is required'); return; } @@ -101,15 +102,15 @@ export const getTensorboardHandlers = ( const create: Handler = async (req, res) => { const { logdir, namespace, tfversion } = req.query; if (!logdir) { - res.status(404).send('logdir argument is required'); + res.status(400).send('logdir argument is required'); return; } if (!namespace) { - res.status(404).send('namespace argument is required'); + res.status(400).send('namespace argument is required'); return; } if (!tfversion) { - res.status(404).send('tfversion (tensorflow version) argument is required'); + res.status(400).send('tfversion (tensorflow version) argument is required'); return; } @@ -151,11 +152,11 @@ export const getTensorboardHandlers = ( const deleteHandler: Handler = async (req, res) => { const { logdir, namespace } = req.query; if (!logdir) { - res.status(404).send('logdir argument is required'); + res.status(400).send('logdir argument is required'); return; } if (!namespace) { - res.status(404).send('namespace argument is required'); + res.status(400).send('namespace argument is required'); return; } From 037f4a27c515889c1e3a8d7f2a4f8e3f392f023e Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 17:00:57 +0800 Subject: [PATCH 16/17] add unit test for header --- frontend/server/app.test.ts | 20 ++++++++++++++++++-- frontend/server/handlers/artifacts.ts | 1 - frontend/server/handlers/tensorboard.ts | 11 ++++++++++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts index 6f905dcab08..03ff927a89e 100644 --- a/frontend/server/app.test.ts +++ b/frontend/server/app.test.ts @@ -713,7 +713,9 @@ describe('UIServer apis', () => { it('authorizes user requests from KFP auth api', done => { app = new UIServer(loadConfigs(argv, { ENABLE_AUTHZ: 'true' })); const appKfpApi = express(); - appKfpApi.get('/apis/v1beta1/auth', (_, res) => { + const receivedHeaders: any[] = []; + appKfpApi.get('/apis/v1beta1/auth', (req, res) => { + receivedHeaders.push(req.headers); res.status(200).send('{}'); // Authorized }); kfpApiServer = appKfpApi.listen(3001); @@ -722,7 +724,21 @@ describe('UIServer apis', () => { ); requests(app.start()) .get(`/apps/tensorboard?logdir=some-log-dir&namespace=test-ns`) - .expect(200, done); + .set('x-goog-authenticated-user-email', 'accounts.google.com:user@google.com') + .expect(200, err => { + expect(receivedHeaders).toHaveLength(1); + expect(receivedHeaders[0]).toMatchInlineSnapshot(` + Object { + "accept": "*/*", + "accept-encoding": "gzip,deflate", + "connection": "close", + "host": "localhost:3001", + "user-agent": "node-fetch/1.0 (+https://github.com/bitinn/node-fetch)", + "x-goog-authenticated-user-email": "accounts.google.com:user@google.com", + } + `); + done(err); + }); }); it('rejects user requests when KFP auth api rejected', done => { diff --git a/frontend/server/handlers/artifacts.ts b/frontend/server/handlers/artifacts.ts index f255c77ae4a..92cb4b2b672 100644 --- a/frontend/server/handlers/artifacts.ts +++ b/frontend/server/handlers/artifacts.ts @@ -239,7 +239,6 @@ function getGCSArtifactHandler(options: { key: string; bucket: string }, peek: n }; } -const AUTH_EMAIL_HEADER = 'x-goog-authenticated-user-email'; const ARTIFACTS_PROXY_DEFAULTS = { serviceName: 'ml-pipeline-ui-artifact', servicePort: '80', diff --git a/frontend/server/handlers/tensorboard.ts b/frontend/server/handlers/tensorboard.ts index 136c588a9d1..a80319822c6 100644 --- a/frontend/server/handlers/tensorboard.ts +++ b/frontend/server/handlers/tensorboard.ts @@ -33,7 +33,14 @@ async function authorize( }: { resources: AuthorizeRequestResources; verb: AuthorizeRequestVerb; namespace: string }, ): Promise { try { - await authService.authorize(namespace, resources as any, verb as any, { headers: req.headers }); + // Resources and verb are string enums, they are used as string here, that + // requires a force type conversion. If we generated client should accept + // enums instead. + await authService.authorize(namespace, resources as any, verb as any, { + // Pass authentication header. + // TODO: parameterize the header. + headers: { [AUTH_EMAIL_HEADER]: req.headers[AUTH_EMAIL_HEADER] }, + }); console.debug(`Authorized to ${verb} ${resources} in namespace ${namespace}.`); return true; } catch (err) { @@ -186,3 +193,5 @@ export const getTensorboardHandlers = ( delete: deleteHandler, }; }; + +const AUTH_EMAIL_HEADER = 'x-goog-authenticated-user-email'; From ffe44fb25027784e8a3e28eefb0e537aa6876f6a Mon Sep 17 00:00:00 2001 From: Yuan Gong Date: Tue, 28 Apr 2020 18:43:43 +0800 Subject: [PATCH 17/17] Update readme --- frontend/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/README.md b/frontend/README.md index b7b46259c84..52a0ccdea62 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -61,6 +61,7 @@ Then it depends on what you want to develop: | ----------------------- | -------------------------------------------------------------- | ------------------------------------------------------------------ | | Client UI | `NAMESPACE=kubeflow npm run start:proxy` | | | Client UI + Node server | `NAMESPACE=kubeflow npm run start:proxy-and-server` | You need to rerun the script every time you edit node server code. | +| Client UI + Node server (debug mode) | `NAMESPACE=kubeflow npm run start:proxy-and-server-inspect` | Same as above, and you can use chrome to debug the server. | ## Unit testing FAQ There are a few typees of tests during presubmit: