Refactor Code for better reading (#326)

* refactor

* fix test case

__tests__/PowerShell/ServicePrinicipalLogin.test.ts

@@ -1,11 +1,17 @@
 import { ServicePrincipalLogin } from '../../src/PowerShell/ServicePrincipalLogin';
+import { LoginConfig } from '../../src/common/LoginConfig';
 
 jest.mock('../../src/PowerShell/Utilities/Utils');
 jest.mock('../../src/PowerShell/Utilities/PowerShellToolRunner');
 let spnlogin: ServicePrincipalLogin;
 
 beforeAll(() => {
-    spnlogin = new ServicePrincipalLogin("servicePrincipalID", "servicePrinicipalkey", null, "tenantId", "subscriptionId", false, null, null);
+    var loginConfig = new LoginConfig();
+    loginConfig.servicePrincipalId = "servicePrincipalID";
+    loginConfig.servicePrincipalKey = "servicePrinicipalkey";
+    loginConfig.tenantId = "tenantId";
+    loginConfig.subscriptionId = "subscriptionId";
+    spnlogin = new ServicePrincipalLogin(loginConfig);
 });
 
 afterEach(() => {

src/Cli/AzureCliLogin.ts

@@ -0,0 +1,128 @@
+import * as exec from '@actions/exec';
+import { LoginConfig } from "../common/LoginConfig";
+import { ExecOptions } from '@actions/exec/lib/interfaces';
+import * as core from '@actions/core';
+import * as io from '@actions/io';
+
+export class AzureCliLogin {
+    loginConfig: LoginConfig;
+    azPath: string;
+    
+    constructor(loginConfig: LoginConfig) {
+        this.loginConfig = loginConfig;
+    }
+
+    async login() {
+        this.azPath = await io.which("az", true);
+        core.debug(`az cli path: ${this.azPath}`);
+
+        let output: string = "";
+        const execOptions: any = {
+            listeners: {
+                stdout: (data: Buffer) => {
+                    output += data.toString();
+                }
+            }
+        };
+        await this.executeAzCliCommand("--version", true, execOptions);
+        core.debug(`az cli version used:\n${output}`);
+
+        this.setAzurestackEnvIfNecessary();
+
+        await this.executeAzCliCommand(`cloud set -n "${this.loginConfig.environment}"`, false);
+        console.log(`Done setting cloud: "${this.loginConfig.environment}"`);
+
+        // Attempting Az cli login
+        var commonArgs = ["--service-principal",
+            "-u", this.loginConfig.servicePrincipalId,
+            "--tenant", this.loginConfig.tenantId
+        ];
+        if (this.loginConfig.allowNoSubscriptionsLogin) {
+            commonArgs = commonArgs.concat("--allow-no-subscriptions");
+        }
+        if (this.loginConfig.enableOIDC) {
+            commonArgs = commonArgs.concat("--federated-token", this.loginConfig.federatedToken);
+        }
+        else {
+            console.log("Note: Azure/login action also supports OIDC login mechanism. Refer https://github.com/azure/login#configure-a-service-principal-with-a-federated-credential-to-use-oidc-based-authentication for more details.")
+            commonArgs = commonArgs.concat(`--password=${this.loginConfig.servicePrincipalKey}`);
+        }
+
+        const loginOptions: ExecOptions = defaultExecOptions();
+        await this.executeAzCliCommand(`login`, true, loginOptions, commonArgs);
+
+        if (!this.loginConfig.allowNoSubscriptionsLogin) {
+            var args = [
+                "--subscription",
+                this.loginConfig.subscriptionId
+            ];
+            await this.executeAzCliCommand(`account set`, true, loginOptions, args);
+        }
+    }
+
+    async setAzurestackEnvIfNecessary() {
+        if (this.loginConfig.environment != "azurestack") {
+            return;
+        }
+        if (!this.loginConfig.resourceManagerEndpointUrl) {
+            throw new Error("resourceManagerEndpointUrl is a required parameter when environment is defined.");
+        }
+
+        console.log(`Unregistering cloud: "${this.loginConfig.environment}" first if it exists`);
+        try {
+            await this.executeAzCliCommand(`cloud set -n AzureCloud`, true);
+            await this.executeAzCliCommand(`cloud unregister -n "${this.loginConfig.environment}"`, false);
+        }
+        catch (error) {
+            console.log(`Ignore cloud not registered error: "${error}"`);
+        }
+
+        console.log(`Registering cloud: "${this.loginConfig.environment}" with ARM endpoint: "${this.loginConfig.resourceManagerEndpointUrl}"`);
+        try {
+            let baseUri = this.loginConfig.resourceManagerEndpointUrl;
+            if (baseUri.endsWith('/')) {
+                baseUri = baseUri.substring(0, baseUri.length - 1); // need to remove trailing / from resourceManagerEndpointUrl to correctly derive suffixes below
+            }
+            let suffixKeyvault = ".vault" + baseUri.substring(baseUri.indexOf('.')); // keyvault suffix starts with .
+            let suffixStorage = baseUri.substring(baseUri.indexOf('.') + 1); // storage suffix starts without .
+            let profileVersion = "2019-03-01-hybrid";
+            await this.executeAzCliCommand(`cloud register -n "${this.loginConfig.environment}" --endpoint-resource-manager "${this.loginConfig.resourceManagerEndpointUrl}" --suffix-keyvault-dns "${suffixKeyvault}" --suffix-storage-endpoint "${suffixStorage}" --profile "${profileVersion}"`, false);
+        }
+        catch (error) {
+            core.error(`Error while trying to register cloud "${this.loginConfig.environment}": "${error}"`);
+        }
+
+        console.log(`Done registering cloud: "${this.loginConfig.environment}"`)
+    }
+
+    async executeAzCliCommand(
+        command: string,
+        silent?: boolean,
+        execOptions: any = {},
+        args: any = []) {
+        execOptions.silent = !!silent;
+        await exec.exec(`"${this.azPath}" ${command}`, args, execOptions);
+    }
+}
+
+function defaultExecOptions(): exec.ExecOptions {
+    return {
+        silent: true,
+        listeners: {
+            stderr: (data: Buffer) => {
+                let error = data.toString();
+                let startsWithWarning = error.toLowerCase().startsWith('warning');
+                let startsWithError = error.toLowerCase().startsWith('error');
+                // printing ERROR
+                if (error && error.trim().length !== 0 && !startsWithWarning) {
+                    if (startsWithError) {
+                        //removing the keyword 'ERROR' to avoid duplicates while throwing error
+                        error = error.slice(5);
+                    }
+                    core.setFailed(error);
+                }
+            }
+        }
+    };
+}
+

src/PowerShell/ServicePrincipalLogin.ts

@@ -4,36 +4,15 @@ import Utils from './Utilities/Utils';
 import PowerShellToolRunner from './Utilities/PowerShellToolRunner';
 import ScriptBuilder from './Utilities/ScriptBuilder';
 import Constants from './Constants';
+import { LoginConfig } from '../common/LoginConfig';
 
 export class ServicePrincipalLogin implements IAzurePowerShellSession {
     static readonly scopeLevel: string = Constants.Subscription;
     static readonly scheme: string = Constants.ServicePrincipal;
-    environment: string;
-    servicePrincipalId: string;
-    servicePrincipalKey: string;
-    tenantId: string;
-    subscriptionId: string;
-    resourceManagerEndpointUrl: string;
-    allowNoSubscriptionsLogin: boolean;
-    federatedToken: string;
+    loginConfig: LoginConfig;
 
-    constructor(servicePrincipalId: string,
-        servicePrincipalKey: string,
-        federatedToken: string,
-        tenantId: string,
-        subscriptionId: string,
-        allowNoSubscriptionsLogin: boolean,
-        environment: string,
-        resourceManagerEndpointUrl: string) {
-
-        this.servicePrincipalId = servicePrincipalId;
-        this.servicePrincipalKey = servicePrincipalKey;
-        this.federatedToken = federatedToken;
-        this.tenantId = tenantId;
-        this.subscriptionId = subscriptionId;
-        this.environment = environment;
-        this.resourceManagerEndpointUrl = resourceManagerEndpointUrl;
-        this.allowNoSubscriptionsLogin = allowNoSubscriptionsLogin;
+    constructor(loginConfig: LoginConfig) {
+        this.loginConfig = loginConfig;
     }
 
     async initialize() {
@@ -53,8 +32,7 @@ export class ServicePrincipalLogin implements IAzurePowerShellSession {
                 },
                 stderr: (data: Buffer) => {
                     let error = data.toString();
-                    if (error && error.trim().length !== 0)
-                    {
+                    if (error && error.trim().length !== 0) {
                         commandStdErr = true;
                         core.error(error);
                     }
@@ -62,16 +40,16 @@ export class ServicePrincipalLogin implements IAzurePowerShellSession {
             }
         };
         const args: any = {
-            servicePrincipalId: this.servicePrincipalId,
-            servicePrincipalKey: this.servicePrincipalKey,
-            federatedToken: this.federatedToken,
-            subscriptionId: this.subscriptionId,
-            environment: this.environment,
+            servicePrincipalId: this.loginConfig.servicePrincipalId,
+            servicePrincipalKey: this.loginConfig.servicePrincipalKey,
+            federatedToken: this.loginConfig.federatedToken,
+            subscriptionId: this.loginConfig.subscriptionId,
+            environment: this.loginConfig.environment,
             scopeLevel: ServicePrincipalLogin.scopeLevel,
-            allowNoSubscriptionsLogin: this.allowNoSubscriptionsLogin,
-            resourceManagerEndpointUrl: this.resourceManagerEndpointUrl
+            allowNoSubscriptionsLogin: this.loginConfig.allowNoSubscriptionsLogin,
+            resourceManagerEndpointUrl: this.loginConfig.resourceManagerEndpointUrl
         }
-        const script: string = new ScriptBuilder().getAzPSLoginScript(ServicePrincipalLogin.scheme, this.tenantId, args);
+        const script: string = new ScriptBuilder().getAzPSLoginScript(ServicePrincipalLogin.scheme, this.loginConfig.tenantId, args);
         await PowerShellToolRunner.init();
         await PowerShellToolRunner.executePowerShellScriptBlock(script, options);
         const result: any = JSON.parse(output.trim());

src/common/LoginConfig.ts

@@ -0,0 +1,93 @@
+import * as core from '@actions/core';
+import { FormatType, SecretParser } from 'actions-secret-parser';
+
+export class LoginConfig {
+    static readonly azureSupportedCloudName = new Set([
+        "azureusgovernment",
+        "azurechinacloud",
+        "azuregermancloud",
+        "azurecloud",
+        "azurestack"]);
+
+    servicePrincipalId: string;
+    servicePrincipalKey: string;
+    tenantId: string;
+    subscriptionId: string;
+    resourceManagerEndpointUrl: string;
+    allowNoSubscriptionsLogin: boolean;
+    enableOIDC: boolean;
+    environment: string;
+    enableAzPSSession: boolean;
+    audience: string;
+    federatedToken: string;
+
+    constructor() {
+        this.enableOIDC = true;
+    }
+
+    async initialize() {
+        this.environment = core.getInput("environment").toLowerCase();
+        this.enableAzPSSession = core.getInput('enable-AzPSSession').toLowerCase() === "true";
+        this.allowNoSubscriptionsLogin = core.getInput('allow-no-subscriptions').toLowerCase() === "true";
+
+        this.servicePrincipalId = core.getInput('client-id', { required: false });
+        this.servicePrincipalKey = null;
+        this.tenantId = core.getInput('tenant-id', { required: false });
+        this.subscriptionId = core.getInput('subscription-id', { required: false });
+
+        this.audience = core.getInput('audience', { required: false });
+        this.federatedToken = null;
+        let creds = core.getInput('creds', { required: false });
+        let secrets = creds ? new SecretParser(creds, FormatType.JSON) : null;
+
+        if (creds) {
+            core.debug('using creds JSON...');
+            this.enableOIDC = false;
+            this.servicePrincipalId = secrets.getSecret("$.clientId", true);
+            this.servicePrincipalKey = secrets.getSecret("$.clientSecret", true);
+            this.tenantId = secrets.getSecret("$.tenantId", true);
+            this.subscriptionId = secrets.getSecret("$.subscriptionId", true);
+            this.resourceManagerEndpointUrl = secrets.getSecret("$.resourceManagerEndpointUrl", false);
+        }
+        this.getFederatedTokenIfNecessary();
+    }
+
+    async getFederatedTokenIfNecessary() {
+        if (!this.enableOIDC) {
+            return;
+        }
+        try {
+            this.federatedToken = await core.getIDToken(this.audience);
+        }
+        catch (error) {
+            core.error(`Please make sure to give write permissions to id-token in the workflow.`);
+            throw error;
+        }
+        if (!!this.federatedToken) {
+            let [issuer, subjectClaim] = await jwtParser(this.federatedToken);
+            console.log("Federated token details: \n issuer - " + issuer + " \n subject claim - " + subjectClaim);
+        }
+        else {
+            throw new Error("Failed to fetch federated token.");
+        }
+    }
+
+    async validate() {
+        if (!this.servicePrincipalId || !this.tenantId || !(this.servicePrincipalKey || this.enableOIDC)) {
+            throw new Error("Not all values are present in the credentials. Ensure clientId, clientSecret and tenantId are supplied.");
+        }
+        if (!this.subscriptionId && !this.allowNoSubscriptionsLogin) {
+            throw new Error("Not all values are present in the credentials. Ensure subscriptionId is supplied.");
+        }
+        if (!LoginConfig.azureSupportedCloudName.has(this.environment)) {
+            throw new Error("Unsupported value for environment is passed.The list of supported values for environment are ‘azureusgovernment', ‘azurechinacloud’, ‘azuregermancloud’, ‘azurecloud’ or ’azurestack’");
+        }
+    }
+}
+
+async function jwtParser(federatedToken: string) {
+    let tokenPayload = federatedToken.split('.')[1];
+    let bufferObj = Buffer.from(tokenPayload, "base64");
+    let decodedPayload = JSON.parse(bufferObj.toString("utf8"));
+    return [decodedPayload['iss'], decodedPayload['sub']];
+}

src/main.ts

@@ -1,37 +1,14 @@
 import * as core from '@actions/core';
-import * as exec from '@actions/exec';
-import { ExecOptions } from '@actions/exec/lib/interfaces';
-import * as io from '@actions/io';
-import { FormatType, SecretParser } from 'actions-secret-parser';
 import { ServicePrincipalLogin } from './PowerShell/ServicePrincipalLogin';
+import { LoginConfig } from './common/LoginConfig';
+import { AzureCliLogin } from './Cli/AzureCliLogin';
 
-var azPath: string;
 var prefix = !!process.env.AZURE_HTTP_USER_AGENT ? `${process.env.AZURE_HTTP_USER_AGENT}` : "";
 var azPSHostEnv = !!process.env.AZUREPS_HOST_ENVIRONMENT ? `${process.env.AZUREPS_HOST_ENVIRONMENT}` : "";
 
 async function main() {
-    try {
-        //Options for error handling
-        const loginOptions: ExecOptions = {
-            silent: true,
-            listeners: {
-                stderr: (data: Buffer) => {
-                    let error = data.toString();
-                    let startsWithWarning = error.toLowerCase().startsWith('warning');
-                    let startsWithError = error.toLowerCase().startsWith('error');
-                    // printing ERROR
-                    if (error && error.trim().length !== 0 && !startsWithWarning) {
-                        if(startsWithError) {
-                            //removing the keyword 'ERROR' to avoid duplicates while throwing error
-                            error = error.slice(5);
-                        }
-                        core.setFailed(error);
-                    }
-                }
-            }
-        }
-        // Set user agent variable
     var isAzCLISuccess = false;
+    try {
         let usrAgentRepo = `${process.env.GITHUB_REPOSITORY}`;
         let actionName = 'AzureLogin';
         let userAgentString = (!!prefix ? `${prefix}+` : '') + `GITHUBACTIONS/${actionName}@v1_${usrAgentRepo}`;
@@ -39,170 +16,20 @@ async function main() {
         core.exportVariable('AZURE_HTTP_USER_AGENT', userAgentString);
         core.exportVariable('AZUREPS_HOST_ENVIRONMENT', azurePSHostEnv);
 
-        azPath = await io.which("az", true);
-        core.debug(`az cli path: ${azPath}`);
-        let azureSupportedCloudName = new Set([
-            "azureusgovernment",
-            "azurechinacloud",
-            "azuregermancloud",
-            "azurecloud",
-            "azurestack"]);
+        // perpare the login configuration
+        var loginConfig = new LoginConfig();
+        await loginConfig.initialize();
+        await loginConfig.validate();
 
-        let output: string = "";
-        const execOptions: any = {
-            listeners: {
-                stdout: (data: Buffer) => {
-                    output += data.toString();
-                }
-            }
-        };
-        await executeAzCliCommand("--version", true, execOptions);
-        core.debug(`az cli version used:\n${output}`);
-
-        let creds = core.getInput('creds', { required: false });
-        let secrets = creds ? new SecretParser(creds, FormatType.JSON) : null;
-        let environment = core.getInput("environment").toLowerCase();
-        const enableAzPSSession = core.getInput('enable-AzPSSession').toLowerCase() === "true";
-        const allowNoSubscriptionsLogin = core.getInput('allow-no-subscriptions').toLowerCase() === "true";
-
-        //Check for the credentials in individual parameters in the workflow.
-        var servicePrincipalId = core.getInput('client-id', { required: false });
-        var servicePrincipalKey = null;
-        var tenantId = core.getInput('tenant-id', { required: false });
-        var subscriptionId = core.getInput('subscription-id', { required: false });
-        var resourceManagerEndpointUrl = "https://management.azure.com/";
-        var enableOIDC = true;
-        var federatedToken = null;
-
-        // If any of the individual credentials (clent_id, tenat_id, subscription_id) is present.
-        if (servicePrincipalId || tenantId || subscriptionId) {
-
-            //If few of the individual credentials (clent_id, tenat_id, subscription_id) are missing in action inputs.
-            if (!(servicePrincipalId && tenantId && (subscriptionId || allowNoSubscriptionsLogin)))
-                throw new Error("Few credentials are missing. ClientId, tenantId are mandatory. SubscriptionId is also mandatory if allow-no-subscriptions is not set.");
-        }
-        else {
-            if (creds) {
-                core.debug('using creds JSON...');
-                enableOIDC = false;
-                servicePrincipalId = secrets.getSecret("$.clientId", true);
-                servicePrincipalKey = secrets.getSecret("$.clientSecret", true);
-                tenantId = secrets.getSecret("$.tenantId", true);
-                subscriptionId = secrets.getSecret("$.subscriptionId", true);
-                resourceManagerEndpointUrl = secrets.getSecret("$.resourceManagerEndpointUrl", false);
-            }
-            else {
-                throw new Error("Credentials are not passed for Login action.");
-            }
-        }
-        //generic checks
-        //servicePrincipalKey is only required in non-oidc scenario.
-        if (!servicePrincipalId || !tenantId || !(servicePrincipalKey || enableOIDC)) {
-            throw new Error("Not all values are present in the credentials. Ensure clientId, clientSecret and tenantId are supplied.");
-        }
-        if (!subscriptionId && !allowNoSubscriptionsLogin) {
-            throw new Error("Not all values are present in the credentials. Ensure subscriptionId is supplied.");
-        }
-        if (!azureSupportedCloudName.has(environment)) {
-            throw new Error("Unsupported value for environment is passed.The list of supported values for environment are ‘azureusgovernment', ‘azurechinacloud’, ‘azuregermancloud’, ‘azurecloud’ or ’azurestack’");
-        }
-
-        // OIDC specific checks
-        if (enableOIDC) {
-            console.log('Using OIDC authentication...')
-            //generating ID-token
-            let audience = core.getInput('audience', { required: false });
-            try{
-                federatedToken = await core.getIDToken(audience);
-            }
-            catch (error) {
-                core.error(`Please make sure to give write permissions to id-token in the workflow.`);
-                throw error;
-            }
-            if (!!federatedToken) {
-                let [issuer, subjectClaim] = await jwtParser(federatedToken);
-                console.log("Federated token details: \n issuer - " + issuer + " \n subject claim - " + subjectClaim);
-            }
-            else{
-                throw new Error("Failed to fetch federated token.");
-            }
-        }
-
-        // Attempting Az cli login
-        if (environment == "azurestack") {
-            if (!resourceManagerEndpointUrl) {
-                throw new Error("resourceManagerEndpointUrl is a required parameter when environment is defined.");
-            }
-
-            console.log(`Unregistering cloud: "${environment}" first if it exists`);
-            try {
-                await executeAzCliCommand(`cloud set -n AzureCloud`, true);
-                await executeAzCliCommand(`cloud unregister -n "${environment}"`, false);
-            }
-            catch (error) {
-                console.log(`Ignore cloud not registered error: "${error}"`);
-            }
-
-            console.log(`Registering cloud: "${environment}" with ARM endpoint: "${resourceManagerEndpointUrl}"`);
-            try {
-                let baseUri = resourceManagerEndpointUrl;
-                if (baseUri.endsWith('/')) {
-                    baseUri = baseUri.substring(0, baseUri.length - 1); // need to remove trailing / from resourceManagerEndpointUrl to correctly derive suffixes below
-                }
-                let suffixKeyvault = ".vault" + baseUri.substring(baseUri.indexOf('.')); // keyvault suffix starts with .
-                let suffixStorage = baseUri.substring(baseUri.indexOf('.') + 1); // storage suffix starts without .
-                let profileVersion = "2019-03-01-hybrid";
-                await executeAzCliCommand(`cloud register -n "${environment}" --endpoint-resource-manager "${resourceManagerEndpointUrl}" --suffix-keyvault-dns "${suffixKeyvault}" --suffix-storage-endpoint "${suffixStorage}" --profile "${profileVersion}"`, false);
-            }
-            catch (error) {
-                core.error(`Error while trying to register cloud "${environment}": "${error}"`);
-            }
-
-            console.log(`Done registering cloud: "${environment}"`)
-        }
-
-        await executeAzCliCommand(`cloud set -n "${environment}"`, false);
-        console.log(`Done setting cloud: "${environment}"`);
-
-        // Attempting Az cli login
-        var commonArgs = ["--service-principal",
-            "-u", servicePrincipalId,
-            "--tenant", tenantId
-        ];
-        if (allowNoSubscriptionsLogin) {
-            commonArgs = commonArgs.concat("--allow-no-subscriptions");
-        }
-        if (enableOIDC) {
-            commonArgs = commonArgs.concat("--federated-token", federatedToken);
-        }
-        else {
-            console.log("Note: Azure/login action also supports OIDC login mechanism. Refer https://github.com/azure/login#configure-a-service-principal-with-a-federated-credential-to-use-oidc-based-authentication for more details.")
-            commonArgs = commonArgs.concat(`--password=${servicePrincipalKey}`);
-        }
-        await executeAzCliCommand(`login`, true, loginOptions, commonArgs);
-
-        if (!allowNoSubscriptionsLogin) {
-            var args = [
-                "--subscription",
-                subscriptionId
-            ];
-            await executeAzCliCommand(`account set`, true, loginOptions, args);
-        }
+        // login to Azure Cli
+        var cliLogin = new AzureCliLogin(loginConfig);
+        await cliLogin.login();
         isAzCLISuccess = true;
-        if (enableAzPSSession) {
-            // Attempting Az PS login
-            console.log(`Running Azure PS Login`);
-            var spnlogin: ServicePrincipalLogin;
 
-            spnlogin = new ServicePrincipalLogin(
-                servicePrincipalId,
-                servicePrincipalKey,
-                federatedToken,
-                tenantId,
-                subscriptionId,
-                allowNoSubscriptionsLogin,
-                environment,
-                resourceManagerEndpointUrl);
+        //login to Azure PowerShell
+        if (loginConfig.enableAzPSSession) {
+            console.log(`Running Azure PS Login`);
+            var spnlogin: ServicePrincipalLogin = new ServicePrincipalLogin(loginConfig);
             await spnlogin.initialize();
             await spnlogin.login();
         }
@@ -224,18 +51,4 @@ async function main() {
     }
 }
 
-async function executeAzCliCommand(
-    command: string,
-    silent?: boolean,
-    execOptions: any = {},
-    args: any = []) {
-    execOptions.silent = !!silent;
-    await exec.exec(`"${azPath}" ${command}`, args, execOptions);
-}
-async function jwtParser(federatedToken: string) {
-    let tokenPayload = federatedToken.split('.')[1];
-    let bufferObj = Buffer.from(tokenPayload, "base64");
-    let decodedPayload = JSON.parse(bufferObj.toString("utf8"));
-    return [decodedPayload['iss'], decodedPayload['sub']];
-}
 main();