mirror of
https://github.com/usememos/memos.git
synced 2025-12-20 07:38:43 +08:00
1a3298554b
This commit addresses all critical and high-priority recommendations from the security review: **Critical Fixes:** - Add nil checks before accessing memo properties in SetMemoAttachments and SetMemoRelations to prevent potential nil pointer dereference - Fix information disclosure in DeleteMemoReaction by returning consistent errors (now returns permission denied instead of not found to avoid revealing reaction existence) **Medium Priority Improvements:** - Add GetReaction() method to store interface for better performance (single reaction lookup instead of list operation) - Implement GetReaction() in all database drivers (SQLite, MySQL, PostgreSQL) - Update DeleteMemoReaction to use the new GetReaction() method **Test Coverage:** - Add comprehensive test coverage for SetMemoAttachments authorization checks - Add comprehensive test coverage for SetMemoRelations authorization checks - Add comprehensive test coverage for DeleteMemoReaction authorization checks - Add comprehensive test coverage for CreateUser registration enforcement All tests follow the same patterns as existing IDP service tests and cover: - Success cases for resource owners - Success cases for superuser/host users - Permission denied cases for non-owners - Unauthenticated access attempts - Not found scenarios Related to PR #5217 security review recommendations.
105 lines
3.3 KiB
Go
105 lines
3.3 KiB
Go
package v1
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"time"
|
|
|
|
"google.golang.org/grpc/codes"
|
|
"google.golang.org/grpc/status"
|
|
"google.golang.org/protobuf/types/known/emptypb"
|
|
"google.golang.org/protobuf/types/known/timestamppb"
|
|
|
|
v1pb "github.com/usememos/memos/proto/gen/api/v1"
|
|
"github.com/usememos/memos/store"
|
|
)
|
|
|
|
func (s *APIV1Service) ListMemoReactions(ctx context.Context, request *v1pb.ListMemoReactionsRequest) (*v1pb.ListMemoReactionsResponse, error) {
|
|
reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{
|
|
ContentID: &request.Name,
|
|
})
|
|
if err != nil {
|
|
return nil, status.Errorf(codes.Internal, "failed to list reactions")
|
|
}
|
|
|
|
response := &v1pb.ListMemoReactionsResponse{
|
|
Reactions: []*v1pb.Reaction{},
|
|
}
|
|
for _, reaction := range reactions {
|
|
reactionMessage := convertReactionFromStore(reaction)
|
|
response.Reactions = append(response.Reactions, reactionMessage)
|
|
}
|
|
return response, nil
|
|
}
|
|
|
|
func (s *APIV1Service) UpsertMemoReaction(ctx context.Context, request *v1pb.UpsertMemoReactionRequest) (*v1pb.Reaction, error) {
|
|
user, err := s.GetCurrentUser(ctx)
|
|
if err != nil {
|
|
return nil, status.Errorf(codes.Internal, "failed to get current user")
|
|
}
|
|
if user == nil {
|
|
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
|
}
|
|
reaction, err := s.Store.UpsertReaction(ctx, &store.Reaction{
|
|
CreatorID: user.ID,
|
|
ContentID: request.Reaction.ContentId,
|
|
ReactionType: request.Reaction.ReactionType,
|
|
})
|
|
if err != nil {
|
|
return nil, status.Errorf(codes.Internal, "failed to upsert reaction")
|
|
}
|
|
|
|
reactionMessage := convertReactionFromStore(reaction)
|
|
|
|
return reactionMessage, nil
|
|
}
|
|
|
|
func (s *APIV1Service) DeleteMemoReaction(ctx context.Context, request *v1pb.DeleteMemoReactionRequest) (*emptypb.Empty, error) {
|
|
user, err := s.GetCurrentUser(ctx)
|
|
if err != nil {
|
|
return nil, status.Errorf(codes.Internal, "failed to get current user: %v", err)
|
|
}
|
|
if user == nil {
|
|
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
|
}
|
|
|
|
reactionID, err := ExtractReactionIDFromName(request.Name)
|
|
if err != nil {
|
|
return nil, status.Errorf(codes.InvalidArgument, "invalid reaction name: %v", err)
|
|
}
|
|
|
|
// Get reaction and check ownership
|
|
reaction, err := s.Store.GetReaction(ctx, &store.FindReaction{
|
|
ID: &reactionID,
|
|
})
|
|
if err != nil {
|
|
return nil, status.Errorf(codes.Internal, "failed to get reaction")
|
|
}
|
|
if reaction == nil {
|
|
// Return permission denied to avoid revealing if reaction exists
|
|
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
|
}
|
|
|
|
if reaction.CreatorID != user.ID && !isSuperUser(user) {
|
|
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
|
}
|
|
|
|
if err := s.Store.DeleteReaction(ctx, &store.DeleteReaction{
|
|
ID: reactionID,
|
|
}); err != nil {
|
|
return nil, status.Errorf(codes.Internal, "failed to delete reaction")
|
|
}
|
|
|
|
return &emptypb.Empty{}, nil
|
|
}
|
|
|
|
func convertReactionFromStore(reaction *store.Reaction) *v1pb.Reaction {
|
|
reactionUID := fmt.Sprintf("%d", reaction.ID)
|
|
return &v1pb.Reaction{
|
|
Name: fmt.Sprintf("%s%s", ReactionNamePrefix, reactionUID),
|
|
Creator: fmt.Sprintf("%s%d", UserNamePrefix, reaction.CreatorID),
|
|
ContentId: reaction.ContentID,
|
|
ReactionType: reaction.ReactionType,
|
|
CreateTime: timestamppb.New(time.Unix(reaction.CreatedTs, 0)),
|
|
}
|
|
}
|