Use newer config settings if available (allow local SAMS/SSC dev) (#6…

…2344)

* Totally unrelated refactoring

* Use newer config settings if available

cmd/frontend/graphqlbackend/external_account.go

@@ -2,7 +2,6 @@ package graphqlbackend
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/graph-gophers/graphql-go"
 	"github.com/graph-gophers/graphql-go/relay"
@@ -63,7 +62,7 @@ func (r *externalAccountResolver) CodySubscription(ctx context.Context) (*CodySu
 		return nil, errors.New("this feature is only available on sourcegraph.com")
 	}
 
-	if r.account.ServiceType != "openidconnect" || r.account.ServiceID != fmt.Sprintf("https://%s", ssc.GetSAMSHostName()) {
+	if r.account.ServiceType != "openidconnect" || r.account.ServiceID != ssc.GetSAMSServiceID() {
 		return nil, nil
 	}
 

cmd/frontend/internal/app/ui/router.go

@@ -80,6 +80,13 @@ var aboutRedirects = map[string]string{
 	"help/terms": "terms",
 }
 
+type staticPageInfo struct {
+	// Specify either path OR pathPrefix.
+	path, pathPrefix string
+	name, title      string
+	index            bool
+}
+
 // Router returns the router that serves pages for our web app.
 func Router() *mux.Router {
 	return uirouter.Router
@@ -111,13 +118,8 @@ func InitRouter(db database.DB) {
 	ghAppRouter := r.PathPrefix("/githubapp/").Subrouter()
 	githubapp.SetupGitHubAppRoutes(ghAppRouter, db)
 
-	// Basic pages with static titles
-	for _, p := range []struct {
-		// Specify either path OR pathPrefix.
-		path, pathPrefix string
-		name, title      string
-		index            bool
-	}{
+	// Basic pages with static titles.
+	staticPages := []staticPageInfo{
 		// with index:
 		{pathPrefix: "/insights", name: "insights", title: "Insights", index: true},
 		{pathPrefix: "/search-jobs", name: "search-jobs", title: "Search Jobs", index: true},
@@ -160,7 +162,8 @@ func InitRouter(db database.DB) {
 		{path: "/survey", name: "survey", title: "Survey", index: false},
 		{path: "/survey/{score}", name: "survey-score", title: "Survey", index: false},
 		{path: "/welcome", name: "welcome", title: "Welcome", index: false},
-	} {
+	}
+	for _, p := range staticPages {
 		var handler http.Handler
 		if p.index {
 			handler = brandedIndex(p.title)

cmd/frontend/internal/cody/subscription.go

@@ -2,7 +2,6 @@ package cody
 
 import (
 	"context"
-	"fmt"
 	"sync"
 	"time"
 
@@ -88,14 +87,10 @@ func consolidateSubscriptionDetails(ctx context.Context, user types.User, subscr
 // will return ("", nil). After we migrate all dotcom user accounts to SAMS, that
 // should no longer be possible.
 func getSAMSAccountIDsForUser(ctx context.Context, db database.DB, dotcomUserID int32) ([]string, error) {
-	// NOTE: We hard-code this to look for the SAMS-prod environment, meaning there isn't a way
-	// to test dotcom pulling subscription data from a local SAMS/SSC instance. To support that
-	// we'd need to make the SAMSHostname configurable. (Or somehow identify which OIDC provider
-	// is SAMS.)
 	oidcAccounts, err := db.UserExternalAccounts().List(ctx, database.ExternalAccountsListOptions{
 		UserID:      dotcomUserID,
 		ServiceType: "openidconnect",
-		ServiceID:   fmt.Sprintf("https://%s", ssc.GetSAMSHostName()),
+		ServiceID:   ssc.GetSAMSServiceID(),
 	})
 	if err != nil {
 		return []string{}, errors.Wrap(err, "listing external accounts")
@@ -137,7 +132,6 @@ func SubscriptionForUser(ctx context.Context, db database.DB, user types.User) (
 		// While developing the SSC backend, we only fetch subscription data for users based on a flag.
 		var subscription *ssc.Subscription
 		if samsAccountID != "" {
-
 			subscription, err = sscClient.FetchSubscriptionBySAMSAccountID(ctx, samsAccountID)
 			if err != nil {
 				return nil, errors.Wrap(err, "fetching subscription from SSC")

cmd/frontend/internal/cody/subscription_test.go

@@ -2,7 +2,6 @@ package cody
 
 import (
 	"context"
-	"fmt"
 	"testing"
 	"time"
 
@@ -130,7 +129,7 @@ func TestGetSubscriptionForUser(t *testing.T) {
 					accounts = append(accounts, &extsvc.Account{AccountSpec: extsvc.AccountSpec{
 						AccountID:   MockSSCValue.SAMSAccountID,
 						ServiceType: "openidconnect",
-						ServiceID:   fmt.Sprintf("https://%s/", ssc.GetSAMSHostName()),
+						ServiceID:   ssc.GetSAMSServiceID(),
 					}})
 				}
 

cmd/frontend/internal/dotcom/productsubscription/codygateway_dotcom_user_test.go

@@ -2,7 +2,6 @@ package productsubscription_test
 
 import (
 	"context"
-	"fmt"
 	"math"
 	"testing"
 	"time"
@@ -313,7 +312,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) {
 		AccountSpec: extsvc.AccountSpec{
 			AccountID:   "123",
 			ServiceType: "openidconnect",
-			ServiceID:   fmt.Sprintf("https://%s", ssc.GetSAMSHostName()),
+			ServiceID:   ssc.GetSAMSServiceID(),
 		},
 	})
 	require.NoError(t, err)
@@ -328,7 +327,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) {
 		AccountSpec: extsvc.AccountSpec{
 			AccountID:   "456",
 			ServiceType: "openidconnect",
-			ServiceID:   fmt.Sprintf("https://%s", ssc.GetSAMSHostName()),
+			ServiceID:   ssc.GetSAMSServiceID(),
 		},
 	})
 	require.NoError(t, err)
@@ -343,7 +342,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) {
 		AccountSpec: extsvc.AccountSpec{
 			AccountID:   "789",
 			ServiceType: "openidconnect",
-			ServiceID:   fmt.Sprintf("https://%s", ssc.GetSAMSHostName()),
+			ServiceID:   ssc.GetSAMSServiceID(),
 		},
 	})
 	require.NoError(t, err)
@@ -358,7 +357,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) {
 		AccountSpec: extsvc.AccountSpec{
 			AccountID:   "abc",
 			ServiceType: "openidconnect",
-			ServiceID:   fmt.Sprintf("https://%s", ssc.GetSAMSHostName()),
+			ServiceID:   ssc.GetSAMSServiceID(),
 		},
 	})
 	require.NoError(t, err)

cmd/frontend/internal/httpapi/ssc.go

@@ -27,7 +27,7 @@ func newSSCRefreshCodyRateLimitHandler(logger log.Logger, db database.DB) http.H
 		oidcAccounts, err := db.UserExternalAccounts().List(ctx, database.ExternalAccountsListOptions{
 			AccountID:   samsAccountID,
 			ServiceType: "openidconnect",
-			ServiceID:   fmt.Sprintf("https://%s", ssc.GetSAMSHostName()),
+			ServiceID:   ssc.GetSAMSServiceID(),
 			LimitOffset: &database.LimitOffset{
 				Limit: 1,
 			},

cmd/frontend/internal/ssc/ssc.go

@@ -113,13 +113,52 @@ func (c *client) FetchSubscriptionBySAMSAccountID(ctx context.Context, samsAccou
 	}
 }
 
-func GetSAMSHostName() string {
-	sgconf := conf.Get().SiteConfig()
+// getSSCBaseURL returns the base URL for the SSC backend's REST API for
+// service-to-service requests.
+func getSSCBaseURL() string {
+	config := conf.Get()
+
+	// Prefer the newer "dotcom.codyProConfig.sscBackendOrigin" config setting if available.
+	// This allows for local development (not hard-coding the https scheme).
+	if dotcomConfig := config.Dotcom; dotcomConfig != nil {
+		if codyProConfig := dotcomConfig.CodyProConfig; codyProConfig != nil {
+			return fmt.Sprintf("%s/cody/api", codyProConfig.SscBackendOrigin)
+		}
+	}
+
+	// Fall back to original logic, using the "ssc.apiBaseUrl" setting.
+	// (To be removed when the codyProConfig changes are in production.)
+	siteConfig := config.SiteConfig()
+	baseURL := siteConfig.SscApiBaseUrl
+	if baseURL == "" {
+		baseURL = "https://accounts.sourcegraph.com/cody/api"
+	}
+
+	return baseURL
+}
+
+// GetSAMSServiceID returns the ServiceID of the currently registered SAMS identity provider.
+// This is found in the site configuration, and must match the auth.providers configuration
+// exactly.
+func GetSAMSServiceID() string {
+	config := conf.Get()
+
+	// Prefer the newer "dotcom.codyProConfig.samsBackendOrigin" config setting if available.
+	// This allows for local development (not hard-coding the https scheme).
+	if dotcomConfig := config.Dotcom; dotcomConfig != nil {
+		if codyProConfig := dotcomConfig.CodyProConfig; codyProConfig != nil {
+			return codyProConfig.SamsBackendOrigin
+		}
+	}
+
+	// Fallback to the original logic, using the "ssc.samsHostName" setting.
+	// (To be removed when the codyProConfig changes are in production.)
+	sgconf := config.SiteConfig()
 	if sgconf.SscSamsHostName == "" {
 		// If unset, default to the production hostname.
-		return "accounts.sourcegraph.com"
+		return "https://accounts.sourcegraph.com"
 	}
-	return sgconf.SscSamsHostName
+	return fmt.Sprintf("https://%s", sgconf.SscSamsHostName)
 }
 
 // NewClient returns a new SSC API client. It is important to avoid creating new
@@ -130,8 +169,6 @@ func GetSAMSHostName() string {
 // If no SAMS authorization provider is configured, this function will not panic,
 // but instead will return an error on every call.
 func NewClient() (Client, error) {
-	sgconf := conf.Get().SiteConfig()
-
 	// Fetch the SAMS configuration data.
 	var samsConfig *clientcredentials.Config
 	for _, provider := range conf.Get().AuthProviders {
@@ -140,7 +177,7 @@ func NewClient() (Client, error) {
 			continue
 		}
 
-		if strings.Contains(oidcInfo.Issuer, GetSAMSHostName()) {
+		if oidcInfo.Issuer == GetSAMSServiceID() {
 			samsConfig = &clientcredentials.Config{
 				ClientID:     oidcInfo.ClientID,
 				ClientSecret: oidcInfo.ClientSecret,
@@ -155,17 +192,12 @@ func NewClient() (Client, error) {
 		return &client{}, errors.New("no SAMS authorization provider configured")
 	}
 
-	baseURL := sgconf.SscApiBaseUrl
-	if baseURL == "" {
-		baseURL = "https://accounts.sourcegraph.com/cody/api"
-	}
-
 	// We want this tokenSource to be long lived, so we benefit from reusing existing
 	// SAMS tokens if repeated requests are made within the token's lifetime. (Under
 	// the hood it returns an oauth2.ReuseTokenSource.)
 	tokenSource := samsConfig.TokenSource(context.Background())
 	return &client{
-		baseURL:         baseURL,
+		baseURL:         getSSCBaseURL(),
 		samsTokenSource: tokenSource,
 	}, nil
 }