diff --git a/.github/workflows/azure-login-negative.yml b/.github/workflows/azure-login-negative.yml index 3cb37d5e6..21bed1f0e 100644 --- a/.github/workflows/azure-login-negative.yml +++ b/.github/workflows/azure-login-negative.yml @@ -347,7 +347,12 @@ jobs: script: | core.setFailed('Last action should fail but not. Please check it.') - # Secret of SP1 in creds will be used to sign in SP2 + # logout first to avoid the conflict with SP1 + - name: Azure CLI logout + run: | + az logout + + # SP1 is ignored and SP2 will be used for login, but it will fail since SP2 has no access to the given subscription - name: Login with both creds and individual parameters id: login_12 continue-on-error: true @@ -382,6 +387,37 @@ jobs: with: script: | core.setFailed('Last action should fail but not. Please check it.') + + - name: Login with individual parameters, no subscription-id, no allow-no-subscriptions + id: login_14 + continue-on-error: true + uses: ./ + with: + client-id: ${{ secrets.OIDC_SP2_CLIENT_ID }} + tenant-id: ${{ secrets.OIDC_SP2_TENANT_ID }} + enable-AzPSSession: true + + - name: Check Last step failed + if: steps.login_14.outcome == 'success' + uses: actions/github-script@v3 + with: + script: | + core.setFailed('Last action should fail but not. Please check it.') + + - name: Login with creds, no subscription-id, no allow-no-subscriptions + id: login_15 + continue-on-error: true + uses: ./ + with: + creds: '{"clientId":"${{ secrets.OIDC_SP2_CLIENT_ID }}","clientSecret":"${{ secrets.SP2_CLIENT_SECRET }}","tenantId":"${{ secrets.OIDC_SP2_TENANT_ID }}"}' + enable-AzPSSession: true + + - name: Check Last step failed + if: steps.login_15.outcome == 'success' + uses: actions/github-script@v3 + with: + script: | + core.setFailed('Last action should fail but not. Please check it.') VMTest: strategy: diff --git a/.github/workflows/azure-login-positive.yml b/.github/workflows/azure-login-positive.yml index 8d5581803..fdcca1517 100644 --- a/.github/workflows/azure-login-positive.yml +++ b/.github/workflows/azure-login-positive.yml @@ -61,10 +61,9 @@ jobs: - name: Login with individual parameters uses: ./ with: - client-id: ${{ secrets.OIDC_SP2_CLIENT_ID }} - tenant-id: ${{ secrets.OIDC_SP2_TENANT_ID }} - # subscription-id: ${{ secrets.OIDC_SP2_SUBSCRIPTION_ID }} - allow-no-subscriptions: true + client-id: ${{ secrets.SP1_CLIENT_ID }} + tenant-id: ${{ secrets.SP1_TENANT_ID }} + subscription-id: ${{ secrets.SP1_SUBSCRIPTION_ID }} enable-AzPSSession: true - name: Run Azure Cli again @@ -176,6 +175,29 @@ jobs: throw "Not all checks passed!" } + - name: Login with individual parameters, allow no subscription + uses: ./ + with: + client-id: ${{ secrets.SP1_CLIENT_ID }} + tenant-id: ${{ secrets.SP1_TENANT_ID}} + subscription-id: ${{ secrets.SP1_SUBSCRIPTION_ID }} + allow-no-subscriptions: true + enable-AzPSSession: true + + - name: Run Azure Cli again + run: | + az account show --output none + + - name: Run Azure PowerShell again + uses: azure/powershell@v1.2.0 + with: + azPSVersion: "latest" + inlineScript: | + $checkResult = (Get-AzContext).Environment.Name -eq 'AzureCloud' + if(-not $checkResult){ + throw "Not all checks passed!" + } + - name: Login with individual parameters, no subscription, allow no subscription uses: ./ with: @@ -198,6 +220,27 @@ jobs: throw "Not all checks passed!" } + - name: Login with creds, no subscription, allow no subscription + uses: ./ + with: + creds: '{"clientId":"${{ secrets.OIDC_SP2_CLIENT_ID }}","clientSecret":"${{ secrets.SP2_CLIENT_SECRET }}","tenantId":"${{ secrets.OIDC_SP2_TENANT_ID }}"}' + allow-no-subscriptions: true + enable-AzPSSession: true + + - name: Run Azure Cli + run: | + az account show --output none + + - name: Run Azure PowerShell + uses: azure/powershell@v1.2.0 + with: + azPSVersion: "latest" + inlineScript: | + $checkResult = (Get-AzContext).Environment.Name -eq 'AzureCloud' + if(-not $checkResult){ + throw "Not all checks passed!" + } + VMTest: strategy: matrix: diff --git a/__tests__/LoginConfig.test.ts b/__tests__/LoginConfig.test.ts index 855af818a..0f75f42d3 100644 --- a/__tests__/LoginConfig.test.ts +++ b/__tests__/LoginConfig.test.ts @@ -74,16 +74,29 @@ describe("LoginConfig Test", () => { await testCreds(creds1); }); - - test('initialize with creds, lack of subscriptionId', async () => { + + test('initialize with creds, lack of subscriptionId, but allowNoSubscriptionsLogin=true', async () => { let creds1 = { 'clientId': 'client-id', 'clientSecret': 'client-secret', 'tenantId': 'tenant-id', // 'subscriptionId': 'subscription-id' } - await testCreds(creds1); - + setEnv('environment', 'azurecloud'); + setEnv('enable-AzPSSession', 'true'); + setEnv('allow-no-subscriptions', 'true'); + setEnv('auth-type', 'SERVICE_PRINCIPAL'); + setEnv('creds', JSON.stringify(creds1)); + let loginConfig = new LoginConfig(); + await loginConfig.initialize(); + expect(loginConfig.environment).toBe("azurecloud"); + expect(loginConfig.enableAzPSSession).toBeTruthy(); + expect(loginConfig.allowNoSubscriptionsLogin).toBeTruthy(); + expect(loginConfig.authType).toBe("SERVICE_PRINCIPAL"); + expect(loginConfig.servicePrincipalId).toBe("client-id"); + expect(loginConfig.servicePrincipalSecret).toBe("client-secret"); + expect(loginConfig.tenantId).toBe("tenant-id"); + expect(loginConfig.subscriptionId).toBe(""); }); test('initialize with creds', async () => { diff --git a/src/Cli/AzureCliLogin.ts b/src/Cli/AzureCliLogin.ts index 03a86f201..fc67fbffc 100644 --- a/src/Cli/AzureCliLogin.ts +++ b/src/Cli/AzureCliLogin.ts @@ -126,14 +126,13 @@ export class AzureCliLogin { args.push("--allow-no-subscriptions"); } await this.executeAzCliCommand(args, true, this.loginOptions); - await this.setSubscription(); + if (this.loginConfig.subscriptionId) { + await this.setSubscription(); + } core.info(`Azure CLI login succeeds by using ${methodName}.`); } async setSubscription() { - if (this.loginConfig.allowNoSubscriptionsLogin) { - return; - } let args = ["account", "set", "--subscription", this.loginConfig.subscriptionId]; await this.executeAzCliCommand(args, true, this.loginOptions); core.info("Subscription is set successfully."); @@ -160,7 +159,7 @@ function defaultExecOptions(): exec.ExecOptions { if (error && error.trim().length !== 0 && !startsWithWarning) { if (startsWithError) { //removing the keyword 'ERROR' to avoid duplicates while throwing error - error = error.slice(5); + error = error.slice(7); } core.error(error); } diff --git a/src/common/LoginConfig.ts b/src/common/LoginConfig.ts index be03bb602..c46860593 100644 --- a/src/common/LoginConfig.ts +++ b/src/common/LoginConfig.ts @@ -69,8 +69,8 @@ export class LoginConfig { this.tenantId = this.tenantId ? this.tenantId : secrets.getSecret("$.tenantId", false); this.subscriptionId = this.subscriptionId ? this.subscriptionId : secrets.getSecret("$.subscriptionId", false); this.resourceManagerEndpointUrl = secrets.getSecret("$.resourceManagerEndpointUrl", false); - if (!this.servicePrincipalId || !this.servicePrincipalSecret || !this.tenantId || !this.subscriptionId) { - throw new Error("Not all parameters are provided in 'creds'. Double-check if all keys are defined in 'creds': 'clientId', 'clientSecret', 'subscriptionId', 'tenantId'."); + if (!this.servicePrincipalId || !this.servicePrincipalSecret || !this.tenantId) { + throw new Error("Not all parameters are provided in 'creds'. Double-check if all keys are defined in 'creds': 'clientId', 'clientSecret', 'tenantId'."); } }