Skip to content

Commit

Permalink
Cluster registration, deletion, and enableRegistrationUrl apis return…
Browse files Browse the repository at this point in the history
… sensitive values as separate headers object in addition to in the url query params. (razee-io#1331)

Using the url alone, with included authorization value (orgKey) is still possible, but now deprecated.
A future major release will make the breaking change that removes the query parameter from the URL and require use of the headers returned separately for authorized requests.
The use of headers for sensitive values (e.g. authorization values) will improve security.
  • Loading branch information
carrolp authored Jun 13, 2023
1 parent e86e5af commit 266402f
Show file tree
Hide file tree
Showing 11 changed files with 1,265 additions and 1,141 deletions.
7 changes: 6 additions & 1 deletion app/apollo/models/organization.local.schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,13 @@ OrganizationLocalSchema.statics.getRegistrationUrl = async function(org_id, cont
if (process.env.EXTERNAL_HOST) {
host = process.env.EXTERNAL_HOST;
}
const orgKey = bestOrgKey(org).key;
// Return the orgKey as both url query param and as header. Once all GUI/CLI/API clients are updated to recognize and use 'headers', the orgKey will be removed as a query parameter in a new major version (as it will be a breaking change).
return {
url: `${protocol}://${host}/api/install/razeedeploy-job?orgKey=${bestOrgKey(org).key}`,
url: `${protocol}://${host}/api/install/razeedeploy-job?orgKey=${orgKey}`,
headers: {
'razee-org-key': orgKey
}
};
};

Expand Down
112 changes: 68 additions & 44 deletions app/apollo/resolvers/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,30 @@ const { getRddArgs } = require('../../utils/rdd');


// Get the URL that returns yaml for cleaning up agents from a cluster
const getCleanupUrl = async (org_id, context) => {
const getCleanupDetails = async (org_id, context) => {
const { models } = context;
/*
Note: this code retrieves the _registration_ url and adds `command=remove`, but this results
in a URL with unneded params (e.g. orgkey). It could instead
in a URL with unneded params (e.g. most if not all of the rddArgs). It could instead
generate the `/api/cleanup/razeedeploy-job` url by:
- replacing `/install/` with `/cleanup/` and truncating query params (e.g. orgkey) before adding RDD Args.
- introducing a new `getCleanupUrl` for Organization models to implement (falling back to a different approach if not implemented).
- replacing `/install/` with `/cleanup/` and truncating query params.
- introducing a new `getCleanupDetails` for Organization models to implement (falling back to a different approach if not implemented).
- Somthing else to be determined in the future.
*/
let { url } = await models.Organization.getRegistrationUrl( org_id, context );
url += '&command=remove';
let { url, headers } = await models.Organization.getRegistrationUrl( org_id, context );

// Headers can hold sensitive information, such as an authorization token.
// Cleanup requirees no authorization token, unlike _registering_ a cluster, so explicitly empty and ignore the headers from the getRegistrationUrl.
headers = {};

// Build out the URL, avoiding sensitive data.
url = `${url}${url.includes('?')?'&':'?'}command=remove`;
const rddArgs = await getRddArgs(context);
if (rddArgs.length > 0) {
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});
}
return( url );
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});

return( { url, headers } );
};

const buildSearchFilter = (ordId, condition, searchStr) => {
Expand Down Expand Up @@ -111,15 +116,18 @@ const clusterResolvers = {
logger.info({req_id, user, org_id, clusterId, cluster }, `${queryName} found matching authorized cluster` );

if(cluster){
var { url } = await models.Organization.getRegistrationUrl(org_id, context);
url = url + `&clusterId=${clusterId}`;
let { url } = await models.Organization.getRegistrationUrl(org_id, context);

// Build out the URL, avoiding sensitive data.
url = `${url}${url.includes('?')?'&':'?'}clusterId=${clusterId}`;
const rddArgs = await getRddArgs(context);
if (rddArgs.length > 0) {
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});
}
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});

if (!cluster.registration) cluster.registration = {};
// Note: the registration.url should not be used -- the URL will not function without first 'priming' it by calling the `enableRegistrationUrl` API, making the value returned here unusable.
// It will be removed in a future update.
cluster.registration.url = url;
}

Expand Down Expand Up @@ -176,15 +184,18 @@ const clusterResolvers = {
await validAuth(me, org_id, ACTIONS.READ, TYPES.CLUSTER, queryName, context, [cluster.id, cluster.name, clusterName]);

if(cluster){
var { url } = await models.Organization.getRegistrationUrl(org_id, context);
url = url + `&clusterId=${cluster.id}`;
let { url } = await models.Organization.getRegistrationUrl(org_id, context);

// Build out the URL, avoiding sensitive data.
url = `${url}${url.includes('?')?'&':'?'}clusterId=${cluster.id}`;
const rddArgs = await getRddArgs(context);
if (rddArgs.length > 0) {
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});
}
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});

if (!cluster.registration) cluster.registration = {};
// Note: the registration.url should not be used -- the URL will not function without first 'priming' it by calling the `enableRegistrationUrl` API, making the value returned here unusable.
// It will be removed in a future update.
cluster.registration.url = url;
}

Expand Down Expand Up @@ -457,13 +468,16 @@ const clusterResolvers = {
// Allow graphQL plugins to retrieve more information. deleteClusterByClusterId can delete clusters. Include details of each deleted resource in pluginContext.
context.pluginContext = {cluster: {name: deletedCluster.registration.name, uuid: deletedCluster.cluster_id, registration: deletedCluster.registration}};

const cleanupDetails = await getCleanupDetails( org_id, context );

logger.info({req_id, user, org_id, cluster_id}, `${queryName} returning`);
return {
deletedClusterCount: deletedCluster ? (deletedCluster.cluster_id === cluster_id ? 1 : 0) : 0,
deletedResourceCount: deletedResources.modifiedCount,
deletedResourceYamlHistCount: deletedResourceYamlHist.modifiedCount,
deletedServiceSubscriptionCount: deletedServiceSubscription.deletedCount,
url: await getCleanupUrl( org_id, context ),
url: cleanupDetails.url,
headers: cleanupDetails.headers,
};
}
catch( error ) {
Expand Down Expand Up @@ -524,13 +538,16 @@ const clusterResolvers = {
const deletedResourceYamlHist = await models.ResourceYamlHist.updateMany({ org_id }, {$set: { deleted: true }}, { upsert: false });
logger.info({req_id, user, org_id, deletedResourceYamlHist}, 'ResourceYamlHist soft-deletion complete');

const cleanupDetails = await getCleanupDetails( org_id, context );

logger.info({req_id, user, org_id}, `${queryName} returning`);
return {
deletedClusterCount: deletedClusters.deletedCount,
deletedResourceCount: deletedResources.modifiedCount,
deletedResourceYamlHistCount: deletedResourceYamlHist.modifiedCount,
deletedServiceSubscriptionCount: deletedServiceSubscription.deletedCount,
url: await getCleanupUrl( org_id, context ),
url: cleanupDetails.url,
headers: cleanupDetails.headers,
};
}
catch( error ) {
Expand Down Expand Up @@ -620,20 +637,24 @@ const clusterResolvers = {
logger.info({req_id, user, org_id, registration}, `${queryName} retrieving registration url`);

const org = await models.Organization.findById(org_id);
var { url } = await models.Organization.getRegistrationUrl(org_id, context);
url = url + `&clusterId=${cluster_id}`;

let { url, headers } = await models.Organization.getRegistrationUrl(org_id, context);

// Headers can hold sensitive information. If no headers specified, use empty object.
headers = headers || {};

// Build out the URL, avoiding sensitive data.
url = `${url}${url.includes('?')?'&':'?'}clusterId=${cluster_id}`;
const rddArgs = await getRddArgs(context);
if (rddArgs.length > 0) {
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});
}
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});

// Allow graphQL plugins to retrieve more information. registerCluster can create clusters. Include details of each created resource in pluginContext.
context.pluginContext = {cluster: {name: registration.name, uuid: cluster_id, registration: registration}};

logger.info({req_id, user, org_id, registration, cluster_id}, `${queryName} returning`);
return { url, orgId: org_id, clusterId: cluster_id, orgKey: bestOrgKey( org ).key, regState: reg_state, registration };
return { url, headers, orgId: org_id, clusterId: cluster_id, orgKey: bestOrgKey( org ).key, regState: reg_state, registration };
}
catch( error ) {
logger.error({ req_id, user, org_id, error }, `${queryName} error encountered: ${error.message}`);
Expand Down Expand Up @@ -665,20 +686,23 @@ const clusterResolvers = {
{$set: {reg_state: CLUSTER_REG_STATES.REGISTERING}});

if (updatedCluster) {
var { url } = await models.Organization.getRegistrationUrl(org_id, context);
url = url + `&clusterId=${cluster_id}`;
let { url, headers } = await models.Organization.getRegistrationUrl(org_id, context);

// Headers can hold sensitive information. If no headers specified, use empty object.
headers = headers || {};

// Build out the URL, avoiding sensitive data.
url = `${url}${url.includes('?')?'&':'?'}clusterId=${cluster_id}`;
const rddArgs = await getRddArgs(context);
if (rddArgs.length > 0) {
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});
}
rddArgs.forEach(arg => {
url += `&args=${arg}`;
});

// Allow graphQL plugins to retrieve more information. enableRegistrationUrl can update clusters. Include details of each updated resource in pluginContext.
context.pluginContext = {cluster: {name: updatedCluster.registration.name, uuid: cluster_id, registration: updatedCluster.registration}};

logger.info({ req_id, user, org_id, cluster_id }, `${queryName} returning`);
return { url };
return { url, headers };
} else {
logger.info({ req_id, user, org_id, cluster_id }, `${queryName} returning (no update)`);
return null;
Expand Down
3 changes: 3 additions & 0 deletions app/apollo/schema/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ const clusterSchema = gql`
deletedResourceYamlHistCount: Int,
deletedServiceSubscriptionCount: Int,
url: String!
headers: JSON!
}
type RegisterClusterResponse {
url: String!
headers: JSON!
orgId: String!
orgKey: String!
clusterId: String!
Expand All @@ -98,6 +100,7 @@ const clusterSchema = gql`
type EnableRegistrationUrlResponse {
url: String!
headers: JSON!
}
extend type Query {
Expand Down
1 change: 0 additions & 1 deletion app/apollo/test/channel.remote.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,6 @@ describe('channel remote graphql test suite', () => {
expect(sub01).to.be.an('object');
expect(sub01.remote).to.be.an('object');
expect(sub01.remote.remoteType).to.equal('github');
console.log( `PLC sub01.remote.parameters: ${JSON.stringify( sub01.remote.parameters, null, 2 )}` );
expect(sub01.remote.parameters.length).to.equal(2); // One from the Config merged with one from the Version
} catch (error) {
if (error.response) {
Expand Down
10 changes: 8 additions & 2 deletions app/apollo/test/cluster.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ const org_01_orgkey = 'orgApiKey-0a9f5ee7-c879-4302-907c-238178ec9071';
const org_01_orgkey2 = {
orgKeyUuid: 'fcb8af1e-e4f1-4e7b-8c52-0e8360b48a13',
name: 'testOrgKey2',
primary: true
primary: true,
key: 'dummy-key-value'
};

// If external auth model specified, use it. Else use built-in auth model.
Expand Down Expand Up @@ -803,6 +804,7 @@ describe('cluster graphql test suite', () => {
expect(deleteClusterByClusterId.deletedResourceYamlHistCount).to.equal(0);
expect(deleteClusterByClusterId.deletedServiceSubscriptionCount).to.equal(0);
expect(deleteClusterByClusterId.url).to.be.an('string');
expect(Object.getOwnPropertyNames(deleteClusterByClusterId.headers).length).to.equal(0);
} catch (error) {
if (error.response) {
console.error('error encountered: ', error.response.data);
Expand Down Expand Up @@ -861,6 +863,7 @@ describe('cluster graphql test suite', () => {
expect(deleteClusters.deletedResourceYamlHistCount).to.equal(0);
expect(deleteClusters.deletedServiceSubscriptionCount).to.equal(0);
expect(deleteClusters.url).to.be.an('string');
expect(Object.getOwnPropertyNames(deleteClusters.headers).length).to.equal(0);
} catch (error) {
if (error.response) {
console.error('error encountered: ', error.response.data);
Expand All @@ -879,6 +882,7 @@ describe('cluster graphql test suite', () => {
});
console.log( `response: ${JSON.stringify( registerCluster.data, null, 2 )}` );
expect(registerCluster.data.data.registerCluster.url).to.be.an('string');
expect(registerCluster.data.data.registerCluster.headers['razee-org-key']).to.be.an('string');

const result = await clusterApi.byClusterName(token, {
orgId: org01._id,
Expand Down Expand Up @@ -1054,13 +1058,15 @@ describe('cluster graphql test suite', () => {
clusterId: clusterIdEnableRegUrl,
});

expect(enableRegistrationUrl.url).to.be.an('string');
expect(enableRegistrationUrl.headers['razee-org-key']).to.be.an('string');

const result = await clusterApi.byClusterID(token, {
orgId: org01._id,
clusterId: clusterIdEnableRegUrl,
});
const clusterByClusterId = result.data.data.clusterByClusterId;

expect(enableRegistrationUrl.url).to.be.an('string');
expect(clusterByClusterId.regState).to.equal('registering');

} catch (error) {
Expand Down
4 changes: 4 additions & 0 deletions app/apollo/test/clusterApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ const clusterFunc = grahqlUrl => {
deletedResourceYamlHistCount
deletedServiceSubscriptionCount
url
headers
}
}
`,
Expand All @@ -362,6 +363,7 @@ const clusterFunc = grahqlUrl => {
deletedResourceYamlHistCount
deletedServiceSubscriptionCount
url
headers
}
}
`,
Expand All @@ -381,6 +383,7 @@ const clusterFunc = grahqlUrl => {
mutation($orgId: String!, $registration: JSON!, $idempotent: Boolean) {
registerCluster(orgId: $orgId, registration: $registration, idempotent: $idempotent) {
url
headers
}
}
`,
Expand All @@ -401,6 +404,7 @@ const clusterFunc = grahqlUrl => {
mutation($orgId: String!,$clusterId: String!) {
enableRegistrationUrl(orgId: $orgId clusterId: $clusterId) {
url
headers
}
}
`,
Expand Down
3 changes: 3 additions & 0 deletions app/apollo/test/externalAuth/organization.local.schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ OrganizationLocalSchema.statics.getRegistrationUrl = async function(org_id, cont
}
return {
url: `${protocol}://${host}/api/install/razeedeploy-job?orgKey=${bestOrgKeyValue}`,
headers: {
'razee-org-key': bestOrgKeyValue
}
};
};

Expand Down
10 changes: 8 additions & 2 deletions app/routes/v1/systemSubscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,19 @@ type: Opaque
Serves a System Subscription that returns a CronJob that updates the operators: Cluster Subscription, Remote Resource and Watch-Keeper
*/
const getOperatorsSubscription = async(req, res) => {
// Get the image and command for the update cronjob from the current values returned from the razeedeploy-job api
const protocol = req.protocol || 'http';
let host = req.header('host') || 'localhost:3333';
if (process.env.EXTERNAL_HOST) {
host = process.env.EXTERNAL_HOST;
}
const url = `${protocol}://${host}/api/install/razeedeploy-job?orgKey=${bestOrgKey(req.org).key}`;
let job = await axios.get(url);
let job = await axios( {
method: 'get',
url: `${protocol}://${host}/api/install/razeedeploy-job`,
headers: {
'razee-org-key': bestOrgKey(req.org).key
}
});
job = yaml.loadAll(job.data);
const image = job[4].spec.template.spec.containers[0].image;
const command = job[4].spec.template.spec.containers[0].command[0];
Expand Down
1 change: 1 addition & 0 deletions app/utils/rdd/rdd-default.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ async function getRddJobUrl() {
return( process.env.RDD_JOB_URL || 'https://github.com/razee-io/razeedeploy-delta/releases/latest/download/job.yaml' );
}

// Return additional RDD query parameters, avoiding sensitive information which should be returned in headers instead
async function getRddArgs() {
return( process.env.RDD_STATIC_ARGS ? process.env.RDD_STATIC_ARGS.split(',') : [] );
}
Expand Down
1 change: 1 addition & 0 deletions app/utils/rdd/rdd-sample.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ async function getRddJobUrl(context) {
return( `https://github.com/razee-io/razeedeploy-delta/releases/${bestVersion}/download/job.yaml` );
}

// Return additional RDD query parameters, avoiding sensitive information which should be returned in headers instead
async function getRddArgs(context) {
const { req_id, logger } = context;
logger.warn( {req_id}, 'using SAMPLE implementation, should only happen during dev/test' );
Expand Down
Loading

0 comments on commit 266402f

Please sign in to comment.