Hash API keys secret using sha512 instead of bcrypt (#1226)

Signed-off-by: Sergio Castaño Arteaga <tegioz@icloud.com>

database/migrations/functions/api_keys/add_api_key.sql

@@ -3,7 +3,7 @@ create or replace function add_api_key(p_api_key jsonb)
 returns setof json as $$
 declare
     v_api_key_id uuid;
-    v_secret_clear_text_b64 text := encode(gen_random_bytes(32), 'base64');
+    v_api_key_secret text := encode(gen_random_bytes(32), 'base64');
 begin
     insert into api_key (
         name,
@@ -11,13 +11,13 @@ begin
         user_id
     ) values (
         p_api_key->>'name',
-        crypt(v_secret_clear_text_b64, gen_salt('bf')),
+        encode(sha512(v_api_key_secret::bytea), 'hex'),
         (p_api_key->>'user_id')::uuid
     ) returning api_key_id into v_api_key_id;
 
     return query select json_build_object(
         'api_key_id', v_api_key_id,
-        'secret', v_secret_clear_text_b64
+        'secret', v_api_key_secret
     );
 end
 $$ language plpgsql;

internal/user/manager.go

@@ -3,10 +3,12 @@ package user
 import (
 	"bytes"
 	"context"
+	"crypto/sha512"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"net/url"
+	"strings"
 	"time"
 
 	"github.com/artifacthub/hub/internal/email"
@@ -84,9 +86,18 @@ func (m *Manager) CheckAPIKey(ctx context.Context, apiKeyID, apiKeySecret string
 	}
 
 	// Check if the secret provided is valid
-	err = bcrypt.CompareHashAndPassword([]byte(apiKeySecretHashed), []byte(apiKeySecret))
-	if err != nil {
-		return &hub.CheckAPIKeyOutput{Valid: false}, nil
+	switch {
+	case strings.HasPrefix(apiKeySecretHashed, "$2a$"):
+		// Bcrypt hash, will be deprecated soon
+		err = bcrypt.CompareHashAndPassword([]byte(apiKeySecretHashed), []byte(apiKeySecret))
+		if err != nil {
+			return &hub.CheckAPIKeyOutput{Valid: false}, nil
+		}
+	default:
+		// SHA512 hash
+		if fmt.Sprintf("%x", sha512.Sum512([]byte(apiKeySecret))) != apiKeySecretHashed {
+			return &hub.CheckAPIKeyOutput{Valid: false}, nil
+		}
 	}
 
 	return &hub.CheckAPIKeyOutput{

internal/user/manager_test.go

@@ -2,6 +2,7 @@ package user
 
 import (
 	"context"
+	"crypto/sha512"
 	"errors"
 	"fmt"
 	"testing"
@@ -73,7 +74,7 @@ func TestCheckAPIKey(t *testing.T) {
 		db.AssertExpectations(t)
 	})
 
-	t.Run("valid key", func(t *testing.T) {
+	t.Run("valid key (secret hashed with bcrypt)", func(t *testing.T) {
 		t.Parallel()
 		db := &tests.DBMock{}
 		secretHashed, _ := bcrypt.GenerateFromPassword([]byte("secret"), bcrypt.DefaultCost)
@@ -86,6 +87,20 @@ func TestCheckAPIKey(t *testing.T) {
 		assert.Equal(t, "userID", output.UserID)
 		db.AssertExpectations(t)
 	})
+
+	t.Run("valid key (secret hashed with sha512)", func(t *testing.T) {
+		t.Parallel()
+		db := &tests.DBMock{}
+		secretHashed := fmt.Sprintf("%x", sha512.Sum512([]byte("secret")))
+		db.On("QueryRow", ctx, getAPIKeyInfoDBQ, "keyID").Return([]interface{}{"userID", secretHashed}, nil)
+		m := NewManager(db, nil)
+
+		output, err := m.CheckAPIKey(ctx, "keyID", "secret")
+		assert.NoError(t, err)
+		assert.True(t, output.Valid)
+		assert.Equal(t, "userID", output.UserID)
+		db.AssertExpectations(t)
+	})
 }
 
 func TestCheckAvailability(t *testing.T) {