Skip to content

Conversation

@asithade
Copy link
Contributor

Added default claims policies for id_token in values.yaml.

Added default claims policies for id_token in values.yaml.

Signed-off-by: Asitha de Silva <[email protected]>
@asithade asithade requested a review from emsearcy as a code owner October 24, 2025 11:22
Copilot AI review requested due to automatic review settings October 24, 2025 11:22
@asithade asithade requested a review from a team as a code owner October 24, 2025 11:22
@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

Walkthrough

Adds a default Authelia OIDC claims policy mapping four ID token claims and references that policy from the lfx OIDC client in the Helm values file.

Changes

Cohort / File(s) Summary
Authelia OIDC values
charts/lfx-platform/values.yaml
Adds claims_policies.default.id_token under authelia.identity_providers.oidc.jwks with claims: email, email_verified, name, preferred_username; adds claims_policy: default to the lfx client definition.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Review focus: charts/lfx-platform/values.yaml — ensure YAML structure and indentation for claims_policies and the claims_policy client reference.

Possibly related PRs

  • add lfx client for authelia #39: Introduces the initial lfx client configuration in the same values file, which this PR extends with a claims policy reference.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'Add default claims policies for id_token' directly and clearly summarizes the main change in the changeset. The title accurately reflects that default claims policies for id_token are being added to the configuration file, which is the primary purpose of this PR.
Description check ✅ Passed The pull request description 'Added default claims policies for id_token in values.yaml' is directly related to the changeset. It provides a brief but accurate description of what was modified (the values.yaml file) and what was added (default claims policies for id_token), which aligns with the actual changes shown in the raw summary.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/auhelia-id-claims

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 69dc354 and 3e8987d.

📒 Files selected for processing (1)
  • charts/lfx-platform/values.yaml (2 hunks)
🔇 Additional comments (2)
charts/lfx-platform/values.yaml (2)

411-417: Claims policy definition looks good.

The four standard OIDC claims (email, email_verified, name, preferred_username) are appropriate for an id_token. The YAML structure is correct, with claims_policies properly positioned as a sibling to jwks under the oidc section.


459-479: Policy reference applied to lfx client; consider other clients.

The claims_policy: default reference is correctly applied to the lfx client, which requests openid and profile scopes. However, the docs client (line 480–499) explicitly includes id_token in its response_types, and token_helper (lines 443–458) also requests openid scope. Both could potentially benefit from referencing the same claims policy.

Is this intentional (i.e., only lfx should use the policy), or should docs and token_helper also reference it in a separate change?


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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds default claims policies for OpenID Connect (OIDC) id_tokens in the Authelia configuration. The changes configure which user claims will be included in id_tokens by default and apply this policy to the Heimdall client.

  • Added a default claims policy specifying standard OIDC claims (email, email_verified, name, preferred_username) to be included in id_tokens
  • Applied the default claims policy to the Heimdall client
  • Added a new scope update:current_user_metadata to the Heimdall client's allowed scopes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

- profile
- offline_access
- access:api
- update:current_user_metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense to me. access:api matches the audience defined on the next line (lfx-api). However update:current_user_metadata is for the management API, which we don't even have defined in Authelia. I assume we would just skip making those API calls or getting the tokens locally. However, in the event that we add another API/audience in dev, like a mock Auth0 API, then we could add this (in other words, I'd be expecting to see audience getting updated, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added by accident.

Copy link
Contributor

@emsearcy emsearcy left a comment

Choose a reason for hiding this comment

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

Adding claims_policy to make these claims be in the ID token, instead of only in userinfo, looks good, but adding Management API scopes doesn't make sense to me.

Removed 'update:current_user_metadata' from the scopes list.

Signed-off-by: Asitha de Silva <[email protected]>
@asithade asithade requested a review from emsearcy November 3, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants