Skip to content

Commit

Permalink
v2 - Add support for AAD password, AAD default, and AAD service princ…
Browse files Browse the repository at this point in the history 
…ipal auth (#100)

* Use tedious mssql library instead of sqlcmd

* Fix mssql connection

* Fix SqlUtils tests

* Use config instead of connection string

* Replace conn string builder with mssql config

* Connect to master db

* Restore connection string validation regex

* AAD auth

* Add support for client and tenant IDs

* Add more debug messaging

* Fix connection string find array

* Make client-id and tenant-id action inputs

* Fix error handling

* More fixes

* Use try catch instead

* Add tests to pr-check.yml

* Change script action from sqlcmd to mssql query

* Update action.yml

* Fully qualify Table1 in sql script

* Add more debug logging

* Clone config before changing db to master

* Cleanup

* Set TEST_DB name before cleanup

* Use runner.temp

* Always cleanup

* Add tests for different auth types

* Mask tenant and client IDs

* Add AAD password test to pr-check.yml

* Fix yaml

* Limit max-parallel to 2

* Add test for service principal

* PR comments

* Fix typo
  • Loading branch information
@zijchen
zijchen authored Jun 27, 2022
1 parent e87bd5b commit 8f95c0f
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 37 deletions.
24 changes: 19 additions & 5 deletions .github/workflows/pr-check.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,18 @@ jobs:
strategy:
matrix:
os: [windows-latest, ubuntu-latest]
auth_type: [sql_login, aad_password, service_principal] # Unique for each parallel job run, part of TEST_DB name to ensure no collisions from parallel jobs
include:
# Includes the connection string Github secret name, effectively a switch statement on auth_type
- auth_type: sql_login
connection_string_secret: AZURE_SQL_CONNECTION_STRING_NO_DATABASE
- auth_type: aad_password
connection_string_secret: AAD_PASSWORD_CONNECTION_STRING_NO_DATABASE
- auth_type: service_principal
connection_string_secret: SERVICE_PRINCIPAL_CONNECTION_STRING_NO_DATABASE

env:
TEST_DB: 'SqlActionTest-${{ matrix.os }}'
TEST_DB: 'SqlActionTest-${{ matrix.os }}-${{ matrix.auth_type }}'

steps:
- name: Checkout from PR branch
Expand All @@ -42,21 +52,24 @@ jobs:
- name: Test DACPAC Action
uses: ./
with:
connection-string: '${{ secrets.AZURE_SQL_CONNECTION_STRING_NO_DATABASE }}Initial Catalog=${{ env.TEST_DB }};'
connection-string: '${{ secrets[matrix.connection_string_secret] }}Initial Catalog=${{ env.TEST_DB }};'
tenant-id: '${{ secrets.AAD_TENANT_ID }}'
dacpac-package: ./__testdata__/sql-action.dacpac

# Build and publish sqlproj that should create a new view
- name: Test Build and Publish
uses: ./
with:
connection-string: '${{ secrets.AZURE_SQL_CONNECTION_STRING_NO_DATABASE }}Initial Catalog=${{ env.TEST_DB }};'
connection-string: '${{ secrets[matrix.connection_string_secret] }}Initial Catalog=${{ env.TEST_DB }};'
tenant-id: '${{ secrets.AAD_TENANT_ID }}'
project-file: ./__testdata__/TestProject/sql-action.sqlproj

# Execute testsql.sql via SQLCMD on server
- name: Test SQL Action
uses: ./
with:
connection-string: '${{ secrets.AZURE_SQL_CONNECTION_STRING_NO_DATABASE }}Initial Catalog=${{ env.TEST_DB }};'
connection-string: '${{ secrets[matrix.connection_string_secret] }}Initial Catalog=${{ env.TEST_DB }};'
tenant-id: '${{ secrets.AAD_TENANT_ID }}'
sql-file: ./__testdata__/testsql.sql

- name: Set database name for cleanup
Expand All @@ -67,5 +80,6 @@ jobs:
if: always()
uses: ./
with:
connection-string: '${{ secrets.AZURE_SQL_CONNECTION_STRING_NO_DATABASE }}Initial Catalog=master;'
connection-string: '${{ secrets[matrix.connection_string_secret] }}Initial Catalog=master;'
tenant-id: '${{ secrets.AAD_TENANT_ID }}'
sql-file: ${{ runner.temp }}/cleanup.sql
141 changes: 123 additions & 18 deletions __tests__/SqlConnectionConfig.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@ import SqlConnectionConfig from '../src/SqlConnectionConfig';
jest.mock('@actions/core');

describe('SqlConnectionConfig tests', () => {
afterEach(() => {
jest.restoreAllMocks();
});

describe('validate correct connection strings', () => {
const validConnectionStrings = [
[`Server=test1.database.windows.net;User Id=user;Password="ab'=abcdf''c;123";Initial catalog=testdb`, 'validates values enclosed with double quotes ', `ab'=abcdf''c;123`],
[`Server=test1.database.windows.net;User Id=user;Password='abc;1""2"adf=33';Initial catalog=testdb`, 'validates values enclosed with single quotes ', `abc;1""2"adf=33`],
[`Server=test1.database.windows.net;User Id=user;Password="abc;1""2""adf(012j^72''asj;')'=33";Initial catalog=testdb`, 'validates values beginning with double quotes and also contains escaped double quotes', `abc;1"2"adf(012j^72''asj;')'=33`],
[`Server=test1.database.windows.net;User Id=user;Password='ab""c;1''2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, 'validates values beginning with single quotes and also contains escaped single quotes', `ab""c;1'2'"'adf("0""12j^72'asj;')'=33`],
[`Server=test1.database.windows.net;User Id=user;Password=JustANormal123@#$password;Initial catalog=testdb`, 'validates values not beginning quotes and not containing quotes or semi-colon', `JustANormal123@#$password`],
[`User Id=user;Password=JustANormal123@#$password;Server=test1.database.windows.net;Initial catalog=testdb`, 'validates connection string without server', `JustANormal123@#$password`]
[`Server=test1.database.windows.net;User Id=user;Password=placeholder;Initial catalog=testdb`, 'validates values not beginning quotes and not containing quotes or semi-colon', `placeholder`],
[`Server=test1.database.windows.net;Database=testdb;User Id=user;Password=placeholder;Authentication=SQL Password`, 'validates SQL password authentication', `placeholder`],
[`Server=test1.database.windows.net;Database=testdb;User Id=user;Password=placeholder;Authentication=SQLPassword`, 'validates SQL password authentication with one word', `placeholder`],
[`Server=test1.database.windows.net;Database=testdb;User Id=user;Password=placeholder;Authentication='SQL Password'`, 'validates SQL password authentication with quotes', `placeholder`],
];

it.each(validConnectionStrings)('Input `%s` %s', (connectionStringInput, testDescription, passwordOutput) => {
Expand All @@ -29,30 +34,130 @@ describe('SqlConnectionConfig tests', () => {

describe('throw for invalid connection strings', () => {
const invalidConnectionStrings = [
[`Server=test1.database.windows.net;User Id=user;Password="ab'=abcdf''c;123;Initial catalog=testdb`, 'validates values beginning with double quotes but not ending with double quotes'],
[`Server=test1.database.windows.net;User Id=user;Password='abc;1""2"adf=33;Initial catalog=testdb`, 'validates values beginning with single quote but not ending with single quote'],
[`Server=test1.database.windows.net;User Id=user;Password="abc;1""2"adf(012j^72''asj;')'=33";Initial catalog=testdb`, 'validates values enclosed in double quotes but does not escape double quotes in between'],
[`Server=test1.database.windows.net;User Id=user;Password='ab""c;1'2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, 'validates values enclosed in single quotes but does not escape single quotes in between'],
[`Server=test1.database.windows.net;User Id=user;Password=NotANormal123@;#$password;Initial catalog=testdb`, 'validates values not enclosed in quotes and containing semi-colon'],
[`Server=test1.database.windows.net;Password=password;Initial catalog=testdb`, 'missing user id'],
[`Server=test1.database.windows.net;User Id=user;Initial catalog=testdb`, 'missing password'],
[`Server=test1.database.windows.net;User Id=user;Password=password;`, 'missing initial catalog']
[`Server=test1.database.windows.net;User Id=user;Password="ab'=abcdf''c;123;Initial catalog=testdb`, `Invalid connection string. A valid connection string is a series of keyword/value pairs separated by semi-colons. If there are any special characters like quotes or semi-colons in the keyword value, enclose the value within quotes. Refer to this link for more info on connection string https://aka.ms/sqlconnectionstring`, 'validates values beginning with double quotes but not ending with double quotes'],
[`Server=test1.database.windows.net;User Id=user;Password='abc;1""2"adf=33;Initial catalog=testdb`, `Invalid connection string. A valid connection string is a series of keyword/value pairs separated by semi-colons. If there are any special characters like quotes or semi-colons in the keyword value, enclose the value within quotes. Refer to this link for more info on connection string https://aka.ms/sqlconnectionstring`, 'validates values beginning with single quote but not ending with single quote'],
[`Server=test1.database.windows.net;User Id=user;Password="abc;1""2"adf(012j^72''asj;')'=33";Initial catalog=testdb`, `Invalid connection string. A valid connection string is a series of keyword/value pairs separated by semi-colons. If there are any special characters like quotes or semi-colons in the keyword value, enclose the value within quotes. Refer to this link for more info on connection string https://aka.ms/sqlconnectionstring`, 'validates values enclosed in double quotes but does not escape double quotes in between'],
[`Server=test1.database.windows.net;User Id=user;Password='ab""c;1'2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, `Invalid connection string. A valid connection string is a series of keyword/value pairs separated by semi-colons. If there are any special characters like quotes or semi-colons in the keyword value, enclose the value within quotes. Refer to this link for more info on connection string https://aka.ms/sqlconnectionstring`, 'validates values enclosed in single quotes but does not escape single quotes in between'],
[`Server=test1.database.windows.net;User Id=user;Password=NotANormal123@;#$password;Initial catalog=testdb`, `Invalid connection string. A valid connection string is a series of keyword/value pairs separated by semi-colons. If there are any special characters like quotes or semi-colons in the keyword value, enclose the value within quotes. Refer to this link for more info on connection string https://aka.ms/sqlconnectionstring`, 'validates values not enclosed in quotes and containing semi-colon'],
[`Server=test1.database.windows.net;Password=password;Initial catalog=testdb`, `Invalid connection string. Please ensure 'User' or 'User ID' is provided in the connection string.`, 'missing user id'],
[`Server=test1.database.windows.net;User Id=user;Initial catalog=testdb`, `Invalid connection string. Please ensure 'Password' is provided in the connection string.`, 'missing password'],
[`Server=test1.database.windows.net;User Id=user;Password=password;`, `Invalid connection string. Please ensure 'Database' or 'Initial Catalog' is provided in the connection string.`, 'missing initial catalog'],
[`Server=test1.database.windows.net;Database=testdb;Authentication=Fake Auth Type;Password=password;`, `Authentication type 'Fake Auth Type' is not supported.`, 'Unsupported authentication type'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='Active Directory Password';Password=password;`, `Invalid connection string. Please ensure 'User' or 'User ID' is provided in the connection string.`, 'AAD password auth missing user'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='Active Directory Password';User Id=user;`, `Invalid connection string. Please ensure 'Password' is provided in the connection string.`, 'AAD password auth missing password'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='SQL Password';Password=password;`, `Invalid connection string. Please ensure 'User' or 'User ID' is provided in the connection string.`, 'SQL password auth missing user'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='SQL Password';User Id=user;`, `Invalid connection string. Please ensure 'Password' is provided in the connection string.`, 'SQL password auth missing password'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='ActiveDirectoryServicePrincipal';Password=placeholder;`, `Invalid connection string. Please ensure client ID is provided in the 'User' or 'User ID' field of the connection string.`, 'Service principal auth without client ID'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='ActiveDirectoryServicePrincipal';User Id=clientId;`, `Invalid connection string. Please ensure client secret is provided in the 'Password' field of the connection string.`, 'Service principal auth without client secret']
];

it.each(invalidConnectionStrings)('Input `%s` %s', (connectionString) => {
expect(() => new SqlConnectionConfig(connectionString)).toThrow();
it.each(invalidConnectionStrings)('Input `%s` %s', (connectionString, expectedError) => {
expect(() => new SqlConnectionConfig(connectionString)).toThrow(expectedError);
})
})

it('should mask connection string password', () => {
const setSecretSpy = jest.spyOn(core, 'setSecret');
new SqlConnectionConfig('User Id=user;Password=1234;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(setSecretSpy).toHaveBeenCalled();
});

it('should call into mssql module to parse connection string', () => {
const parseConnectionStringSpy = jest.spyOn(ConnectionPool, 'parseConnectionString');
new SqlConnectionConfig('User Id=user;Password=1234;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(parseConnectionStringSpy).toHaveBeenCalled();
});

it('should mask connection string password', () => {
const setSecretSpy = jest.spyOn(core, 'setSecret');
new SqlConnectionConfig('User Id=user;Password=placeholder;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(setSecretSpy).toHaveBeenCalledWith('placeholder');
});

it('should mask client id', () => {
const setSecretSpy = jest.spyOn(core, 'setSecret');
const getInputSpy = jest.spyOn(core, 'getInput').mockImplementation((name, options) => {
switch (name) {
case 'client-id': return 'testClientId';
default: return '';
}
});
new SqlConnectionConfig('User Id=user;Password=placeholder;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(getInputSpy).toHaveBeenCalled();
expect(setSecretSpy).toHaveBeenCalledWith('testClientId');
});

it('should mask tenant id', () => {
const setSecretSpy = jest.spyOn(core, 'setSecret');
const getInputSpy = jest.spyOn(core, 'getInput').mockImplementation((name, options) => {
switch (name) {
case 'tenant-id': return 'testTenantId';
default: return '';
}
});
new SqlConnectionConfig('User Id=user;Password=placeholder;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(getInputSpy).toHaveBeenCalled();
expect(setSecretSpy).toHaveBeenCalledWith('testTenantId');
});

describe('parse authentication in connection strings', () => {
// For ease of testing, all user/tenant IDs will be 'user' and password/secrets will be 'placeholder'
const connectionStrings = [
[`Server=test1.database.windows.net;Database=testdb;Authentication="Active Directory Password";User Id=user;Password="placeholder";`, 'azure-active-directory-password', 'Validates AAD password with double quotes'],
[`Server=test1.database.windows.net;Database=testdb;Authentication=Active Directory Password;User Id=user;Password="placeholder";`, 'azure-active-directory-password', 'Validates AAD password with no quotes'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='ActiveDirectoryPassword';User Id=user;Password="placeholder";`, 'azure-active-directory-password', 'Validates AAD password with one word'],
[`Server=test1.database.windows.net;Database=testdb;Authentication="Active Directory Service Principal";User Id=user;Password="placeholder";`, 'azure-active-directory-service-principal-secret', 'Validates AAD service principal with double quotes'],
[`Server=test1.database.windows.net;Database=testdb;Authentication=Active Directory Service Principal;User Id=user;Password="placeholder";`, 'azure-active-directory-service-principal-secret', 'Validates AAD service principal with single quotes'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='ActiveDirectoryServicePrincipal';User Id=user;Password="placeholder";`, 'azure-active-directory-service-principal-secret', 'Validates AAD service principal with one word'],
[`Server=test1.database.windows.net;Database=testdb;Authentication="Active Directory Default"`, 'azure-active-directory-default', 'Validates default AAD with double quotes'],
[`Server=test1.database.windows.net;Database=testdb;Authentication=Active Directory Default`, 'azure-active-directory-default', 'Validates default AAD with single quotes'],
[`Server=test1.database.windows.net;Database=testdb;Authentication='ActiveDirectoryDefault'`, 'azure-active-directory-default', 'Validates default AAD with one word'],
];

it.each(connectionStrings)('should parse different authentication types successfully', (connectionStringInput, expectedAuthType) => {
const config = new SqlConnectionConfig(connectionStringInput);

expect(config.Config.server).toMatch('test1.database.windows.net');
expect(config.Config.database).toMatch('testdb');
expect(config.ConnectionString).toMatch(connectionStringInput);
expect(config.Config['authentication']).toBeDefined();
expect(config.Config['authentication']!.type).toMatch(expectedAuthType);
expect(config.Config['authentication']!.options).toBeDefined();
switch (expectedAuthType) {
case 'azure-active-directory-password': {
expect(config.Config['authentication']!.options!.userName).toMatch('user');
expect(config.Config['authentication']!.options!.password).toMatch('placeholder');
break;
}
case 'azure-active-directory-service-principal-secret': {
expect(config.Config['authentication']!.options!.clientId).toMatch('user');
expect(config.Config['authentication']!.options!.clientSecret).toMatch('placeholder');
break;
}
case 'azure-active-directory-default': {
// AAD default uses environment variables, nothing needs to be passed in
break;
}
}
});
})

it('should include client and tenant IDs in AAD connection', () => {
const clientId = '00000000-0000-0000-0000-000000000000';
const tenantId = '11111111-1111-1111-1111-111111111111';
const getInputSpy = jest.spyOn(core, 'getInput').mockImplementation((name, options) => {
switch (name) {
case 'client-id': return clientId;
case 'tenant-id': return tenantId;
default: return '';
}
});

const config = new SqlConnectionConfig(`Server=test1.database.windows.net;Database=testdb;Authentication="Active Directory Password";User Id=user;Password="abcd"`);
expect(getInputSpy).toHaveBeenCalledTimes(2);
expect(config.Config.server).toMatch('test1.database.windows.net');
expect(config.Config.database).toMatch('testdb');
expect(config.Config['authentication']).toBeDefined();
expect(config.Config['authentication']!.type).toMatch('azure-active-directory-password');
expect(config.Config['authentication']!.options).toBeDefined();
expect(config.Config['authentication']!.options!.userName).toMatch('user');
expect(config.Config['authentication']!.options!.password).toMatch('abcd');
expect(config.Config['authentication']!.options!.clientId).toMatch(clientId);
expect(config.Config['authentication']!.options!.tenantId).toMatch(tenantId);
});

})
Loading

0 comments on commit 8f95c0f

Please sign in to comment.