667/1 To square a bit error handling on the API side of our service. Fix #667

tests/test_api_urls.py

@@ -0,0 +1,90 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+'''Tests for our API URL endpoints.'''
+
+import json
+import os.path
+import sys
+import unittest
+
+# Add webcompat module to import path
+sys.path.append(os.path.realpath(os.pardir))
+import webcompat
+
+
+# Any request that depends on parsing HTTP Headers (basically anything
+# on the index route, will need to include the following: environ_base=headers
+headers = {'HTTP_USER_AGENT': ('Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; '
+                               'rv:31.0) Gecko/20100101 Firefox/31.0'),
+           'HTTP_ACCEPT': 'application/json'}
+
+
+class TestAPIURLs(unittest.TestCase):
+    def setUp(self):
+        webcompat.app.config['TESTING'] = True
+        self.app = webcompat.app.test_client()
+
+    def tearDown(self):
+        pass
+
+    def test_api_issues_search(self):
+        '''API issue search with bad keywords returns JSON 404.'''
+        rv = self.app.get('/api/issues/search/foobar', environ_base=headers)
+        self.assertEqual(rv.status_code, 404)
+        self.assertEqual(rv.content_type, 'application/json')
+
+    def test_api_issues_out_of_range(self):
+        '''API issue for a non existent number returns JSON 404.'''
+        # If we reach 1,000,000 webcompat issues we can celebrate
+        rv = self.app.get('/api/issues/1000000', environ_base=headers)
+        json_body = json.loads(rv.data)
+        self.assertEqual(rv.status_code, 404)
+        self.assertEqual(rv.content_type, 'application/json')
+        self.assertEqual(json_body['status'], 404)
+
+    def test_api_wrong_route(self):
+        '''API with wrong route returns JSON 404.'''
+        rv = self.app.get('/api/foobar', environ_base=headers)
+        json_body = json.loads(rv.data)
+        self.assertEqual(rv.status_code, 404)
+        self.assertEqual(rv.content_type, 'application/json')
+        self.assertEqual(json_body['status'], 404)
+
+    def test_api_wrong_category(self):
+        '''API with wrong category returns JSON 404.'''
+        rv = self.app.get('/api/issues/category/foobar', environ_base=headers)
+        json_body = json.loads(rv.data)
+        self.assertEqual(rv.status_code, 404)
+        self.assertEqual(rv.content_type, 'application/json')
+        self.assertEqual(json_body['status'], 404)
+
+    def test_api_labels_without_auth(self):
+        '''API access to labels without auth returns JSON 401.'''
+        rv = self.app.get('/api/issues/labels', environ_base=headers)
+        json_body = json.loads(rv.data)
+        self.assertEqual(rv.status_code, 401)
+        self.assertEqual(rv.content_type, 'application/json')
+        self.assertEqual(json_body['status'], 401)
+
+    def test_api_search_wrong_parameter(self):
+        '''API with wrong parameter returns JSON 404.'''
+        rv = self.app.get('/api/issues/search?z=foobar', environ_base=headers)
+        json_body = json.loads(rv.data)
+        self.assertEqual(rv.status_code, 404)
+        self.assertEqual(rv.content_type, 'application/json')
+        self.assertEqual(json_body['status'], 404)
+
+    def test_api_category_mine(self):
+        '''API access category/mine without auth returns JSON 401.'''
+        rv = self.app.get('/api/issues/category/mine', environ_base=headers)
+        json_body = json.loads(rv.data)
+        self.assertEqual(rv.status_code, 401)
+        self.assertEqual(rv.content_type, 'application/json')
+        self.assertEqual(json_body['status'], 401)
+
+if __name__ == '__main__':
+    unittest.main()

webcompat/api/endpoints.py

@@ -24,7 +24,6 @@
 from webcompat.helpers import get_headers
 from webcompat.helpers import get_request_headers
 from webcompat.helpers import get_user_info
-from webcompat.helpers import json_not_found
 from webcompat.helpers import normalize_api_params
 from webcompat.issues import filter_new
 from webcompat.issues import proxy_request
@@ -53,7 +52,11 @@ def proxy_issue(number):
     else:
         issue = proxy_request('get', '/{0}'.format(number),
                               headers=request_headers)
-    return (issue.content, issue.status_code, get_headers(issue))
+    if issue.status_code == 200:
+        return (issue.content, issue.status_code, get_headers(issue))
+    else:
+        # we might want to be less tolerant here.
+        abort(404)
 
 
 @api.route('/issues/<int:number>/edit', methods=['PATCH'])
@@ -96,14 +99,17 @@ def user_issues():
 
     Not cached.
     '''
-    get_user_info()
-    path = 'repos/{0}?creator={1}&state=all'.format(
-        ISSUES_PATH, session['username']
-    )
-    request_headers = get_request_headers(g.request_headers)
-    issues = github.raw_request('GET', path, headers=request_headers)
-    return (issues.content, issues.status_code, get_headers(issues))
-
+    if g.user:
+        get_user_info()
+        path = 'repos/{0}?creator={1}&state=all'.format(
+            ISSUES_PATH, session['username']
+        )
+        request_headers = get_request_headers(g.request_headers)
+        issues = github.raw_request('GET', path, headers=request_headers)
+        return (issues.content, issues.status_code, get_headers(issues))
+    else:
+        # Credentials are need to be able to get the issues
+        abort(401)
 
 @api.route('/issues/category/<issue_category>')
 def get_issue_category(issue_category):
@@ -173,6 +179,9 @@ def get_search_results(query_string=None, params=None):
     '''
     params = params or request.args.copy()
     query_string = query_string or params.get('q')
+    # Fail early if no appropriate query_string
+    if not query_string:
+        abort(404)
     search_uri = 'https://api.github.com/search/issues'
 
     # restrict results to our repo.
@@ -215,7 +224,7 @@ def get_category_from_search(issue_category):
         return get_search_results(query_string, params)
     else:
         # no known keyword we send not found
-        return json_not_found()
+        abort(404)
 
 
 @api.route('/issues/<int:number>/comments', methods=['GET', 'POST'])

webcompat/helpers.py

@@ -11,7 +11,6 @@
 import urlparse
 
 from babel.dates import format_timedelta
-from flask import jsonify
 from flask import session
 from ua_parser import user_agent_parser
 
@@ -299,14 +298,3 @@ def format_link_header(link_header_data):
     links = ['<{0}>; rel="{1}"'.format(data['link'], data['rel'])
              for data in link_header_data]
     return ', '.join(links)
-
-
-@app.errorhandler(404)
-def json_not_found(error=None):
-    message = {
-        'status': 404,
-        'message': 'Not Found',
-        }
-    resp = jsonify(message)
-    resp.status_code = 404
-    return resp

webcompat/static/js/lib/issues.js

@@ -220,7 +220,7 @@ issues.MainView = Backbone.View.extend({
       }
     }).error(function(response) {
       var msg;
-      if (response.responseJSON.message === "Not Found") {
+      if (response.responseJSON.message === "API call. Not Found") {
         location.href = "/404";
         return;
       } else {

webcompat/views.py

@@ -10,6 +10,7 @@
 from flask.ext.github import GitHubError
 from flask import flash
 from flask import g
+from flask import jsonify
 from flask import redirect
 from flask import render_template
 from flask import request
@@ -231,8 +232,40 @@ def jumpship(e):
     return redirect(url_for('index'))
 
 
+@app.errorhandler(401)
+def unauthorized(err):
+    if (request.path.startswith('/api/') and
+       request.accept_mimetypes.accept_json and
+       not request.accept_mimetypes.accept_html):
+        message = {
+            'status': 401,
+            'message': 'API call. Unauthorized. Please log in.',
+        }
+        resp = jsonify(message)
+        resp.status_code = 401
+        return resp
+    else:
+        message = {
+            'status': 400,
+            'message': 'API call. Bad Request.',
+        }
+        resp = jsonify(message)
+        resp.status_code = 400
+        return resp
+
+
 @app.errorhandler(404)
 def not_found(err):
+    if (request.path.startswith('/api/') and
+       request.accept_mimetypes.accept_json and
+       not request.accept_mimetypes.accept_html):
+        message = {
+            'status': 404,
+            'message': 'API call. Not Found',
+        }
+        resp = jsonify(message)
+        resp.status_code = 404
+        return resp
     message = "We can't find what you are looking for."
     return render_template('error.html',
                            error_code=404,