Skip to content

Improve signing/encryption related token configurations - #4423

Merged
thiva-k merged 1 commit into
thunder-id:mainfrom
thiva-k:fix-alg
Jul 30, 2026
Merged

Improve signing/encryption related token configurations#4423
thiva-k merged 1 commit into
thunder-id:mainfrom
thiva-k:fix-alg

Conversation

@thiva-k

@thiva-k thiva-k commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Purpose

Improve the signing and encryption related configurations for OAuth tokens across the Console, backend, and docs.

  • Signing is server-derived, not per-app configurable. Removed the misleading UserInfo signing-algorithm dropdown and the associated runtime trap; the Console now shows the deployment signing algorithm read-only (from OIDC discovery). Backend validation relaxed so JWS/NESTED_JWT UserInfo no longer require a per-app signingAlg.
  • Clearer encryption UX. Developer-friendly response-format labels + descriptions, and a certificate-required hint that points to the correct tab (Advanced Settings for applications, Credentials for agents).
  • Fixed the certificate deadlock (APP-1024). A client_secret/none client can now attach a certificate for token encryption; removing a certificate an encrypted format still depends on is blocked in the UI with a clear warning.
  • Docs. Reworded the OAuth/OIDC pages to state signing uses the deployment signing key, removed the non-existent id_token_signed_response_alg DCR field, and noted encryption requires an RSA client key.

Related Issues

@thiva-k thiva-k added Type/Improvement trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes labels Jul 30, 2026
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

OAuth/OIDC validation now permits certificates with none authentication and removes UserInfo signing-algorithm requirements. The console derives signing algorithms from discovery, guards certificate-dependent encrypted formats, clears stale fields, updates translations and tests, and aligns protocol documentation.

Changes

OAuth/OIDC configuration

Layer / File(s) Summary
Validation and error contract updates
backend/internal/inboundclient/..., backend/internal/{agent,application}/..., backend/internal/system/i18n/...
none authentication rejects client secrets but permits certificates; JWS and nested JWT UserInfo configurations no longer require signingAlg; mappings, messages, and tests were updated.
Certificate dependency safeguards
frontend/apps/console/src/features/agents/..., frontend/apps/console/src/features/applications/..., frontend/packages/i18n/...
Certificate removal is blocked while encrypted token formats depend on the certificate, with warning alerts and tests for blocked and allowed changes.
Token format and signing configuration
frontend/apps/console/src/features/applications/components/edit-application/token-settings/..., frontend/apps/console/src/features/applications/constants/..., frontend/packages/i18n/...
Signing algorithms are read from OIDC discovery, encrypted formats are disabled without certificates, UserInfo signing selection is removed, stale crypto fields are cleared, and related UI translations and tests are updated.
OAuth/OIDC documentation alignment
docs/content/guides/protocols/oauth-oidc/*.mdx
Guides describe deployment-key signing, certificate requirements, metadata-based algorithm selection, and updated registration parameters.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EditTokenSettings
  participant OIDCDiscovery
  participant TokenUserAttributesSection
  participant CertificateSection
  EditTokenSettings->>OIDCDiscovery: fetch signing algorithm metadata
  OIDCDiscovery-->>EditTokenSettings: return supported signing algorithm
  EditTokenSettings->>TokenUserAttributesSection: pass signing and certificate state
  TokenUserAttributesSection->>CertificateSection: enforce certificate dependency
Loading

Possibly related PRs

Suggested reviewers: donomalvindula, brionmario, rajithacharith, thamindudilshan

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description covers Purpose and Related Issues, but it omits the required Approach, Checklist, and Security checks sections. Add the missing template sections, especially Approach, Checklist, and Security checks, and fill in any relevant testing and documentation details.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes align with #3707 by removing per-app UserInfo signing config, clarifying response formats, and updating docs and certificate handling.
Out of Scope Changes check ✅ Passed The backend, UI, test, and docs changes all support the signing/encryption work; no unrelated additions stand out.
Title check ✅ Passed The title is concise and accurately summarizes the main change around signing/encryption token configuration.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
docs/content/guides/protocols/oauth-oidc/userinfo.mdx (1)

61-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Split the encryption setup into explicit steps.

This combines selecting algorithms with configuring a certificate, and the certificate location differs between applications and agents. Separate the actions and identify the relevant certificate section.

🤖 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 `@docs/content/guides/protocols/oauth-oidc/userinfo.mdx` at line 61, Update the
UserInfo protocol guide’s encrypted-response instructions to separate choosing
JWE encryption algorithms from configuring the certificate. Identify the
appropriate certificate configuration section, including the distinct
certificate locations for applications and agents.

Source: Coding guidelines

frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx (1)

69-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider extracting the shared CertificateSection instead of maintaining two copies.

frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx now carries a byte-identical implementation of this component, including the new encryptionDependsOnCert/blockedRemoval logic and the same applications:edit.advanced.certificate.* keys. The next behavior change has to be applied twice.

🤖 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
`@frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx`
around lines 69 - 141, Extract the shared CertificateSection implementation,
including the encryptionDependsOnCert and blockedRemoval logic, into a reusable
component. Replace both the application and agent CertificateSection copies with
imports of that shared component, preserving the existing props, behavior, and
applications:edit.advanced.certificate.* translation keys.
frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx (2)

246-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The "which formats are encrypted" rule is now spelled out in four places. Each site re-derives 'JWE' | 'NESTED_JWT' inline, so adding or renaming an encrypted format means finding every copy; the agent-side EditCredentialsSettings carries a fifth copy. isEncryptedFormat already exists here and is the natural thing to export from token-constants.ts.

  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx#L246-L246: move isEncryptedFormat into token-constants.ts and import it here.
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx#L522-L522: replace the inline value !== 'JWE' && value !== 'NESTED_JWT' check with !isEncryptedFormat(value).
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx#L550-L550: same replacement in handleUserInfoConfigChange.
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx#L146-L150: build encryptionDependsOnCert as isEncryptedFormat(idTokenResponseType) || isEncryptedFormat(userInfoResponseType).
🤖 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
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx`
at line 246, Centralize the encrypted-format rule by moving isEncryptedFormat
from TokenUserAttributesSection.tsx into token-constants.ts, exporting it, and
importing it where needed. In TokenUserAttributesSection.tsx,
EditTokenSettings.tsx at lines 522 and 550, and EditAdvancedSettings.tsx at
lines 146-150, replace each inline JWE/NESTED_JWT check with isEncryptedFormat;
build encryptionDependsOnCert using the helper for both response types.

214-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Untranslated tab names are interpolated into a translated sentence. certificateLocation is passed and defaulted as a raw English display string, then substituted into edit.token.encryption_requires_certificate via {{location}}. In any non-English locale the hint renders as translated prose with an English tab name inside it, and it will also drift from the actual tab labels (applications:edit.page.tabs.advanced, agents:edit.page.tabs.credentials). Passing a translation key (or a 'advanced' | 'credentials' discriminator resolved with t() at render time) keeps the whole sentence localized.

  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx#L214-L216: change certificateLocation to a key/discriminator and resolve it with t() before interpolating into the alert at Lines 570-574 and 769-773.
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx#L148-L148: replace the 'Advanced Settings' default with the corresponding key/discriminator.
  • frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx#L98-L98: replace certificateLocation="Credentials" with the corresponding key/discriminator.
🤖 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
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx`
around lines 214 - 216, Replace the raw certificateLocation display strings with
a translation key or advanced/credentials discriminator across
TokenUserAttributesSection.tsx (lines 214-216), EditTokenSettings.tsx (line
148), and EditTokensSettings.tsx (line 98). In TokenUserAttributesSection,
resolve the discriminator through t() before interpolating it into the alerts at
the existing encryption_requires_certificate usages, using the matching
applications:edit.page.tabs.advanced or agents:edit.page.tabs.credentials
labels.
🤖 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 `@docs/content/guides/protocols/oauth-oidc/token-formats.mdx`:
- Line 48: Update the sentence in the token formats documentation to hyphenate
the compound modifier as “RSA-based,” without changing the surrounding guidance
about the client certificate or selected algorithm.

In
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx`:
- Around line 339-364: Update the discovery-fetch useEffect around
fetchSigningAlg to use the existing cancelled-flag pattern: set the flag in the
cleanup function, check it before applying the response via setSigningAlg, and
prevent stale requests from updating state after unmount or getServerUrl
changes. Remove the unreachable outer fetchSigningAlg().catch handler while
preserving the existing internal error logging.

In
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx`:
- Around line 789-793: Update the two UserInfo placeholder translation calls in
TokenUserAttributesSection, including the renderValue lookup and the
corresponding lookup near the encryption settings, to provide explicit fallback
default strings via the established t() convention. Keep the existing
translation keys and rendered placeholder behavior unchanged.

---

Nitpick comments:
In `@docs/content/guides/protocols/oauth-oidc/userinfo.mdx`:
- Line 61: Update the UserInfo protocol guide’s encrypted-response instructions
to separate choosing JWE encryption algorithms from configuring the certificate.
Identify the appropriate certificate configuration section, including the
distinct certificate locations for applications and agents.

In
`@frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx`:
- Around line 69-141: Extract the shared CertificateSection implementation,
including the encryptionDependsOnCert and blockedRemoval logic, into a reusable
component. Replace both the application and agent CertificateSection copies with
imports of that shared component, preserving the existing props, behavior, and
applications:edit.advanced.certificate.* translation keys.

In
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx`:
- Line 246: Centralize the encrypted-format rule by moving isEncryptedFormat
from TokenUserAttributesSection.tsx into token-constants.ts, exporting it, and
importing it where needed. In TokenUserAttributesSection.tsx,
EditTokenSettings.tsx at lines 522 and 550, and EditAdvancedSettings.tsx at
lines 146-150, replace each inline JWE/NESTED_JWT check with isEncryptedFormat;
build encryptionDependsOnCert using the helper for both response types.
- Around line 214-216: Replace the raw certificateLocation display strings with
a translation key or advanced/credentials discriminator across
TokenUserAttributesSection.tsx (lines 214-216), EditTokenSettings.tsx (line
148), and EditTokensSettings.tsx (line 98). In TokenUserAttributesSection,
resolve the discriminator through t() before interpolating it into the alerts at
the existing encryption_requires_certificate usages, using the matching
applications:edit.page.tabs.advanced or agents:edit.page.tabs.credentials
labels.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 38dbcaaa-8259-4716-87fe-9e9d4d5c4a15

📥 Commits

Reviewing files that changed from the base of the PR and between fa3fe8c and bd86dec.

📒 Files selected for processing (24)
  • backend/internal/agent/service.go
  • backend/internal/agent/service_test.go
  • backend/internal/application/service.go
  • backend/internal/application/service_test.go
  • backend/internal/inboundclient/error_constants.go
  • backend/internal/inboundclient/service.go
  • backend/internal/inboundclient/service_test.go
  • backend/internal/system/i18n/core/defaults.go
  • docs/content/guides/protocols/oauth-oidc/dynamic-client-registration.mdx
  • docs/content/guides/protocols/oauth-oidc/openid-connect.mdx
  • docs/content/guides/protocols/oauth-oidc/token-formats.mdx
  • docs/content/guides/protocols/oauth-oidc/userinfo.mdx
  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx
  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/EditCredentialsSettings.tsx
  • frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/__tests__/CertificateSection.test.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/__tests__/EditTokenSettings.test.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/__tests__/TokenUserAttributesSection.test.tsx
  • frontend/apps/console/src/features/applications/constants/token-constants.ts
  • frontend/packages/i18n/src/locales/en-US.ts
💤 Files with no reviewable changes (1)
  • frontend/apps/console/src/features/applications/constants/token-constants.ts

Comment thread docs/content/guides/protocols/oauth-oidc/token-formats.mdx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (1)
frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx (1)

46-115: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Stale "blocked removal" warning in both CertificateSection implementations. Both components gate the warning Alert on local blockedRemoval state that is only cleared inside this component's own onChange handler, not when the underlying encryptionDependsOnCert prop later becomes false (e.g. after the user changes the ID token/UserInfo format elsewhere, as the warning itself instructs). The warning then keeps showing a no-longer-applicable message until the user re-opens this certificate dropdown.

  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx#L46-L115: change the Alert condition to {blockedRemoval && encryptionDependsOnCert && (...)} so it clears automatically once the format dependency resolves.
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx#L78-L142: apply the same {blockedRemoval && encryptionDependsOnCert && (...)} gate.
🤖 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
`@frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx`
around lines 46 - 115, The blocked-removal warning remains visible after its
encryption dependency is resolved. In both CertificateSection
implementations—frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx:46-115
and
frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx:78-142—update
the warning Alert condition to require both blockedRemoval and
encryptionDependsOnCert, preserving the existing alert content and behavior.
🤖 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
`@frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx`:
- Line 46: Gate the blocked-removal Alert in CertificateSection on both
blockedRemoval and the live encryptionDependsOnCert prop, so it disappears
immediately when the underlying dependency resolves. Update the related
warning/rendering logic around the certificate type change handler without
relying solely on the local setBlockedRemoval reset; apply the same pattern in
the corresponding advanced-settings CertificateSection.

In
`@frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx`:
- Line 78: Update CertificateSection’s blockedRemoval state handling so it is
cleared whenever encryptionDependsOnCert becomes false, including changes
originating from ID token or UserInfo response types in
EditAdvancedSettings.tsx. Add the reset alongside the existing dropdown logic
and preserve the current warning behavior while the dependency remains true.

In
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx`:
- Around line 801-803: Update the hasCertificate prop passed by
EditTokenSettings to require both oauth2Config.certificate.type and a non-empty
certificate value. Ensure JWE/NESTED_JWT formats cannot be enabled with a blank
certificate, and JWKS_URI still reaches the SSRF-safe URL validation instead of
bypassing it.

---

Outside diff comments:
In
`@frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx`:
- Around line 46-115: The blocked-removal warning remains visible after its
encryption dependency is resolved. In both CertificateSection
implementations—frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx:46-115
and
frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx:78-142—update
the warning Alert condition to require both blockedRemoval and
encryptionDependsOnCert, preserving the existing alert content and behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 603422fb-dc59-4d33-bde8-279b96fbdc16

📥 Commits

Reviewing files that changed from the base of the PR and between bd86dec and b976de8.

📒 Files selected for processing (24)
  • backend/internal/agent/service.go
  • backend/internal/agent/service_test.go
  • backend/internal/application/service.go
  • backend/internal/application/service_test.go
  • backend/internal/inboundclient/error_constants.go
  • backend/internal/inboundclient/service.go
  • backend/internal/inboundclient/service_test.go
  • backend/internal/system/i18n/core/defaults.go
  • docs/content/guides/protocols/oauth-oidc/dynamic-client-registration.mdx
  • docs/content/guides/protocols/oauth-oidc/openid-connect.mdx
  • docs/content/guides/protocols/oauth-oidc/token-formats.mdx
  • docs/content/guides/protocols/oauth-oidc/userinfo.mdx
  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx
  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/EditCredentialsSettings.tsx
  • frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/__tests__/CertificateSection.test.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/__tests__/EditTokenSettings.test.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/__tests__/TokenUserAttributesSection.test.tsx
  • frontend/apps/console/src/features/applications/constants/token-constants.ts
  • frontend/packages/i18n/src/locales/en-US.ts
💤 Files with no reviewable changes (1)
  • frontend/apps/console/src/features/applications/constants/token-constants.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • docs/content/guides/protocols/oauth-oidc/openid-connect.mdx
  • docs/content/guides/protocols/oauth-oidc/dynamic-client-registration.mdx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx
  • docs/content/guides/protocols/oauth-oidc/userinfo.mdx
  • backend/internal/system/i18n/core/defaults.go
  • docs/content/guides/protocols/oauth-oidc/token-formats.mdx
  • frontend/packages/i18n/src/locales/en-US.ts

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.47826% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...tion/token-settings/TokenUserAttributesSection.tsx 88.23% 2 Missing ⚠️
...ents/edit-agent/credentials/CertificateSection.tsx 83.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx (1)

142-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a shared isEncryptedFormat helper.

The 'JWE' | 'NESTED_JWT' check is now duplicated here, in EditCredentialsSettings.tsx, in EditTokenSettings.tsx (both change handlers), and in TokenUserAttributesSection.tsx. Exporting one predicate from constants/token-constants.ts keeps these in sync when a new encrypted format is added.

🤖 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
`@frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx`
around lines 142 - 151, Extract and export a shared isEncryptedFormat predicate
from constants/token-constants.ts that recognizes JWE and NESTED_JWT response
types, then replace the duplicated format checks in EditAdvancedSettings,
EditCredentialsSettings, both EditTokenSettings change handlers, and
TokenUserAttributesSection with this helper.
frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx (1)

98-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Interpolated tab name won't localize.

certificateLocation is a hardcoded English string injected into the translated encryption_requires_certificate sentence, so non-English locales will render a mixed-language hint. Passing the existing tab label key (agents:edit.page.tabs.credentials) resolved via t() would keep it localized.

🤖 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
`@frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx`
at line 98, Update the certificateLocation value in EditTokensSettings to use
the existing credentials tab translation via t() with the
agents:edit.page.tabs.credentials key, instead of the hardcoded "Credentials"
string, while preserving the surrounding encryption_requires_certificate
translation flow.
frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx (1)

560-568: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align this condition with the UserInfo allow-list.

The ID token branch excludes JWE (fail-open: any future format shows "Signed with"), while the UserInfo branch at Lines 769-770 uses an explicit JWS | NESTED_JWT allow-list. Matching the allow-list style here keeps both consistent and correct if a new format is introduced.

♻️ Proposed change
-                            {signingAlg && (idTokenResponseType ?? 'JWT') !== 'JWE' && (
+                            {signingAlg &&
+                              ((idTokenResponseType ?? 'JWT') === 'JWT' || idTokenResponseType === 'NESTED_JWT') && (
🤖 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
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx`
around lines 560 - 568, Update the signing-algorithm display condition in the ID
token section of TokenUserAttributesSection so it uses the same explicit
JWS/NESTED_JWT allow-list as the UserInfo branch, rather than excluding only
JWE. Preserve the existing signingAlg requirement and display behavior for those
supported formats.
🤖 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.

Nitpick comments:
In
`@frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx`:
- Line 98: Update the certificateLocation value in EditTokensSettings to use the
existing credentials tab translation via t() with the
agents:edit.page.tabs.credentials key, instead of the hardcoded "Credentials"
string, while preserving the surrounding encryption_requires_certificate
translation flow.

In
`@frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx`:
- Around line 142-151: Extract and export a shared isEncryptedFormat predicate
from constants/token-constants.ts that recognizes JWE and NESTED_JWT response
types, then replace the duplicated format checks in EditAdvancedSettings,
EditCredentialsSettings, both EditTokenSettings change handlers, and
TokenUserAttributesSection with this helper.

In
`@frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx`:
- Around line 560-568: Update the signing-algorithm display condition in the ID
token section of TokenUserAttributesSection so it uses the same explicit
JWS/NESTED_JWT allow-list as the UserInfo branch, rather than excluding only
JWE. Preserve the existing signingAlg requirement and display behavior for those
supported formats.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a5230190-5dee-4477-9c30-8780b0cbb631

📥 Commits

Reviewing files that changed from the base of the PR and between b976de8 and ab944bc.

📒 Files selected for processing (24)
  • backend/internal/agent/service.go
  • backend/internal/agent/service_test.go
  • backend/internal/application/service.go
  • backend/internal/application/service_test.go
  • backend/internal/inboundclient/error_constants.go
  • backend/internal/inboundclient/service.go
  • backend/internal/inboundclient/service_test.go
  • backend/internal/system/i18n/core/defaults.go
  • docs/content/guides/protocols/oauth-oidc/dynamic-client-registration.mdx
  • docs/content/guides/protocols/oauth-oidc/openid-connect.mdx
  • docs/content/guides/protocols/oauth-oidc/token-formats.mdx
  • docs/content/guides/protocols/oauth-oidc/userinfo.mdx
  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/CertificateSection.tsx
  • frontend/apps/console/src/features/agents/components/edit-agent/credentials/EditCredentialsSettings.tsx
  • frontend/apps/console/src/features/agents/components/edit-agent/tokens/EditTokensSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/CertificateSection.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/__tests__/CertificateSection.test.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/TokenUserAttributesSection.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/__tests__/EditTokenSettings.test.tsx
  • frontend/apps/console/src/features/applications/components/edit-application/token-settings/__tests__/TokenUserAttributesSection.test.tsx
  • frontend/apps/console/src/features/applications/constants/token-constants.ts
  • frontend/packages/i18n/src/locales/en-US.ts
💤 Files with no reviewable changes (1)
  • frontend/apps/console/src/features/applications/constants/token-constants.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • docs/content/guides/protocols/oauth-oidc/openid-connect.mdx
  • docs/content/guides/protocols/oauth-oidc/dynamic-client-registration.mdx
  • docs/content/guides/protocols/oauth-oidc/token-formats.mdx
  • frontend/apps/console/src/features/applications/components/edit-application/advanced-settings/tests/CertificateSection.test.tsx
  • docs/content/guides/protocols/oauth-oidc/userinfo.mdx

@thiva-k
thiva-k added this pull request to the merge queue Jul 30, 2026
Merged via the queue into thunder-id:main with commit f702d4f Jul 30, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes Type/Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve the signing and encryption related configurations in the UI

2 participants