Skip to content
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

v2 - Add support for AAD password, AAD default, and AAD service principal auth #100

Merged
merged 38 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
8f5773a
Use tedious mssql library instead of sqlcmd
zijchen May 20, 2022
ad0198a
Fix mssql connection
zijchen May 23, 2022
499ffe5
Fix SqlUtils tests
zijchen May 24, 2022
deb020e
Use config instead of connection string
zijchen May 31, 2022
fceb850
Replace conn string builder with mssql config
zijchen May 31, 2022
01f886e
Connect to master db
zijchen May 31, 2022
203615f
Restore connection string validation regex
zijchen Jun 1, 2022
5ed7191
AAD auth
zijchen Jun 2, 2022
8f7e533
Add support for client and tenant IDs
zijchen Jun 3, 2022
6160da5
Add more debug messaging
zijchen Jun 3, 2022
3687029
Fix connection string find array
zijchen Jun 3, 2022
f3754ce
Make client-id and tenant-id action inputs
zijchen Jun 4, 2022
ed579aa
Fix error handling
zijchen Jun 4, 2022
d5def40
More fixes
zijchen Jun 4, 2022
42984b4
Use try catch instead
zijchen Jun 4, 2022
f0257b0
Merge remote-tracking branch 'origin/v2' into zijchen/aad
zijchen Jun 7, 2022
45ef62a
Add tests to pr-check.yml
zijchen Jun 7, 2022
c4b876c
Change script action from sqlcmd to mssql query
zijchen Jun 8, 2022
123e0b7
Update action.yml
zijchen Jun 8, 2022
ae18e5e
Fully qualify Table1 in sql script
zijchen Jun 9, 2022
a10e6a3
Add more debug logging
zijchen Jun 9, 2022
191dd56
Clone config before changing db to master
zijchen Jun 9, 2022
ec36260
Cleanup
zijchen Jun 9, 2022
7853d01
Set TEST_DB name before cleanup
zijchen Jun 9, 2022
da83f88
Use runner.temp
zijchen Jun 9, 2022
ef3752a
Always cleanup
zijchen Jun 9, 2022
704bb4e
Merge branch 'zijchen/sqlcmd' into zijchen/aad
zijchen Jun 13, 2022
5d76466
Add tests for different auth types
zijchen Jun 14, 2022
c12cba5
Merge branch 'v2' into zijchen/aad
zijchen Jun 14, 2022
416417e
Mask tenant and client IDs
zijchen Jun 15, 2022
f1bbe17
Add AAD password test to pr-check.yml
zijchen Jun 15, 2022
19bfc52
Merge branch 'zijchen/aad' of https://github.com/Azure/sql-action int…
zijchen Jun 15, 2022
e9f3012
Fix yaml
zijchen Jun 15, 2022
1f18a04
Limit max-parallel to 2
zijchen Jun 15, 2022
4a6c162
Add test for service principal
zijchen Jun 16, 2022
6ce3efd
Merge branch 'v2' into zijchen/aad
zijchen Jun 21, 2022
f910523
PR comments
zijchen Jun 21, 2022
68aa3d6
Fix typo
zijchen Jun 23, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions .github/workflows/pr-check.yml
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in a future PR I'll rework this so each part of the connection string is its own secret and we build the connection string at runtime. Right now you'll just have to trust that I didn't specify the same connection string 3 times 😆

- 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
140 changes: 123 additions & 17 deletions __tests__/SqlConnectionConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import SqlConnectionConfig from '../src/SqlConnectionConfig';
jest.mock('@actions/core');

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

describe('validate correct connection strings', () => {
const validConnectionStrings = [
Expand All @@ -13,7 +16,10 @@ describe('SqlConnectionConfig tests', () => {
[`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`]
[`User Id=user;Password=JustANormal123@#$password;Server=test1.database.windows.net;Initial catalog=testdb`, 'validates connection string without server', `JustANormal123@#$password`],
zijchen marked this conversation as resolved.
Show resolved Hide resolved
zijchen marked this conversation as resolved.
Show resolved Hide resolved
[`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 +35,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 this link for more info on conneciton string https://aka.ms/sqlconnectionstring`, 'validates values beginning with double quotes but not ending with double quotes'],
zijchen marked this conversation as resolved.
Show resolved Hide resolved
[`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 this link for more info on conneciton 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 this link for more info on conneciton 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 this link for more info on conneciton 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 this link for more info on conneciton 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