Merge pull request #8 from mirekys/fix-3113

Merge cs3org#3121 into my debug branch
michielbdejong · Dec 9, 2022 · fc0579f · fc0579f
2 parents 4ee835a + 78b8ad1
commit fc0579f
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 28 deletions.
diff --git a/changelog/unreleased/fix-mentix-domain-normalization.md b/changelog/unreleased/fix-mentix-domain-normalization.md
@@ -0,0 +1,6 @@
+Bugfix: Add missing domain normalization to mentix provider authorizer
+
+The Mentix OCM Provider authorizer lacked provider domain normalization.
+This led to incorrect provider domain matching when authorizing OCM providers.
+
+https://github.com/cs3org/reva/pull/3121
diff --git a/internal/http/services/ocmd/invites.go b/internal/http/services/ocmd/invites.go
@@ -26,7 +26,6 @@ import (
 	"io"
 	"mime"
 	"net/http"
-	"net/url"
 
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	invitepb "github.com/cs3org/go-cs3apis/cs3/ocm/invite/v1beta1"
@@ -237,14 +236,8 @@ func (h *invitesHandler) acceptInvite(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	recipientProviderURL, err := url.Parse(recipientProvider)
-	if err != nil {
-		WriteError(w, r, APIErrorServerError, fmt.Sprintf("error parseing recipientProvider URL: %s", recipientProvider), err)
-		return
-	}
-
 	providerInfo := ocmprovider.ProviderInfo{
-		Domain: recipientProviderURL.Hostname(),
+		Domain: recipientProvider,
 		Services: []*ocmprovider.Service{
 			{
 				Host: clientIP,

diff --git a/pkg/ocm/provider/authorizer/json/json.go b/pkg/ocm/provider/authorizer/json/json.go
@@ -113,9 +113,9 @@ func (a *authorizer) GetInfoByDomain(ctx context.Context, domain string) (*ocmpr
 	return nil, errtypes.NotFound(domain)
 }
 
-func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovider.ProviderInfo) error {
+func (a *authorizer) IsProviderAllowed(ctx context.Context, pi *ocmprovider.ProviderInfo) error {
 	var err error
-	normalizedDomain, err := normalizeDomain(provider.Domain)
+	normalizedDomain, err := normalizeDomain(pi.Domain)
 	if err != nil {
 		return err
 	}
@@ -133,10 +133,10 @@ func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovide
 
 	switch {
 	case !providerAuthorized:
-		return errtypes.NotFound(provider.GetDomain())
+		return errtypes.NotFound(pi.GetDomain())
 	case !a.conf.VerifyRequestHostname:
 		return nil
-	case len(provider.Services) == 0:
+	case len(pi.Services) == 0:
 		return errtypes.NotSupported("No IP provided")
 	}
 
@@ -147,6 +147,7 @@ func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovide
 			if err != nil {
 				return err
 			}
+			break
 		}
 	}
 	if ocmHost == "" {
@@ -169,8 +170,9 @@ func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovide
 	}
 
 	for _, ip := range ipList {
-		if ip == provider.Services[0].Host {
+		if ip == pi.Services[0].Host {
 			providerAuthorized = true
+			break
 		}
 	}
 	if !providerAuthorized {
@@ -194,8 +196,8 @@ func (a *authorizer) getOCMProviders(providers []*ocmprovider.ProviderInfo) (po
 	return
 }
 
-func (a *authorizer) getOCMHost(provider *ocmprovider.ProviderInfo) (string, error) {
-	for _, s := range provider.Services {
+func (a *authorizer) getOCMHost(pi *ocmprovider.ProviderInfo) (string, error) {
+	for _, s := range pi.Services {
 		if s.Endpoint.Type.Name == "OCM" {
 			ocmHost, err := url.Parse(s.Host)
 			if err != nil {

diff --git a/pkg/ocm/provider/authorizer/mentix/mentix.go b/pkg/ocm/provider/authorizer/mentix/mentix.go
@@ -96,6 +96,22 @@ type authorizer struct {
 	conf                *config
 }
 
+func normalizeDomain(d string) (string, error) {
+	var urlString string
+	if strings.Contains(d, "://") {
+		urlString = d
+	} else {
+		urlString = "https://" + d
+	}
+
+	u, err := url.Parse(urlString)
+	if err != nil {
+		return "", err
+	}
+
+	return u.Hostname(), nil
+}
+
 func (a *authorizer) fetchProviders() ([]*ocmprovider.ProviderInfo, error) {
 	if (a.providers != nil) && (time.Now().Unix() < a.providersExpiration) {
 		return a.providers, nil
@@ -111,7 +127,7 @@ func (a *authorizer) fetchProviders() ([]*ocmprovider.ProviderInfo, error) {
 	res, err := a.client.HTTPClient.Do(req)
 	if err != nil {
 		err = errors.Wrap(err,
-			fmt.Sprintf("error fetching provider list from: %s", a.client.BaseURL))
+			fmt.Sprintf("mentix: error fetching provider list from: %s", a.client.BaseURL))
 		return nil, err
 	}
 
@@ -131,14 +147,19 @@ func (a *authorizer) fetchProviders() ([]*ocmprovider.ProviderInfo, error) {
 
 func (a *authorizer) GetInfoByDomain(ctx context.Context, domain string) (*ocmprovider.ProviderInfo, error) {
 	log := appctx.GetLogger(ctx)
+	normalizedDomain, err := normalizeDomain(domain)
 	providers, err := a.fetchProviders()
 	if err != nil {
 		return nil, err
 	}
 
+	providers, err := a.fetchProviders()
+	if err != nil {
+		return nil, err
+	}
 	for _, p := range providers {
 		log.Info().Msgf("Getting info for domain %s among authorized domains", domain)
-		if strings.Contains(p.Domain, domain) {
+		if strings.Contains(p.Domain, normalizedDomain) {
 			log.Info().Msgf("Considering against %s - YES", p.Domain)
 			return p, nil
 		} else {
@@ -148,15 +169,19 @@ func (a *authorizer) GetInfoByDomain(ctx context.Context, domain string) (*ocmpr
 	return nil, errtypes.NotFound(domain)
 }
 
-func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovider.ProviderInfo) error {
+func (a *authorizer) IsProviderAllowed(ctx context.Context, pi *ocmprovider.ProviderInfo) error {
 	log := appctx.GetLogger(ctx)
 	providers, err := a.fetchProviders()
 	if err != nil {
 		return err
 	}
+	normalizedDomain, err := normalizeDomain(pi.Domain)
+	if err != nil {
+		return err
+	}
 
 	var providerAuthorized bool
-	if provider.Domain != "" {
+	if normalizedDomain != "" {
 		log.Info().Msgf("Considering %s against authorized domains", provider.Domain)
 		for _, p := range providers {
 			if p.Domain == provider.Domain {
@@ -173,24 +198,27 @@ func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovide
 
 	switch {
 	case !providerAuthorized:
-		return errtypes.NotFound(provider.GetDomain())
+		return errtypes.NotFound(pi.GetDomain())
 	case !a.conf.VerifyRequestHostname:
 		return nil
-	case len(provider.Services) == 0:
-		return errtypes.NotSupported("No IP provided")
+	case len(pi.Services) == 0:
+		return errtypes.NotSupported(
+			fmt.Sprintf("mentix: provider %s has no supported services", pi.GetDomain()))
 	}
 
 	var ocmHost string
 	for _, p := range providers {
-		if p.Domain == provider.Domain {
+		if p.Domain == normalizedDomain {
 			ocmHost, err = a.getOCMHost(p)
 			if err != nil {
 				return err
 			}
+			break
 		}
 	}
 	if ocmHost == "" {
-		return errtypes.InternalError("mentix: ocm host not specified for mesh provider")
+		return errtypes.NotSupported(
+			fmt.Sprintf("mentix: provider %s is missing OCM endpoint", pi.GetDomain()))
 	}
 
 	providerAuthorized = false
@@ -200,7 +228,8 @@ func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovide
 	} else {
 		addr, err := net.LookupIP(ocmHost)
 		if err != nil {
-			return errors.Wrap(err, "json: error looking up client IP")
+			return errors.Wrap(err,
+				fmt.Sprintf("mentix: error looking up IPs for OCM endpoint %s", ocmHost))
 		}
 		for _, a := range addr {
 			ipList = append(ipList, a.String())
@@ -209,12 +238,16 @@ func (a *authorizer) IsProviderAllowed(ctx context.Context, provider *ocmprovide
 	}
 
 	for _, ip := range ipList {
-		if ip == provider.Services[0].Host {
+		if ip == pi.Services[0].Host {
 			providerAuthorized = true
+			break
 		}
 	}
 	if !providerAuthorized {
-		return errtypes.NotFound("OCM Host")
+		return errtypes.BadRequest(
+			fmt.Sprintf(
+				"Invalid requesting OCM endpoint IP %s of provider %s",
+				pi.Services[0].Host, pi.GetDomain()))
 	}
 
 	return nil
@@ -243,7 +276,7 @@ func (a *authorizer) getOCMHost(provider *ocmprovider.ProviderInfo) (string, err
 		if s.Endpoint.Type.Name == "OCM" {
 			ocmHost, err := url.Parse(s.Host)
 			if err != nil {
-				return "", errors.Wrap(err, "json: error parsing OCM host URL")
+				return "", errors.Wrap(err, fmt.Sprintf("mentix: error parsing OCM host URL %s", s.Host))
 			}
 			return ocmHost.Host, nil
 		}