diff --git a/internal/auth/service_apikeys_api.go b/internal/auth/service_apikeys_api.go index 22948bf35..26d994f3f 100644 --- a/internal/auth/service_apikeys_api.go +++ b/internal/auth/service_apikeys_api.go @@ -2,6 +2,7 @@ package auth import ( "context" + "encoding/json" "fmt" "time" ) @@ -41,11 +42,30 @@ 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 +// 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) { - // Type assert the request - createReq, ok := req.(APICreateAPIKeyRequest) - if !ok { - return nil, fmt.Errorf("invalid request type") + var createReq APICreateAPIKeyRequest + switch r := req.(type) { + case APICreateAPIKeyRequest: + // Fast path: caller already supplies our concrete type (service tests, + // future callers that import this package directly). + createReq = r + default: + // Cross-package path: re-encode to JSON and back so any struct with + // compatible json tags (e.g. api.CreateAPIKeyRequest) is accepted. + b, err := json.Marshal(r) + if err != nil { + return nil, fmt.Errorf("invalid request: %w", err) + } + if err := json.Unmarshal(b, &createReq); err != nil { + return nil, fmt.Errorf("invalid request: %w", err) + } } // Create the API key diff --git a/internal/auth/service_apikeys_api_test.go b/internal/auth/service_apikeys_api_test.go index 800724e9d..783850a6d 100644 --- a/internal/auth/service_apikeys_api_test.go +++ b/internal/auth/service_apikeys_api_test.go @@ -114,6 +114,57 @@ func TestService_CreateAPIKeyAPI(t *testing.T) { }) } +// TestService_CreateAPIKeyAPI_CrossPackageType is the regression test for +// issue #1440. The HTTP handler (internal/api) unmarshals the request body +// into api.CreateAPIKeyRequest - a struct in a different package that has +// the same json tags as APICreateAPIKeyRequest but is a distinct Go type. +// The former type-assertion implementation always returned "invalid request +// type", causing a 500. The fix uses JSON re-encoding so any struct with +// compatible json fields is accepted. +func TestService_CreateAPIKeyAPI_CrossPackageType(t *testing.T) { + ctx := context.Background() + mockStore := new(MockStore) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + service := &Service{store: mockStore} + + user := &User{ + ID: "user-123", + Email: "test@example.com", + Active: true, + GroupIDs: []string{DefaultAdminGroupID}, + } + mockStore.On("GetUserByID", ctx, "user-123").Return(user, nil) + mockStore.On("GetGroup", ctx, DefaultAdminGroupID).Return(&Group{ + ID: DefaultAdminGroupID, + Permissions: []Permission{{Action: ActionAdmin, Resource: ResourceAll}}, + }, nil) + mockStore.On("CreateAPIKey", ctx, mock.AnythingOfType("*auth.UserAPIKey")).Return(nil) + + // Simulate what the api.Handler does: an anonymous struct with the same + // json field names but a different Go type than APICreateAPIKeyRequest. + // This is exactly the value passed by the handler in production (issue #1440). + crossPkgReq := struct { + Name string `json:"name"` + ExpiresAt *time.Time `json:"expires_at,omitempty"` + Permissions []struct { + Action string `json:"action"` + Resource string `json:"resource"` + } `json:"permissions,omitempty"` + }{ + Name: "My API Key", + } + + result, err := service.CreateAPIKeyAPI(ctx, "user-123", crossPkgReq) + + // Before the fix this returned "invalid request type" (500); now it must succeed. + require.NoError(t, err, "CreateAPIKeyAPI must accept cross-package types with compatible JSON fields (issue #1440)") + require.NotNil(t, result) + resp, ok := result.(*APICreateAPIKeyResponse) + require.True(t, ok) + assert.NotEmpty(t, resp.APIKey) + assert.Equal(t, "My API Key", resp.Info.Name) +} + func TestService_ListUserAPIKeysAPI(t *testing.T) { ctx := context.Background() diff --git a/internal/server/adapter_test.go b/internal/server/adapter_test.go index 1e8b54dbb..69fb24133 100644 --- a/internal/server/adapter_test.go +++ b/internal/server/adapter_test.go @@ -360,13 +360,27 @@ func TestAuthServiceAdapter_UpdateGroupAPI(t *testing.T) { func TestAuthServiceAdapter_CreateAPIKeyAPI(t *testing.T) { adapter, mockStore := createMockAuthService(t) ctx := context.Background() + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + // CreateAPIKey calls GetUserByID then validateAPIKeyPermissions (GetGroup) + // then CreateAPIKey. All three stores must be mocked for the happy path. + mockStore.On("GetUserByID", ctx, "user-1").Return(&auth.User{ + ID: "user-1", + Email: "test@example.com", + Active: true, + 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) mockStore.On("CreateAPIKey", ctx, mock.AnythingOfType("*auth.UserAPIKey")).Return(nil) - _, err := adapter.CreateAPIKeyAPI(ctx, "user-1", map[string]interface{}{ - "name": "my-key", + result, err := adapter.CreateAPIKeyAPI(ctx, "user-1", auth.APICreateAPIKeyRequest{ + Name: "my-key", }) - _ = err + require.NoError(t, err) + require.NotNil(t, result) } func TestAuthServiceAdapter_ListUserAPIKeysAPI(t *testing.T) {