From 036d2f6108289f17c3351e6219d11419df9760c7 Mon Sep 17 00:00:00 2001 From: shark Date: Mon, 11 May 2026 13:26:46 +0800 Subject: [PATCH] feat: update api key --- README.md | 8 ++ cmd/keystone-edge/main.go | 3 +- docs/designs/cloud-sync-ui-implementation.md | 6 ++ internal/cloud/auth_client.go | 32 ++---- internal/cloud/auth_client_test.go | 72 ++----------- internal/config/config.go | 64 +---------- internal/config/config_test.go | 106 ++----------------- 7 files changed, 47 insertions(+), 244 deletions(-) diff --git a/README.md b/README.md index 77bf306..d1158c3 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,14 @@ Configuration is loaded from environment variables. See [`docker/.env.example`]( | `KEYSTONE_MYSQL_HOST` | `localhost` | MySQL host | | `KEYSTONE_MYSQL_PASSWORD` | *required* | MySQL password | +### Cloud Sync Credentials + +When cloud sync is enabled, `KEYSTONE_CLOUD_API_KEY` is required. Keystone treats +this value as an opaque credential issued by the cloud platform and forwards it +to `AuthService.ExchangeCredential` as `credential_base64`. Keystone does not +decode, split, validate, or derive `site_id` / secret values from this key; the +cloud AuthService owns credential interpretation and validation. + ## Project Structure ``` diff --git a/cmd/keystone-edge/main.go b/cmd/keystone-edge/main.go index 8cdc9d8..002de8d 100644 --- a/cmd/keystone-edge/main.go +++ b/cmd/keystone-edge/main.go @@ -120,8 +120,7 @@ func main() { UseTLS: cfg.Sync.CloudUseTLS, TLSCAFile: cfg.Sync.CloudTLSCAFile, TLSServerName: cfg.Sync.CloudTLSServerName, - SiteID: cfg.Sync.SiteID, - APISecret: cfg.Sync.APISecret, + APIKey: cfg.Sync.APIKey, RefreshBefore: 60 * time.Second, }) diff --git a/docs/designs/cloud-sync-ui-implementation.md b/docs/designs/cloud-sync-ui-implementation.md index c17f42e..9439228 100644 --- a/docs/designs/cloud-sync-ui-implementation.md +++ b/docs/designs/cloud-sync-ui-implementation.md @@ -741,6 +741,12 @@ TODO: decide later whether any config should become editable from Synapse. If that is added, keep secret values masked and require admin permissions, audit logs, validation, and clear restart/reload semantics. +Credential handling: `KEYSTONE_CLOUD_API_KEY` is a cloud-issued opaque +credential. Keystone should only check that it is present when sync is enabled, +then forward it to cloud auth as `credential_base64`. Keystone must not decode +the key, derive `site_id`, extract a secret, or enforce the key's internal +encoding format; the cloud AuthService owns credential validation. + ### 7.6 Require Admin Permission Cloud sync trigger APIs should require an authenticated admin role. Batch sync can diff --git a/internal/cloud/auth_client.go b/internal/cloud/auth_client.go index 31fcc43..4bcc5b5 100644 --- a/internal/cloud/auth_client.go +++ b/internal/cloud/auth_client.go @@ -9,9 +9,8 @@ package cloud import ( "context" - "encoding/base64" - "encoding/binary" "fmt" + "strings" "sync" "time" @@ -31,10 +30,8 @@ type AuthClientConfig struct { TLSCAFile string // TLSServerName is an optional TLS server name override (SNI / verification). TLSServerName string - // SiteID is the numeric site identifier assigned to this edge deployment. - SiteID int64 - // APISecret is the raw API key secret for credential exchange. - APISecret string // #nosec G117 -- in-process auth config only; not JSON-marshaled to clients + // APIKey is an opaque cloud-issued credential forwarded to AuthService. + APIKey string // #nosec G117 -- in-process auth config only; not JSON-marshaled to clients // RefreshBefore is how long before expiry to proactively refresh the token. RefreshBefore time.Duration } @@ -92,7 +89,7 @@ func (c *AuthClient) shouldRefresh(token *AuthToken) bool { } func (c *AuthClient) refreshToken(ctx context.Context) (*AuthToken, error) { - credentialBase64 := c.buildCredentialBase64() + credentialBase64 := c.credentialBase64() if credentialBase64 == "" { return nil, fmt.Errorf("credential_base64 must not be empty") } @@ -178,21 +175,8 @@ func (c *AuthClient) Close() error { return err } -// buildCredentialBase64 encodes the credential as base64url(int64_be(site_id) + "." + api_secret). -// This mirrors the Rust SDK encoding in auth-client tests. -func (c *AuthClient) buildCredentialBase64() string { - if c.cfg.APISecret == "" { - return "" - } - var raw []byte - buf := make([]byte, 8) - // SiteID is specified as an int64 and encoded as its big-endian byte representation. - // We allow negative values (two's complement), matching typical i64-to-bytes behavior. - // - //nolint:gosec // G115: intentional bit-preserving cast for wire encoding - binary.BigEndian.PutUint64(buf, uint64(c.cfg.SiteID)) - raw = append(raw, buf...) - raw = append(raw, '.') - raw = append(raw, []byte(c.cfg.APISecret)...) - return base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(raw) +// credentialBase64 returns the cloud-issued API key unchanged. Keystone treats +// it as an opaque credential; the cloud AuthService owns parsing and validation. +func (c *AuthClient) credentialBase64() string { + return strings.TrimSpace(c.cfg.APIKey) } diff --git a/internal/cloud/auth_client_test.go b/internal/cloud/auth_client_test.go index 48bff32..70026a5 100644 --- a/internal/cloud/auth_client_test.go +++ b/internal/cloud/auth_client_test.go @@ -5,87 +5,35 @@ package cloud import ( - "encoding/base64" - "encoding/binary" "strings" "testing" "time" ) -func TestBuildCredentialBase64_Format(t *testing.T) { +func TestCredentialBase64_ReturnsOpaqueAPIKey(t *testing.T) { client := NewAuthClient(AuthClientConfig{ - SiteID: 42, - APISecret: "secret-1", + APIKey: "cloud-issued-opaque-key", }) - got := client.buildCredentialBase64() - if got == "" { - t.Fatal("buildCredentialBase64 returned empty string") - } - - // Decode and verify structure: int64_be(42) + "." + "secret-1" - raw, err := base64.URLEncoding.WithPadding(base64.NoPadding).DecodeString(got) - if err != nil { - t.Fatalf("failed to decode base64: %v", err) - } - - if len(raw) < 10 { // 8 bytes site_id + 1 byte '.' + at least 1 byte secret - t.Fatalf("decoded credential too short: %d bytes", len(raw)) - } - - siteID := binary.BigEndian.Uint64(raw[:8]) - if siteID != 42 { - t.Errorf("siteID = %d, want 42", siteID) - } - - if raw[8] != '.' { - t.Errorf("separator = %c, want '.'", raw[8]) - } - - secret := string(raw[9:]) - if secret != "secret-1" { - t.Errorf("secret = %q, want %q", secret, "secret-1") + got := client.credentialBase64() + want := "cloud-issued-opaque-key" + if got != want { + t.Errorf("credential_base64 = %q, want %q", got, want) } } -func TestBuildCredentialBase64_MatchesRustSDK(t *testing.T) { - // The Rust SDK test uses: - // let mut raw = 42_i64.to_be_bytes().to_vec(); - // raw.push(b'.'); - // raw.extend_from_slice(b"secret-1"); - // base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(raw) - +func TestCredentialBase64_TrimsWhitespace(t *testing.T) { client := NewAuthClient(AuthClientConfig{ - SiteID: 42, - APISecret: "secret-1", + APIKey: " cloud-issued-opaque-key ", }) - got := client.buildCredentialBase64() - - // Build expected value same way - var raw []byte - buf := make([]byte, 8) - binary.BigEndian.PutUint64(buf, 42) - raw = append(raw, buf...) - raw = append(raw, '.') - raw = append(raw, []byte("secret-1")...) - want := base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(raw) + got := client.credentialBase64() + want := "cloud-issued-opaque-key" if got != want { t.Errorf("credential_base64 = %q, want %q", got, want) } } -func TestBuildCredentialBase64_EmptySecret(t *testing.T) { - client := NewAuthClient(AuthClientConfig{ - SiteID: 1, - APISecret: "", - }) - got := client.buildCredentialBase64() - if got != "" { - t.Errorf("expected empty credential for empty secret, got %q", got) - } -} - func TestShouldRefresh_NotExpired(t *testing.T) { client := NewAuthClient(AuthClientConfig{ RefreshBefore: 60 * time.Second, diff --git a/internal/config/config.go b/internal/config/config.go index f1ce24a..d5f6d21 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,7 +6,6 @@ package config import ( - "encoding/base64" "fmt" "os" "strconv" @@ -77,9 +76,7 @@ type SyncConfig struct { CloudUseTLS bool // enable TLS for cloud gRPC connections CloudTLSCAFile string // optional CA bundle path for TLS verification CloudTLSServerName string // optional TLS server name override (SNI / verification) - APIKey string `json:"-"` // base64url-encoded API key (decoded into SiteID + APISecret at load time; never JSON-marshaled) - SiteID int64 // site identifier decoded from APIKey - APISecret string `json:"-"` // site secret decoded from APIKey (never JSON-marshaled) + APIKey string `json:"-"` // opaque cloud-issued credential; never JSON-marshaled MaxConcurrent int // max concurrent uploads WorkerIntervalSec int // sync worker poll interval in seconds RequestTimeoutSec int // per-RPC timeout in seconds @@ -267,15 +264,11 @@ func (c *Config) Validate() error { if strings.TrimSpace(c.Sync.GatewayEndpoint) == "" { return fmt.Errorf("sync gateway endpoint is required when sync is enabled") } - if strings.TrimSpace(c.Sync.APIKey) == "" { + apiKey := strings.TrimSpace(c.Sync.APIKey) + if apiKey == "" { return fmt.Errorf("KEYSTONE_CLOUD_API_KEY is required when sync is enabled") } - siteID, apiSecret, err := decodeAPIKey(c.Sync.APIKey) - if err != nil { - return fmt.Errorf("KEYSTONE_CLOUD_API_KEY is invalid: %w", err) - } - c.Sync.SiteID = siteID - c.Sync.APISecret = apiSecret + c.Sync.APIKey = apiKey if c.Sync.BatchSize <= 0 { return fmt.Errorf("sync batch size must be greater than 0 when sync is enabled") } @@ -346,52 +339,3 @@ func getEnvBool(key string, fallback bool) bool { } return fallback } - -// decodeAPIKey decodes a base64url-no-pad API key into its component parts. -// -// The wire format (produced by the data-platform credential issuer) is: -// -// base64url_no_pad( i64_big_endian(site_id) + "." + site_secret_utf8 ) -// -// Returns the site_id (signed int64) and site_secret string, or an error if -// the key is malformed. -func decodeAPIKey(apiKey string) (siteID int64, apiSecret string, err error) { - apiKey = strings.TrimSpace(apiKey) - if apiKey == "" { - return 0, "", fmt.Errorf("api key must not be empty") - } - - // Restore standard base64 padding (URL-safe, no-pad variant). - padLen := (-len(apiKey)) % 4 - if padLen < 0 { - padLen += 4 - } - padded := apiKey + strings.Repeat("=", padLen) - - decoded, err := base64.URLEncoding.DecodeString(padded) - if err != nil { - return 0, "", fmt.Errorf("base64 decode failed: %w", err) - } - - // Minimum: 8 bytes site_id + 1 byte '.' + at least 1 byte secret. - if len(decoded) <= 9 || decoded[8] != '.' { - return 0, "", fmt.Errorf("invalid format: expected i64_be + '.' + secret") - } - - // First 8 bytes: signed int64 big-endian. - siteID = int64(decoded[0])<<56 | int64(decoded[1])<<48 | int64(decoded[2])<<40 | - int64(decoded[3])<<32 | int64(decoded[4])<<24 | int64(decoded[5])<<16 | - int64(decoded[6])<<8 | int64(decoded[7]) - if siteID <= 0 { - return 0, "", fmt.Errorf("site_id decoded from api key must be greater than 0, got %d", siteID) - } - - // Remaining bytes after the '.' separator: UTF-8 secret. - secretBytes := decoded[9:] - apiSecret = string(secretBytes) - if strings.TrimSpace(apiSecret) == "" { - return 0, "", fmt.Errorf("site_secret decoded from api key must not be empty") - } - - return siteID, apiSecret, nil -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ba2269a..54e5474 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -252,80 +252,6 @@ func TestConfigValidate(t *testing.T) { } } -func TestDecodeAPIKey(t *testing.T) { - tests := []struct { - name string - apiKey string - wantSiteID int64 - wantAPISecret string - wantErr bool - wantErrContain string - }{ - { - // site_id=42, secret="test-secret-value" - // Generated: base64url_no_pad( i64_be(42) + "." + "test-secret-value" ) - name: "valid key", - apiKey: "AAAAAAAAACoudGVzdC1zZWNyZXQtdmFsdWU", - wantSiteID: 42, - wantAPISecret: "test-secret-value", - wantErr: false, - }, - { - name: "empty key", - apiKey: "", - wantErr: true, - wantErrContain: "must not be empty", - }, - { - name: "whitespace only", - apiKey: " ", - wantErr: true, - wantErrContain: "must not be empty", - }, - { - name: "invalid base64", - apiKey: "!!!notbase64!!!", - wantErr: true, - wantErrContain: "base64 decode failed", - }, - { - name: "too short (no separator)", - apiKey: "AAAAAAAA", - wantErr: true, - wantErrContain: "invalid format", - }, - { - name: "wrong separator byte", - apiKey: "AAAAAAAAAX9zZWNyZXQ", // 8 zero bytes + 'A' (0x41) instead of '.' (0x2E) + "secret" - wantErr: true, - wantErrContain: "invalid format", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - siteID, apiSecret, err := decodeAPIKey(tt.apiKey) - if (err != nil) != tt.wantErr { - t.Fatalf("decodeAPIKey() error = %v, wantErr %v", err, tt.wantErr) - } - if tt.wantErr { - if tt.wantErrContain != "" && err != nil { - if !contains(err.Error(), tt.wantErrContain) { - t.Errorf("decodeAPIKey() error = %q, want to contain %q", err.Error(), tt.wantErrContain) - } - } - return - } - if siteID != tt.wantSiteID { - t.Errorf("decodeAPIKey() siteID = %d, want %d", siteID, tt.wantSiteID) - } - if apiSecret != tt.wantAPISecret { - t.Errorf("decodeAPIKey() apiSecret = %q, want %q", apiSecret, tt.wantAPISecret) - } - }) - } -} - func TestValidateSyncAPIKey(t *testing.T) { validBase := Config{ Server: ServerConfig{Mode: "edge"}, @@ -363,7 +289,7 @@ func TestValidateSyncAPIKey(t *testing.T) { } }) - t.Run("sync enabled — invalid API key", func(t *testing.T) { + t.Run("sync enabled — arbitrary opaque API key accepted", func(t *testing.T) { cfg := validBase cfg.Sync = SyncConfig{ Enabled: true, @@ -379,18 +305,21 @@ func TestValidateSyncAPIKey(t *testing.T) { RetryBaseSec: 30, RetryMaxSec: 1800, } - if err := cfg.Validate(); err == nil { - t.Error("Validate() expected error for invalid API key, got nil") + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate() unexpected error = %v", err) + } + if cfg.Sync.APIKey != "notvalidbase64!!!" { + t.Errorf("APIKey = %q, want %q", cfg.Sync.APIKey, "notvalidbase64!!!") } }) - t.Run("sync enabled — valid API key populates SiteID and APISecret", func(t *testing.T) { + t.Run("sync enabled — trims API key whitespace", func(t *testing.T) { cfg := validBase cfg.Sync = SyncConfig{ Enabled: true, AuthEndpoint: "auth:443", GatewayEndpoint: "gateway:443", - APIKey: "AAAAAAAAACoudGVzdC1zZWNyZXQtdmFsdWU", // site_id=42, secret="test-secret-value" + APIKey: " cloud-issued-key ", BatchSize: 10, MaxRetries: 5, MaxConcurrent: 2, @@ -403,27 +332,12 @@ func TestValidateSyncAPIKey(t *testing.T) { if err := cfg.Validate(); err != nil { t.Fatalf("Validate() unexpected error = %v", err) } - if cfg.Sync.SiteID != 42 { - t.Errorf("SiteID = %d, want 42", cfg.Sync.SiteID) - } - if cfg.Sync.APISecret != "test-secret-value" { - t.Errorf("APISecret = %q, want %q", cfg.Sync.APISecret, "test-secret-value") + if cfg.Sync.APIKey != "cloud-issued-key" { + t.Errorf("APIKey = %q, want %q", cfg.Sync.APIKey, "cloud-issued-key") } }) } -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || - func() bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false - }()) -} - func TestGetEnv(t *testing.T) { // Test non-existent environment variable got := getEnv("NONEXISTENT_ENV_VAR_12345", "default")