From 6d2e6574998ff0478481d7ff7f0b327019006e05 Mon Sep 17 00:00:00 2001 From: Nick Muerdter Date: Fri, 18 Sep 2015 16:19:55 -0400 Subject: [PATCH 1/2] Setup error data for easier overriding of URL variables. Now there's a concept of "common" error data variables, which we can use so that contact URLs (and other things like this) only have to be defined once per API backend, which makes it much easier to define variables only once which can then be embedded in other areas of the error response. See: https://github.com/18F/api.data.gov/issues/285 This also begins to cleanup and standardize the error data variables to snake_case moving forward (instead of some of our odd mixtures of camel-case that had built up over time). --- config/default.yml | 19 ++-- .../middleware/https_requirements.js | 1 + lib/gatekeeper/utils.js | 24 ++++- test/server/formatted_errors.js | 91 +++++++++++++++++-- 4 files changed, 118 insertions(+), 17 deletions(-) diff --git a/config/default.yml b/config/default.yml index 61ef5514..d47f2355 100644 --- a/config/default.yml +++ b/config/default.yml @@ -85,6 +85,9 @@ apiSettings: error_data: + common: + signup_url: "{{base_url}}" + contact_url: "{{base_url}}/contact/" not_found: status_code: 404 code: NOT_FOUND @@ -92,32 +95,32 @@ apiSettings: api_key_missing: status_code: 403 code: API_KEY_MISSING - message: No api_key was supplied. Get one at {{baseUrl}} + message: No api_key was supplied. Get one at {{signup_url}} api_key_invalid: status_code: 403 code: API_KEY_INVALID - message: An invalid api_key was supplied. Get one at {{baseUrl}} + message: An invalid api_key was supplied. Get one at {{signup_url}} api_key_disabled: status_code: 403 code: API_KEY_DISABLED - message: The api_key supplied has been disabled. Contact us at {{baseUrl}}/contact for assistance + message: The api_key supplied has been disabled. Contact us at {{contact_url}} for assistance api_key_unverified: status_code: 403 code: API_KEY_UNVERIFIED - message: The api_key supplied has not been verified yet. Please check your e-mail to verify the API key. Contact us at {{baseUrl}}/contact for assistance + message: The api_key supplied has not been verified yet. Please check your e-mail to verify the API key. Contact us at {{contact_url}} for assistance api_key_unauthorized: status_code: 403 code: API_KEY_UNAUTHORIZED - message: The api_key supplied is not authorized to access the given service. Contact us at {{baseUrl}}/contact for assistance + message: The api_key supplied is not authorized to access the given service. Contact us at {{contact_url}} for assistance over_rate_limit: status_code: 429 code: OVER_RATE_LIMIT - message: You have exceeded your rate limit. Try again later or contact us at {{baseUrl}}/contact for assistance + message: You have exceeded your rate limit. Try again later or contact us at {{contact_url}} for assistance internal_server_error: status_code: 500 code: INTERNAL_SERVER_ERROR - message: An unexpected error has occurred. Try again later or contact us at {{baseUrl}}/contact for assistance + message: An unexpected error has occurred. Try again later or contact us at {{contact_url}} for assistance https_required: status_code: 400 code: HTTPS_REQUIRED - message: "Requests must be made over HTTPS. Try accessing the API at: {{httpsUrl}}" + message: "Requests must be made over HTTPS. Try accessing the API at: {{https_url}}" diff --git a/lib/gatekeeper/middleware/https_requirements.js b/lib/gatekeeper/middleware/https_requirements.js index e27d75a7..651bc07f 100644 --- a/lib/gatekeeper/middleware/https_requirements.js +++ b/lib/gatekeeper/middleware/https_requirements.js @@ -94,6 +94,7 @@ _.extend(HttpsRequirements.prototype, { response.end(body); } else { utils.errorHandler(request, response, 'https_required', { + https_url: httpsUrl, httpsUrl: httpsUrl, }); } diff --git a/lib/gatekeeper/utils.js b/lib/gatekeeper/utils.js index b86d7069..3915bfee 100644 --- a/lib/gatekeeper/utils.js +++ b/lib/gatekeeper/utils.js @@ -44,14 +44,34 @@ exports.errorHandler = function(request, response, error, data) { // introduce in multi-line templates and XML doesn't like if there's any // leading space before the XML declaration. var templateContent = settings.error_templates[format].replace(/^\s+|\s+$/g, ''); + + var commonErrorData = settings.error_data.common; + if(!commonErrorData || !_.isPlainObject(commonErrorData)) { + commonErrorData = {}; + logger.error({ error_type: error }, 'Common error data not found.'); + } + var errorData = settings.error_data[error]; if(!errorData || !_.isPlainObject(errorData)) { errorData = settings.error_data.internal_server_error; logger.error({ error_type: error }, 'Error data not found for error type: ' + error); } + data = _.merge({ - baseUrl: request.base, - }, data || {}, errorData); + base_url: request.base, + }, data || {}, commonErrorData, errorData); + + // Support legacy camel-case capitalization of variables. Moving forward, + // we're trying to clean things up and standardize on snake-case. + if(!data.baseUrl) { + data.baseUrl = data.base_url; + } + if(!data.signupUrl) { + data.signupUrl = data.signup_url; + } + if(!data.contactUrl) { + data.contactUrl = data.contact_url; + } var prop; for(prop in data) { diff --git a/test/server/formatted_errors.js b/test/server/formatted_errors.js index 375960f1..2a434477 100644 --- a/test/server/formatted_errors.js +++ b/test/server/formatted_errors.js @@ -99,15 +99,92 @@ describe('formatted error responses', function() { }); describe('data variables', function() { - shared.runServer(); + shared.runServer({ + apis: [ + { + frontend_host: 'localhost', + backend_host: 'example.com', + url_matches: [ + { + frontend_prefix: '/', + backend_prefix: '/', + } + ], + settings: { + error_data: { + api_key_missing: { + embedded: 'base_url: {{base_url}} signup_url: {{signup_url}} contact_url: {{contact_url}}', + }, + }, + error_templates: { + json: '{' + + '"base_url": {{base_url}},' + + '"baseUrl": {{baseUrl}},' + + '"signup_url": {{signup_url}},' + + '"signupUrl": {{signupUrl}},' + + '"contact_url": {{contact_url}},' + + '"contactUrl": {{contactUrl}},' + + '"embedded": {{embedded}} ' + + '}', + }, + }, + }, + ], + }); + + it('substitutes the base_url variable', function(done) { + request.get('http://localhost:9333/base_url.json', function(error, response, body) { + var data = JSON.parse(body); + data.base_url.should.eql('http://localhost:9333'); + done(); + }); + }); it('substitutes the baseUrl variable', function(done) { - Factory.create('api_user', { disabled_at: new Date() }, function(user) { - request.get('http://localhost:9333/hello.json?api_key=' + user.api_key, function(error, response, body) { - var data = JSON.parse(body); - data.error.message.should.include(' http://localhost:9333/contact '); - done(); - }); + request.get('http://localhost:9333/baseUrl.json', function(error, response, body) { + var data = JSON.parse(body); + data.baseUrl.should.eql('http://localhost:9333'); + done(); + }); + }); + + it('substitutes the signup_url variable', function(done) { + request.get('http://localhost:9333/signup_url.json', function(error, response, body) { + var data = JSON.parse(body); + data.signup_url.should.eql('http://localhost:9333'); + done(); + }); + }); + + it('substitutes the signupUrl variable', function(done) { + request.get('http://localhost:9333/signupUrl.json', function(error, response, body) { + var data = JSON.parse(body); + data.signupUrl.should.eql('http://localhost:9333'); + done(); + }); + }); + + it('substitutes the contact_url variable', function(done) { + request.get('http://localhost:9333/contact_url.json', function(error, response, body) { + var data = JSON.parse(body); + data.contact_url.should.eql('http://localhost:9333/contact/'); + done(); + }); + }); + + it('substitutes the contactUrl variable', function(done) { + request.get('http://localhost:9333/contactUrl.json', function(error, response, body) { + var data = JSON.parse(body); + data.contactUrl.should.eql('http://localhost:9333/contact/'); + done(); + }); + }); + + it('substitutes variables embedded inside of other variables', function(done) { + request.get('http://localhost:9333/embedded.json', function(error, response, body) { + var data = JSON.parse(body); + data.embedded.should.eql('base_url: http://localhost:9333 signup_url: http://localhost:9333 contact_url: http://localhost:9333/contact/'); + done(); }); }); }); From f5926a88a2b25921c14ff72296c33d0e890b5d88 Mon Sep 17 00:00:00 2001 From: Nick Muerdter Date: Fri, 18 Sep 2015 19:36:39 -0400 Subject: [PATCH 2/2] Fix for the legacy camel case variables not working the same. Due to the order in which these were being looped over, if these variables contained other internal variables, the legacy copies weren't working properly (since they were bein modified later). --- lib/gatekeeper/utils.js | 8 +++++--- test/server/formatted_errors.js | 12 +++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/gatekeeper/utils.js b/lib/gatekeeper/utils.js index 3915bfee..8bb881d3 100644 --- a/lib/gatekeeper/utils.js +++ b/lib/gatekeeper/utils.js @@ -9,7 +9,7 @@ var _ = require('lodash'), Negotiator = require('negotiator'), url = require('url'); -exports.errorHandler = function(request, response, error, data) { +exports.errorHandler = function(request, response, error, errorSpecificData) { var availableMediaTypes = ['application/json', 'application/xml', 'text/csv', 'text/html']; // Prefer the format from the extension given in the URL. @@ -57,9 +57,9 @@ exports.errorHandler = function(request, response, error, data) { logger.error({ error_type: error }, 'Error data not found for error type: ' + error); } - data = _.merge({ + var data = _.merge({ base_url: request.base, - }, data || {}, commonErrorData, errorData); + }, commonErrorData); // Support legacy camel-case capitalization of variables. Moving forward, // we're trying to clean things up and standardize on snake-case. @@ -73,6 +73,8 @@ exports.errorHandler = function(request, response, error, data) { data.contactUrl = data.contact_url; } + data = _.merge(data, errorSpecificData || {}, errorData); + var prop; for(prop in data) { try { diff --git a/test/server/formatted_errors.js b/test/server/formatted_errors.js index 2a434477..cb403bbf 100644 --- a/test/server/formatted_errors.js +++ b/test/server/formatted_errors.js @@ -114,6 +114,7 @@ describe('formatted error responses', function() { error_data: { api_key_missing: { embedded: 'base_url: {{base_url}} signup_url: {{signup_url}} contact_url: {{contact_url}}', + embedded_legacy: 'baseUrl: {{baseUrl}} signupUrl: {{signupUrl}} contactUrl: {{contactUrl}}', }, }, error_templates: { @@ -124,7 +125,8 @@ describe('formatted error responses', function() { '"signupUrl": {{signupUrl}},' + '"contact_url": {{contact_url}},' + '"contactUrl": {{contactUrl}},' + - '"embedded": {{embedded}} ' + + '"embedded": {{embedded}},' + + '"embedded_legacy": {{embedded_legacy}} ' + '}', }, }, @@ -187,6 +189,14 @@ describe('formatted error responses', function() { done(); }); }); + + it('substitutes legacy camel case variables embedded inside of other variables', function(done) { + request.get('http://localhost:9333/embedded_legacy.json', function(error, response, body) { + var data = JSON.parse(body); + data.embedded_legacy.should.eql('baseUrl: http://localhost:9333 signupUrl: http://localhost:9333 contactUrl: http://localhost:9333/contact/'); + done(); + }); + }); }); describe('format validation', function() {