diff --git a/internal/auth/service_api_fields_test.go b/internal/auth/service_api_fields_test.go index 34ac56050..4b2e41a15 100644 --- a/internal/auth/service_api_fields_test.go +++ b/internal/auth/service_api_fields_test.go @@ -5,6 +5,7 @@ import ( "reflect" "sort" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -223,3 +224,40 @@ func TestAPIAdapters_UpdateGroup_AllFieldsReachStore(t *testing.T) { assert.Equal(t, existing().AllowedAccounts, got.AllowedAccounts) }) } + +func TestAPIAdapters_CreateAPIKey_AllFieldsReachStore(t *testing.T) { + ctx := context.Background() + expiresAt := time.Now().Add(24 * time.Hour) + req := APICreateAPIKeyRequest{ + Name: "scoped", + Password: testAPIKeyPassword, + Permissions: constrainedAPIPermissions(t), + ExpiresAt: &expiresAt, + } + checks := map[string]func(*testing.T, *UserAPIKey){ + "Name": func(t *testing.T, k *UserAPIKey) { assert.Equal(t, req.Name, k.Name) }, + "Password": func(t *testing.T, k *UserAPIKey) {}, // re-verified against the user's hash, never stored + "Permissions": func(t *testing.T, k *UserAPIKey) { assert.Equal(t, constrainedPermissions, k.Permissions) }, + "ExpiresAt": func(t *testing.T, k *UserAPIKey) { assert.True(t, req.ExpiresAt.Equal(*k.ExpiresAt)) }, + } + requireFieldsCovered(t, req, checks) + + store := new(MockStore) + svc := &Service{store: store} + store.On("GetUserByID", ctx, "user-1").Return(&User{ + ID: "user-1", Email: "test@example.com", Active: true, + PasswordHash: testAPIKeyPasswordHash, GroupIDs: []string{DefaultAdminGroupID}, + }, nil) + store.On("GetGroup", ctx, DefaultAdminGroupID).Return(&Group{ + ID: DefaultAdminGroupID, Permissions: []Permission{{Action: ActionAdmin, Resource: ResourceAll}}, + }, nil) + var got *UserAPIKey + store.On("CreateAPIKey", ctx, mock.AnythingOfType("*auth.UserAPIKey")). + Run(func(args mock.Arguments) { got = args.Get(1).(*UserAPIKey) }).Return(nil) + _, err := svc.CreateAPIKeyAPI(ctx, "user-1", req) + require.NoError(t, err) + require.NotNil(t, got) + for name, check := range checks { + t.Run(name, func(t *testing.T) { check(t, got) }) + } +} diff --git a/internal/auth/service_apikeys_api.go b/internal/auth/service_apikeys_api.go index 1d3ae91fd..2f5dc4cc5 100644 --- a/internal/auth/service_apikeys_api.go +++ b/internal/auth/service_apikeys_api.go @@ -92,10 +92,10 @@ func normalizeAPIKeyPermissions(perms []Permission) []Permission { // APICreateAPIKeyRequest represents the API request to create an API key. type APICreateAPIKeyRequest struct { - ExpiresAt *time.Time `json:"expires_at,omitempty"` - Name string `json:"name"` - Password string `json:"password"` //nolint:gosec // G117: intentional credential field in request struct -- decoded by the handler and verified against the caller's own stored hash, never re-stored - Permissions []Permission `json:"permissions,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty"` + Name string `json:"name"` + Password string `json:"password"` //nolint:gosec // G117: intentional credential field in request struct -- decoded by the handler and verified against the caller's own stored hash, never re-stored + Permissions []APIPermission `json:"permissions,omitempty"` } // APIKeyInfo represents public API key information (without sensitive data). @@ -183,9 +183,12 @@ type APIListAPIKeysResponse struct { // CreateAPIKeyAPI creates a new API key and returns API-friendly response. // // req may be any struct whose JSON representation is compatible with -// APICreateAPIKeyRequest (name, expires_at, permissions). The handler lives -// in a different package (api) and passes api.CreateAPIKeyRequest, which -// carries the same JSON fields but is a distinct Go type. A direct type +// APICreateAPIKeyRequest (name, expires_at, permissions in the API wire shape: +// APIPermission, with accounts/max_amount constraint tags, not the storage +// shape). The handler lives in a different package (api) and passes +// api.CreateAPIKeyRequest, which carries the same JSON fields but is a +// distinct Go type. Permissions must stay APIPermission: decoding them into +// Permission silently dropped the account and amount constraints. A direct type // assertion would always fail and return a 500; JSON re-encoding handles any // caller-package type transparently (issue #1440). func (s *Service) CreateAPIKeyAPI(ctx context.Context, userID string, req any) (any, error) { @@ -208,7 +211,7 @@ func (s *Service) CreateAPIKeyAPI(ctx context.Context, userID string, req any) ( } // Create the API key - apiKey, keyInfo, err := s.CreateAPIKey(ctx, userID, createReq.Name, createReq.Password, createReq.Permissions, createReq.ExpiresAt) + apiKey, keyInfo, err := s.CreateAPIKey(ctx, userID, createReq.Name, createReq.Password, apiPermissionsToPermissions(createReq.Permissions), createReq.ExpiresAt) if err != nil { return nil, err } diff --git a/internal/auth/service_apikeys_api_test.go b/internal/auth/service_apikeys_api_test.go index 7c5af2cb1..e16d11b0d 100644 --- a/internal/auth/service_apikeys_api_test.go +++ b/internal/auth/service_apikeys_api_test.go @@ -35,7 +35,7 @@ func TestService_CreateAPIKeyAPI(t *testing.T) { req := APICreateAPIKeyRequest{ Name: "Test API Key", Password: testAPIKeyPassword, - Permissions: permissions, + Permissions: []APIPermission{{Action: ActionView, Resource: ResourceRecommendations}}, ExpiresAt: &expiresAt, } @@ -74,7 +74,7 @@ func TestService_CreateAPIKeyAPI(t *testing.T) { req := APICreateAPIKeyRequest{ Name: "Test API Key", Password: testAPIKeyPassword, - Permissions: []Permission{{Action: ActionView, Resource: ResourceRecommendations}}, + Permissions: []APIPermission{{Action: ActionView, Resource: ResourceRecommendations}}, ExpiresAt: &expiresAt, } @@ -106,7 +106,7 @@ func TestService_CreateAPIKeyAPI(t *testing.T) { req := APICreateAPIKeyRequest{ Name: "Test API Key", - Permissions: []Permission{}, + Permissions: []APIPermission{}, } mockStore.On("GetUserByID", ctx, "user-123").Return(user, nil) diff --git a/internal/server/adapter_test.go b/internal/server/adapter_test.go index 12bb76e58..a1dfd4fee 100644 --- a/internal/server/adapter_test.go +++ b/internal/server/adapter_test.go @@ -420,13 +420,61 @@ func TestAuthServiceAdapter_CreateAPIKeyAPI(t *testing.T) { result, err := adapter.CreateAPIKeyAPI(ctx, "user-1", auth.APICreateAPIKeyRequest{ Name: "my-key", Password: password, - Permissions: []auth.Permission{{Action: auth.ActionView, Resource: auth.ResourceRecommendations}}, + Permissions: []auth.APIPermission{{Action: auth.ActionView, Resource: auth.ResourceRecommendations}}, ExpiresAt: &expiresAt, }) require.NoError(t, err) require.NotNil(t, result) } +// The handler passes api.CreateAPIKeyRequest, whose constraint JSON tags +// (accounts, max_amount) differ from auth.PermissionConstraints. Every +// constraint field must reach the store; a dropped one widens the key's scope. +func TestAuthServiceAdapter_CreateAPIKeyAPI_KeepsPermissionConstraints(t *testing.T) { + adapter, mockStore := createMockAuthService(t) + ctx := context.Background() + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + const password = "CorrectHorse@123" + mockStore.On("GetUserByID", ctx, "user-1").Return(&auth.User{ + ID: "user-1", Email: "test@example.com", Active: true, + PasswordHash: auth.TestPasswordHash(t, password), + GroupIDs: []string{auth.DefaultAdminGroupID}, + }, nil) + mockStore.On("GetGroup", ctx, auth.DefaultAdminGroupID).Return(&auth.Group{ + ID: auth.DefaultAdminGroupID, + Permissions: []auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}, + }, nil) + var stored *auth.UserAPIKey + mockStore.On("CreateAPIKey", ctx, mock.AnythingOfType("*auth.UserAPIKey")). + Run(func(args mock.Arguments) { stored = args.Get(1).(*auth.UserAPIKey) }).Return(nil) + + expiresAt := time.Now().Add(24 * time.Hour) + _, err := adapter.CreateAPIKeyAPI(ctx, "user-1", api.CreateAPIKeyRequest{ + Name: "scoped", + Password: password, + ExpiresAt: &expiresAt, + Permissions: []api.Permission{{ + Action: auth.ActionView, Resource: auth.ResourceRecommendations, + Constraints: &api.PermissionConstraint{ + Accounts: []string{"123"}, Providers: []string{"aws"}, Services: []string{"ec2"}, + Regions: []string{"us-east-1"}, MaxAmount: 500, + }, + }}, + }) + require.NoError(t, err) + require.NotNil(t, stored) + assert.Equal(t, "scoped", stored.Name) + require.NotNil(t, stored.ExpiresAt) + assert.Equal(t, []auth.Permission{{ + Action: auth.ActionView, Resource: auth.ResourceRecommendations, + Constraints: &auth.PermissionConstraints{ + AccountIDs: []string{"123"}, Providers: []string{"aws"}, Services: []string{"ec2"}, + Regions: []string{"us-east-1"}, MaxPurchaseAmount: 500, + }, + }}, stored.Permissions) +} + func TestAuthServiceAdapter_ListUserAPIKeysAPI(t *testing.T) { adapter, mockStore := createMockAuthService(t) ctx := context.Background()