Skip to content

Commit d10ef2b

Browse files
committed
Fix codeRabbit comments
1 parent a53e898 commit d10ef2b

File tree

5 files changed

+2
-147
lines changed

5 files changed

+2
-147
lines changed

docs/gateway/policies/apikey-authentication.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,8 +584,6 @@ All generated API keys follow a consistent format:
584584
- **Total Length**: 92 characters
585585
- **Example**: `apip_b9abae64a955aded2eb700aff88235ce3f7e6a8ca0f2f52ba31f73bcbb960360_jh~cPInvccQ09goMO5-4mQ`
586586

587-
The API key uses a URL-safe character set and does not contain underscores or other special characters that might cause issues in various contexts.
588-
589587
### API Key Security
590588

591589
The platform implements comprehensive security measures for API key management:

gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ func (sm *APIKeySnapshotManager) RevokeAPIKey(apiId, apiKeyName string) error {
145145
sm.logger.Warn("API key not found for revocation",
146146
zap.String("api_id", apiId),
147147
zap.String("api_key", apiKeyName))
148-
return nil
149148
}
150149

151150
// Update the snapshot to reflect the new state

gateway/gateway-controller/pkg/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ func (c *Config) validateAPIKeyConfig() error {
11561156
}
11571157
}
11581158
if !isValidAlgorithm {
1159-
return fmt.Errorf("api_key_hashing.algorithm must be one of: %s, got: %s",
1159+
return fmt.Errorf("api_key.algorithm must be one of: %s, got: %s",
11601160
strings.Join(validAlgorithms, ", "), c.GatewayController.APIKey.Algorithm)
11611161
}
11621162
return nil

gateway/gateway-controller/pkg/storage/apikey_store.go

Lines changed: 0 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,12 @@
1919
package storage
2020

2121
import (
22-
"crypto/sha256"
23-
"crypto/subtle"
24-
"encoding/base64"
25-
"encoding/hex"
26-
"errors"
2722
"fmt"
28-
"golang.org/x/crypto/bcrypt"
29-
"strings"
3023
"sync"
3124
"sync/atomic"
3225

3326
"github.com/wso2/api-platform/gateway/gateway-controller/pkg/models"
3427
"go.uber.org/zap"
35-
"golang.org/x/crypto/argon2"
3628
)
3729

3830
// APIKeyStore manages API keys in memory with thread-safe operations
@@ -202,140 +194,6 @@ func (s *APIKeyStore) removeFromAPIMapping(apiKey *models.APIKey) {
202194
}
203195
}
204196

205-
// compareAPIKeys compares API keys for external use
206-
// Returns true if the plain API key matches the hash, false otherwise
207-
// If hashing is disabled, performs plain text comparison
208-
func compareAPIKeys(providedAPIKey, storedAPIKey string) bool {
209-
if providedAPIKey == "" || storedAPIKey == "" {
210-
return false
211-
}
212-
213-
// Check if it's an SHA-256 hash (format: $sha256$<salt_hex>$<hash_hex>)
214-
if strings.HasPrefix(storedAPIKey, "$sha256$") {
215-
return compareSHA256Hash(providedAPIKey, storedAPIKey)
216-
}
217-
218-
// Check if it's a bcrypt hash (starts with $2a$, $2b$, or $2y$)
219-
if strings.HasPrefix(storedAPIKey, "$2a$") ||
220-
strings.HasPrefix(storedAPIKey, "$2b$") ||
221-
strings.HasPrefix(storedAPIKey, "$2y$") {
222-
return compareBcryptHash(providedAPIKey, storedAPIKey)
223-
}
224-
225-
// Check if it's an Argon2id hash
226-
if strings.HasPrefix(storedAPIKey, "$argon2id$") {
227-
err := compareArgon2id(providedAPIKey, storedAPIKey)
228-
return err == nil
229-
}
230-
231-
// If no hash format is detected and hashing is enabled, try plain text comparison as fallback
232-
// This handles migration scenarios where some keys might still be stored as plain text
233-
return subtle.ConstantTimeCompare([]byte(providedAPIKey), []byte(storedAPIKey)) == 1
234-
}
235-
236-
// compareSHA256Hash validates an encoded SHA-256 hash and compares it to the provided password.
237-
// Expected format: $sha256$<salt_hex>$<hash_hex>
238-
// Returns true if the plain API key matches the hash, false otherwise
239-
func compareSHA256Hash(apiKey, encoded string) bool {
240-
if apiKey == "" || encoded == "" {
241-
return false
242-
}
243-
244-
// Parse the hash format: $sha256$<salt_hex>$<hash_hex>
245-
parts := strings.Split(encoded, "$")
246-
if len(parts) != 4 || parts[1] != "sha256" {
247-
return false
248-
}
249-
250-
// Decode salt and hash from hex
251-
salt, err := hex.DecodeString(parts[2])
252-
if err != nil {
253-
return false
254-
}
255-
256-
storedHash, err := hex.DecodeString(parts[3])
257-
if err != nil {
258-
return false
259-
}
260-
261-
// Compute hash of the provided key with the stored salt
262-
hasher := sha256.New()
263-
hasher.Write([]byte(apiKey))
264-
hasher.Write(salt)
265-
computedHash := hasher.Sum(nil)
266-
267-
// Constant-time comparison
268-
return subtle.ConstantTimeCompare(computedHash, storedHash) == 1
269-
}
270-
271-
// compareBcryptHash validates an encoded bcrypt hash and compares it to the provided password.
272-
// Returns true if the plain API key matches the hash, false otherwise
273-
func compareBcryptHash(apiKey, encoded string) bool {
274-
if apiKey == "" || encoded == "" {
275-
return false
276-
}
277-
278-
// Compare the provided key with the stored bcrypt hash
279-
err := bcrypt.CompareHashAndPassword([]byte(encoded), []byte(apiKey))
280-
return err == nil
281-
}
282-
283-
// compareArgon2id parses an encoded Argon2id hash and compares it to the provided password.
284-
// Expected format: $argon2id$v=19$m=<m>,t=<t>,p=<p>$<salt_b64>$<hash_b64>
285-
func compareArgon2id(apiKey, encoded string) error {
286-
parts := strings.Split(encoded, "$")
287-
if len(parts) != 6 || parts[1] != "argon2id" {
288-
return fmt.Errorf("invalid argon2id hash format")
289-
}
290-
291-
// parts[2] -> v=19
292-
var version int
293-
if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil {
294-
return err
295-
}
296-
if version != argon2.Version {
297-
return fmt.Errorf("unsupported argon2 version: %d", version)
298-
}
299-
300-
// parts[3] -> m=<m>,t=<t>,p=<p>
301-
var mem uint32
302-
var iters uint32
303-
var threads uint8
304-
var t, m, p uint32
305-
if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &m, &t, &p); err != nil {
306-
return err
307-
}
308-
mem = m
309-
iters = t
310-
threads = uint8(p)
311-
312-
// decode salt and hash (try RawStd then Std)
313-
salt, err := decodeBase64(parts[4])
314-
if err != nil {
315-
return err
316-
}
317-
hash, err := decodeBase64(parts[5])
318-
if err != nil {
319-
return err
320-
}
321-
322-
derived := argon2.IDKey([]byte(apiKey), salt, iters, mem, threads, uint32(len(hash)))
323-
if subtle.ConstantTimeCompare(derived, hash) == 1 {
324-
return nil
325-
}
326-
return errors.New("API key mismatch")
327-
}
328-
329-
// decodeBase64 decodes a base64 string, trying RawStdEncoding first, then StdEncoding
330-
func decodeBase64(s string) ([]byte, error) {
331-
b, err := base64.RawStdEncoding.DecodeString(s)
332-
if err == nil {
333-
return b, nil
334-
}
335-
// try StdEncoding as a fallback
336-
return base64.StdEncoding.DecodeString(s)
337-
}
338-
339197
// GetCompositeKey generates a composite key for storing/retrieving API keys
340198
func GetCompositeKey(apiId, keyName string) string {
341199
return fmt.Sprintf("%s:%s", apiId, keyName)

gateway/policies/api-key-auth/v0.1.0/apikey_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func generateShortUniqueID() string {
3030
randomBytes := make([]byte, 16)
3131
_, err := rand.Read(randomBytes)
3232
if err != nil {
33-
return "defaulttestid123456" // fallback for tests
33+
return "defaulttestid123456789" // fallback for tests
3434
}
3535

3636
// Encode as base64url without padding and replace underscores with tildes

0 commit comments

Comments
 (0)