-
Notifications
You must be signed in to change notification settings - Fork 921
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a no file fluent API for application configuration. #1422
Conversation
This pull request introduces 2 alerts when merging 6b6a5c8 into 24f90c4 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 4839b72 into 24f90c4 - view on LGTM.com new alerts:
|
switch GDS tests turned out extensions are yet missing |
This pull request introduces 2 alerts when merging bf6b715 into 24f90c4 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 991b96f into 24f90c4 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 2ba7279 into 24f90c4 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 42615e2 into 24f90c4 - view on LGTM.com new alerts:
|
var uri = SecurityPolicies.GetUri(name); | ||
if (uri != null) | ||
{ | ||
deprecatedPolicyList.Add(uri); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deprecatedPolicyList is not added to main list.
AddSecurityPolicies() is called internally only with deprecated = false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, intentionally added a AddPolicy instead of allowing to add all deprecated policies. However the function might be useful for testing, so I keep it around for a bit,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated API design --> I removed addDeprecated in favor of AddPolicy, to avoid to make it too easy to have unsecure policy settings.
ApplicationCertificate = new CertificateIdentifier() | ||
{ | ||
StoreType = appStoreType, | ||
StorePath = DefaultCertificateStorePath(TrustlistType.Application, pkiRoot), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be appRoot here instead of pkiRoot, right?
StorePath = DefaultCertificateStorePath(TrustlistType.Application, appRoot),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, good catch! Also if appRoot == null, the pkiRoot should be used
This pull request introduces 2 alerts when merging a6efd3d into 24f90c4 - view on LGTM.com new alerts:
|
No description provided.