Skip to content

fix: PagerBug: API token Last Used On/By IP fields never update after use against Kubernetes resources (pager #2971) - #7040

Draft
devtronpageragent[bot] wants to merge 1 commit into
mainfrom
agent/pager-2971
Draft

devtronpageragent[bot] wants to merge 1 commit into
mainfrom
agent/pager-2971

Conversation

@devtronpageragent

Copy link
Copy Markdown

Agent-authored draft fix for pager issue #2971

This pull request was written by an automated agent and reviewed by nobody. Devtron's test suites do not cover this code (make test-unit runs only ./pkg/pipeline; dashboard CI never runs tests), so you are the gate. Read the causal chain below and decide whether it holds.

Tracking issue: https://github.com/devtron-labs/sprint-tasks/issues/2971

Root cause

Two defects on the same field pair. (1) enterprise-only: /orchestrator/k8s/proxy is listed in WhitelistChecker's prefix list, so the auth middleware skips CheckUserStatusAndUpdateLoginAudit — the only thing that writes a user_audit row for a token — and handleK8sProxyRequest, which does its own Bearer-token authentication, never records usage either. A token used only against Kubernetes resources via the kubectl proxy therefore has no user_audit row at all, so GetLatestByUserId returns nil and the listing leaves both lastUsedAt and lastUsedByIp unset (dashboard renders 'Never used' / '-'). (2) both repos: ApiTokenService.GetAllActiveApiTokens reports user_audit.created_on as "Last Used On", but the live write path (updateUserAudit -> UserAuditRepositoryImpl.Update) updates the row in place and only bumps updated_on, preserving created_on. The old insert-per-use writer saveUserAudit, which is what made created_on mean "last use", is dead code. So even once a row exists the timestamp is frozen at the first use and never updates.

From symptom to defect

  1. kubectl-style access to Kubernetes resources with a Devtron API token is served by the proxy routes registered under the /orchestrator/k8s prefix.
    devtron-enterprise/api/k8s/application/k8sApplicationRouter.go · InitK8sApplicationRouter
    Lines 124-128: k8sAppRouter.PathPrefix(fmt.Sprintf("/proxy/%s/{%s}", bean.Cluster, bean.ClusterIdentifier)).HandlerFunc(impl.k8sApplicationRestHandler.HandleK8sProxyRequest) (and the same for env).
  2. Those proxy URLs are whitelisted, so the auth middleware short-circuits before the DB user-status/audit step.
    devtron-enterprise/pkg/auth/user/UserAuthService.go · WhitelistChecker
    Line 395 in the prefixUrls slice: "/orchestrator/k8s/proxy", matched by strings.Contains(url, a) at line 400.
  3. With the path whitelisted, the block that calls userStatusCheckInDb is skipped entirely and the request is passed straight to the handler.
    devtron-services/authenticator/middleware/AuthMiddleware.go · Authorizer
    Line 59 if token != "" && authEnabled && !whitelistChecker(r.URL.Path) { guards the userStatusCheckInDb(token) call at line 75; line 93 } else if whitelistChecker(r.URL.Path) { next.ServeHTTP(w, r) }.
  4. That skipped callback is the only place the orchestrator records a user_audit row for a request that is not handled by a handler calling GetLoggedInUser.
    devtron-enterprise/pkg/auth/user/UserService.go · CheckUserStatusAndUpdateLoginAudit
    Line 2206: impl.SaveLoginAudit(emailId, "localhost", userId), reached only from App.go:123 where it is passed to authMiddleware.Authorizer.
  5. The proxy handler re-implements authentication from the Authorization header but records no usage, so nothing writes user_audit for proxy traffic.
    devtron-enterprise/api/k8s/application/k8sApplicationRestHandler.go · handleK8sProxyRequest
    Line 1770-1771 token := getDevtronLoginTokenFromK8sProxyRequest(r) / handler.userService.GetEmailAndGroupClaimsFromToken(token); the rest of the function only does RBAC and proxyServer.ServeHTTP — no call to GetLoggedInUser or SaveLoginAudit.
  6. The listing leaves both fields unset when no audit row exists, which the dashboard renders as blank.
    devtron-enterprise/pkg/apiToken/ApiTokenService.go · GetAllActiveApiTokens
    Line 161 if latestAuditLog != nil { guards both apiToken.LastUsedAt and apiToken.LastUsedByIp; UserAuditServiceImpl.GetLatestByUserId returns nil, nil on pg.ErrNoRows.
  7. Unset fields are displayed as 'Never used' and '-', i.e. the blank columns in the report.
    dashboard/src/Pages/GlobalConfigurations/Authorization/APITokens/APITokenList.tsx · APITokenList
    Lines 129-135: {list.lastUsedAt ? moment(...) : 'Never used'} and {list.lastUsedByIp ? list.lastUsedByIp : '-'}.
  8. Second defect: even when an audit row exists, the timestamp shown is created_on, which the write path never changes after the row is inserted.
    devtron/pkg/apiToken/ApiTokenService.go · GetAllActiveApiTokens
    Was line 177 lastUsedAtStr := latestAuditLog.CreatedOn.String(), while UserAuditRepositoryImpl.Update (UserAuditRepository.go:53-65) sets userAudit.CreatedOn = userAuditPresentInDB.CreatedOn and only advances UpdatedOn.
  9. Confirms the regression: the insert-per-use writer that once made created_on equal the latest use is now unreferenced, leaving created_on frozen at first use.
    devtron/pkg/auth/user/UserService.go · saveUserAudit
    saveUserAudit is defined at line 1678 and has no callers anywhere in either repo (grep for saveUserAudit matches only the two definitions); the live path is go impl.updateUserAudit(r, userId) at line 1250.

Explanations considered and rejected

  • The dashboard never renders the fields, or the JSON keys do not match, so the backend value is discarded. — ruled out by APITokenList.tsx:129-135 reads list.lastUsedAt / list.lastUsedByIp and only falls back to 'Never used' / '-' when they are falsy; the Go openapi model emits exactly those keys (api/openapi/openapiClient/model_api_token.go:34,36json:"lastUsedAt,omitempty", json:"lastUsedByIp,omitempty"). The neighbouring expireAtInMs from the same response renders fine on the same row.
  • apiTokenFromDb.User is not loaded, so userId is 0 and the audit lookup finds nothing for every token. — ruled out by ApiTokenRepositoryImpl.FindAllActive selects Column("api_token.*", "User") with an explicit Relation("User", ...) (ApiTokenRepository.go:77-85), and the same apiTokenFromDb.User.EmailId is used for UserIdentifier, which does display correctly on the listing page.
  • The audit write fails at the repository layer for all users, so no user_audit row is ever created. — ruled out by UserAuditRepositoryImpl.Update explicitly handles the empty case — else if err == pg.ErrNoRows { userAudit.CreatedOn = userAudit.UpdatedOn; err = impl.dbConnection.Insert(userAudit) } (UserAuditRepository.go:60-63) — and it is driven by the middleware on every non-whitelisted authenticated request. If it failed universally, no token would ever show a value, and the same read path feeds the listing for tokens used against non-k8s APIs.
  • The resource-browser endpoints (/orchestrator/k8s/resource, /events, /pods/logs) are the k8s path in the ticket and they skip the audit. — ruled out by Those handlers do skip GetLoggedInUser (k8sApplicationRestHandler.go — GetResource at 167 in OSS / 336 in enterprise never calls it), but their URLs are not in WhitelistChecker, so CheckUserStatusAndUpdateLoginAudit still writes a user_audit row for them. They can produce a stale timestamp and a 'localhost' IP, but not an absent row, so they cannot explain both fields being blank. Only the /orchestrator/k8s/proxy prefix is whitelisted.
  • Customer-specific cluster, credential or configuration state. — ruled out by The whitelist entry and the proxy handler's lack of any audit call are unconditional code paths with no env var, feature flag or cluster input gating them.
  • The token itself is rejected, so nothing is recorded because the request never authenticates. — ruled out by SessionManager.VerifyToken dispatches api-token issuers to ParseApiToken (devtron-services/authenticator/middleware/sessionmanager.go:224-225), and the reporter observes the Kubernetes access succeeding; a failed auth would surface as 401/400, not as a silently missing audit field.

What this PR changes in devtron

API-token listing now reports user_audit.updated_on as 'Last Used On' instead of created_on, falling back to created_on when updated_on is zero (rows predating migration 87, which added the column without a backfill). The write path updates the audit row in place and only advances updated_on, so created_on was frozen at the token's first ever use and the field never changed afterwards.

Risk: Behaviour change visible to anyone reading the API-token listing: 'Last Used On' will jump from the first-use timestamp to the most recent one. Any consumer that was (incorrectly) treating this field as 'first used' loses that value — nothing in-tree does; the only caller is api/apiToken/ApiTokenRestHandler.go:76. If a deployment somehow has user_audit rows whose updated_on is NULL and which are never touched again, they now take the created_on fallback, i.e. unchanged from today.

  • pkg/apiToken/ApiTokenService.go

Verified

  • Build: not run. This change has not been compiled. Not built. This environment has no Go toolchain and the checkouts have no vendor/ directory; the task instructions forbid running go build/go test here. Compilation is left to the repository's own CI on the draft PR.
  • Tests: none run.
  • user2.UserAudit (pkg/auth/user/UserAuditService.go:27-32) has an UpdatedOn time.Time field, so .IsZero() is valid and no new import is needed.
  • UserAuditServiceImpl.GetLatestByUserId already populates UpdatedOn from the DB row (UserAuditService.go:99-104), so the value is available at the call site.
  • UserAuditRepositoryImpl.Update advances UpdatedOn on every call and re-copies the existing CreatedOn back onto the row (UserAuditRepository.go:53-65), which is why created_on never moves.
  • saveUserAudit — the insert-per-use writer under which created_on did equal the latest use — has no callers in either devtron or devtron-enterprise; the live path is go impl.updateUserAudit(r, userId) (UserService.go:1250).
  • No test or other caller depends on the old value: the only consumer of GetAllActiveApiTokens is api/apiToken/ApiTokenRestHandler.go:76.

NOT verified

Read this section before the diff.

  • That the file compiles (no Go toolchain here).
  • That the rendered string format is unchanged in practice — both branches call time.Time.String() exactly as before, but I did not observe the actual JSON or the dashboard rendering it.
  • Whether any customer database has user_audit rows with a NULL updated_on; the fallback is defensive, not observed.

Open assumptions:

  • Which Kubernetes access path the customer actually used. The blank-field symptom is only fully explained by the kubectl proxy (/orchestrator/k8s/proxy, enterprise-only). If they instead used the resource-browser endpoints (/orchestrator/k8s/resource etc.), a user_audit row does exist and they would have seen a stale date and the literal string 'localhost' in the IP column, not blanks — in that case my ApiTokenService change fixes the timestamp but the IP column will still read 'localhost'.
  • Whether the customer is on enterprise. The proxy handler and the whitelist entry exist only in devtron-enterprise; OSS devtron has the constant BaseForK8sProxy but no route, handler or whitelist entry, so OSS only carries the frozen-timestamp half.
  • Whether recording the audit at authentication time (rather than only after the RBAC check passes) is the semantics the team wants for the proxy. I matched the middleware, which records for every authenticated request regardless of the later authorisation outcome.
  • Request volume through the k8s proxy in production. Each proxied request now schedules one SELECT + UPDATE against user_audit. Every other Devtron API route already pays this cost synchronously in the middleware, but the proxy was previously exempt.
  • Whether any pre-migration-87 user_audit rows exist with NULL updated_on in customer databases (scripts/sql/87_alter_user_audit.up.sql adds the column with no backfill). I added a zero-time fallback to created_on rather than assume they do not.

Verification plan for QA

  1. On a build with this change, create an API token and call any authenticated orchestrator endpoint with it (e.g. GET /orchestrator/app/list with header token: <api-token>).
  2. Open Global Configurations -> API tokens and note the 'Last Used On' value.
  3. Wait a minute, repeat the same request, reload the listing, and confirm 'Last Used On' has advanced. Before this change it stayed pinned at the first value.
  4. In the orchestrator database, confirm the single row for that token's user: select id, user_id, client_ip, created_on, updated_on from user_audit where user_id = <token user id>; — created_on should be unchanged and updated_on should match what the UI now shows.
  5. Regression check: for a token that has never been used, the listing must still show 'Never used' (no user_audit row -> both fields stay unset).

⚠️ This change is marked security-sensitive (auth, RBAC, casbin, secrets, or tenancy). A wrong fix here is a security hole that passes a green build. Do not merge on the strength of CI.


🤖 Generated by the agentic pager-duty fix engine. Draft, agent-authored, do not merge without review.

… use against Kubernetes resources

Automated fix for pager issue devtron-labs/sprint-tasks#2971.
Root cause and verification plan are in the pull request body.

Refs: https://github.com/devtron-labs/sprint-tasks/issues/2971
@devtronpageragent devtronpageragent Bot added the agent-authored Opened by the pager-duty fix agent. Draft; never merge without review. label Sep 17, 2026
@devtronpageragent

Copy link
Copy Markdown
Author

Paired with https://github.com/devtron-labs/devtron-enterprise/pull/3410 . These repositories are a hard fork at the same module path, so this fix needs both diffs — merging one without the other leaves the other half broken.

@bito-code-review

Copy link
Copy Markdown

Bito Automatic Review Skipped - Draft PR

Bito didn't auto-review because this pull request is in draft status.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change draft PR review settings here, or contact your Bito workspace admin at shivam@devtron.ai.

@github-actions

Copy link
Copy Markdown

Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-authored Opened by the pager-duty fix agent. Draft; never merge without review. PR:Issue-verification-failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants