Improve SSO Config validation #572
Conversation
| if (api.configType === SsoType.OpenIdConnect) { | ||
| api.authority = view.openId.authority; | ||
| api.clientId = view.openId.clientId; | ||
| api.clientSecret = view.openId.clientSecret; | ||
| api.metadataAddress = view.openId.metadataAddress; | ||
| api.redirectBehavior = view.openId.redirectBehavior; | ||
| api.getClaimsFromUserInfoEndpoint = view.openId.getClaimsFromUserInfoEndpoint; | ||
| api.additionalScopes = view.openId.additionalScopes; | ||
| api.additionalUserIdClaimTypes = view.openId.additionalUserIdClaimTypes; | ||
| api.additionalEmailClaimTypes = view.openId.additionalEmailClaimTypes; | ||
| api.additionalNameClaimTypes = view.openId.additionalNameClaimTypes; | ||
| api.acrValues = view.openId.acrValues; | ||
| api.expectedReturnAcrValue = view.openId.expectedReturnAcrValue; | ||
| } else if (api.configType === SsoType.Saml2) { | ||
| api.spNameIdFormat = view.saml.spNameIdFormat; | ||
| api.spOutboundSigningAlgorithm = view.saml.spOutboundSigningAlgorithm; | ||
| api.spSigningBehavior = view.saml.spSigningBehavior; | ||
| api.spMinIncomingSigningAlgorithm = view.saml.spMinIncomingSigningAlgorithm; | ||
| api.spWantAssertionsSigned = view.saml.spWantAssertionsSigned; | ||
| api.spValidateCertificates = view.saml.spValidateCertificates; | ||
|
|
||
| api.idpEntityId = view.saml.idpEntityId; | ||
| api.idpBindingType = view.saml.idpBindingType; | ||
| api.idpSingleSignOnServiceUrl = view.saml.idpSingleSignOnServiceUrl; | ||
| api.idpSingleLogoutServiceUrl = view.saml.idpSingleLogoutServiceUrl; | ||
| api.idpArtifactResolutionServiceUrl = view.saml.idpArtifactResolutionServiceUrl; | ||
| api.idpX509PublicCert = view.saml.idpX509PublicCert; | ||
| api.idpOutboundSigningAlgorithm = view.saml.idpOutboundSigningAlgorithm; | ||
| api.idpAllowUnsolicitedAuthnResponse = view.saml.idpAllowUnsolicitedAuthnResponse; | ||
| api.idpWantAuthnRequestsSigned = view.saml.idpWantAuthnRequestsSigned; | ||
|
|
||
| // Value is inverted in the api model (disable instead of allow) | ||
| api.idpDisableOutboundLogoutRequests = !view.saml.idpAllowOutboundLogoutRequests; | ||
| } |
There was a problem hiding this comment.
Would this work? We send defaults from the server which we should receive on the client to show the correct defaults.
There was a problem hiding this comment.
Hmm I didn't realise that. I've provided the defaults client-side in the related web PR (see here). To me it makes more sense to do this client-side, because they are form defaults and the form belongs to the client. I also think it's required for the new design of the form (now that it only saves the selected configuration). I didn't notice any unexpected behaviour during my testing.
Note that the read-only fields (callbackPath, entityId, metadataUrl, etc) are still populated from the data provided by the server (for both configurations), they're outside the ssoConfig object.
What do you think?
There was a problem hiding this comment.
Although if we do that then I guess we should remove the defaults server-side and make sure that won't affect anything else.
There was a problem hiding this comment.
I'm fine with that. Can you open a PR on the server with the removal of the defaults?
There was a problem hiding this comment.
@Hinton I came back to this and have done the following:
- server PR as discussed above: Improve SSO Config validation server#1879
- refactored the web PR to use reusable components rather than copy/pasting throughout the template, should now be much easier to review, and can be lifted into the component library once we have it.
… into fix/sso-validation
Co-authored-by: Oscar Hinton <oscar@oscarhinton.com>
… into fix/sso-validation
|
We should be good code wise on this. Haven't tested the functionality. |
Type of change
Objective
See bitwarden/web#1332.
Code changes
Directives:
angular/src/directives/a11y-invalid.directive.ts: automatically apply thearia-invalidattribute depending on the state of the linkedformControl. (Used on all controls to make them accessible.)angular/src/directives/input-strip-spaces.directive.ts: automatically strip spaces from certain inputs (used on URL fields)Validators:
angular/src/validators/dirty.validator.ts: design requirements were to validate on blur, but only once the user has actually changed the field value. This is achieved by setting the form toupdateOn: 'blur'and then running any required validator through this factory, which only runs the validator if the field is dirty.angular/src/validators/requiredIf.validator.ts: used for conditional validation, usually depending on another field.Enums:
common/src/enums/ssoEnums.ts: extract the SsoApi enums into their own class and export them so we can use them in the component. AddNoneas an option toSsoType.Models:
common/src/models/view/ssoConfigView.ts: this is a new view model because the form structure no longer reflected the api model in 2 main ways:idpDisableOutboundLogoutRequests. Design wanted it to read "allow" rather than "deny". Using separate models lets us make this change without having to change the logic all the way up into the server.common/src/models/api/ssoConfigApi.ts: add helper method to create a new api model from a view modelBefore you submit
npm run lint) (required)