fix: correct api URL handling in CLI (#570)

r0ssing · web-flow · commit 1d64a0137d80 · 2026-03-24T11:00:04.000+03:00
diff --git a/synkronus-cli/internal/auth/auth.go b/synkronus-cli/internal/auth/auth.go
@@ -6,19 +6,13 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strings"
 	"time"
 
 	"github.com/OpenDataEnsemble/ode/synkronus-cli/internal/utils"
 	"github.com/golang-jwt/jwt/v5"
 	"github.com/spf13/viper"
 )
 
-// normalizeBaseURL trims trailing slashes to avoid double slashes when joining paths.
-func normalizeBaseURL(base string) string {
-	return strings.TrimRight(base, "/")
-}
-
 // TokenResponse represents the response from the authentication endpoint
 type TokenResponse struct {
 	Token        string `json:"token"`
@@ -35,8 +29,8 @@ type Claims struct {
 
 // Login authenticates with the Synkronus API and returns a token
 func Login(username, password string) (*TokenResponse, error) {
-	apiURL := normalizeBaseURL(utils.EnsureScheme(viper.GetString("api.url")))
-	loginURL := fmt.Sprintf("%s/api/auth/login", apiURL)
+	apiURL := utils.APIBaseURL(viper.GetString("api.url"))
+	loginURL := fmt.Sprintf("%s/auth/login", apiURL)
 
 	// Prepare login request
 	loginData := map[string]string{
@@ -97,8 +91,8 @@ func Login(username, password string) (*TokenResponse, error) {
 
 // RefreshToken refreshes the JWT token
 func RefreshToken() (*TokenResponse, error) {
-	apiURL := normalizeBaseURL(utils.EnsureScheme(viper.GetString("api.url")))
-	refreshURL := fmt.Sprintf("%s/api/auth/refresh", apiURL)
+	apiURL := utils.APIBaseURL(viper.GetString("api.url"))
+	refreshURL := fmt.Sprintf("%s/auth/refresh", apiURL)
 	refreshToken := viper.GetString("auth.refresh_token")
 
 	// Prepare refresh request
diff --git a/synkronus-cli/internal/cmd/health.go b/synkronus-cli/internal/cmd/health.go
@@ -3,7 +3,6 @@ package cmd
 import (
 	"fmt"
 	"net/http"
-	"strings"
 	"time"
 
 	"github.com/OpenDataEnsemble/ode/synkronus-cli/internal/utils"
@@ -17,8 +16,8 @@ func init() {
 		Short: "Check the health of the Synkronus API",
 		Long:  `Verify connectivity to the Synkronus API server.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			apiURL := strings.TrimRight(utils.EnsureScheme(viper.GetString("api.url")), "/")
-			healthURL := apiURL + "/health"
+			origin := utils.OriginURL(viper.GetString("api.url"))
+			healthURL := origin + "/health"
 
 			utils.PrintInfo("Checking API health at %s...", healthURL)
 
diff --git a/synkronus-cli/internal/utils/url.go b/synkronus-cli/internal/utils/url.go
@@ -14,3 +14,36 @@ func EnsureScheme(raw string) string {
 	}
 	return "https://" + raw
 }
+
+// NormalizeURL trims trailing slashes from a URL string (after EnsureScheme).
+func NormalizeURL(raw string) string {
+	return strings.TrimRight(EnsureScheme(raw), "/")
+}
+
+// APIBaseURL returns the base URL for Synkronus HTTP API routes in openapi/synkronus.yaml,
+// which are all under the /api prefix from the deployment origin.
+// If the configured URL already ends with /api, it is left unchanged so paths are not doubled.
+func APIBaseURL(raw string) string {
+	base := NormalizeURL(raw)
+	if base == "" {
+		return base
+	}
+	if strings.HasSuffix(strings.ToLower(base), "/api") {
+		return base
+	}
+	return base + "/api"
+}
+
+// OriginURL strips a trailing /api segment (case-insensitive) so callers can reach routes
+// served at the site root, e.g. GET /health in the OpenAPI spec.
+func OriginURL(raw string) string {
+	base := NormalizeURL(raw)
+	if base == "" {
+		return base
+	}
+	lower := strings.ToLower(base)
+	if strings.HasSuffix(lower, "/api") {
+		return strings.TrimRight(base[:len(base)-len("/api")], "/")
+	}
+	return base
+}
diff --git a/synkronus-cli/internal/utils/url_test.go b/synkronus-cli/internal/utils/url_test.go
@@ -0,0 +1,40 @@
+package utils
+
+import "testing"
+
+func TestAPIBaseURL(t *testing.T) {
+	tests := []struct {
+		raw  string
+		want string
+	}{
+		{"https://example.com", "https://example.com/api"},
+		{"https://example.com/", "https://example.com/api"},
+		{"https://example.com/api", "https://example.com/api"},
+		{"https://example.com/api/", "https://example.com/api"},
+		{"http://localhost:8080", "http://localhost:8080/api"},
+		{"", ""},
+	}
+	for _, tt := range tests {
+		if got := APIBaseURL(tt.raw); got != tt.want {
+			t.Errorf("APIBaseURL(%q) = %q, want %q", tt.raw, got, tt.want)
+		}
+	}
+}
+
+func TestOriginURL(t *testing.T) {
+	tests := []struct {
+		raw  string
+		want string
+	}{
+		{"https://example.com", "https://example.com"},
+		{"https://example.com/api", "https://example.com"},
+		{"https://example.com/api/", "https://example.com"},
+		{"http://localhost:8080/api", "http://localhost:8080"},
+		{"", ""},
+	}
+	for _, tt := range tests {
+		if got := OriginURL(tt.raw); got != tt.want {
+			t.Errorf("OriginURL(%q) = %q, want %q", tt.raw, got, tt.want)
+		}
+	}
+}
diff --git a/synkronus-cli/pkg/client/client.go b/synkronus-cli/pkg/client/client.go
@@ -57,7 +57,7 @@ type Client struct {
 
 // NewClient creates a new Synkronus API client
 func NewClient() *Client {
-	baseURL := strings.TrimRight(utils.EnsureScheme(viper.GetString("api.url")), "/")
+	baseURL := utils.APIBaseURL(viper.GetString("api.url"))
 	return &Client{
 		BaseURL:    baseURL,
 		APIVersion: viper.GetString("api.version"),
@@ -70,7 +70,7 @@ func NewClient() *Client {
 // doRequest performs an HTTP request with authentication
 // GetVersion retrieves version information from the Synkronus server
 func (c *Client) GetVersion() (*SystemVersionInfo, error) {
-	req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/version", c.BaseURL), nil)
+	req, err := http.NewRequest("GET", fmt.Sprintf("%s/version", c.BaseURL), nil)
 	if err != nil {
 		return nil, fmt.Errorf("error creating version request: %w", err)
 	}
@@ -145,7 +145,7 @@ func (c *Client) GetAppBundleManifest() (map[string]interface{}, error) {
 
 // GetAppBundleVersions retrieves available app bundle versions
 func (c *Client) GetAppBundleVersions() (map[string]interface{}, error) {
-	url := fmt.Sprintf("%s/api/app-bundle/versions", c.BaseURL)
+	url := fmt.Sprintf("%s/app-bundle/versions", c.BaseURL)
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
 		return nil, err
@@ -216,7 +216,7 @@ func (c *Client) DownloadAppBundleFile(path, destPath string, preview bool) erro
 	return nil
 }
 
-// downloadBinaryToFile performs an authenticated GET on path (must start with /, e.g. /api/dataexport/parquet)
+// downloadBinaryToFile performs an authenticated GET on path (must start with /, e.g. /dataexport/parquet)
 // and streams the body to destPath. Uses no overall HTTP timeout so large ZIP exports can complete.
 func (c *Client) downloadBinaryToFile(path string, destPath string) error {
 	url := fmt.Sprintf("%s%s", c.BaseURL, path)
@@ -271,17 +271,17 @@ func (c *Client) downloadBinaryToFile(path string, destPath string) error {
 
 // DownloadParquetExport downloads the Parquet export ZIP archive to the specified destination path
 func (c *Client) DownloadParquetExport(destPath string) error {
-	return c.downloadBinaryToFile("/api/dataexport/parquet", destPath)
+	return c.downloadBinaryToFile("/dataexport/parquet", destPath)
 }
 
 // DownloadRawJSONExport downloads the per-observation JSON ZIP export to the specified destination path
 func (c *Client) DownloadRawJSONExport(destPath string) error {
-	return c.downloadBinaryToFile("/api/dataexport/raw-json", destPath)
+	return c.downloadBinaryToFile("/dataexport/raw-json", destPath)
 }
 
 // DownloadAttachmentsExport downloads a ZIP of all current attachments to the specified destination path
 func (c *Client) DownloadAttachmentsExport(destPath string) error {
-	return c.downloadBinaryToFile("/api/attachments/export-zip", destPath)
+	return c.downloadBinaryToFile("/attachments/export-zip", destPath)
 }
 
 // UploadAppBundle uploads a new app bundle
@@ -348,7 +348,7 @@ func (c *Client) UploadAppBundle(bundlePath string) (map[string]interface{}, err
 
 // SwitchAppBundleVersion switches to a specific app bundle version
 func (c *Client) SwitchAppBundleVersion(version string) (map[string]interface{}, error) {
-	url := fmt.Sprintf("%s/api/app-bundle/switch/%s", c.BaseURL, version)
+	url := fmt.Sprintf("%s/app-bundle/switch/%s", c.BaseURL, version)
 
 	req, err := http.NewRequest("POST", url, nil)
 	if err != nil {
@@ -376,7 +376,7 @@ func (c *Client) SwitchAppBundleVersion(version string) (map[string]interface{},
 
 // SyncPull pulls updated records from the server
 func (c *Client) SyncPull(clientID string, currentVersion int64, schemaTypes []string, limit int, pageToken string) (map[string]interface{}, error) {
-	requestURL := fmt.Sprintf("%s/api/sync/pull", c.BaseURL)
+	requestURL := fmt.Sprintf("%s/sync/pull", c.BaseURL)
 
 	// Build query parameters
 	var queryParams []string
@@ -455,7 +455,7 @@ func (c *Client) SyncPull(clientID string, currentVersion int64, schemaTypes []s
 
 // SyncPush pushes records to the server
 func (c *Client) SyncPush(clientID string, transmissionID string, records []map[string]interface{}) (map[string]interface{}, error) {
-	url := fmt.Sprintf("%s/api/sync/push", c.BaseURL)
+	url := fmt.Sprintf("%s/sync/push", c.BaseURL)
 
 	// Prepare request body
 	reqBody := map[string]interface{}{
diff --git a/synkronus-cli/pkg/client/user.go b/synkronus-cli/pkg/client/user.go
@@ -28,7 +28,7 @@ type UserChangePasswordRequest struct {
 
 // CreateUser calls POST /users to create a new user (admin)
 func (c *Client) CreateUser(reqBody UserCreateRequest) (map[string]interface{}, error) {
-	url := fmt.Sprintf("%s/api/users", c.BaseURL)
+	url := fmt.Sprintf("%s/users", c.BaseURL)
 	body, err := json.Marshal(reqBody)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal request: %w", err)
@@ -60,7 +60,7 @@ func (c *Client) CreateUser(reqBody UserCreateRequest) (map[string]interface{},
 
 // DeleteUser calls DELETE /users/delete/{username} (admin)
 func (c *Client) DeleteUser(username string) error {
-	url := fmt.Sprintf("%s/api/users/delete/%s", c.BaseURL, username)
+	url := fmt.Sprintf("%s/users/delete/%s", c.BaseURL, username)
 	request, err := http.NewRequest("DELETE", url, nil)
 	if err != nil {
 		return fmt.Errorf("failed to create request: %w", err)
@@ -80,7 +80,7 @@ func (c *Client) DeleteUser(username string) error {
 
 // ResetUserPassword calls POST /users/reset-password (admin)
 func (c *Client) ResetUserPassword(reqBody UserResetPasswordRequest) error {
-	url := fmt.Sprintf("%s/api/users/reset-password", c.BaseURL)
+	url := fmt.Sprintf("%s/users/reset-password", c.BaseURL)
 	body, err := json.Marshal(reqBody)
 	if err != nil {
 		return fmt.Errorf("failed to marshal request: %w", err)
@@ -105,7 +105,7 @@ func (c *Client) ResetUserPassword(reqBody UserResetPasswordRequest) error {
 
 // ChangeOwnPassword calls POST /users/change-password (self)
 func (c *Client) ChangeOwnPassword(reqBody UserChangePasswordRequest) error {
-	url := fmt.Sprintf("%s/api/users/change-password", c.BaseURL)
+	url := fmt.Sprintf("%s/users/change-password", c.BaseURL)
 	body, err := json.Marshal(reqBody)
 	if err != nil {
 		return fmt.Errorf("failed to marshal request: %w", err)
@@ -130,7 +130,7 @@ func (c *Client) ChangeOwnPassword(reqBody UserChangePasswordRequest) error {
 
 // ListUsers calls GET /users (admin only)
 func (c *Client) ListUsers() ([]map[string]interface{}, error) {
-	url := fmt.Sprintf("%s/api/users", c.BaseURL)
+	url := fmt.Sprintf("%s/users", c.BaseURL)
 	request, err := http.NewRequest("GET", url, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
