fix(auth): align gated features with current license state#2000
Merged
fix(auth): align gated features with current license state#2000
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds grace period support for license expiration, allowing licenses to remain functional for a configurable duration after expiry. It introduces Docker test environment helpers, updates license feature gating throughout the API/auth/OIDC handlers, modifies license state evaluation logic, and aligns UI terminology to reflect both license and trial variations. Changes
Sequence Diagram(s)sequenceDiagram
participant API as API Handler
participant LM as License Manager
participant State as License State
participant Svc as Feature Service<br/>(Audit/SSO/OIDC)
API->>LM: isFeatureEnabled(Feature)?
LM->>State: IsFeatureEnabled(Feature)?
State->>State: Check: expired?
alt License Not Expired
State-->>LM: true (feature enabled)
else License Expired
State->>State: Check: within graceDuration()?
alt Within Grace Period
State-->>LM: true (feature enabled)
else Grace Period Expired
State-->>LM: false (feature disabled)
end
end
LM-->>API: feature allowed?
alt Feature Allowed
API->>Svc: Proceed with feature
Svc-->>API: Success
else Feature Denied
API-->>API: Gate/Skip feature<br/>(null service or early return)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
internal/intg/ct_test.go (1)
503-507: Consider removing duplicate daemon probes whererequireDockerClient(t)is already used.In these blocks,
requireDockerDaemon(t)is immediately followed byrequireDockerClient(t), which re-checks daemon availability. Collapsing to a singlerequireDockerClient(t)call would reduce redundant probe calls.Also applies to: 1088-1093, 1287-1292, 1400-1405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/intg/ct_test.go` around lines 503 - 507, Remove redundant requireDockerDaemon(t) calls when they are immediately followed by requireDockerClient(t); keep only requireDockerClient(t) (which already probes the daemon) and remove the duplicate requireDockerDaemon(t) before it in the same setup blocks (e.g., the block that calls test.Setup(t), requireDockerDaemon(t), then requireDockerClient(t)). Apply the same change to other similar blocks in this file where requireDockerClient(t) follows requireDockerDaemon(t) so only requireDockerClient(t) remains.internal/intg/docker_helper_test.go (1)
14-29: Optional: extract shared daemon-probe logic to a small internal helper.Both functions duplicate creation/probe logic; consolidating it would reduce maintenance drift if probe behavior changes later.
Also applies to: 31-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/intg/docker_helper_test.go` around lines 14 - 29, The requireDockerDaemon function duplicates docker creation/probe logic—extract that into a small internal helper (e.g., newDockerClientOrSkip or probeDockerDaemon) that encapsulates client.New(client.FromEnv), context creation/timeout, Info probe and t.Skip behavior; replace requireDockerDaemon and the other duplicate block (lines 31-49) to call the new helper, returning the client, cancel/cleanup function, and/or context so callers can defer Close()/cancel; ensure helper uses the same t *testing.T to call t.Skipf with the original error messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/license/state.go`:
- Around line 109-119: The issue is that graceDuration() reads
s.claims.GraceDays which is currently shallow-copied by cloneClaims, allowing
external mutation to affect entitlement checks; update cloneClaims to deep-copy
the GraceDays pointer (allocate a new int64/int/whatever the type is and copy
the value) so the internal State.claims holds its own independent pointer, and
ensure Claims()/Update() continue to use cloneClaims so
IsFeatureEnabled/IsGracePeriod and graceDuration() observe an immutable copy.
In `@internal/service/frontend/server.go`:
- Around line 1078-1082: The agent audit path is using the raw auditSvc passed
into initAgentAPI(...) before WithLicenseManager sets up srv.licenseManager,
allowing agent tool executions to bypass license-based audit gating; fix by
making initAgentAPI receive the live license.Checker (from
srv.licenseManager.Checker()) or by moving the initAgentAPI(...) call to after
WithLicenseManager is applied (i.e., after srv.licenseManager is wired), and
ensure any agent audit hooks created inside initAgentAPI use that
license.Checker instead of the standalone auditSvc; update references to
initAgentAPI, auditSvc, srv.licenseManager, license.Checker, and any agent audit
hook registration so they use the live checker.
In `@ui/src/components/LicenseBanner.tsx`:
- Around line 64-72: The banner currently uses a hardcoded/graceEnd value that
can be incorrect; update LicenseBanner (and its callers) to accept the
backend-provided grace end (e.g., a prop named graceEnd or graceEndDate) and
render that value instead of the hardcoded 14-day date, and if the prop is not
provided render a non-specific message without a concrete date (e.g., omit
{graceEnd} and say "soon" or remove the date fragment) so the UI is accurate for
trials/non-default grace periods; locate references to LicenseBanner, the
variables licenseLabel, graceEnd, renewalLabel and LICENSE_CONSOLE_URL to apply
the prop addition and add a safe fallback when undefined.
In `@ui/src/pages/license/index.tsx`:
- Around line 119-129: The deactivate control is only shown when license.valid
is true, so when a file/env-backed license has expired into license.gracePeriod
the deactivate card becomes hidden; update the visibility check used for that
card (where it currently checks license.valid) to allow both active and grace
states (e.g., use license.valid || license.gracePeriod) so the deactivate
section remains available during grace period, and verify any related rendering
branches that treat gracePeriod distinctly still render the correct status
badges (license.gracePeriod, license.valid, license.community).
---
Nitpick comments:
In `@internal/intg/ct_test.go`:
- Around line 503-507: Remove redundant requireDockerDaemon(t) calls when they
are immediately followed by requireDockerClient(t); keep only
requireDockerClient(t) (which already probes the daemon) and remove the
duplicate requireDockerDaemon(t) before it in the same setup blocks (e.g., the
block that calls test.Setup(t), requireDockerDaemon(t), then
requireDockerClient(t)). Apply the same change to other similar blocks in this
file where requireDockerClient(t) follows requireDockerDaemon(t) so only
requireDockerClient(t) remains.
In `@internal/intg/docker_helper_test.go`:
- Around line 14-29: The requireDockerDaemon function duplicates docker
creation/probe logic—extract that into a small internal helper (e.g.,
newDockerClientOrSkip or probeDockerDaemon) that encapsulates
client.New(client.FromEnv), context creation/timeout, Info probe and t.Skip
behavior; replace requireDockerDaemon and the other duplicate block (lines
31-49) to call the new helper, returning the client, cancel/cleanup function,
and/or context so callers can defer Close()/cancel; ensure helper uses the same
t *testing.T to call t.Skipf with the original error messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b7a7194-d04b-41ef-bedd-e9dde9003ba6
📒 Files selected for processing (24)
internal/intg/ct_test.gointernal/intg/docker_helper_test.gointernal/intg/mltcmd_test.gointernal/intg/redis_test.gointernal/intg/s3_test.gointernal/intg/sftp_test.gointernal/intg/ssh_test.gointernal/license/claims.gointernal/license/claims_test.gointernal/license/manager.gointernal/license/manager_test.gointernal/license/state.gointernal/license/state_test.gointernal/service/frontend/api/v1/api.gointernal/service/frontend/api/v1/auth.gointernal/service/frontend/auth/oidc.gointernal/service/frontend/server.gointernal/service/frontend/templates.gointernal/service/frontend/terminal/handler.goui/src/App.tsxui/src/components/LicenseBanner.tsxui/src/hooks/useLicense.tsui/src/pages/license/index.tsxui/src/pages/users/index.tsx
# Conflicts: # internal/runtime/agent/agent.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Licensed paths were not enforced consistently after expiry. Some UI affordances only checked feature claims, OIDC gating was still tied to startup-time state, and heartbeat expiry handling kept stale data without making current entitlement availability explicit. That left long-running servers and the frontend out of sync with actual license validity.
Testing
go test ./... -count=1pnpm -C ui exec tsc --noEmitSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores