This repository has been archived by the owner on Mar 15, 2024. It is now read-only.
Refactor orgnaization policy management #1147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cscharf
reviewed
Aug 24, 2021
addisonbeck
reviewed
Aug 25, 2021
Comment on lines
+185
to
+196
| this.policyListService.addPolicies([ | ||
| new TwoFactorAuthenticationPolicy(), | ||
| new MasterPasswordPolicy(), | ||
| new PasswordGeneratorPolicy(), | ||
| new SingleOrgPolicy(), | ||
| new RequireSsoPolicy(), | ||
| new PersonalOwnershipPolicy(), | ||
| new DisableSendPolicy(), | ||
| new SendOptionsPolicy(), | ||
| new ResetPasswordPolicy(), | ||
| ]); | ||
|
|
There was a problem hiding this comment.
I know you weren't happy about loading policies here, but I think it's perfect besides being in app.component. We get clear control and vision on what policies are supported & the addPolicies call taking an array is pretty clean.
Just curious, could we not populate the service in policies.component? What about in a constructor for the policy list service?
Comment on lines
+10
to
+18
|
|
||
| <div class="modal-body"> | ||
| <div class="modal-body" *ngIf="loading"> | ||
| <i class="fa fa-spinner fa-spin text-muted" title="{{'loading' | i18n}}" aria-hidden="true"></i> | ||
| <span class="sr-only">{{'loading' | i18n}}</span> | ||
| </div> | ||
| <div [hidden]="loading"> | ||
| <p>{{policy.description | i18n}}</p> | ||
| <ng-template #policyForm></ng-template> |
There was a problem hiding this comment.
Hooray for organized html 🥳
addisonbeck
approved these changes
Aug 25, 2021
domdomegg
added a commit
to domdomegg/vaultwarden
that referenced
this pull request
Sep 24, 2021
Boolean-toggle enterprise policies (like 'Two-Step Login' and 'Personal Ownership') don't provide a data attribute in the new version of the web client. This updates the backend to expect these to be optional. Web change introduced in bitwarden/web#1147 which added https://github.com/bitwarden/web/blob/2cbe023a38792ff70a2a4907f4748354bb4e0573/src/app/organizations/policies/base-policy.component.ts#L48-L50
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Refactors the policy management page for organizations. Instead of having a single component
PolicyEditComponentcontaining a multitude of if statements, we now have aBasePolicyandBasePolicyComponentwhich encapsulates a single Policy. ThePoliciesComponentsdisplays a list of components which it retrives from thePolicyListService, andPolicyEditComponentlets theBasePolicyComponenthandle the policy specific details.I've also removed the deprecation warning for policies since it's no longer accurate.
Noteworthy Code Changes
componentFactoryResolver.Testing Considerations
Since this PR does some drastic changes to the policy management pages, we will have to do a full regression surrounding it.
Asana: https://app.asana.com/0/1198901840263430/1200796078487139