Skip to content
This repository was archived by the owner on Jun 17, 2022. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1d3de8e
Extract SsoConfig enums to own file
eliykat Nov 30, 2021
69f41a9
Add changeClearErrorsDirective
eliykat Dec 1, 2021
6954ba0
Export SsoUrls
eliykat Dec 2, 2021
ecf002d
Add ChangeStripSpaces directive
eliykat Dec 2, 2021
d8acd71
Move custom validators to jslib
eliykat Dec 2, 2021
0256ea8
Fix typing
eliykat Dec 2, 2021
994d4d3
Add a11y-invalid directive
eliykat Dec 2, 2021
0ff7138
Add and implement dirtyValidator
eliykat Dec 7, 2021
e2fe543
Strip spaces on input event instead of change
eliykat Dec 7, 2021
f27a9a1
Fix linting
eliykat Dec 7, 2021
6c4c5c7
Add None to SsoType enum
eliykat Dec 8, 2021
aee297e
Create ssoConfigView model and factory methods
eliykat Dec 8, 2021
56fd31b
Add prebuilt dirty validators
eliykat Dec 8, 2021
7c719c1
Do not export internal SsoUrls class
eliykat Dec 9, 2021
c2646f0
Merge commit '8b2dfc6cdcb8ff5b604364c2ea6d343473aee7cd' into fix/sso-…
eliykat Jan 11, 2022
4e67874
Run prettier
eliykat Jan 11, 2022
4b1850a
Merge commit '193434461dbd9c48fe5dcbad95693470aec422ac' into fix/sso-…
eliykat Jan 11, 2022
45b5225
Remove old TODO
eliykat Jan 11, 2022
24c0f66
Merge branch 'master' into fix/sso-validation
eliykat Jan 12, 2022
ef579ab
Merge branch 'master' into fix/sso-validation
eliykat Feb 18, 2022
d4e034e
Refactor dirtyRequired
eliykat Feb 21, 2022
1b499d1
Add interface for select options
eliykat Feb 21, 2022
dba76df
Fix prettier
eliykat Feb 21, 2022
cc8678a
Fix syntax
eliykat Feb 21, 2022
1e2fbaa
Update comment
eliykat Feb 21, 2022
678e1c2
Rename interface
eliykat Feb 22, 2022
ea9cce7
Merge branch 'fix/sso-validation' of https://github.com/bitwarden/jsl…
eliykat Feb 22, 2022
6f54045
Don't build SsoConfigData if null
eliykat Feb 22, 2022
581635e
Fix prettier
eliykat Feb 22, 2022
72779f6
Add null check in unsubscribe
eliykat Feb 22, 2022
da8c480
Fix selector
eliykat Feb 22, 2022
ad2e89e
Add jsdoc comments
eliykat Feb 22, 2022
0501b4d
Merge branch 'fix/sso-validation' of https://github.com/bitwarden/jsl…
eliykat Feb 22, 2022
23ef3e7
Fix linting
eliykat Feb 22, 2022
ae0843f
Run prettier
eliykat Feb 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions angular/src/directives/a11y-invalid.directive.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Directive, ElementRef, OnDestroy, OnInit } from "@angular/core";
import { NgControl } from "@angular/forms";
import { Subscription } from "rxjs";

@Directive({
selector: "[appA11yInvalid]",
})
export class A11yInvalidDirective implements OnDestroy, OnInit {
private sub: Subscription;

constructor(private el: ElementRef<HTMLInputElement>, private formControlDirective: NgControl) {}

ngOnInit() {
this.sub = this.formControlDirective.control.statusChanges.subscribe((status) => {
if (status === "INVALID") {
this.el.nativeElement.setAttribute("aria-invalid", "true");
} else if (status === "VALID") {
this.el.nativeElement.setAttribute("aria-invalid", "false");
}
});
}

ngOnDestroy() {
this.sub?.unsubscribe();
}
}
12 changes: 12 additions & 0 deletions angular/src/directives/input-strip-spaces.directive.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Directive, ElementRef, HostListener } from "@angular/core";

@Directive({
selector: "input[appInputStripSpaces]",
})
export class InputStripSpacesDirective {
constructor(private el: ElementRef<HTMLInputElement>) {}

@HostListener("input") onInput() {
this.el.nativeElement.value = this.el.nativeElement.value.replace(/ /g, "");
}
}
5 changes: 5 additions & 0 deletions angular/src/interfaces/selectOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export interface SelectOptions {
name: string;
value: any;
disabled?: boolean;
}
25 changes: 25 additions & 0 deletions angular/src/validators/dirty.validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { AbstractControl, ValidationErrors, ValidatorFn, Validators } from "@angular/forms";
import { requiredIf } from "./requiredIf.validator";

/**
* A higher order function that takes a ValidatorFn and returns a new validator.
* The new validator only runs the ValidatorFn if the control is dirty. This prevents error messages from being
* displayed to the user prematurely.
*/
function dirtyValidator(validator: ValidatorFn) {
return (control: AbstractControl): ValidationErrors | null => {
return control.dirty ? validator(control) : null;
};
}

export function dirtyRequiredIf(predicate: (predicateCtrl: AbstractControl) => boolean) {
return dirtyValidator(requiredIf(predicate));
}

/**
* Equivalent to dirtyValidator(Validator.required), however using dirtyValidator returns a new function
* each time which prevents formControl.hasError from properly comparing functions for equality.
*/
export function dirtyRequired(control: AbstractControl): ValidationErrors | null {
return control.dirty ? Validators.required(control) : null;
}
10 changes: 10 additions & 0 deletions angular/src/validators/requiredIf.validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { AbstractControl, ValidationErrors, Validators } from "@angular/forms";

/**
* Returns a new validator which will apply Validators.required only if the predicate is true.
*/
export function requiredIf(predicate: (predicateCtrl: AbstractControl) => boolean) {
return (control: AbstractControl): ValidationErrors | null => {
return predicate(control) ? Validators.required(control) : null;
};
}
34 changes: 34 additions & 0 deletions common/src/enums/ssoEnums.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
export enum SsoType {
None = 0,
OpenIdConnect = 1,
Saml2 = 2,
}

export enum OpenIdConnectRedirectBehavior {
RedirectGet = 0,
FormPost = 1,
}

export enum Saml2BindingType {
HttpRedirect = 1,
HttpPost = 2,
Artifact = 4,
}

export enum Saml2NameIdFormat {
NotConfigured = 0,
Unspecified = 1,
EmailAddress = 2,
X509SubjectName = 3,
WindowsDomainQualifiedName = 4,
KerberosPrincipalName = 5,
EntityIdentifier = 6,
Persistent = 7,
Transient = 8,
}

export enum Saml2SigningBehavior {
IfIdpWantAuthnRequestsSigned = 0,
Always = 1,
Never = 3,
}
84 changes: 51 additions & 33 deletions common/src/models/api/ssoConfigApi.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,58 @@
import { BaseResponse } from "../response/baseResponse";

enum SsoType {
OpenIdConnect = 1,
Saml2 = 2,
}

enum OpenIdConnectRedirectBehavior {
RedirectGet = 0,
FormPost = 1,
}

enum Saml2BindingType {
HttpRedirect = 1,
HttpPost = 2,
Artifact = 4,
}

enum Saml2NameIdFormat {
NotConfigured = 0,
Unspecified = 1,
EmailAddress = 2,
X509SubjectName = 3,
WindowsDomainQualifiedName = 4,
KerberosPrincipalName = 5,
EntityIdentifier = 6,
Persistent = 7,
Transient = 8,
}

enum Saml2SigningBehavior {
IfIdpWantAuthnRequestsSigned = 0,
Always = 1,
Never = 3,
}
import {
OpenIdConnectRedirectBehavior,
Saml2BindingType,
Saml2NameIdFormat,
Saml2SigningBehavior,
SsoType,
} from "../../enums/ssoEnums";
import { SsoConfigView } from "../view/ssoConfigView";

export class SsoConfigApi extends BaseResponse {
static fromView(view: SsoConfigView, api = new SsoConfigApi()) {
api.configType = view.configType;

api.keyConnectorEnabled = view.keyConnectorEnabled;
api.keyConnectorUrl = view.keyConnectorUrl;

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;
}
Comment on lines +19 to +52
Copy link
Member

Choose a reason for hiding this comment

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

Would this work? We send defaults from the server which we should receive on the client to show the correct defaults.

Copy link
Member Author

@eliykat eliykat Jan 12, 2022

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Although if we do that then I guess we should remove the defaults server-side and make sure that won't affect anything else.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with that. Can you open a PR on the server with the removal of the defaults?

Copy link
Member Author

@eliykat eliykat Feb 22, 2022

Choose a reason for hiding this comment

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

@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.


return api;
}
configType: SsoType;

keyConnectorEnabled: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ export class OrganizationSsoResponse extends BaseResponse {
constructor(response: any) {
super(response);
this.enabled = this.getResponseProperty("Enabled");
this.data = new SsoConfigApi(this.getResponseProperty("Data"));
this.data =
this.getResponseProperty("Data") != null
? new SsoConfigApi(this.getResponseProperty("Data"))
: null;
this.urls = new SsoUrls(this.getResponseProperty("Urls"));
}
}
Expand Down
107 changes: 107 additions & 0 deletions common/src/models/view/ssoConfigView.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { View } from "./view";

import { SsoConfigApi } from "../api/ssoConfigApi";

import {
OpenIdConnectRedirectBehavior,
Saml2BindingType,
Saml2NameIdFormat,
Saml2SigningBehavior,
SsoType,
} from "../../enums/ssoEnums";

export class SsoConfigView extends View {
configType: SsoType;

keyConnectorEnabled: boolean;
keyConnectorUrl: string;

openId: {
authority: string;
clientId: string;
clientSecret: string;
metadataAddress: string;
redirectBehavior: OpenIdConnectRedirectBehavior;
getClaimsFromUserInfoEndpoint: boolean;
additionalScopes: string;
additionalUserIdClaimTypes: string;
additionalEmailClaimTypes: string;
additionalNameClaimTypes: string;
acrValues: string;
expectedReturnAcrValue: string;
};

saml: {
spNameIdFormat: Saml2NameIdFormat;
spOutboundSigningAlgorithm: string;
spSigningBehavior: Saml2SigningBehavior;
spMinIncomingSigningAlgorithm: boolean;
spWantAssertionsSigned: boolean;
spValidateCertificates: boolean;

idpEntityId: string;
idpBindingType: Saml2BindingType;
idpSingleSignOnServiceUrl: string;
idpSingleLogoutServiceUrl: string;
idpArtifactResolutionServiceUrl: string;
idpX509PublicCert: string;
idpOutboundSigningAlgorithm: string;
idpAllowUnsolicitedAuthnResponse: boolean;
idpAllowOutboundLogoutRequests: boolean;
idpWantAuthnRequestsSigned: boolean;
};

constructor(api: SsoConfigApi) {
super();
if (api == null) {
return;
}

this.configType = api.configType;

this.keyConnectorEnabled = api.keyConnectorEnabled;
this.keyConnectorUrl = api.keyConnectorUrl;

if (this.configType === SsoType.OpenIdConnect) {
this.openId = {
authority: api.authority,
clientId: api.clientId,
clientSecret: api.clientSecret,
metadataAddress: api.metadataAddress,
redirectBehavior: api.redirectBehavior,
getClaimsFromUserInfoEndpoint: api.getClaimsFromUserInfoEndpoint,
additionalScopes: api.additionalScopes,
additionalUserIdClaimTypes: api.additionalUserIdClaimTypes,
additionalEmailClaimTypes: api.additionalEmailClaimTypes,
additionalNameClaimTypes: api.additionalNameClaimTypes,
acrValues: api.acrValues,
expectedReturnAcrValue: api.expectedReturnAcrValue,
};
} else if (this.configType === SsoType.Saml2) {
this.saml = {
spNameIdFormat: api.spNameIdFormat,
spOutboundSigningAlgorithm: api.spOutboundSigningAlgorithm,
spSigningBehavior: api.spSigningBehavior,
spMinIncomingSigningAlgorithm: api.spMinIncomingSigningAlgorithm,
spWantAssertionsSigned: api.spWantAssertionsSigned,
spValidateCertificates: api.spValidateCertificates,

idpEntityId: api.idpEntityId,
idpBindingType: api.idpBindingType,
idpSingleSignOnServiceUrl: api.idpSingleSignOnServiceUrl,
idpSingleLogoutServiceUrl: api.idpSingleLogoutServiceUrl,
idpArtifactResolutionServiceUrl: api.idpArtifactResolutionServiceUrl,
idpX509PublicCert: api.idpX509PublicCert,
idpOutboundSigningAlgorithm: api.idpOutboundSigningAlgorithm,
idpAllowUnsolicitedAuthnResponse: api.idpAllowUnsolicitedAuthnResponse,
idpWantAuthnRequestsSigned: api.idpWantAuthnRequestsSigned,

// Value is inverted in the view model (allow instead of disable)
idpAllowOutboundLogoutRequests:
api.idpDisableOutboundLogoutRequests == null
? null
: !api.idpDisableOutboundLogoutRequests,
};
}
}
}