Certificate support in Azure RM endpoint #7678
Conversation
vincent1173
added
Area: Release
Area: AzureAppService
vincent1173
requested review from
kmkumaran,
Ajay-MS,
sachinma,
asranja and
thesattiraju
| "archivePackages": [ | ||
| { | ||
| "archiveName": "openssl-1.0.2l-x64_86-win64.zip", | ||
| "url": "https://indy.fulgan.com/SSL/openssl-1.0.2l-x64_86-win64.zip", |
There was a problem hiding this comment.
// TODO : update openssl package to vstsagent blob
| @@ -104,6 +105,11 @@ | |||
| "resolved": "https://registry.npmjs.org/buffer-crc32/-/buffer-crc32-0.2.13.tgz", | |||
| "integrity": "sha1-DTM+PwDqxQqhRUq9MO+MKl2ackI=" | |||
| }, | |||
| "buffer-equal-constant-time": { | |||
There was a problem hiding this comment.
Are these new dependencies? Did you run them through OSS scans and should they be included in TPN?
There was a problem hiding this comment.
checked with OSS , all the new modules are IP scanned. will check with PMs for TPN
There was a problem hiding this comment.
few newly introduced modules require IP scan and requested for same.
Will update TPN in a separate PR.
| @@ -16,7 +23,7 @@ export class ApplicationTokenCredentials { | |||
| public msiClientId: string; | |||
| private token_deferred: Q.Promise<string>; | |||
|
|
|||
| constructor(clientId: string, domain: string, secret: string, baseUrl: string, authorityUrl: string, activeDirectoryResourceId: string, isAzureStackEnvironment: boolean, scheme?: string, msiClientId?: string) { | |||
| constructor(clientId: string, domain: string, secret: string, baseUrl: string, authorityUrl: string, activeDirectoryResourceId: string, isAzureStackEnvironment: boolean, scheme?: string, msiClientId?: string, authType?: string, certFilePath?: string,isADFSEnabled?: boolean) { | |||
There was a problem hiding this comment.
nit - space before isADFSEnabled
| var deferred = Q.defer<string>(); | ||
| let webRequest = new webClient.WebRequest(); | ||
| webRequest.method = "POST"; | ||
| webRequest.uri = this.authorityUrl + (this.isADFSEnabled ? "" : this.domain) + "/oauth2/token/"; |
There was a problem hiding this comment.
Can you give example of how the Uri is for AAD and ADFS?
There was a problem hiding this comment.
| } | ||
| } | ||
|
|
||
| this.isADFSEnabled = isADFSEnabled; |
There was a problem hiding this comment.
I see the use case of this Boolean is to determine the tenant id (or domain) later. Why can't the tenant Id (or domain) be directly stored instead?
There was a problem hiding this comment.
tenantID might be used in future reference, so didnt make it null for loginAuth
| @@ -49,10 +63,13 @@ export class AzureRMEndpoint { | |||
| this.endpoint.activeDirectoryResourceID = this.endpoint.url; | |||
| } | |||
|
|
|||
| this.endpoint.isADFSEnabled = this.endpoint.environmentAuthorityUrl.endsWith('/adfs'); | |||
There was a problem hiding this comment.
Is this assumption (that URL ends with adfs) true always? Is there any documentation on this?
| @@ -9,6 +9,20 @@ | |||
| "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz", | |||
| "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=" | |||
| }, | |||
| "base64-url": { | |||
There was a problem hiding this comment.
Similar query as above. Are there new modules and have we gone through the compliance process for these?
| @@ -39,6 +40,19 @@ export class AzureRMEndpoint { | |||
| activeDirectoryResourceID: tl.getEndpointDataParameter(this._connectedServiceName, 'activeDirectoryServiceEndpointResourceId', true) | |||
| } as AzureEndpoint; | |||
|
|
|||
|
|
|||
| this.endpoint.authenticationType = tl.getEndpointAuthorizationParameter(this._connectedServiceName, 'authenticationType', true); | |||
| if(this.endpoint.authenticationType && this.endpoint.authenticationType == constants.AzureServicePrinicipalAuthentications.servicePrincipalCertificate) { | |||
There was a problem hiding this comment.
| "joi": "6.10.1", | ||
| "jws": "3.1.3", | ||
| "lodash.once": "4.1.1", | ||
| "ms": "0.7.2", |
| "requires": { | ||
| "joi": "6.10.1", | ||
| "jws": "3.1.5", | ||
| "ms": "0.7.3", |
No description provided.