Updates to the cli environment configuration

cmd/environment/environment.md

@@ -19,6 +19,15 @@ If no environment is provided the CLI will use your tenant name as the name of t
 sail environment create
 ```
 
+When the CLI asks for the tenant name, you can also paste your tenant URL. This works for tenants that are not on the default `identitynow.com` domain. The CLI derives the environment name, the tenant URL, and the API URL from it. It then checks the API URL with the tenant. If the check passes, the CLI skips the URL confirmation prompts.
+
+```bash
+sail environment create
+Tenant name or URL (ie: acme, or https://acme.identitynow-demo.com): () https://devrel-ga-25031.identitynow-demo.com/
+
+✔ Found tenant at https://devrel-ga-25031.identitynow-demo.com (API: https://devrel-ga-25031.api.identitynow-demo.com)
+```
+
 ## Switching environments
 
 You can switch between environments by calling `use` and supplying the name of an existing environment you want to switch to.

internal/client/client.go

@@ -289,7 +289,10 @@ func (c *SpClient) resolveUrl(rawURL string) (string, bool, error) {
 	if u.Scheme == "" && u.Host == "" && u.Opaque == "" {
 		base := strings.TrimSuffix(config.GetBaseUrl(), "/")
 		if base == "" {
-			return "", false, fmt.Errorf("no tenant base url is configured, run \"sail configure\" or set SAIL_BASE_URL")
+			if err := config.CheckEnvironment(); err != nil {
+				return "", false, err
+			}
+			return "", false, fmt.Errorf("no tenant base url is configured, run \"sail env update\" or set SAIL_BASE_URL")
 		}
 		return base + trimmed, true, nil
 	}
@@ -311,7 +314,7 @@ func (c *SpClient) resolveUrl(rawURL string) (string, bool, error) {
 
 	base, err := url.Parse(strings.TrimSuffix(config.GetBaseUrl(), "/"))
 	if err != nil || base.Host == "" {
-		return "", false, fmt.Errorf("no valid tenant base url is configured, run \"sail configure\" or set SAIL_BASE_URL")
+		return "", false, fmt.Errorf("no valid tenant base url is configured, run \"sail env create\" or set SAIL_BASE_URL")
 	}
 
 	if !sameOrigin(base, u) {

internal/config/config.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 
@@ -159,7 +160,7 @@ func InitAPIClient(experimental bool) (*sailpoint.APIClient, error) {
 
 	token, err := GetAuthToken()
 	if err != nil {
-		log.Debug("unable to retrieve accesstoken", "error", err)
+		return apiClient, err
 	}
 
 	configuration := sailpoint.NewCLIConfiguration(sailpoint.ClientConfiguration{Token: token, BaseURL: GetBaseUrl()})
@@ -432,10 +433,50 @@ func TestSecretsStorage() bool {
 	}
 }
 
+// CheckEnvironment returns an error that tells the user what to do when no
+// usable environment is configured. Setting SAIL_BASE_URL skips the check,
+// because the CLI can then run from environment variables alone.
+func CheckEnvironment() error {
+	if os.Getenv("SAIL_BASE_URL") != "" {
+		return nil
+	}
+
+	environments := GetEnvironments()
+	if len(environments) == 0 {
+		return fmt.Errorf("no environment is configured\n\n" +
+			"Run `sail env create` to set up an environment, or set SAIL_BASE_URL, SAIL_CLIENT_ID, and SAIL_CLIENT_SECRET to use environment variables")
+	}
+
+	active := GetActiveEnvironment()
+	if environments[active] == nil {
+		names := make([]string, 0, len(environments))
+		for name := range environments {
+			names = append(names, name)
+		}
+		sort.Strings(names)
+
+		problem := "no active environment is selected"
+		if strings.TrimSpace(active) != "" {
+			problem = fmt.Sprintf("the active environment %q does not exist", active)
+		}
+
+		return fmt.Errorf("%s\n\nConfigured environments: %s\n\n"+
+			"Run `sail env use <name>` to select one, or `sail env create` to add a new one", problem, strings.Join(names, ", "))
+	}
+
+	return nil
+}
+
 func Validate() error {
 	var errors int
 	authType := GetAuthType()
 
+	if err := CheckEnvironment(); err != nil {
+		return err
+	}
+
+	env := GetActiveEnvironment()
+
 	supportsSecrets := TestSecretsStorage()
 
 	switch authType {
@@ -447,7 +488,7 @@ func Validate() error {
 		}
 
 		if GetBaseUrl() == "" {
-			log.Error("configured environment is missing BaseURL")
+			log.Error("configured environment is missing BaseURL, run `sail env update " + env + "` to set it")
 			errors++
 		}
 
@@ -477,18 +518,18 @@ func Validate() error {
 		}
 
 		if GetBaseUrl() == "" {
-			log.Error("configured environment is missing BaseURL")
+			log.Error("configured environment is missing BaseURL, run `sail env update " + env + "` to set it")
 			errors++
 		}
 
 		if GetTenantUrl() == "" {
-			log.Error("configured environment is missing TenantURL")
+			log.Error("configured environment is missing TenantURL, run `sail env update " + env + "` to set it")
 			errors++
 		}
 
 	default:
 
-		log.Error("invalid authtype '%s' configured", authType)
+		log.Error("invalid authtype configured, run `sail set auth` to choose oauth or pat", "authtype", authType)
 		errors++
 
 	}

internal/config/environment_test.go

@@ -0,0 +1,119 @@
+package config
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/spf13/viper"
+	keyring "github.com/zalando/go-keyring"
+)
+
+// resetViper clears all configuration for the test and again when it ends.
+func resetViper(t *testing.T) {
+	t.Helper()
+	viper.Reset()
+	t.Cleanup(viper.Reset)
+}
+
+func TestCheckEnvironmentNoEnvironments(t *testing.T) {
+	resetViper(t)
+	t.Setenv("SAIL_BASE_URL", "")
+
+	err := CheckEnvironment()
+	if err == nil || !strings.Contains(err.Error(), "no environment is configured") || !strings.Contains(err.Error(), "sail env create") {
+		t.Fatalf("CheckEnvironment() error = %v, want no-environment guidance", err)
+	}
+}
+
+func TestCheckEnvironmentUnknownActive(t *testing.T) {
+	resetViper(t)
+	t.Setenv("SAIL_BASE_URL", "")
+	viper.Set("environments.beta.baseurl", "https://beta.api.identitynow.com")
+	viper.Set("environments.alpha.baseurl", "https://alpha.api.identitynow.com")
+	SetActiveEnvironment("gone")
+
+	err := CheckEnvironment()
+	if err == nil {
+		t.Fatal("CheckEnvironment() error = nil, want error")
+	}
+	for _, want := range []string{`"gone" does not exist`, "alpha, beta", "sail env use"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("CheckEnvironment() error = %q, want it to contain %q", err, want)
+		}
+	}
+}
+
+func TestCheckEnvironmentNoActive(t *testing.T) {
+	resetViper(t)
+	t.Setenv("SAIL_BASE_URL", "")
+	viper.Set("environments.alpha.baseurl", "https://alpha.api.identitynow.com")
+	SetActiveEnvironment("")
+
+	err := CheckEnvironment()
+	if err == nil || !strings.Contains(err.Error(), "no active environment is selected") {
+		t.Fatalf("CheckEnvironment() error = %v, want no-active guidance", err)
+	}
+}
+
+func TestCheckEnvironmentValid(t *testing.T) {
+	resetViper(t)
+	t.Setenv("SAIL_BASE_URL", "")
+	viper.Set("environments.alpha.baseurl", "https://alpha.api.identitynow.com")
+	SetActiveEnvironment("Alpha")
+
+	if err := CheckEnvironment(); err != nil {
+		t.Fatalf("CheckEnvironment() error = %v", err)
+	}
+}
+
+func TestCheckEnvironmentEnvVars(t *testing.T) {
+	resetViper(t)
+	t.Setenv("SAIL_BASE_URL", "https://acme.api.identitynow.com")
+
+	if err := CheckEnvironment(); err != nil {
+		t.Fatalf("CheckEnvironment() error = %v, want nil when SAIL_BASE_URL is set", err)
+	}
+}
+
+func TestMissingSecretError(t *testing.T) {
+	err := missingSecretError("client ID", "SAIL_CLIENT_ID", "alpha", keyring.ErrNotFound)
+	for _, want := range []string{`client ID is stored for environment "alpha"`, "sail set pat", "SAIL_CLIENT_ID"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("missingSecretError() = %q, want it to contain %q", err, want)
+		}
+	}
+
+	other := errors.New("keychain locked")
+	if err := missingSecretError("client ID", "SAIL_CLIENT_ID", "alpha", other); !errors.Is(err, other) {
+		t.Errorf("missingSecretError() = %v, want it to wrap the keyring error", err)
+	}
+}
+
+func TestPatTokenErrorInvalidClient(t *testing.T) {
+	body := []byte(`{"error_description":"Full authentication is required to access this resource","error":"invalid_client"}`)
+
+	err := patTokenError(400, body, "devrel", false)
+	for _, want := range []string{"rejected the PAT client ID or client secret", `"devrel"`, "sail set pat"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("patTokenError() = %q, want it to contain %q", err, want)
+		}
+	}
+
+	err = patTokenError(400, body, "devrel", true)
+	if !strings.Contains(err.Error(), "SAIL_CLIENT_ID and SAIL_CLIENT_SECRET") {
+		t.Errorf("patTokenError() = %q, want environment variable guidance", err)
+	}
+}
+
+func TestPatTokenErrorOther(t *testing.T) {
+	err := patTokenError(500, []byte(`{"error":"server_error","error_description":"try again later"}`), "devrel", false)
+	if !strings.Contains(err.Error(), "status 500") || !strings.Contains(err.Error(), "try again later") {
+		t.Errorf("patTokenError() = %q, want status and description", err)
+	}
+
+	err = patTokenError(502, []byte("bad gateway"), "devrel", false)
+	if !strings.Contains(err.Error(), "bad gateway") {
+		t.Errorf("patTokenError() = %q, want raw body", err)
+	}
+}

internal/config/oauth.go

@@ -365,6 +365,19 @@ func discoverAuthorizeEndpoint(baseURL string) (string, error) {
 	return info.AuthorizeEndpoint, nil
 }
 
+// CheckTenantAPI reports whether baseURL answers {baseURL}/oauth/info like an
+// Identity Security Cloud tenant. It is used to confirm a generated API URL
+// before an environment is saved.
+func CheckTenantAPI(baseURL string) error {
+	parsed, err := assertHTTPSURL(strings.TrimSuffix(baseURL, "/"), "tenant API URL")
+	if err != nil {
+		return err
+	}
+
+	_, err = discoverAuthorizeEndpoint(parsed.Scheme + "://" + parsed.Host)
+	return err
+}
+
 // randomURLSafeString returns byteLength random bytes as a base64url string.
 func randomURLSafeString(byteLength int) (string, error) {
 	buf := make([]byte, byteLength)

internal/config/oauth_test.go

@@ -3,6 +3,8 @@ package config
 import (
 	"encoding/base64"
 	"encoding/json"
+	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 )
@@ -220,3 +222,60 @@ func TestPromptForPasteCodeTrimsInput(t *testing.T) {
 		t.Fatalf("promptForPasteCode() = %q, want %q", got, "sp1.abc")
 	}
 }
+
+// withTenantServer starts a TLS server that serves /oauth/info with the given
+// status and body, and points oauthHTTPClient at it for the test.
+func withTenantServer(t *testing.T, status int, body string) string {
+	t.Helper()
+
+	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/oauth/info" {
+			http.NotFound(w, r)
+			return
+		}
+		w.WriteHeader(status)
+		_, _ = w.Write([]byte(body))
+	}))
+	t.Cleanup(server.Close)
+
+	previous := oauthHTTPClient
+	oauthHTTPClient = server.Client()
+	t.Cleanup(func() { oauthHTTPClient = previous })
+
+	return server.URL
+}
+
+func TestCheckTenantAPIAcceptsTenant(t *testing.T) {
+	baseURL := withTenantServer(t, http.StatusOK, `{"authorizeEndpoint":"https://acme.example.com/oauth/authorize"}`)
+
+	if err := CheckTenantAPI(baseURL + "/"); err != nil {
+		t.Fatalf("CheckTenantAPI() error = %v", err)
+	}
+}
+
+func TestCheckTenantAPIRejectsNonTenant(t *testing.T) {
+	cases := map[string]struct {
+		status int
+		body   string
+	}{
+		"not found":        {http.StatusNotFound, "not found"},
+		"missing endpoint": {http.StatusOK, `{}`},
+		"not json":         {http.StatusOK, "<html></html>"},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			baseURL := withTenantServer(t, tc.status, tc.body)
+
+			if err := CheckTenantAPI(baseURL); err == nil {
+				t.Fatal("CheckTenantAPI() error = nil, want error")
+			}
+		})
+	}
+}
+
+func TestCheckTenantAPIRejectsHTTP(t *testing.T) {
+	if err := CheckTenantAPI("http://acme.api.identitynow.com"); err == nil {
+		t.Fatal("CheckTenantAPI() error = nil, want error")
+	}
+}