@@ -576,14 +576,75 @@ func (h *Handler) getRIExchangeHistory(ctx context.Context, req *events.LambdaFu
576576 return & RIExchangeHistoryResponse {Records : records }, nil
577577}
578578
579- // approveRIExchange handles approval of a pending RI exchange via token.
580- func (h * Handler ) approveRIExchange (ctx context.Context , id , token string ) (any , error ) {
581- record , err := h .validateExchangeApproval (ctx , id , token )
579+ // approveRIExchange handles approval of a pending RI exchange.
580+ //
581+ // Three-mode dispatch mirroring approvePurchase (issue #286, issue #300):
582+ //
583+ // 1. Session present AND RBAC-authorized (admin / approve-any / approve-own
584+ // match) -> session-authed approve, regardless of whether a token is also
585+ // in the URL. Closes issue #300.
586+ // 2. token != "" -> legacy email-link flow. validateExchangeApproval enforces
587+ // the token-equality check; the permission-denied fall-through ensures a
588+ // logged-in user without approve-* can still use an email link they hold.
589+ // 3. token == "" AND no qualifying session -> 403 via
590+ // approveRIExchangeViaSession's requireSession gate.
591+ func (h * Handler ) approveRIExchange (ctx context.Context , req * events.LambdaFunctionURLRequest , id , token string ) (any , error ) {
592+ if session := h .tryGetSession (ctx , req ); session != nil {
593+ // Quick RBAC pre-check (no record fetch needed): does this session hold
594+ // ANY approve right? If yes, hand off to approveRIExchangeViaSession
595+ // which will re-check ownership with the actual record. If 403, fall
596+ // through to the token branch so email-link holders can still approve.
597+ switch err := h .sessionHasApproveRight (ctx , session ); {
598+ case err == nil :
599+ return h .approveRIExchangeViaSession (ctx , req , id , session )
600+ case isPermissionDenied (err ):
601+ // Logged-in user without approve-* may still hold a valid email token.
602+ default :
603+ return nil , err
604+ }
605+ }
606+
607+ if token != "" {
608+ record , err := h .validateExchangeApproval (ctx , id , token )
609+ if err != nil {
610+ return nil , err
611+ }
612+
613+ transitioned , err := h .config .TransitionRIExchangeStatus (ctx , id , "pending" , "processing" )
614+ if err != nil {
615+ return nil , fmt .Errorf ("failed to transition exchange status: %w" , err )
616+ }
617+ if transitioned == nil {
618+ return nil , NewClientError (409 , "exchange already processed, expired, or was cancelled by a newer analysis run" )
619+ }
620+
621+ return h .executeApprovedExchange (ctx , id , record )
622+ }
623+
624+ return h .approveRIExchangeViaSession (ctx , req , id , nil )
625+ }
626+
627+ // approveRIExchangeViaSession is the session-authed branch of approveRIExchange
628+ // (issue #300). Enforces the approve-any/approve-own RBAC matrix, then atomically
629+ // transitions pending -> processing and executes the exchange. Stamps
630+ // session.Email onto the approved_by column as an audit trail.
631+ //
632+ // The session parameter may be non-nil (already validated by the caller) or nil
633+ // (requireSession will validate it and return 401 if absent).
634+ func (h * Handler ) approveRIExchangeViaSession (ctx context.Context , req * events.LambdaFunctionURLRequest , id string , session * Session ) (any , error ) {
635+ var err error
636+ if session == nil {
637+ session , err = h .requireSession (ctx , req )
638+ if err != nil {
639+ return nil , err
640+ }
641+ }
642+
643+ record , err := h .fetchAndAuthorizeRIExchange (ctx , session , id )
582644 if err != nil {
583645 return nil , err
584646 }
585647
586- // Atomic transition: pending -> processing (checks expiry in WHERE clause)
587648 transitioned , err := h .config .TransitionRIExchangeStatus (ctx , id , "pending" , "processing" )
588649 if err != nil {
589650 return nil , fmt .Errorf ("failed to transition exchange status: %w" , err )
@@ -592,7 +653,112 @@ func (h *Handler) approveRIExchange(ctx context.Context, id, token string) (any,
592653 return nil , NewClientError (409 , "exchange already processed, expired, or was cancelled by a newer analysis run" )
593654 }
594655
595- return h .executeApprovedExchange (ctx , id , record )
656+ result , execErr := h .executeApprovedExchange (ctx , id , record )
657+
658+ // Stamp approver attribution (best-effort: the exchange itself already
659+ // executed, so a stamp failure is logged but not surfaced to the caller).
660+ if execErr == nil {
661+ if stampErr := h .config .StampRIExchangeApprovedBy (ctx , id , session .Email ); stampErr != nil {
662+ logging .Errorf ("failed to stamp approved_by on exchange %s: %v" , id , stampErr )
663+ }
664+ }
665+
666+ return result , execErr
667+ }
668+
669+ // fetchAndAuthorizeRIExchange looks up the pending exchange record by id, checks
670+ // that it is in "pending" state, and then verifies that session is authorised to
671+ // approve it. Extracted from approveRIExchangeViaSession to keep that function
672+ // under the cyclomatic-complexity limit.
673+ func (h * Handler ) fetchAndAuthorizeRIExchange (ctx context.Context , session * Session , id string ) (* config.RIExchangeRecord , error ) {
674+ if err := validateUUID (id ); err != nil {
675+ return nil , err
676+ }
677+
678+ record , err := h .config .GetRIExchangeRecord (ctx , id )
679+ if err != nil {
680+ return nil , fmt .Errorf ("failed to look up exchange record: %w" , err )
681+ }
682+ if record == nil {
683+ return nil , NewClientError (404 , "exchange record not found" )
684+ }
685+
686+ if record .Status != "pending" {
687+ return nil , NewClientError (409 , fmt .Sprintf ("exchange %s cannot be approved (status=%s)" , id , record .Status ))
688+ }
689+
690+ if err := h .authorizeSessionApproveRIExchange (ctx , session , record ); err != nil {
691+ return nil , err
692+ }
693+
694+ return record , nil
695+ }
696+
697+ // sessionHasApproveRight returns nil when the session holds ANY approve right
698+ // on purchases (admin / approve-any / approve-own) without checking ownership.
699+ // Used by the three-mode dispatch in approveRIExchange to decide whether to route
700+ // to approveRIExchangeViaSession before fetching the record.
701+ func (h * Handler ) sessionHasApproveRight (ctx context.Context , session * Session ) error {
702+ if session .Role == "admin" {
703+ return nil
704+ }
705+ if h .auth == nil {
706+ return NewClientError (500 , "authentication service not configured" )
707+ }
708+ hasAny , err := h .auth .HasPermissionAPI (ctx , session .UserID , auth .ActionApproveAny , auth .ResourcePurchases )
709+ if err != nil {
710+ return fmt .Errorf ("permission check failed: %w" , err )
711+ }
712+ if hasAny {
713+ return nil
714+ }
715+ hasOwn , err := h .auth .HasPermissionAPI (ctx , session .UserID , auth .ActionApproveOwn , auth .ResourcePurchases )
716+ if err != nil {
717+ return fmt .Errorf ("permission check failed: %w" , err )
718+ }
719+ if hasOwn {
720+ return nil
721+ }
722+ return NewClientError (403 , "permission denied: requires approve-any or approve-own on purchases" )
723+ }
724+
725+ // authorizeSessionApproveRIExchange returns nil when the session is permitted to
726+ // approve the given RI exchange record under the approve-any / approve-own RBAC rules
727+ // (issue #300). Mirrors authorizeSessionApprove from handler_purchases.go.
728+ //
729+ // The RI exchange shares ResourcePurchases because approval is conceptually
730+ // "approving a purchase action on a different resource type" per the issue spec,
731+ // which prefers reusing the existing verbs to keep the matrix small.
732+ func (h * Handler ) authorizeSessionApproveRIExchange (ctx context.Context , session * Session , record * config.RIExchangeRecord ) error {
733+ if session .Role == "admin" {
734+ return nil
735+ }
736+ if h .auth == nil {
737+ return NewClientError (500 , "authentication service not configured" )
738+ }
739+
740+ hasAny , err := h .auth .HasPermissionAPI (ctx , session .UserID , auth .ActionApproveAny , auth .ResourcePurchases )
741+ if err != nil {
742+ return fmt .Errorf ("permission check failed: %w" , err )
743+ }
744+ if hasAny {
745+ return nil
746+ }
747+
748+ hasOwn , err := h .auth .HasPermissionAPI (ctx , session .UserID , auth .ActionApproveOwn , auth .ResourcePurchases )
749+ if err != nil {
750+ return fmt .Errorf ("permission check failed: %w" , err )
751+ }
752+ if ! hasOwn {
753+ return NewClientError (403 , "permission denied: requires approve-any or approve-own on purchases" )
754+ }
755+
756+ // approve-own: only allow if the session user created this exchange.
757+ if record .CreatedByUserID == nil || * record .CreatedByUserID != session .UserID {
758+ return NewClientError (403 , "permission denied: cannot approve another user's pending exchange" )
759+ }
760+
761+ return nil
596762}
597763
598764// validateExchangeApproval validates ID, token, and record state for an exchange approval.
0 commit comments