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

Handle login redirects via Referer not ?next param. #279

Merged
merged 8 commits into from
Sep 22, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions grunt-tasks/concat.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ module.exports = function(grunt) {
'<%= jsPath %>/vendor/mousetrap-min.js',
'<%= jsPath %>/vendor/backbone.mousetrap.js',
'<%= jsPath %>/lib/homepage.js',
'<%= jsPath %>/lib/bugform.js',
'<%= jsPath %>/lib/shared.js'
'<%= jsPath %>/lib/bugform.js'
],
dest: '<%= jsPath %>/<%= pkg.name %>.js'
},
Expand Down
1 change: 0 additions & 1 deletion grunt-tasks/jshint.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ module.exports = function(grunt) {
'<%= jsPath %>/lib/comments.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/issues.js',
'<%= jsPath %>/lib/shared.js',
'<%= jsPath %>/lib/diagnose.js'
]
});
Expand Down
16 changes: 15 additions & 1 deletion run.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,21 @@
# 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/.

import argparse

from webcompat import app

if __name__ == '__main__':
app.run()
parser = argparse.ArgumentParser()
parser.add_argument('-t', '--testmode', action='store_true',
help='Run server in "test mode".')
args = parser.parse_args()

if args.testmode:
# disable HttpOnly setting for session cookies so Selenium
# can interact with them. *ONLY* do this for testing.
app.config['SESSION_COOKIE_HTTPONLY'] = False
print("Starting server in ~*TEST MODE*~")
app.run()
else:
app.run()
21 changes: 7 additions & 14 deletions tests/functional/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ define([
.end();
},

'logging in on the homepage': function() {
'my issues (when logged in)': function() {
return this.remote
.setFindTimeout(intern.config.wc.pageLoadTimeout)
.get(require.toUrl(url))
Expand All @@ -46,6 +46,12 @@ define([
assert.equal(text, "Logout");
})
.end()
.findByCssSelector('#my-issues h3').getVisibleText()
.then(function (text) {
assert.equal(text, 'Submitted by Me');
})
.sleep(1000)
.end()
},

'reporter addon link is shown': function () {
Expand Down Expand Up @@ -74,19 +80,6 @@ define([
});
},

'browse issues (my issues)': function() {
return this.remote
.setFindTimeout(intern.config.wc.pageLoadTimeout)
.get(require.toUrl(url))
.findByCssSelector('.nav__section--right .nav__link').click()
.end()
.findByCssSelector('#my-issues h3').getVisibleText()
.then(function (text) {
assert.equal(text, 'Submitted by Me');
})
.end()
},

'browse issues (needs diagnosis)': function() {
return this.remote
.get(require.toUrl(url))
Expand Down
79 changes: 37 additions & 42 deletions tests/functional/lib/reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ define([
.end()
.findByCssSelector('#summary').click()
.end()
.findByCssSelector('.Report-form > div:nth-child(1) > div:nth-child(1) > div:nth-child(1)').getAttribute('class')
.then(function (className) {
assert.include(className, 'has-error');
})
.end()
.findByCssSelector('#url').type('hi')
.end()
.findByCssSelector('#summary').click()
Expand Down Expand Up @@ -74,44 +69,44 @@ define([
})
},

'anonymous report': function () {
var issueNumber;
// 'anonymous report': function () {
// var issueNumber;

return this.remote
.setFindTimeout(intern.config.wc.pageLoadTimeout)
.get(require.toUrl(url + "?open=1"))
.findByCssSelector('#url').type('http://miketaylr.com')
.end()
.findByCssSelector('#summary').type('Hello from Intern')
.end()
.findByCssSelector('#submitanon').click()
.end()
.findByCssSelector('.wc-content--body h2').getVisibleText()
.then(function (text) {
// Make sure we got to the /thanks/<number> route
assert.equal(text, 'Thank you.');
})
.end()
.findByCssSelector('.wc-content--body a').getVisibleText()
.then(function (text) {
// Grab the issue number from the end of the URL link
issueNumber = text.split('/').pop();
})
.end()
.findByCssSelector('.wc-content--body a').getVisibleText().click()
.end()
.findByCssSelector('.js-issue-title').getVisibleText()
.then(function (text) {
// Make sure GitHub has the correct title
assert.equal(text, 'miketaylr.com - Hello from Intern');
})
.end()
.findByCssSelector('.gh-header-number').getVisibleText()
.then(function (text) {
// Make sure GitHub has the correct issue number
assert.equal(text, '#' + issueNumber);
})
}
// return this.remote
// .setFindTimeout(intern.config.wc.pageLoadTimeout)
// .get(require.toUrl(url + "?open=1"))
// .findByCssSelector('#url').type('some broken URL')
// .end()
// .findByCssSelector('#summary').type('this site doesnt work ' + Math.random())
// .end()
// .findByCssSelector('#submitanon').click()
// .end()
// .findByCssSelector('.wc-content--body h2').getVisibleText()
// .then(function (text) {
// // Make sure we got to the /thanks/<number> route
// assert.equal(text, 'Thank you.');
// })
// .end()
// .findByCssSelector('.wc-content--body a').getVisibleText()
// .then(function (text) {
// // Grab the issue number from the end of the URL link
// issueNumber = text.split('/').pop();
// })
// .end()
// .findByCssSelector('.wc-content--body a').getVisibleText().click()
// .end()
// .findByCssSelector('.js-issue-title').getVisibleText()
// .then(function (text) {
// // Make sure GitHub has the correct title
// assert.include(text, 'some broken URL - this site doesnt work');
// })
// .end()
// .findByCssSelector('.gh-header-number').getVisibleText()
// .then(function (text) {
// // Make sure GitHub has the correct issue number
// assert.equal(text, '#' + issueNumber);
// })
// }

});
});
18 changes: 18 additions & 0 deletions webcompat/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
# 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/.

import urlparse

from flask import session
from ua_parser import user_agent_parser

from webcompat import github

JSON_MIME = 'application/json'
Expand Down Expand Up @@ -79,3 +82,18 @@ def get_headers(response):
'cache-control': response.headers.get('cache-control'),
'content-type': JSON_MIME}
return headers


def get_referer(request):
'''Return the Referer URI based on the passed in Request object.

Also validate that it came from our own server. If it didn't, return None.
'''
host_whitelist = ('webcompat.com', 'staging.webcompat.com',
'127.0.0.1', 'localhost')
if request.referrer:
host = urlparse.urlparse(request.referrer).hostname
if host in host_whitelist:
return request.referrer
else:
return None
11 changes: 0 additions & 11 deletions webcompat/static/js/lib/shared.js

This file was deleted.

1 change: 0 additions & 1 deletion webcompat/templates/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
<script src="{{ url_for('static', filename='js/vendor/marked-min.js') }}"></script>
<script src="{{ url_for('static', filename='js/lib/homepage.js') }}"></script>
<script src="{{ url_for('static', filename='js/lib/bugform.js') }}"></script>
<script src="{{ url_for('static', filename='js/lib/shared.js') }}"></script>
{%- endif %}
{% for category, message in get_flashed_messages(with_categories=True) %}
<div class="flash {{ category }}">{{ message }}</div>
Expand Down
14 changes: 6 additions & 8 deletions webcompat/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from helpers import get_browser
from helpers import get_browser_name
from helpers import get_os
from helpers import get_referer
from helpers import get_user_info
from issues import report_issue
from models import db_session
Expand All @@ -47,6 +48,7 @@ def before_request():
g.user = None
if 'user_id' in session:
g.user = User.query.get(session['user_id'])
g.referer = get_referer(request) or url_for('index')


@app.after_request
Expand All @@ -71,31 +73,27 @@ def format_date(datestring):

@app.route('/login')
def login():
next_url = request.args.get('next') or url_for('index')
if session.get('user_id', None) is None:
session['next_url'] = next_url
return github.authorize('public_repo')
else:
return redirect(next_url)
return redirect(g.referer)


@app.route('/logout')
def logout():
next_url = request.args.get('next') or url_for('index')
session.clear()
flash(u'You were successfully logged out.', 'info')
return redirect(next_url)
return redirect(g.referer)


# OAuth2 callback handler that GitHub requires.
# If this moves, it needs to change in GitHub settings as well
@app.route('/callback')
@github.authorized_handler
def authorized(access_token):
next_url = session.get('next_url') or url_for('index')
if access_token is None:
flash(u'Something went wrong trying to sign into GitHub. :(', 'error')
return redirect(next_url)
return redirect(g.referer)
user = User.query.filter_by(github_access_token=access_token).first()
if user is None:
user = User(access_token)
Expand All @@ -105,7 +103,7 @@ def authorized(access_token):
if session.get('form_data', None) is not None:
return redirect(url_for('file_issue'))
else:
return redirect(next_url)
return redirect(g.referer)


# This route won't ever be viewed by a human being--there's not
Expand Down