[TT-15141] Toggling default policy from inactive to active does not activate JWT in some cases - revert session save

[MaciekMis]

User description

TT-15141

Summary	Toggling default policy from inactive to active does not activate JWT in some cases
Type	Bug
Status	Ready for Testing
Points	N/A
Labels	2025_long_tail, 2025_r5_candidate, AI-Complexity-Medium, AI-Priority-High, codilime_refined, customer_bug, jira_escalated

---

Description

This PR reverts session save change.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

---

PR Type

Bug fix, Enhancement

---

Description

• Remove unnecessary session Touch call

• Prevent unintended session save side effects

• Avoid performance impact from extra writes

---

Diagram Walkthrough

flowchart LR
  ApplyPolicy["Apply policy to session"] -- "previously called" --> TouchSession["session.Touch()"]
  ApplyPolicy -- "now" --> ReturnOnly["return without touching session"]

Loading

File Walkthrough

	Relevant files
Bug fix	apply.go Stop touching session after applying policies                        internal/policy/apply.go Remove session.Touch() at end of Apply. Preserve error when no valid policies applied. Keep access rights update logic unchanged. +0/-2

---

[buger]

I'm a bot and I 👍 this PR title. 🤖

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Behavioral Change Removing the call to session.Touch() changes session update/expiry semantics. Validate that session TTL refresh and last-seen timestamps are still updated where required, especially when policies are applied without other session mutations. if len(applyState.didAcl) > 0 { session.AccessRights = rights } if len(rights) == 0 && policyIDs != nil { return errors.New("key has no valid policies to be applied") } return nil

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Conditionally update session activity Removing session.Touch() may prevent session last-activity updates, potentially leaving active sessions stale. If the intent is to avoid unnecessary persistence, still update in-memory last-seen to maintain correct JWT/session freshness. Consider conditionally touching only when policies were applied or access rights changed. internal/policy/apply.go [242-247] if len(rights) == 0 && policyIDs != nil { return errors.New("key has no valid policies to be applied") } +// Only update session activity when access rights were modified. +if len(applyState.didAcl) > 0 { + session.Touch() +} + return nil Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that session.Touch() was removed and proposes a conditional touch tied to applyState.didAcl, which aligns with nearby logic and could prevent stale sessions. However, the need to touch here is speculative without broader context, so impact is moderate rather than critical.	Low

[github-actions]

API Changes

no api changes detected

[probelabs]

🔍 Code Analysis Results

This pull request addresses a bug where activating a default policy would not correctly enable JWT authentication in certain scenarios. The root cause was a premature saving of the user session state.

Files Changed Analysis

• File: internal/policy/apply.go
• Change: 2 lines deleted.
• Summary: The change is minimal, removing a single function call, session.Touch(), from the Apply method of the policy service. While small, this has a significant impact on the session management lifecycle.

---

Architecture & Impact Assessment

What this PR accomplishes

This PR fixes a bug that caused inconsistent session states, specifically affecting JWT activation when a policy was changed. It corrects the session persistence flow by ensuring the session is saved only after all modifications for a given request have been applied.

Key technical changes introduced

The core change is the removal of session.Touch() from internal/policy/Service.Apply. The Touch() method marks the session object as "modified," flagging it to be saved to the session store (e.g., Redis).

Previously, the policy service would immediately mark the session as modified after applying policy data. This created a race condition: the session could be saved before other middleware (like the JWT middleware) had a chance to add its own data to the session.

By removing this call, the responsibility for marking the session as modified is moved up the call stack to the middleware orchestrator. This ensures that the session is only flagged for saving after the entire middleware chain has processed it, resulting in a complete and correct state being persisted.

Affected system components

• Policy Engine: The internal/policy service is directly modified.
• Session Management: The lifecycle of user.SessionState objects is impacted, improving its robustness.
• Authentication Middleware (JWT): The JWT middleware and potentially other auth-related components will now function correctly, as their changes to the session state will be reliably persisted.

Visualization

The following diagrams illustrate the change in the request flow.

Old Flow (Buggy):

sequenceDiagram
    participant Middleware
    participant SessionManager
    participant PolicyService
    participant JWT_Middleware
    participant SessionStore

    Note over Middleware: Old Flow (with bug)
    Middleware->>SessionManager: Get/Create Session
    SessionManager-->>Middleware: session
    Middleware->>PolicyService: ApplyPolicies(session)
    PolicyService->>PolicyService: Applies policy data
    PolicyService->>Middleware: session.Touch() called inside
    Note right of PolicyService: Session is now marked as modified
    Middleware->>SessionManager: SaveSessionIfModified(session)
    SessionManager->>SessionStore: SAVE session (prematurely)
    Middleware->>JWT_Middleware: ProcessRequest(session)
    JWT_Middleware->>JWT_Middleware: Adds JWT data to session
    Note right of JWT_Middleware: Too late! Session is already saved without JWT data.

Loading

New Flow (Corrected):

sequenceDiagram
    participant Middleware
    participant SessionManager
    participant PolicyService
    participant JWT_Middleware
    participant SessionStore

    Note over Middleware: New Flow (with fix)
    Middleware->>SessionManager: Get/Create Session
    SessionManager-->>Middleware: session
    Middleware->>PolicyService: ApplyPolicies(session)
    PolicyService->>PolicyService: Applies policy data
    PolicyService-->>Middleware: returns
    Note right of PolicyService: session.Touch() is NOT called
    Middleware->>JWT_Middleware: ProcessRequest(session)
    JWT_Middleware->>JWT_Middleware: Adds JWT data to session
    Middleware->>Middleware: session.Touch() called by orchestrator
    Note right of Middleware: Session marked as modified *after* all changes
    Middleware->>SessionManager: SaveSessionIfModified(session)
    SessionManager->>SessionStore: SAVE session (with all data)

Loading

---

Scope Discovery & Context Expansion

The change is located in the core policy application logic (internal/policy/apply.go), which is invoked by the BaseMiddleware in gateway/middleware.go. The BaseMiddleware.ApplyPolicies function is called whenever a session is retrieved or created, such as in CheckSessionAndIdentityForValidKey.

This means the impact of this change extends to all authentication mechanisms that rely on policies. By centralizing the "dirty" flag (modified) management outside of individual services like the policy engine, the system becomes less prone to similar race conditions in the future. Any middleware that modifies the session can do so with the confidence that the final state will be persisted correctly at the end of the request processing cycle.

Metadata

• Review Effort: 2 / 5
• Primary Label: bug

---

Powered by Visor from Probelabs

Last updated: 2025-10-14T09:14:48.010Z | Triggered by: opened | Commit: e655429

[probelabs]

🔍 Code Analysis Results

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

✅ Quality Check Passed

No quality issues found – changes LGTM.

✅ Style Check Passed

No style issues found – changes LGTM.

✅ Dependency Check Passed

No dependency issues found – changes LGTM.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.

---

Powered by Visor from Probelabs

Last updated: 2025-10-14T09:14:49.059Z | Triggered by: opened | Commit: e655429

[probelabs]

🔍 Code Analysis Results

---

Powered by Visor from Probelabs

Last updated: 2025-10-14T09:14:50.005Z | Triggered by: opened | Commit: e655429

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[MaciekMis]

/release to release-5.8

[MaciekMis]

/release to release-5.10

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@MaciekMis Created merge PRs

[tykbot]

@MaciekMis Seems like there is conflict and it require manual merge.