feat(secretserver): support authentication via access token

[sumanpal97]

Problem Statement

The Delinea Secret Server provider only supported username/password
authentication. Delinea Secret Server also issues access tokens, and many
users prefer (or are required) to authenticate with a token rather than
storing account credentials. There was no way to configure that in the
SecretStore/ClusterSecretStore.

Related Issue

Fixes #3977

Proposed Changes

• Add an optional token field to the Secret Server provider spec
  (both v1 and v1beta1 APIs). It accepts either an inline value
  or a secretRef pointing at a Kubernetes Secret.
• When token is set, username and password are no longer required
  and are ignored; the token is passed to the Delinea SDK's
  UserCredential.Token, which it already supports natively.
• username and password are relaxed to optional; validation now
  requires either a token or a username/password pair, and rejects an
  empty/ambiguous token reference.
• Namespace-dependency detection (doesConfigDependOnNamespace) is made
  nil-safe and token-aware.
• Added unit tests covering token-via-value, token-via-secretRef,
  dangling token refs, and the updated validation paths.
• Updated the Secret Server provider docs with an "Authenticating with an
  Access Token" section, and regenerated CRDs, deepcopy, snapshots, and
  the deploy bundle.

Checklist

• I have read the contribution guidelines
• All commits are signed with git commit --signoff
• My changes have reasonable test coverage
• All tests pass with make test
• I ensured my PR is ready for review with make reviewable

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds token-based authentication to the Secret Server provider as an alternative to username and password credentials. The provider now resolves token values or secret references, applies token precedence, validates token references, and handles namespace dependencies. API types, generated deepcopy code, CRD schemas, bundled CRDs, documentation, provider tests, and configuration snapshots are updated accordingly.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements Secret Server API-token authentication, satisfying issue #3977's request for token-based login.
Out of Scope Changes check	✅ Passed	The changes are focused on token auth support and its required docs, tests, CRDs, and generated code, with no clear unrelated additions.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

providers/v1/secretserver/provider.go (1)

177-188: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Skip username/password in doesConfigDependOnNamespace when token auth is set.

getConfig treats Token as exclusive, but this helper still flags namespace-less Username/Password refs and can reject a ClusterSecretStore even when those fields are ignored. Short-circuit on Token the same way as getConfig.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@providers/v1/secretserver/provider.go` around lines 177 - 188, The
doesConfigDependOnNamespace helper is still validating Username and Password
secret refs even when Token auth is configured, which can incorrectly reject a
ClusterSecretStore. Update doesConfigDependOnNamespace in provider.go to
short-circuit on cfg.Token the same way getConfig does, so namespace checks for
Username and Password are skipped whenever Token is set and treated as
exclusive.

🧹 Nitpick comments (1)

deploy/crds/bundle.yaml (1)

7301-7340: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift

Consider CEL validation for token vs username/password mutual requirement.

The regenerated schema removes password/username from required (now only serverURL is required) but doesn't add an x-kubernetes-validations rule enforcing "token or (username and password)" — unlike sibling providers in this same file (e.g. doppler.auth, openBao.auth, pulumi.auth) that use CEL rules for equivalent either/or credential constraints. Without such a rule, a SecretStore with none of token/username/password set will pass API-server admission and only fail later in the provider controller.

This file is auto-generated by controller-gen; any validation should be added via a kubebuilder +kubebuilder:validation:XValidation marker on the SecretServerProvider Go type (not in this file) and regenerated with make generate/make manifests, rather than hand-edited here.

Also applies to: 11822-11861, 20116-20155, 24637-24676

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/crds/bundle.yaml` around lines 7301 - 7340, Add a CEL validation on
SecretServerProvider to enforce the credential invariant “token or (username and
password)” since the regenerated schema only marks serverURL as required. Mirror
the sibling auth schemas in this bundle by adding a kubebuilder XValidation
marker on the SecretServerProvider Go type, then regenerate the CRD via the
normal controller-gen workflow instead of editing bundle.yaml by hand. Ensure
the rule covers the token, username, and password fields together so admission
rejects empty credentials before the controller sees the SecretStore.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@providers/v1/secretserver/provider.go`:
- Around line 177-188: The doesConfigDependOnNamespace helper is still
validating Username and Password secret refs even when Token auth is configured,
which can incorrectly reject a ClusterSecretStore. Update
doesConfigDependOnNamespace in provider.go to short-circuit on cfg.Token the
same way getConfig does, so namespace checks for Username and Password are
skipped whenever Token is set and treated as exclusive.

---

Nitpick comments:
In `@deploy/crds/bundle.yaml`:
- Around line 7301-7340: Add a CEL validation on SecretServerProvider to enforce
the credential invariant “token or (username and password)” since the
regenerated schema only marks serverURL as required. Mirror the sibling auth
schemas in this bundle by adding a kubebuilder XValidation marker on the
SecretServerProvider Go type, then regenerate the CRD via the normal
controller-gen workflow instead of editing bundle.yaml by hand. Ensure the rule
covers the token, username, and password fields together so admission rejects
empty credentials before the controller sees the SecretStore.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07669c49-69b0-4c39-9320-2f4390f764da

📥 Commits

Reviewing files that changed from the base of the PR and between 4468655 and 316420f.

📒 Files selected for processing (14)

• apis/externalsecrets/v1/secretsstore_secretserver_types.go
• apis/externalsecrets/v1/zz_generated.deepcopy.go
• apis/externalsecrets/v1beta1/secretsstore_secretserver_types.go
• apis/externalsecrets/v1beta1/zz_generated.deepcopy.go
• config/crds/bases/external-secrets.io_clustersecretstores.yaml
• config/crds/bases/external-secrets.io_secretstores.yaml
• deploy/crds/bundle.yaml
• docs/provider/secretserver.md
• providers/v1/secretserver/provider.go
• providers/v1/secretserver/provider_test.go
• tests/__snapshot__/clustersecretstore-v1.yaml
• tests/__snapshot__/clustersecretstore-v1beta1.yaml
• tests/__snapshot__/secretstore-v1.yaml
• tests/__snapshot__/secretstore-v1beta1.yaml

[alekc]

Two requested changes below; the docs and the token happy-path/validation tests are otherwise solid. The main one is dropping the v1beta1 additions (that version is unserved and deprecated); the other is a token-awareness fix in doesConfigDependOnNamespace.

[alekc]

Please drop the v1beta1 changes. SecretStore/ClusterSecretStore v1beta1 are +kubebuilder:unservedversion and +kubebuilder:deprecatedversion (the generated CRD has served: false, storage: false), so this field is unreachable by any client and is never the stored version. New provider fields go to v1 only (e.g. the OpenBao Kubernetes auth in #6591 touched v1 alone). Reverting the v1beta1 type, its deepcopy, the v1beta1 CRD sections, and the *-v1beta1 snapshots keeps this to v1.

[evrardj-roche]

100%

[sumanpal97]

Done. Reverted all v1beta1 changes, secretsstore_secretserver_types.go, zz_generated.deepcopy.go. The token field exists only in v1.

[alekc]

doesConfigDependOnNamespace checks username/password even under token auth, but getConfig ignores them once token is set. So a ClusterSecretStore with a valid token.secretRef (namespace set) plus a leftover username.secretRef without a namespace is rejected with errClusterStoreRequiresNamespace, even though the username is ignored. Mirror getConfig's precedence: when cfg.Token != nil, only check cfg.Token here. A test for that case would be worth adding too.

[sumanpal97]

Fixed. doesConfigDependOnNamespace now short-circuits when Token is set , it only inspects the token ref, so a ClusterSecretStore with a valid token.secretRef plus a namespace-less username.secretRef is no longer wrongly rejected. Added unit tests covering both directions (token ref with/without namespace, and precedence over username/password).

[alekc]

also, just to confirm, did you test your changes live against secretserver?

[evrardj-roche]

100%

[evrardj-roche]

please use our esutils here, we need to check whether some can do that.

Else you can just exfiltrate secrets :)

[sumanpal97]

Good catch, thanks. loadConfigSecret now runs every secretRef (token, username, password) through validateStoreSecretRef, which calls esutils.ValidateReferentSecretSelector before the value is resolved. So, a ClusterSecretStore can't be pointed at secrets in arbitrary namespaces. It's enforced in the credential-loading path now, not just at config validation.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deploy/crds/bundle.yaml`:
- Line 11783: Make the v1 Kubebuilder markers for username and password optional
in the relevant Go API definitions, update their stale descriptions, then
regenerate all affected CRDs under deploy/crds/** with controller-gen; do not
edit the generated bundle.yaml manually.

🪄 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: cdfd962e-f96e-4d5e-85b9-fba1316b62ab

📥 Commits

Reviewing files that changed from the base of the PR and between 316420f and ba3706e.

📒 Files selected for processing (5)

• config/crds/bases/external-secrets.io_clustersecretstores.yaml
• config/crds/bases/external-secrets.io_secretstores.yaml
• deploy/crds/bundle.yaml
• providers/v1/secretserver/provider.go
• providers/v1/secretserver/provider_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• providers/v1/secretserver/provider_test.go
• providers/v1/secretserver/provider.go

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deploy/crds/bundle.yaml`:
- Line 11783: Make the v1 Kubebuilder markers for username and password optional
in the relevant Go API definitions, update their stale descriptions, then
regenerate all affected CRDs under deploy/crds/** with controller-gen; do not
edit the generated bundle.yaml manually.

🪄 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: cdfd962e-f96e-4d5e-85b9-fba1316b62ab

📥 Commits

Reviewing files that changed from the base of the PR and between 316420f and ba3706e.

📒 Files selected for processing (5)

• config/crds/bases/external-secrets.io_clustersecretstores.yaml
• config/crds/bases/external-secrets.io_secretstores.yaml
• deploy/crds/bundle.yaml
• providers/v1/secretserver/provider.go
• providers/v1/secretserver/provider_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• providers/v1/secretserver/provider_test.go
• providers/v1/secretserver/provider.go

🛑 Comments failed to post (1)

deploy/crds/bundle.yaml (1)

11783-11783: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Regenerate the bundle with token-compatible authentication requirements.

Both schemas still require username and password, so Kubernetes admission rejects token-only stores before provider validation runs. They also retain stale descriptions. Ensure the v1 Kubebuilder markers make these fields optional, then regenerate the CRDs rather than editing this generated bundle manually.

Based on learnings, CRD files under deploy/crds/** must be regenerated from the Go API definitions using controller-gen.

Also applies to: 11821-11821, 11853-11856, 24560-24560, 24598-24598, 24630-24633

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/crds/bundle.yaml` at line 11783, Make the v1 Kubebuilder markers for
username and password optional in the relevant Go API definitions, update their
stale descriptions, then regenerate all affected CRDs under deploy/crds/** with
controller-gen; do not edit the generated bundle.yaml manually.

Source: Learnings

[sumanpal97]

also, just to confirm, did you test your changes live against secretserver?

Not live , I don't have a Delinea tenant. I relied on the SDK's native token support, plus unit tests for the token path, refs, namespace dependency, and the cross-namespace referent guard.

[evrardj-roche]

@sumanpal97 are you using secretserver? If not, why did you submit this? Just curious.

[sumanpal97]

@sumanpal97 are you using secretserver? If not, why did you submit this? Just curious.

Hi @evrardj-roche , No I don't use Delinea myself — I picked up #3977 because it was an open 'good first issue' requesting token auth, and the SDK already supports it natively ('UserCredential.Token'). I can't test against a live tenant, so I covered it with unit tests.

[evrardj-roche]

@sumanpal97 you are helping a lot here, thank you! This already helps me understand a bit more about context.

Can you tell me more about that 'good first issue' ? We did not classify as 'good first issue', so I am wondering where does it come from.

I might look inquisitive, but I am eager to know the thinking behind it. And by answering this, you will help cover patterns that might be really helpful for the community.

For example, if a website is listing those issues as good first issue, i would rather know the criteria they use, and talk with them, so this way we don't arrive to a flurry of untested in real life (but tested in unit tests) contributions. The problem is unit tests only cover so much, and real life regressions are a pain. And those are generally not covered by unit tests, else we would not have this conversation :)

[alekc]

uhm

XD

[evrardj-roche]

@alekc lol.

@sumanpal97 sorry for that question, you can ignore it ! ><

I will do one more round of review, I think I saw something worth changing to avoid issues in the future, but it starts to take shape! Sorry for the back and forth

let's do one more round with the updates!

[sumanpal97]

@evrardj-roche No problem at all, and thanks for the follow-up. I appreciate the thorough review, happy to address whatever comes up in the next round.

[evrardj-roche]

@sumanpal97 thanks for your understanding and your help! I ahave this in my backlog of reviews.

[evrardj-roche]

I don't have much to say anymore, that's good!

However, I would prefer avoiding any accidental footguns, and have proper validation from kubebuilder.

Can you adapt the SecretServerProvider kubebuilder annotations to be more strict and match the current comments, please?

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/crds/bases/external-secrets.io_secretstores.yaml`:
- Around line 5462-5465: Remove the provider-specific XValidation marker from
the SecretServerProvider struct in secretsstore_secretserver_types.go, leaving
runtime credential checks in provider.go unchanged. Regenerate the CRDs with
controller-gen so the generated external-secrets.io_secretstores.yaml no longer
contains the token/username/password CEL rule; do not edit the generated YAML
manually.

🪄 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: 252a092b-1492-4f00-a144-9792d659e1c2

📥 Commits

Reviewing files that changed from the base of the PR and between b0b8bb8 and 4f711df.

📒 Files selected for processing (4)

• apis/externalsecrets/v1/secretsstore_secretserver_types.go
• config/crds/bases/external-secrets.io_clustersecretstores.yaml
• config/crds/bases/external-secrets.io_secretstores.yaml
• deploy/crds/bundle.yaml

🚧 Files skipped from review as they are similar to previous changes (3)

• apis/externalsecrets/v1/secretsstore_secretserver_types.go
• deploy/crds/bundle.yaml
• config/crds/bases/external-secrets.io_clustersecretstores.yaml

[evrardj-roche]

I think it's the last set, @sumanpal97

Code has changed since then

[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

[sumanpal97]

@evrardj-roche Can you please help with the approval again.
check-diff was failing because docs/api/spec.md wasn't regenerated for the new token field. Regenerated and pushed — generated-doc update only, no logic changes. Should be green now.

[evrardj-roche]

let's do it

[sumanpal97]

Thanks @evrardj-roche and @alekc for the welcoming and thorough reviews , a wonderful first open-source experience. Looking forward to contributing more! 🙌