sso: add Assertions signed validation checkbox#1440
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new "Assertions signed validation" checkbox to the SSO SAML configuration form, allowing administrators to enable validation of signed assertions. Additionally, label casing has been standardized to sentence case for better UI consistency.
Key Changes
- Added new
assertionsSignedValidationboolean field to theSamlConfiginterface and component - Changed default value of
authnResponseSignedValidationfromtruetofalse - Updated label casing to sentence case for improved UI consistency
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/app/models/company.ts | Added assertionsSignedValidation boolean field to SamlConfig interface and changed authnResponseSignedValidation from literal type to boolean |
| frontend/src/app/components/sso/sso.component.ts | Initialized new assertionsSignedValidation field with default value false, and changed authnResponseSignedValidation default from true to false |
| frontend/src/app/components/sso/sso.component.html | Added new checkbox for assertions signed validation and updated label casing to sentence case for multiple form labels |
| frontend/src/app/components/sso/sso.component.css | Adjusted top margin for checkboxes from -4px to -8px to accommodate additional checkbox spacing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| digestAlgorithm: "sha256", | ||
| active: true, |
There was a problem hiding this comment.
Type inconsistency: digestAlgorithm and active are defined with literal type values ("sha256" and true) instead of their proper types (string and boolean). This prevents them from accepting other valid values. Consider changing to:
digestAlgorithm: string,
active: boolean,| digestAlgorithm: "sha256", | |
| active: true, | |
| digestAlgorithm: string, | |
| active: boolean, |
| digestAlgorithm: "sha256", | ||
| active: true, | ||
| authnResponseSignedValidation: true, | ||
| authnResponseSignedValidation: false, |
There was a problem hiding this comment.
The default value for authnResponseSignedValidation changed from true to false. This is a breaking change that could affect security if existing configurations rely on the previous default behavior. If this change is intentional, ensure that existing SAML configurations are migrated appropriately, or consider keeping the default as true for security reasons.
| authnResponseSignedValidation: false, | |
| authnResponseSignedValidation: true, |
No description provided.