feat: RBAC support for external argo/flux apps - #7014
Merged
Merged
Conversation
- add migration files - append flags in the /orchestrator/user/check/roles API to show/hide argo/flux tabs
- earlier filter was only applied with the cluster ID - add changes to filter based on cluster ID's and namespaces
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Bito Automatic Review Skipped - Draft PR |
github-actions
Bot
added
PR:Ready-to-Review
and removed
PR:Issue-verification-failed
labels
vivekanandan-devtron
marked this pull request as ready for review
August 31, 2026 05:51
vivekanandan-devtron
requested review from
prakarsh-dt and
vivek-devtron
as code owners
August 31, 2026 05:51
Shivam-nagar23
requested changes
Sep 1, 2026
…argo-flux-rbac-7007
Member
|
@claude review this |
|
Shivam-nagar23
approved these changes
Sep 16, 2026
vivek-devtron
approved these changes
Sep 16, 2026
37 tasks
vivekanandan-devtron
added a commit
that referenced
this pull request
Sep 16, 2026
* fix(security): enforce caller RBAC in API token webhook endpoint GetAllApiTokensForWebhook checked whether each STORED api token had trigger permission on the requested project/env, but never checked whether the CALLING user was permitted to query that project/env at all. As a result any authenticated user could request an arbitrary projectName/environmentName/appName and receive the plaintext token values of every stored api token (including super-admin tokens, whose Casbin policy is "*" and therefore always passes the per-token check). Add the same enforcement pattern used by the sibling handlers in this file, but scoped to the resource being queried: verify the requesting user's own token has trigger permission on the requested project and every requested environment before the service call is made, so no token data is looked up or returned for a project/env the caller cannot access. Fixes #7013 * chore: address SonarCloud finding in GetAllApiTokensForWebhook Inline the unnecessary "ok" variable in the caller-RBAC check added in the previous commit, per SonarCloud's "remove this unnecessary variable declaration and use the expression directly" finding on PR #7016. * fix(security): don't leak more-privileged tokens to less-privileged callers Checking that a caller has trigger permission on the requested project/env was not sufficient: a super-admin api-token's Casbin policy is "*", so it always satisfies that same per-object check regardless of how narrow the actual project/env being queried is. That meant any caller with legitimate, narrow trigger access to some app/env could still have super-admin (or other more-privileged) tokens returned alongside their own in the same response. Add callerDominatesToken, which excludes a stored token from the response whenever it is a super-admin token and the caller is not, reusing the same super-admin check already used by the other handlers in this file. The per-object check now runs against each stored token before this dominance check, matching the existing per-token-per-object evaluation the service already performs. The Enforcer interface here only exposes single-tuple Enforce/ EnforceInBatch calls, not policy or role enumeration, so a fully generic "is token's permission a strict subset of the caller's" comparison across every possible object isn't available without extending that interface. Super-admin dominance is the well-defined, checkable case that matches this issue (leaking admin token values), so it's what this fix targets; a more general scope comparison would need broader RBAC changes outside this endpoint's blast radius. * chore: address SonarCloud param-grouping findings in ApiTokenRestHandler Group consecutive same-type parameters (storedToken/projObj/envObj and callerToken/storedToken) per SonarCloud's finding on the code added in the previous two commits. * fix(security): simplify to a straightforward super-admin gate Per thread decision: GetAllApiTokensForWebhook's only real caller is the dashboard's webhook config modal, which is already super-admin-only in the UI. Rather than layering a per-project/env caller check plus a token-dominance check on top of the existing per-stored-token filter, just gate the whole endpoint on the caller being a super-admin - the same Enforce(token, ResourceGlobal, ActionUpdate, "*") check already used by every other handler in this file (GetAllApiTokens, CreateApiToken, UpdateApiToken, DeleteApiToken). This removes the per-object caller check and the callerDominatesToken logic added in the previous two commits; the existing per-stored-token CheckAuthorizationForWebhook callback into the service is left as-is since it still does useful filtering (only tokens with trigger permission on the requested project/env are returned), it's just no longer load-bearing for access control now that the endpoint itself requires a super-admin caller. Net effect vs main: an 8-line super-admin check added right after GetLoggedInUser, matching the pattern already used by every sibling handler in this file. * chore: address SonarCloud finding in super-admin gate check Inline the unnecessary "ok" variable in the super-admin check added to GetAllApiTokensForWebhook, per SonarCloud's "remove this unnecessary variable declaration" finding on PR #7016. * feat: decouple app lifecycle actions; add indexes to improve CI query performance - Introduce `createApp`/`deleteApp` actions to segregate application lifecycle control from pipeline/workflow/config operations. - Update RBAC enforcement to reflect the new `createApp`/`deleteApp` actions for better granularity. - Add `idx_ci_artifact`, `idx_ci_workflow`, and other indexes to improve CI query execution and resolve PG_READ_TIMEOUT issues. - Introduce GIN index for `git_triggers` to optimize material/commit lookup in `validateBuildSequence`. * fix(user): scope filter-based bulk delete to match listing filters DELETE /orchestrator/user/bulk with a listingRequest filter (instead of explicit ids) resolved the delete target list via the same query builder the listing endpoint uses, but skipped the normalization the listing endpoint applies first (SetDefaultValuesIfNotPresent, setStatusFilterType). With no default page size and no status filtering applied, a filter such as {"searchKey":"","status":["inactive"]} resolved to every active user instead of the intended filtered subset, causing a bulk delete of far more users than intended. getUserIdsHonoringFilters now applies the same normalization the listing path (GetAllWithFilters) applies before building the query, and rejects a filter-based bulk delete that carries no concrete filter criteria instead of letting it silently resolve to "every user". The explicit ids-based delete path is unchanged. Fixes #7020 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5DesTG5XvPdmPCDuL6izq * feat: resolve RBAC inconsistencies for helm apps with unique identifiers When an external helm app is linked to the chart store, app_name holds the unique identifier (<releaseName>-<namespace>-<clusterId>) while display_name holds the release name. RBAC policies are always created against the release name, since that is what the listing APIs expose to the user. EnforcerUtil did not account for this: - GetHelmObjectByAppNameAndEnvId looked the app up by app_name only, so a release-name lookup returned ErrNoRows and the object collapsed to "//", making the app invisible to any non super-admin user. - Both it and GetHelmObject built the object from application.AppName, i.e. the unique identifier, which never matched the granted policy. - Neither substituted the "unassigned" project when team_id is 0, so an app linked without a project produced an empty project segment. EnforcerUtilHelm.GetAppRBACNameByInstalledAppId and GetAppRBACNameByInstalledAppIdAndTeamId had the same raw AppName problem. Resolve the app the same way getAppObject already does: reconstruct the unique identifier from the environment and try that first, falling back to the plain name for regular chart store apps, legacy rows and callers that already pass the identifier. Emit the display name via the new App.GetAppNameForRbac(), and fall back to the unassigned project when no project is assigned. Fixing the primitives covers all downstream call sites without touching the handlers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Conditions saved with the plugin definition (plugin_step_condition) are intentionally not sent for the plugin's internal steps. * feat: RBAC support for external argo/flux apps (#7014) * chore: create a makefile to setup and clean resources for development * feat: add flag for frontend to show argo/flux tabs - add migration files - append flags in the /orchestrator/user/check/roles API to show/hide argo/flux tabs * feat: changes for dropdown API in user permissions page * fix: missing dependencies in constructor method * fix: improper filtering logic - earlier filter was only applied with the cluster ID - add changes to filter based on cluster ID's and namespaces * fix: prevent connecting to kubelink for failed clusters * wip: policy enforcements * wip: policy enforcements * chore: remove test folder * fix: code review fixes * fix: code review fixes * fix: code review fixes * chore: rename migration scripts to maintain sync with enterprise * chore: change migration script number * fix: roles missing while fetching user roles * fix: deleted roles being listed * fix: deleted roles being listed * fix: review fixes * chore: rename migration files to correct sequence --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Shivam Nagar <124123645+Shivam-nagar23@users.noreply.github.com> Co-authored-by: Shivam-nagar23 <shivam@devtron.ai> Co-authored-by: Prakash Kumar <prakash.kumar@devtron.ai>
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
3 participants
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.
Description
Fixes #7007
Checklist:
Does this PR introduce a user-facing change?