From 79ec2afe9f26102121cf0a6acb60133c644a6a97 Mon Sep 17 00:00:00 2001 From: TP Honey Date: Tue, 3 Dec 2024 12:53:01 +0000 Subject: [PATCH 1/2] (feat) allow the saving of multiple tokens --- cmd/changes_get_change.go | 2 +- cmd/changes_manual_change.go | 3 +- cmd/changes_submit_plan.go | 2 +- cmd/pterm.go | 4 -- cmd/root.go | 123 +++++++++++++++++++++++++---------- cmd/root_test.go | 95 +++++++++++++++++++++++++++ 6 files changed, 185 insertions(+), 44 deletions(-) diff --git a/cmd/changes_get_change.go b/cmd/changes_get_change.go index 5a1c90b0..6b48ab44 100644 --- a/cmd/changes_get_change.go +++ b/cmd/changes_get_change.go @@ -43,7 +43,7 @@ const assetVersion = "17c7fd2c365d4f4cdd8e414ca5148f825fa4febd" func GetChange(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - app := getAppUrl(viper.GetString("frontend"), viper.GetString("app")) + app := viper.GetString("app") riskLevels := []sdp.Risk_Severity{} for _, level := range viper.GetStringSlice("risk-levels") { diff --git a/cmd/changes_manual_change.go b/cmd/changes_manual_change.go index ff2d7ec8..bf3be117 100644 --- a/cmd/changes_manual_change.go +++ b/cmd/changes_manual_change.go @@ -25,8 +25,7 @@ var manualChangeCmd = &cobra.Command{ func ManualChange(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - - app := getAppUrl(viper.GetString("frontend"), viper.GetString("app")) + app := viper.GetString("app") method, err := methodFromString(viper.GetString("query-method")) if err != nil { diff --git a/cmd/changes_submit_plan.go b/cmd/changes_submit_plan.go index 6f936a43..290d31f9 100644 --- a/cmd/changes_submit_plan.go +++ b/cmd/changes_submit_plan.go @@ -91,7 +91,7 @@ func tryLoadText(ctx context.Context, fileName string) string { func SubmitPlan(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - app := getAppUrl(viper.GetString("frontend"), viper.GetString("app")) + app := viper.GetString("app") ctx, oi, _, err := login(ctx, cmd, []string{"changes:write"}, nil) if err != nil { diff --git a/cmd/pterm.go b/cmd/pterm.go index 44a3e139..e04867c3 100644 --- a/cmd/pterm.go +++ b/cmd/pterm.go @@ -321,23 +321,19 @@ func extractClaims(token string) (*sdp.CustomClaims, error) { // contains the right scopes. Therefore we just parse the payload // directly sections := strings.Split(token, ".") - if len(sections) != 3 { return nil, errors.New("token is not a JWT") } // Decode the payload decodedPayload, err := base64.RawURLEncoding.DecodeString(sections[1]) - if err != nil { return nil, fmt.Errorf("error decoding token payload: %w", err) } // Parse the payload claims := new(sdp.CustomClaims) - err = json.Unmarshal(decodedPayload, claims) - if err != nil { return nil, fmt.Errorf("error parsing token payload: %w", err) } diff --git a/cmd/root.go b/cmd/root.go index c5f4c47c..10c817e9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -89,7 +89,12 @@ func PreRunSetup(cmd *cobra.Command, args []string) { log.AllLevels[:log.GetLevel()+1]..., ))) } - + // set up app, it may be ambiguous if frontend is set + app := getAppUrl(viper.GetString("frontend"), viper.GetString("app")) + if app == "" { + log.Fatal("no app specified, please use --app or set OVM_APP") + } + viper.Set("app", app) // capture span in global variable to allow Execute() below to end it ctx, cmdSpan = tracing.Tracer().Start(ctx, fmt.Sprintf("CLI %v", cmd.CommandPath()), trace.WithAttributes( attribute.String("ovm.config", fmt.Sprintf("%v", tracedSettings())), @@ -435,13 +440,11 @@ func ensureToken(ctx context.Context, oi sdp.OvermindInstance, requiredScopes [] // that token locally for use later, and will use the cached token if possible func getOauthToken(ctx context.Context, oi sdp.OvermindInstance, requiredScopes []string) (*oauth2.Token, error) { var localScopes []string - - // Check for a locally saved token in ~/.overmind - if home, err := os.UserHomeDir(); err == nil { - var localToken *oauth2.Token - - localToken, localScopes, err = readLocalToken(home, requiredScopes) - + var localToken *oauth2.Token + home, err := os.UserHomeDir() + if err == nil { + // Check for a locally saved token in ~/.overmind + localToken, localScopes, err = readLocalTokenFile(home, viper.GetString("app"), requiredScopes) if err != nil { log.WithContext(ctx).Debugf("Error reading local token, ignoring: %v", err) } else { @@ -532,28 +535,13 @@ func getOauthToken(ctx context.Context, oi sdp.OvermindInstance, requiredScopes ) } - // Save the token locally - if home, err := os.UserHomeDir(); err == nil { - // Create the directory if it doesn't exist - err = os.MkdirAll(filepath.Join(home, ".overmind"), 0700) - if err != nil { - log.WithContext(ctx).WithError(err).Error("Failed to create ~/.overmind directory") - } - - // Write the token to a file - path := filepath.Join(home, ".overmind", "token.json") - file, err := os.Create(path) - if err != nil { - log.WithContext(ctx).WithError(err).Errorf("Failed to create token file at %v", path) - } - - // Encode the token - err = json.NewEncoder(file).Encode(token) + // Save the token to the local file, if the home directory is available + if home != "" { + err = saveLocalTokenFile(home, viper.GetString("app"), token) if err != nil { - log.WithContext(ctx).WithError(err).Errorf("Failed to encode token file at %v", path) + // we don't worry if we cannot save the token, it will just be requested again + log.WithContext(ctx).WithError(err).Error("Error saving token") } - - log.WithContext(ctx).Debugf("Saved token to %v", path) } return token, nil @@ -599,11 +587,20 @@ func getAPIKeyToken(ctx context.Context, oi sdp.OvermindInstance, apiKey string, return token, nil } -func readLocalToken(homeDir string, requiredScopes []string) (*oauth2.Token, []string, error) { +type TokenFile struct { + AuthEntries map[string]*TokenEntry `json:"auth_entries"` +} + +type TokenEntry struct { + Token *oauth2.Token `json:"token"` + AddedDate time.Time `json:"added_date"` +} + +func readLocalTokenFile(homeDir, app string, requiredScopes []string) (*oauth2.Token, []string, error) { // Read in the token JSON file path := filepath.Join(homeDir, ".overmind", "token.json") - token := new(oauth2.Token) + tokenFile := new(TokenFile) // Check that the file exists if _, err := os.Stat(path); err != nil { @@ -615,19 +612,25 @@ func readLocalToken(homeDir string, requiredScopes []string) (*oauth2.Token, []s if err != nil { return nil, nil, fmt.Errorf("error opening token file at %v: %w", path, err) } + defer file.Close() // Decode the file - err = json.NewDecoder(file).Decode(token) + err = json.NewDecoder(file).Decode(tokenFile) if err != nil { return nil, nil, fmt.Errorf("error decoding token file at %v: %w", path, err) } + authEntry, ok := tokenFile.AuthEntries[app] + if !ok { + return nil, nil, fmt.Errorf("no token found for app %v", app) + } + // Check to see if the token is still valid - if !token.Valid() { + if !authEntry.Token.Valid() { return nil, nil, errors.New("token is no longer valid") } - claims, err := extractClaims(token.AccessToken) + claims, err := extractClaims(authEntry.Token.AccessToken) if err != nil { return nil, nil, fmt.Errorf("error extracting claims from token: %w", err) } @@ -638,7 +641,7 @@ func readLocalToken(homeDir string, requiredScopes []string) (*oauth2.Token, []s currentScopes := strings.Split(claims.Scope, " ") // Check that we actually got the claims we asked for. - ok, missing, err := HasScopesFlexible(token, requiredScopes) + ok, missing, err := HasScopesFlexible(authEntry.Token, requiredScopes) if err != nil { return nil, currentScopes, fmt.Errorf("error checking token scopes: %w", err) } @@ -646,8 +649,56 @@ func readLocalToken(homeDir string, requiredScopes []string) (*oauth2.Token, []s return nil, currentScopes, fmt.Errorf("local token is missing this permission: '%v'", missing) } - log.Debugf("Using local token from %v", path) - return token, currentScopes, nil + log.Debugf("Using local token from %v for %s", path, app) + return authEntry.Token, currentScopes, nil +} + +func saveLocalTokenFile(homeDir, app string, token *oauth2.Token) error { + // Read in the existing token file if it exists + path := filepath.Join(homeDir, ".overmind", "token.json") + + tokenFile := &TokenFile{ + AuthEntries: make(map[string]*TokenEntry), + } + + if _, err := os.Stat(path); err == nil { + file, err := os.Open(path) + if err == nil { + // file exists, read it + defer file.Close() + + err = json.NewDecoder(file).Decode(tokenFile) + if err != nil { + return fmt.Errorf("error decoding token file at %v: %w", path, err) + } + } + } else { + err = os.MkdirAll(filepath.Dir(path), 0755) + if err != nil { + return fmt.Errorf("unexpected fail creating directories: %w", err) + } + } + + // Update the token for the given app + tokenFile.AuthEntries[app] = &TokenEntry{ + Token: token, + AddedDate: time.Now(), + } + + // Write the updated token file + file, err := os.Create(path) + if err != nil { + return fmt.Errorf("error creating token file at %v: %w", path, err) + } + defer file.Close() + + err = json.NewEncoder(file).Encode(tokenFile) + if err != nil { + return fmt.Errorf("error encoding token file at %v: %w", path, err) + } + + log.Debugf("Saved token to %v for %s", path, app) + return nil } func getAppUrl(frontend, app string) string { diff --git a/cmd/root_test.go b/cmd/root_test.go index e39e5141..033eff3a 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "testing" + "time" "github.com/overmindtech/sdp-go" "golang.org/x/oauth2" @@ -110,3 +111,97 @@ func Test_getAppUrl(t *testing.T) { }) } } + +func TestSaveTokenFile(t *testing.T) { + // Setup temporary directory for testing + tempDir := t.TempDir() + app := "https://localhost.df.overmind-demo.com:3000" + + claims := sdp.CustomClaims{ + Scope: "scope1 scope2", + AccountName: "test", + } + jsonClaims, err := json.Marshal(claims) + if err != nil { + t.Fatalf("unexpected fail marshalling claims: %v", err) + } + claimsSection := base64.RawURLEncoding.EncodeToString([]byte(jsonClaims)) + accessToken := fmt.Sprintf("%s.%s.%s", "header", claimsSection, "signature") + token := &oauth2.Token{ + AccessToken: accessToken, + Expiry: time.Now().Add(1 * time.Hour), + } + + // Test saving the token file + err = saveLocalTokenFile(tempDir, app, token) + if err != nil { + t.Fatalf("unexpected fail saving token file: %v", err) + } + // Test reading the token file + readAppToken, readClaims, err := readLocalTokenFile(tempDir, app, nil) + if err != nil { + t.Fatalf("unexpected fail reading token file: %v", err) + } + if readAppToken.AccessToken != token.AccessToken { + t.Fatalf("expected: %v, got: %v", token.AccessToken, readAppToken.AccessToken) + } + if readClaims[0] != "scope1" { + t.Fatalf("expected: %v, got: %v", "scope1", readClaims[0]) + } + if readClaims[1] != "scope2" { + t.Fatalf("expected: %v, got: %v", "scope2", readClaims[1]) + } + + // lets read a token from a non existent app + nonExistentToken, _, err := readLocalTokenFile(tempDir, "otherApp", nil) + if err == nil { + t.Fatalf("expected error, got nil") + } + if nonExistentToken == readAppToken { + t.Fatalf("expected different tokens, got the same") + } + + // lets write the token to a different app + otherApp := "otherApp" + err = saveLocalTokenFile(tempDir, otherApp, token) + if err != nil { + t.Fatalf("unexpected fail saving token file: %v", err) + } + readAppToken, _, err = readLocalTokenFile(tempDir, otherApp, nil) + if err != nil { + t.Fatalf("unexpected fail reading token file: %v", err) + } + if readAppToken.AccessToken != token.AccessToken { + t.Fatalf("expected: %v, got: %v", token.AccessToken, readAppToken.AccessToken) + } + + // lets update the first app token + claims = sdp.CustomClaims{ + Scope: "scope3 scope4", + AccountName: "test", + } + jsonClaims, err = json.Marshal(claims) + if err != nil { + t.Fatalf("unexpected fail marshalling claims: %v", err) + } + claimsSection = base64.RawURLEncoding.EncodeToString([]byte(jsonClaims)) + accessToken = fmt.Sprintf("%s.%s.%s", "header", claimsSection, "signature") + newToken := &oauth2.Token{ + AccessToken: accessToken, + Expiry: time.Now().Add(1 * time.Hour), + } + err = saveLocalTokenFile(tempDir, app, newToken) + if err != nil { + t.Fatalf("unexpected fail saving token file: %v", err) + } + _, lastClaims, err := readLocalTokenFile(tempDir, app, nil) + if err != nil { + t.Fatalf("unexpected fail reading token file: %v", err) + } + if lastClaims[0] != "scope3" { + t.Fatalf("expected: %v, got: %v", "scope3", lastClaims[0]) + } + if lastClaims[1] != "scope4" { + t.Fatalf("expected: %v, got: %v", "scope4", lastClaims[1]) + } +} From 4edeb60d586caa4202c8b1917d19037c3fc4f819 Mon Sep 17 00:00:00 2001 From: TP Honey Date: Tue, 3 Dec 2024 17:39:32 +0000 Subject: [PATCH 2/2] (fix) improve information when logging in --- .github/workflows/e2e.yaml | 5 +++-- cmd/root.go | 42 +++++++++++++++++++------------------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 81118551..1d87921e 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -69,8 +69,9 @@ jobs: tfplan.json \ > ./overmindtech-change-url - ./overmind changes get-change \ - --change "$(< ./overmindtech-change-url)" \ + # we only want the last line of the previous command + ./overmind changes get-change \ + --change "$(tail -n 1 ./overmindtech-change-url)" \ --format markdown \ > ./overmindtech-message diff --git a/cmd/root.go b/cmd/root.go index 10c817e9..4c0c0fc9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -446,7 +446,9 @@ func getOauthToken(ctx context.Context, oi sdp.OvermindInstance, requiredScopes // Check for a locally saved token in ~/.overmind localToken, localScopes, err = readLocalTokenFile(home, viper.GetString("app"), requiredScopes) if err != nil { - log.WithContext(ctx).Debugf("Error reading local token, ignoring: %v", err) + if !os.IsNotExist(err) { + pterm.Info.Println(fmt.Sprintf("Skipping using local token: %v. Re-authenticating.", err)) + } } else { // If we already have the right scopes, return the token return localToken, nil @@ -549,12 +551,10 @@ func getOauthToken(ctx context.Context, oi sdp.OvermindInstance, requiredScopes // Gets a token using an API key func getAPIKeyToken(ctx context.Context, oi sdp.OvermindInstance, apiKey string, requiredScopes []string) (*oauth2.Token, error) { - log.WithContext(ctx).Debug("using provided token for authentication") - var token *oauth2.Token - + app := viper.GetString("app") if !strings.HasPrefix(apiKey, "ovm_api_") { - return nil, errors.New("OVM_API_KEY does not match pattern 'ovm_api_*'") + return nil, errors.New("--api-key or OVM_API_KEY or API_KEY does not match pattern 'ovm_api_*'") } // exchange api token for JWT @@ -565,9 +565,8 @@ func getAPIKeyToken(ctx context.Context, oi sdp.OvermindInstance, apiKey string, }, }) if err != nil { - return nil, fmt.Errorf("error authenticating the API token: %w", err) + return nil, fmt.Errorf("error authenticating the API token for %s: %w", app, err) } - log.WithContext(ctx).Debug("successfully got a token from the API key") token = &oauth2.Token{ AccessToken: resp.Msg.GetAccessToken(), @@ -578,12 +577,12 @@ func getAPIKeyToken(ctx context.Context, oi sdp.OvermindInstance, apiKey string, // permission auth0 will just not assign those scopes rather than fail ok, missing, err := HasScopesFlexible(token, requiredScopes) if err != nil { - return nil, fmt.Errorf("error checking token scopes: %w", err) + return nil, fmt.Errorf("error checking token scopes for %s: %w", app, err) } if !ok { - return nil, fmt.Errorf("authenticated successfully, but your API key is missing this permission: '%v'", missing) + return nil, fmt.Errorf("authenticated successfully against %s, but your API key is missing this permission: '%v'", app, missing) } - + pterm.Info.Println(fmt.Sprintf("Using Overmind API key for %s", app)) return token, nil } @@ -596,6 +595,7 @@ type TokenEntry struct { AddedDate time.Time `json:"added_date"` } +// readLocalTokenFile is also used in the gateway assistant cli tool. It is copied over, so if you change it here, you should also change it there. func readLocalTokenFile(homeDir, app string, requiredScopes []string) (*oauth2.Token, []string, error) { // Read in the token JSON file path := filepath.Join(homeDir, ".overmind", "token.json") @@ -610,19 +610,19 @@ func readLocalTokenFile(homeDir, app string, requiredScopes []string) (*oauth2.T // Read the file file, err := os.Open(path) if err != nil { - return nil, nil, fmt.Errorf("error opening token file at %v: %w", path, err) + return nil, nil, fmt.Errorf("error opening token file at %q: %w", path, err) } defer file.Close() // Decode the file err = json.NewDecoder(file).Decode(tokenFile) if err != nil { - return nil, nil, fmt.Errorf("error decoding token file at %v: %w", path, err) + return nil, nil, fmt.Errorf("error decoding token file at %q: %w", path, err) } authEntry, ok := tokenFile.AuthEntries[app] if !ok { - return nil, nil, fmt.Errorf("no token found for app %v", app) + return nil, nil, fmt.Errorf("no token found for app %s in %q", app, path) } // Check to see if the token is still valid @@ -632,7 +632,7 @@ func readLocalTokenFile(homeDir, app string, requiredScopes []string) (*oauth2.T claims, err := extractClaims(authEntry.Token.AccessToken) if err != nil { - return nil, nil, fmt.Errorf("error extracting claims from token: %w", err) + return nil, nil, fmt.Errorf("error extracting claims from token: %s in %q: %w", app, path, err) } if claims.Scope == "" { return nil, nil, errors.New("token does not have any scopes") @@ -643,13 +643,13 @@ func readLocalTokenFile(homeDir, app string, requiredScopes []string) (*oauth2.T // Check that we actually got the claims we asked for. ok, missing, err := HasScopesFlexible(authEntry.Token, requiredScopes) if err != nil { - return nil, currentScopes, fmt.Errorf("error checking token scopes: %w", err) + return nil, currentScopes, fmt.Errorf("error checking token scopes: %s in %q: %w", app, path, err) } if !ok { - return nil, currentScopes, fmt.Errorf("local token is missing this permission: '%v'", missing) + return nil, currentScopes, fmt.Errorf("local token is missing this permission: '%v'. %s in %q", missing, app, path) } - log.Debugf("Using local token from %v for %s", path, app) + pterm.Info.Println(fmt.Sprintf("Using local token for %s in %q", app, path)) return authEntry.Token, currentScopes, nil } @@ -669,7 +669,7 @@ func saveLocalTokenFile(homeDir, app string, token *oauth2.Token) error { err = json.NewDecoder(file).Decode(tokenFile) if err != nil { - return fmt.Errorf("error decoding token file at %v: %w", path, err) + return fmt.Errorf("error decoding token file at %q: %w", path, err) } } } else { @@ -688,16 +688,16 @@ func saveLocalTokenFile(homeDir, app string, token *oauth2.Token) error { // Write the updated token file file, err := os.Create(path) if err != nil { - return fmt.Errorf("error creating token file at %v: %w", path, err) + return fmt.Errorf("error creating token file at %q: %w", path, err) } defer file.Close() err = json.NewEncoder(file).Encode(tokenFile) if err != nil { - return fmt.Errorf("error encoding token file at %v: %w", path, err) + return fmt.Errorf("error encoding token file at %q: %w", path, err) } - log.Debugf("Saved token to %v for %s", path, app) + pterm.Info.Println(fmt.Sprintf("Saving token locally for %s at %q", app, path)) return nil }