Always register the metadata endpoint (flyteorg#392)

* Always register the metadata endpoint Signed-off-by: Haytham Abuelfutuh <[email protected]> * Rename Signed-off-by: Haytham Abuelfutuh <[email protected]> * Using released flyteidl Signed-off-by: Haytham Abuelfutuh <[email protected]> * lint Signed-off-by: Haytham Abuelfutuh <[email protected]> * Initialize authz metadata server Signed-off-by: Haytham Abuelfutuh <[email protected]>
eapolinario · Apr 11, 2022 · f04414e · f04414e
1 parent c89898e
commit f04414e
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 26 deletions.
diff --git a/auth/authzserver/metadata_provider.go b/auth/authzserver/metadata_provider.go
@@ -7,6 +7,8 @@ import (
 	"net/url"
 	"strings"
 
+	"github.com/flyteorg/flyteadmin/pkg/config"
+
 	"github.com/flyteorg/flyteadmin/auth"
 
 	authConfig "github.com/flyteorg/flyteadmin/auth/config"
@@ -15,10 +17,11 @@ import (
 )
 
 type OAuth2MetadataProvider struct {
-	cfg *authConfig.Config
+	cfg       *authConfig.Config
+	serverCfg *config.ServerConfig
 }
 
-// Override auth func to enforce anonymous access on the implemented APIs
+// AuthFuncOverride overrides auth func to enforce anonymous access on the implemented APIs
 // Ref: https://github.com/grpc-ecosystem/go-grpc-middleware/blob/master/auth/auth.go#L31
 func (s OAuth2MetadataProvider) AuthFuncOverride(ctx context.Context, fullMethodName string) (context.Context, error) {
 	return ctx, nil
@@ -90,11 +93,13 @@ func (s OAuth2MetadataProvider) GetPublicClientConfig(context.Context, *service.
 		RedirectUri:              s.cfg.AppAuth.ThirdParty.FlyteClientConfig.RedirectURI,
 		Scopes:                   s.cfg.AppAuth.ThirdParty.FlyteClientConfig.Scopes,
 		AuthorizationMetadataKey: s.cfg.GrpcAuthorizationHeader,
+		ServiceHttpEndpoint:      s.serverCfg.ServiceHTTPEndpoint.String(),
 	}, nil
 }
 
-func NewService(config *authConfig.Config) OAuth2MetadataProvider {
+func NewService(serverCfg *config.ServerConfig, config *authConfig.Config) OAuth2MetadataProvider {
 	return OAuth2MetadataProvider{
-		cfg: config,
+		cfg:       config,
+		serverCfg: serverCfg,
 	}
 }
diff --git a/auth/authzserver/metadata_provider_test.go b/auth/authzserver/metadata_provider_test.go
@@ -8,16 +8,17 @@ import (
 	"strings"
 	"testing"
 
-	config2 "github.com/flyteorg/flytestdlib/config"
+	"github.com/flyteorg/flyteadmin/pkg/config"
+
+	stdConfig "github.com/flyteorg/flytestdlib/config"
 
-	"github.com/flyteorg/flyteadmin/auth/config"
 	authConfig "github.com/flyteorg/flyteadmin/auth/config"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/service"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestOAuth2MetadataProvider_FlyteClient(t *testing.T) {
-	provider := NewService(&authConfig.Config{
+	provider := NewService(&config.ServerConfig{}, &authConfig.Config{
 		AppAuth: authConfig.OAuth2Options{
 			ThirdParty: authConfig.ThirdPartyConfigOptions{
 				FlyteClientConfig: authConfig.FlyteClientConfig{
@@ -39,8 +40,8 @@ func TestOAuth2MetadataProvider_FlyteClient(t *testing.T) {
 
 func TestOAuth2MetadataProvider_OAuth2Metadata(t *testing.T) {
 	t.Run("Self AuthServer", func(t *testing.T) {
-		provider := NewService(&authConfig.Config{
-			AuthorizedURIs: []config2.URL{{URL: *config.MustParseURL("https://issuer/")}},
+		provider := NewService(&config.ServerConfig{}, &authConfig.Config{
+			AuthorizedURIs: []stdConfig.URL{{URL: *authConfig.MustParseURL("https://issuer/")}},
 		})
 
 		ctx := context.Background()
@@ -74,12 +75,12 @@ func TestOAuth2MetadataProvider_OAuth2Metadata(t *testing.T) {
 	http.DefaultClient = s.Client()
 
 	t.Run("External AuthServer", func(t *testing.T) {
-		provider := NewService(&authConfig.Config{
-			AuthorizedURIs: []config2.URL{{URL: *config.MustParseURL("https://issuer/")}},
+		provider := NewService(&config.ServerConfig{}, &authConfig.Config{
+			AuthorizedURIs: []stdConfig.URL{{URL: *authConfig.MustParseURL("https://issuer/")}},
 			AppAuth: authConfig.OAuth2Options{
 				AuthServerType: authConfig.AuthorizationServerTypeExternal,
 				ExternalAuthServer: authConfig.ExternalAuthorizationServer{
-					BaseURL: config2.URL{URL: *config.MustParseURL(s.URL)},
+					BaseURL: stdConfig.URL{URL: *authConfig.MustParseURL(s.URL)},
 				},
 			},
 		})
@@ -91,14 +92,14 @@ func TestOAuth2MetadataProvider_OAuth2Metadata(t *testing.T) {
 	})
 
 	t.Run("External AuthServer fallback url", func(t *testing.T) {
-		provider := NewService(&authConfig.Config{
-			AuthorizedURIs: []config2.URL{{URL: *config.MustParseURL("https://issuer/")}},
+		provider := NewService(&config.ServerConfig{}, &authConfig.Config{
+			AuthorizedURIs: []stdConfig.URL{{URL: *authConfig.MustParseURL("https://issuer/")}},
 			AppAuth: authConfig.OAuth2Options{
 				AuthServerType: authConfig.AuthorizationServerTypeExternal,
 			},
 			UserAuth: authConfig.UserAuthConfig{
 				OpenID: authConfig.OpenIDOptions{
-					BaseURL: config2.URL{URL: *config.MustParseURL(s.URL)},
+					BaseURL: stdConfig.URL{URL: *authConfig.MustParseURL(s.URL)},
 				},
 			},
 		})

diff --git a/auth/handlers.go b/auth/handlers.go
@@ -54,8 +54,9 @@ func RegisterHandlers(ctx context.Context, handler interfaces.HandlerRegisterer,
 	handler.HandleFunc("/logout", GetLogoutEndpointHandler(ctx, authCtx))
 }
 
-// Look for access token and refresh token, if both are present and the access token is expired, then attempt to
-// refresh. Otherwise do nothing and proceed to the next handler. If successfully refreshed, proceed to the landing page.
+// RefreshTokensIfExists looks for access token and refresh token, if both are present and the access token is expired,
+// then attempt to refresh. Otherwise do nothing and proceed to the next handler. If successfully refreshed, proceed to
+// the landing page.
 func RefreshTokensIfExists(ctx context.Context, authCtx interfaces.AuthenticationContext, authHandler http.HandlerFunc) http.HandlerFunc {
 
 	return func(writer http.ResponseWriter, request *http.Request) {

diff --git a/go.mod b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/cloudevents/sdk-go/v2 v2.8.0
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/evanphx/json-patch v4.9.0+incompatible
-	github.com/flyteorg/flyteidl v0.24.17
+	github.com/flyteorg/flyteidl v0.24.18
 	github.com/flyteorg/flyteplugins v0.10.16
 	github.com/flyteorg/flytepropeller v0.16.36
 	github.com/flyteorg/flytestdlib v0.4.21

diff --git a/go.sum b/go.sum
@@ -369,8 +369,8 @@ github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8S
 github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/flyteorg/flyteidl v0.23.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
 github.com/flyteorg/flyteidl v0.24.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
-github.com/flyteorg/flyteidl v0.24.17 h1:Xx70bJbuQGyvS8uAyU4AN74rot6KnzJ9r/L9gcCdEsU=
-github.com/flyteorg/flyteidl v0.24.17/go.mod h1:vHSugApgS3hRITIafzQDU8DZD/W8wFRfFcgaFU35Dww=
+github.com/flyteorg/flyteidl v0.24.18 h1:5HMlc11OcSzRHR5VRRAac2gH6yO2XFcQ3fWlcl9tj+k=
+github.com/flyteorg/flyteidl v0.24.18/go.mod h1:vHSugApgS3hRITIafzQDU8DZD/W8wFRfFcgaFU35Dww=
 github.com/flyteorg/flyteplugins v0.10.16 h1:rwNI2MACPbcST2O6CEUsNW6bccz7ZLni0GiY3orevfw=
 github.com/flyteorg/flyteplugins v0.10.16/go.mod h1:YBWV8QnFakDJfLyua8pYddiWqszAqseBKIJPNMERlos=
 github.com/flyteorg/flytepropeller v0.16.36 h1:5uE8JsutrPVyLVrRJ8BgvhZUOmTBFkEkn5xmIOo21nU=

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -26,6 +26,12 @@ type ServerConfig struct {
 	DeprecatedThirdPartyConfig authConfig.ThirdPartyConfigOptions `json:"thirdPartyConfig" pflag:",Deprecated please use auth.appAuth.thirdPartyConfig instead."`
 
 	DataProxy DataProxyConfig `json:"dataProxy" pflag:",Defines data proxy configuration."`
+
+	// ServiceHTTPEndpoint allows specifying the http endpoint this admin instance is accessible on. This is useful
+	// for when there is no ingress setup or when the service is serving http and grpc over two different ports.
+	// Setting it here allows gRPC clients to retrieve the http endpoint to be able to present it to end-users (e.g.
+	// open up the browser to the workflow overview page)
+	ServiceHTTPEndpoint config.URL `json:"serviceHttpEndpoint" pflag:",Defines the http endpoint the service is accessible at."`
 }
 
 type DataProxyConfig struct {

diff --git a/pkg/config/serverconfig_flags.go b/pkg/config/serverconfig_flags.go
diff --git a/pkg/config/serverconfig_flags_test.go b/pkg/config/serverconfig_flags_test.go
diff --git a/pkg/server/service.go b/pkg/server/service.go
@@ -72,7 +72,7 @@ func blanketAuthorization(ctx context.Context, req interface{}, _ *grpc.UnarySer
 
 // Creates a new gRPC Server with all the configuration
 func newGRPCServer(ctx context.Context, pluginRegistry *plugins.Registry, cfg *config.ServerConfig,
-	storageCfg *storage.Config, authCtx interfaces.AuthenticationContext,
+	storageCfg *storage.Config, authCfg *authConfig.Config, authCtx interfaces.AuthenticationContext,
 	scope promutils.Scope, opts ...grpc.ServerOption) (*grpc.Server, error) {
 	// Not yet implemented for streaming
 	var chainedUnaryInterceptors grpc.UnaryServerInterceptor
@@ -107,8 +107,8 @@ func newGRPCServer(ctx context.Context, pluginRegistry *plugins.Registry, cfg *c
 
 	configuration := runtime2.NewConfigurationProvider()
 	service.RegisterAdminServiceServer(grpcServer, adminservice.NewAdminServer(ctx, pluginRegistry, configuration, cfg.KubeConfig, cfg.Master, dataStorageClient, scope.NewSubScope("admin")))
+	service.RegisterAuthMetadataServiceServer(grpcServer, authzserver.NewService(cfg, authCfg))
 	if cfg.Security.UseAuth {
-		service.RegisterAuthMetadataServiceServer(grpcServer, authCtx.AuthMetadataService())
 		service.RegisterIdentityServiceServer(grpcServer, authCtx.IdentityService())
 	}
 
@@ -243,7 +243,7 @@ func serveGatewayInsecure(ctx context.Context, pluginRegistry *plugins.Registry,
 			}
 		}
 
-		oauth2MetadataProvider := authzserver.NewService(authCfg)
+		oauth2MetadataProvider := authzserver.NewService(cfg, authCfg)
 		oidcUserInfoProvider := auth.NewUserInfoProvider()
 
 		authCtx, err = auth.NewAuthenticationContext(ctx, sm, oauth2Provider, oauth2ResourceServer, oauth2MetadataProvider, oidcUserInfoProvider, authCfg)
@@ -253,7 +253,7 @@ func serveGatewayInsecure(ctx context.Context, pluginRegistry *plugins.Registry,
 		}
 	}
 
-	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageConfig, authCtx, scope)
+	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageConfig, authCfg, authCtx, scope)
 	if err != nil {
 		return fmt.Errorf("failed to create a newGRPCServer. Error: %w", err)
 	}
@@ -346,7 +346,7 @@ func serveGatewaySecure(ctx context.Context, pluginRegistry *plugins.Registry, c
 			}
 		}
 
-		oauth2MetadataProvider := authzserver.NewService(authCfg)
+		oauth2MetadataProvider := authzserver.NewService(cfg, authCfg)
 		oidcUserInfoProvider := auth.NewUserInfoProvider()
 
 		authCtx, err = auth.NewAuthenticationContext(ctx, sm, oauth2Provider, oauth2ResourceServer, oauth2MetadataProvider, oidcUserInfoProvider, authCfg)
@@ -356,7 +356,7 @@ func serveGatewaySecure(ctx context.Context, pluginRegistry *plugins.Registry, c
 		}
 	}
 
-	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageCfg, authCtx, scope, grpc.Creds(credentials.NewServerTLSFromCert(cert)))
+	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageCfg, authCfg, authCtx, scope, grpc.Creds(credentials.NewServerTLSFromCert(cert)))
 	if err != nil {
 		return fmt.Errorf("failed to create a newGRPCServer. Error: %w", err)
 	}