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.

Sign up for GitHub

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

Jump to bottom

[DPLT-1049] Prevent modification of other schemas during provisioning #114

Closed
wants to merge 3 commits into from
Closed

[DPLT-1049] Prevent modification of other schemas during provisioning #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b5e0c72
refactor: Merge schema creation/migration to single request
morgsmccauley Jun 28, 2023
a5990e8
feat: Prevent modification of schemas unrelated to current indexer
morgsmccauley Jun 28, 2023
a42aada
refactor: Rename `roleName`->`hasuraRoleName` to avoid confusion with…
morgsmccauley Jun 28, 2023
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
34 changes: 21 additions & 13 deletions indexer-js-queue-handler/__snapshots__/hasura-client.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,6 @@ exports[`HasuraClient checks if a schema exists 1`] = `
}
`;

exports[`HasuraClient creates a schema 1`] = `
{
"args": {
"read_only": false,
"source": "default",
"sql": "CREATE schema name",
},
"type": "run_sql",
}
`;

exports[`HasuraClient gets table names within a schema 1`] = `
{
"args": {
Expand All @@ -117,8 +106,27 @@ exports[`HasuraClient runs migrations for the specified schema 1`] = `
"read_only": false,
"source": "default",
"sql": "
set schema 'schema';
CREATE TABLE blocks (height numeric)
-- Create the role for the indexer
CREATE ROLE schema_role;

-- Create the schema and assign its ownership to the indexer role
CREATE SCHEMA schema AUTHORIZATION schema_role;

-- Grant necessary privileges to the indexer role
GRANT USAGE ON SCHEMA schema TO schema_role;
ALTER DEFAULT PRIVILEGES IN SCHEMA schema GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO schema_role;

-- Allow the role to create tables
GRANT CREATE ON SCHEMA schema TO schema_role;

-- Switch to the indexer role and schema
SET ROLE schema_role;
SET SCHEMA 'schema';

-- indexer provided migration
CREATE TABLE blocks (height numeric);

RESET ROLE;
",
},
"type": "run_sql",
Expand Down
32 changes: 23 additions & 9 deletions indexer-js-queue-handler/hasura-client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,32 @@ export default class HasuraClient {
return result.length > 1;
};

createSchema (schemaName) {
return this.executeSql(
`CREATE schema ${schemaName}`,
{ readOnly: false }
);
}

runMigrations(schemaName, migration) {
const roleName = `${schemaName}_role`;

return this.executeSql(
`
set schema '${schemaName}';
${migration}
-- Create the role for the indexer
CREATE ROLE ${roleName};

-- Create the schema and assign its ownership to the indexer role
CREATE SCHEMA ${schemaName} AUTHORIZATION ${roleName};

-- Grant necessary privileges to the indexer role
GRANT USAGE ON SCHEMA ${schemaName} TO ${roleName};
ALTER DEFAULT PRIVILEGES IN SCHEMA ${schemaName} GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO ${roleName};

-- Allow the role to create tables
GRANT CREATE ON SCHEMA ${schemaName} TO ${roleName};

-- Switch to the indexer role and schema
SET ROLE ${roleName};
SET SCHEMA '${schemaName}';

-- indexer provided migration
${migration};
Copy link
Collaborator

@gabehamilton gabehamilton Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can this be attacked by calling

SET ROLE someotheruser;
SET SCHEMA otherschema

at the start of a migration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just checked, yes it can.. let me try and figure out how to prevent that

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could disallow SET in schema. There may be other clever syntaxes to do the same thing but it'd be a start. Keeping people from setting config sounds like a good idea as well.


RESET ROLE;
`,
{ readOnly: false }
);
Expand Down
15 changes: 0 additions & 15 deletions indexer-js-queue-handler/hasura-client.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,6 @@ describe('HasuraClient', () => {
process.env = oldEnv;
});

it('creates a schema', async () => {
const fetch = jest
.fn()
.mockResolvedValue({
status: 200,
text: () => JSON.stringify({})
});
const client = new HasuraClient({ fetch })

await client.createSchema('name');

expect(fetch.mock.calls[0][1].headers['X-Hasura-Admin-Secret']).toBe(HASURA_ADMIN_SECRET)
expect(JSON.parse(fetch.mock.calls[0][1].body)).toMatchSnapshot();
});

it('checks if a schema exists', async () => {
const fetch = jest
.fn()
Expand Down
20 changes: 5 additions & 15 deletions indexer-js-queue-handler/provisioner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ export default class Provisioner {
return this.hasuraClient.isSchemaCreated(schemaName);
}

async createSchema(schemaName) {
try {
await this.hasuraClient.createSchema(schemaName);
} catch (error) {
throw new VError(error, `Failed to create schema`);
}
}

async runMigrations(schemaName, migration) {
try {
await this.hasuraClient.runMigrations(schemaName, migration);
Expand All @@ -45,12 +37,12 @@ export default class Provisioner {
}
}

async addPermissionsToTables(schemaName, tableNames, roleName, permissions) {
async addPermissionsToTables(schemaName, tableNames, hasuraRoleName, permissions) {
try {
await this.hasuraClient.addPermissionsToTables(
schemaName,
tableNames,
roleName,
hasuraRoleName,
['select', 'insert', 'update', 'delete']
);
} catch (error) {
Expand All @@ -66,25 +58,23 @@ export default class Provisioner {
}
}

async createAuthenticatedEndpoint(schemaName, roleName, migration) {
async createAuthenticatedEndpoint(schemaName, hasuraRoleName, migration) {
try {
await this.createSchema(schemaName);

await this.runMigrations(schemaName, migration);

const tableNames = await this.getTableNames(schemaName);
await this.trackTables(schemaName, tableNames);

await this.trackForeignKeyRelationships(schemaName);

await this.addPermissionsToTables(schemaName, tableNames, roleName, ['select', 'insert', 'update', 'delete']);
await this.addPermissionsToTables(schemaName, tableNames, hasuraRoleName, ['select', 'insert', 'update', 'delete']);
} catch (error) {
throw new VError(
{
cause: error,
info: {
schemaName,
roleName,
hasuraRoleName,
migration,
}
},
Expand Down
37 changes: 8 additions & 29 deletions indexer-js-queue-handler/provisioner.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('Provision', () => {
it('creates an authenticated endpoint', async () => {
const tableNames = ['blocks'];
const hasuraClient = {
createSchema: jest.fn().mockReturnValueOnce(),
runMigrations: jest.fn().mockReturnValueOnce(),
getTableNames: jest.fn().mockReturnValueOnce(tableNames),
trackTables: jest.fn().mockReturnValueOnce(),
Expand All @@ -26,18 +25,17 @@ describe('Provision', () => {
const provisioner = new Provisioner(hasuraClient);

const schemaName = 'schema';
const roleName = 'role';
const hasuraRoleName = 'role';
const migration = 'CREATE TABLE blocks (height numeric)';
await provisioner.createAuthenticatedEndpoint(schemaName, roleName, migration);
await provisioner.createAuthenticatedEndpoint(schemaName, hasuraRoleName, migration);

expect(hasuraClient.createSchema).toBeCalledWith(schemaName);
expect(hasuraClient.runMigrations).toBeCalledWith(schemaName, migration);
expect(hasuraClient.getTableNames).toBeCalledWith(schemaName);
expect(hasuraClient.trackTables).toBeCalledWith(schemaName, tableNames);
expect(hasuraClient.addPermissionsToTables).toBeCalledWith(
schemaName,
tableNames,
roleName,
hasuraRoleName,
[
'select',
'insert',
Expand All @@ -47,25 +45,6 @@ describe('Provision', () => {
);
});

it('throws an error when it fails to create the schema', async () => {
const error = new Error('some http error');
const hasuraClient = {
createSchema: jest.fn().mockRejectedValue(error),
};
const provisioner = new Provisioner(hasuraClient);

try {
await provisioner.createAuthenticatedEndpoint('name', 'role', 'CREATE TABLE blocks (height numeric)')
} catch (error) {
expect(error.message).toBe('Failed to provision endpoint: Failed to create schema: some http error');
expect(VError.info(error)).toEqual({
schemaName: 'name',
roleName: 'role',
migration: 'CREATE TABLE blocks (height numeric)',
});
}
});

it('throws an error when it fails to run migrations', async () => {
const error = new Error('some http error');
const hasuraClient = {
Expand All @@ -80,7 +59,7 @@ describe('Provision', () => {
expect(error.message).toBe('Failed to provision endpoint: Failed to run migrations: some http error');
expect(VError.info(error)).toEqual({
schemaName: 'name',
roleName: 'role',
hasuraRoleName: 'role',
migration: 'CREATE TABLE blocks (height numeric)',
});
}
Expand All @@ -101,7 +80,7 @@ describe('Provision', () => {
expect(error.message).toBe('Failed to provision endpoint: Failed to fetch table names: some http error');
expect(VError.info(error)).toEqual({
schemaName: 'name',
roleName: 'role',
hasuraRoleName: 'role',
migration: 'CREATE TABLE blocks (height numeric)',
});
}
Expand All @@ -124,7 +103,7 @@ describe('Provision', () => {
expect(error.message).toBe('Failed to provision endpoint: Failed to track tables: some http error');
expect(VError.info(error)).toEqual({
schemaName: 'name',
roleName: 'role',
hasuraRoleName: 'role',
migration: 'CREATE TABLE blocks (height numeric)',
});
}
Expand All @@ -148,7 +127,7 @@ describe('Provision', () => {
expect(error.message).toBe('Failed to provision endpoint: Failed to track foreign key relationships: some http error');
expect(VError.info(error)).toEqual({
schemaName: 'name',
roleName: 'role',
hasuraRoleName: 'role',
migration: 'CREATE TABLE blocks (height numeric)',
});
}
Expand All @@ -174,7 +153,7 @@ describe('Provision', () => {
expect(VError.info(error)).toEqual({
migration: 'CREATE TABLE blocks (height numeric)',
schemaName: 'name',
roleName: 'role',
hasuraRoleName: 'role',
});
}
});
Expand Down