fix(config): write to keychain before config (#1044)

Reviewed-on: https://gitea.com/gitea/tea/pulls/1044
Co-authored-by: techknowlogick <techknowlogick@gitea.com>
Co-committed-by: techknowlogick <techknowlogick@gitea.com>

modules/auth/oauth.go

@@ -412,16 +412,11 @@ func createLoginFromToken(ctx context.Context, name, serverURL string, token *oa
 	}
 	login.SSHHost = parsedURL.Host
 
-	// Add login to config
+	// Save tokens and add login to config
-	if err := config.AddLogin(&login); err != nil {
+	if err := config.AddOAuthLogin(&login, token.AccessToken, token.RefreshToken, token.Expiry); err != nil {
 		return err
 	}
 
-	// Save tokens to credstore
-	if err := config.SaveOAuthToken(login.Name, token.AccessToken, token.RefreshToken, token.Expiry); err != nil {
-		return fmt.Errorf("failed to save token to secure store: %s", err)
-	}
-
 	fmt.Printf("Login as %s on %s successful. Added this login as %s\n", login.User, login.URL, login.Name)
 	return nil
 }

modules/config/credstore.go

@@ -16,6 +16,18 @@ import (
 var (
 	tokenStore     *credstore.SecureStore[credstore.Token]
 	tokenStoreOnce sync.Once
+
+	saveOAuthTokenToStore = func(loginName, accessToken, refreshToken string, expiresAt time.Time) error {
+		return getTokenStore().Save(loginName, credstore.Token{
+			AccessToken:  accessToken,
+			RefreshToken: refreshToken,
+			ExpiresAt:    expiresAt,
+			ClientID:     loginName,
+		})
+	}
+	deleteOAuthTokenFromStore = func(loginName string) error {
+		return getTokenStore().Delete(loginName)
+	}
 )
 
 func getTokenStore() *credstore.SecureStore[credstore.Token] {
@@ -37,17 +49,12 @@ func LoadOAuthToken(loginName string) (*credstore.Token, error) {
 
 // SaveOAuthToken saves OAuth tokens to the secure store.
 func SaveOAuthToken(loginName, accessToken, refreshToken string, expiresAt time.Time) error {
-	return getTokenStore().Save(loginName, credstore.Token{
+	return saveOAuthTokenToStore(loginName, accessToken, refreshToken, expiresAt)
-		AccessToken:  accessToken,
-		RefreshToken: refreshToken,
-		ExpiresAt:    expiresAt,
-		ClientID:     loginName,
-	})
 }
 
 // DeleteOAuthToken removes tokens from the secure store.
 func DeleteOAuthToken(loginName string) error {
-	return getTokenStore().Delete(loginName)
+	return deleteOAuthTokenFromStore(loginName)
 }
 
 // SaveOAuthTokenFromOAuth2 saves an oauth2.Token to credstore, falling back to

modules/config/login.go

@@ -269,6 +269,34 @@ func AddLogin(login *Login) error {
 	})
 }
 
+// AddOAuthLogin saves the OAuth token and login profile as one operation.
+// The profile is only written after secure token storage succeeds.
+func AddOAuthLogin(login *Login, accessToken, refreshToken string, expiresAt time.Time) error {
+	return withConfigLock(func() error {
+		// Check for duplicate login names before touching credential storage.
+		for _, existing := range config.Logins {
+			if strings.EqualFold(existing.Name, login.Name) {
+				return fmt.Errorf("login name '%s' already exists", login.Name)
+			}
+		}
+
+		if err := SaveOAuthToken(login.Name, accessToken, refreshToken, expiresAt); err != nil {
+			return fmt.Errorf("failed to save token to secure store: %w", err)
+		}
+
+		config.Logins = append(config.Logins, *login)
+		if err := saveConfigUnsafe(); err != nil {
+			config.Logins = config.Logins[:len(config.Logins)-1]
+			if deleteErr := DeleteOAuthToken(login.Name); deleteErr != nil {
+				return errors.Join(err, fmt.Errorf("failed to clean up OAuth token after config save failure: %w", deleteErr))
+			}
+			return err
+		}
+
+		return nil
+	})
+}
+
 // SaveLoginTokens updates the token fields for an existing login.
 // This is used after browser-based re-authentication to save new tokens.
 func SaveLoginTokens(login *Login) error {