feat: implement UpdateCurrentUserPAT RPC#1467
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds Personal Access Token (PAT) update functionality across the stack: repository interface and PostgreSQL implementation, service layer with validation and policy management, Connect RPC handler with error mapping, proto message validation, interface mocks, and audit event tracking. The Makefile's PROTON_COMMIT hash is updated to enable proto regeneration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request Test Coverage Report for Build 23331104157Details
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
internal/api/v1beta1connect/user_pat_test.go (1)
1066-1110: This happy-path test doesn't exercise the fields the RPC is supposed to update.
UserPATService.Updateis matched withmock.Anything, the request omitsProjectIds/Metadata, and the assertions only checkId/Title. A handler regression that dropsUserID,ProjectIds, orMetadatawould still pass. Please match the outboundmodels.PATand assert the response carries the updated scope/metadata as well.core/userpat/service_test.go (1)
2061-2093: The happy path never proves stale policies are revoked.This success case starts from an empty old scope, so a regression that appends new policies without deleting the old ones would still pass. Seed
policySvc.Listwith at least one existing policy and assert the matchingDeletecalls beforeCreate.core/userpat/service.go (1)
233-239: Prefer auditing the effective scope fromupdated.
role_idsandproject_idshere are taken fromtoUpdate, while the effective scope is already computed intoupdatedon Lines 184-186. Usingupdated.RoleIDs/updated.ProjectIDskeeps the audit payload aligned with whatever normalization or scope pruning happened during policy replacement.Suggested change
func (s *Service) auditUpdate(ctx context.Context, updated patmodels.PAT, toUpdate patmodels.PAT, oldTitle string, oldRoleIDs []string, oldProjectIDs []string) { if err := s.createAuditRecord(ctx, pkgAuditRecord.PATUpdatedEvent, updated, time.Now().UTC(), map[string]any{ - "role_ids": toUpdate.RoleIDs, - "project_ids": toUpdate.ProjectIDs, + "role_ids": updated.RoleIDs, + "project_ids": updated.ProjectIDs, "old_title": oldTitle, "old_role_ids": oldRoleIDs, "old_project_ids": oldProjectIDs, }); err != nil {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8928912-f18b-4e71-9aaa-6c9f876377b5
⛔ Files ignored due to path filters (1)
proto/v1beta1/frontier.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (14)
Makefilecore/userpat/mocks/repository.gocore/userpat/service.gocore/userpat/service_test.gocore/userpat/userpat.gointernal/api/v1beta1connect/interfaces.gointernal/api/v1beta1connect/mocks/user_pat_service.gointernal/api/v1beta1connect/user_pat.gointernal/api/v1beta1connect/user_pat_test.gointernal/store/postgres/userpat_repository.gopkg/auditrecord/consts.gopkg/server/connect_interceptors/authorization.goproto/v1beta1/frontier.pb.validate.goproto/v1beta1/frontierv1beta1connect/frontier.connect.go
Description:
Summary
UpdateCurrentUserPATRPC to replace a PAT's title, metadata, and scope (roles + projects)Changes
UpdateCurrentUserPATRPC, request (UUID-validated id, title, role_ids, project_ids, metadata), response with updated PATUpdatemethod — updates title + metadata, returns updated row viaRETURNING, handles ErrNotFound/ErrConflictUpdatemethod with helpers:getOwnedPAT,captureOldScope,replacePolicies,auditUpdatePATUpdatedEventwith old_title, old_role_ids, old_project_ids in metadataManual test verification
pat.updatedevent, including old and new title, role_ids, and project_ids