From 9c444c7255765a60ad1d9277e8e54d25dd695b06 Mon Sep 17 00:00:00 2001 From: Ansgar Tasler Date: Jul 10 2019 17:32:37 +0000 Subject: [PATCH 1/7] Add REMOTE_USER authentication support --- diff --git a/pagure/flask_app.py b/pagure/flask_app.py index 6f16786..1025a34 100644 --- a/pagure/flask_app.py +++ b/pagure/flask_app.py @@ -29,6 +29,7 @@ import pagure.login_forms import pagure.mail_logging import pagure.proxy import pagure.utils +from pagure.lib.query import search_user from pagure.config import config as pagure_config from pagure.utils import get_repo_path @@ -100,7 +101,7 @@ def create_app(config=None): oidc.init_app(app) app.before_request(fas_user_from_oidc) - if auth == "local": + elif auth == "local" or auth == "remoteuser": # Only import the login controller if the app is set up for local login import pagure.ui.login as login @@ -234,6 +235,9 @@ def logout(): login.logout() + elif auth == "remoteuser": + logger.warn("You can not log out from remote_user!") + def set_request(): """ Prepare every request. """ @@ -406,7 +410,7 @@ def set_request(): def auth_login(): # pragma: no cover - """ Method to log into the application using FAS OpenID. """ + """ Method to log into the application """ return_point = flask.url_for("ui_ns.index") if "next" in flask.request.args: if pagure.utils.is_safe_url(flask.request.args["next"]): @@ -432,6 +436,28 @@ def auth_login(): # pragma: no cover authenticated = pagure.utils.authenticated() set_user() + if auth == "remoteuser": + # Note: auth_login's URL needs to be protected by remote webserver + username = flask.request.environ.get("REMOTE_USER") + dbuser = None + if not authenticated: + if username: + dbuser = search_user(flask.g.session, username=username) + if not username or not dbuser: + return flask.redirect(flask.url_for("ui_ns.new_remote_user")) + if dbuser.token: + flask.flash( + "User exists. Did you confirm the creation with the url " + "provided by email?", + "error", + ) + return flask.redirect(return_point) + else: + from pagure.ui.login import _set_session + + _set_session(dbuser) + authenticated = pagure.utils.authenticated() + if authenticated: return flask.redirect(return_point) diff --git a/pagure/login_forms.py b/pagure/login_forms.py index 2cd8d5c..5d6f12b 100644 --- a/pagure/login_forms.py +++ b/pagure/login_forms.py @@ -113,3 +113,15 @@ class ChangePasswordForm(FlaskForm): 'Confirm password *', [wtforms.validators.DataRequired(), same_password], ) + + +class NewRemoteUserForm(FlaskForm): + """ Form to set up a new user from REMOTE_USER """ + + fullname = wtforms.StringField( + "Full name", [wtforms.validators.Optional()] + ) + email_address = wtforms.StringField( + 'Email address *', + [wtforms.validators.Required(), wtforms.validators.Email()], + ) diff --git a/pagure/templates/login/remote_user_new.html b/pagure/templates/login/remote_user_new.html new file mode 100644 index 0000000..ec9635c --- /dev/null +++ b/pagure/templates/login/remote_user_new.html @@ -0,0 +1,23 @@ +{% extends "master.html" %} +{% from "_formhelper.html" import render_bootstrap_field %} + +{% block title %}New user from REMOTE_USER{% endblock %} +{% set tag = "home" %} + +{% block content %} +
+
+
+

Create new account for "{{ request.environ.get('REMOTE_USER') }}"

+
+ {{ render_bootstrap_field( + form.fullname) }} + {{ render_bootstrap_field( + form.email_address) }} + + {{ form.csrf_token }} +
+
+
+
+{% endblock %} diff --git a/pagure/templates/master.html b/pagure/templates/master.html index fab6dd8..7166639 100644 --- a/pagure/templates/master.html +++ b/pagure/templates/master.html @@ -82,9 +82,11 @@ My Pull Requests + {% if config.get("PAGURE_AUTH", None) != "remoteuser" %}
Log Out + {% endif %} {% else %} diff --git a/pagure/ui/__init__.py b/pagure/ui/__init__.py index ec64735..e3460bf 100644 --- a/pagure/ui/__init__.py +++ b/pagure/ui/__init__.py @@ -28,6 +28,8 @@ import pagure.ui.repo # noqa: E402 if pagure.config.config["PAGURE_AUTH"] == "local": import pagure.ui.login # noqa: E402 +elif pagure.config.config["PAGURE_AUTH"] == "remoteuser": + import pagure.ui.remoteuser_login # noqa: E402 add_clone_proxy_cmds() diff --git a/pagure/ui/login.py b/pagure/ui/login.py index 8383603..c9dd8d0 100644 --- a/pagure/ui/login.py +++ b/pagure/ui/login.py @@ -85,6 +85,34 @@ def new_user(): return flask.render_template("login/user_new.html", form=form) +def _set_session(user_obj): + """ Shared method to set flask user session data + """ + visit_key = pagure.lib.login.id_generator(40) + now = datetime.datetime.utcnow() + expiry = now + datetime.timedelta(days=30) + session = model.PagureUserVisit( + user_id=user_obj.id, + user_ip=flask.request.remote_addr, + visit_key=visit_key, + expiry=expiry, + ) + flask.g.session.add(session) + try: + flask.g.session.commit() + flask.g.fas_user = user_obj + flask.g.fas_session_id = visit_key + flask.g.fas_user.login_time = now + flask.flash("Welcome %s" % user_obj.username) + except SQLAlchemyError as err: # pragma: no cover + flask.flash( + "Could not set the session in the db, " + "please report this error to an admin", + "error", + ) + _log.exception(err) + + @UI_NS.route("/dologin", methods=["POST"]) def do_login(): """ Log in the user. @@ -137,29 +165,7 @@ def do_login(): flask.g.session.add(user_obj) flask.g.session.flush() - visit_key = pagure.lib.login.id_generator(40) - now = datetime.datetime.utcnow() - expiry = now + datetime.timedelta(days=30) - session = model.PagureUserVisit( - user_id=user_obj.id, - user_ip=flask.request.remote_addr, - visit_key=visit_key, - expiry=expiry, - ) - flask.g.session.add(session) - try: - flask.g.session.commit() - flask.g.fas_user = user_obj - flask.g.fas_session_id = visit_key - flask.g.fas_user.login_time = now - flask.flash("Welcome %s" % user_obj.username) - except SQLAlchemyError as err: # pragma: no cover - flask.flash( - "Could not set the session in the db, " - "please report this error to an admin", - "error", - ) - _log.exception(err) + _set_session(user_obj) return flask.redirect(next_url) else: diff --git a/pagure/ui/remoteuser_login.py b/pagure/ui/remoteuser_login.py new file mode 100644 index 0000000..3ca1751 --- /dev/null +++ b/pagure/ui/remoteuser_login.py @@ -0,0 +1,107 @@ +# -*- coding: utf-8 -*- + +from __future__ import unicode_literals + +import logging + +import flask +from sqlalchemy.exc import SQLAlchemyError +import pagure +import pagure.lib.model as model +import pagure.login_forms as forms +from pagure.flask_app import logout as flask_app_logout +from pagure.ui.login import send_confirmation_email +from pagure.ui import UI_NS + + +""" remote_user authentication for pagure + +Expects the front web server to handle authentication. + +Only /login and /usr/remote/new need to be protected by front web server +as other pages either don't need protection at all or are protected by +pagure itself. + +Example for simple file-based Apache httpd (only what's relevant for auth): + + AuthType Basic + AuthName "Pagure Login" + AuthBasicProvider file + AuthUserFile "/path/to/htpasswd" + Require valid-user + +Of course this also works with other mod_auth modules like Kerberos/SAML/OIDC. + +See also: auth switch in create_app method at flask_app.py +""" + +_log = logging.getLogger(__name__) + + +@UI_NS.route( + "/user/remote/new/", methods=["GET", "POST"], endpoint="new_remote_user" +) +@UI_NS.route( + "/user/remote/new", methods=["GET", "POST"], endpoint="new_remote_user" +) +def new_remote_user(): + """ Create a new user from REMOTE_USER. + """ + form = forms.NewRemoteUserForm() + if form.validate_on_submit(): + + username = flask.request.environ.get("REMOTE_USER") + if not username: + flask.flash( + "Server malconfiguration: Tried to access " + " /user/remote/new without REMOTE_USER. " + "Please inform administrator." + ) + return flask.redirect(flask.url_for("ui_ns.index")) + dbuser = pagure.lib.query.search_user(flask.g.session, username=username) + if dbuser: + flask.flash("User already exists!", "error") + return flask.redirect(flask.url_for("auth_login")) + + email = form.email_address.data + if pagure.lib.query.search_user(flask.g.session, email=email): + flask.flash("Email address already taken.", "error") + flask.redirect(flask.url_for("ui_ns.new_remote_user")) + + user = model.User() + user.user = username + user.token = pagure.lib.login.id_generator(40) + form.populate_obj(obj=user) + user.default_email = form.email_address.data + flask.g.session.add(user) + flask.g.session.flush() + + try: + pagure.lib.query.add_email_to_user( + flask.g.session, user, form.email_address.data + ) + flask.g.session.commit() + send_confirmation_email(user) + flask.flash( + "User created, please check your email to activate the " + "account" + ) + except pagure.exceptions.PagureException as err: + flask.flash(str(err), "error") + _log.exception(err) + except SQLAlchemyError: # pragma: no cover + flask.g.session.rollback() + flask.flash("Could not create user.") + _log.exception("Could not create user.") + + return flask.redirect(flask.url_for("ui_ns.index")) + + return flask.render_template("login/remote_user_new.html", form=form) + + +def logout(): + """ Remove login metadata in case login process failed + """ + flask.g.fas_session_id = None + flask.g.fas_user = None + flask_app_logout() diff --git a/tests/test_pagure_flask_ui_remoteuser_login.py b/tests/test_pagure_flask_ui_remoteuser_login.py new file mode 100644 index 0000000..b6be270 --- /dev/null +++ b/tests/test_pagure_flask_ui_remoteuser_login.py @@ -0,0 +1,291 @@ +# -*- coding: utf-8 -*- + +from __future__ import unicode_literals + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import unittest +import sys +import os +import tests +import pagure.lib +import pagure.ui.remoteuser_login +from mock import patch, MagicMock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + + +class PagureFlaskRemoteUserLogintests(tests.SimplePagureTest): + """ Tests for remote_login authentication of pagure """ + + def setUp(self): + """ Create the application with PAGURE_AUTH set to remoteuser. """ + super(PagureFlaskRemoteUserLogintests, self).setUp() + + app = pagure.flask_app.create_app({ + 'DB_URL': self.dbpath, + 'PAGURE_AUTH': 'remoteuser' + }) + # Remove the log handlers for the tests + app.logger.handlers = [] + + self.app = app.test_client() + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_front_page_without_remote_user(self): + """ Test the front page without remote_user set """ + output1 = self.app.get('/') + o1data = output1.get_data(as_text=True) + self.assertEqual(output1.status_code, 200) + self.assertIn( + 'Home - Pagure', + o1data + ) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_front_page_with_remote_user(self): + """ test the front page with remote_user set """ + o1data = self.app.get('/').get_data(as_text=True) + output2 = self.app.get('/', environ_base={'REMOTE_USER': 'foo'}) + o2data = output2.get_data(as_text=True) + self.assertEqual(output2.status_code, 200) + self.assertIn( + 'Home - Pagure', + o2data + ) + self.assertEqual(o1data, o2data) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_new_user_page(self): + """ Check new user page """ + output = self.app.get( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'New user from REMOTE_USER - Pagure', + output_text + ) + self.assertIn( + 'Create new account for "foo"', + output_text + ) + self.assertIn( + '
', + output_text + ) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_new_user_no_csrf(self): + """ don't accept without csrf token """ + output = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + data={ + 'fullname': 'foo m. atic', + 'email_address': 'foo@bar.com', + }, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'New user from REMOTE_USER - Pagure', + output_text + ) + self.assertIn( + '', + output_text + ) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_new_user_no_email(self): + """ don't accept without email """ + csrf_token = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + ).get_data(as_text=True).split( + 'name="csrf_token" type="hidden" value="' + )[1].split('">')[0] + output = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + data={ + 'fullname': 'foo m. atic', + 'email_address': None, + 'csrf_token': csrf_token + }, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'New user from REMOTE_USER - Pagure', + output_text + ) + self.assertIn( + '', + output_text + ) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_new_user_exists(self): + """ submit form with proper data but existing user """ + csrf_token = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + ).get_data(as_text=True).split( + 'name="csrf_token" type="hidden" value="' + )[1].split('">')[0] + output = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + data={ + 'fullname': 'foo m. atic', + 'email_address': 'bar@baz.test', + 'csrf_token': csrf_token + }, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'New user from REMOTE_USER - Pagure', + output_text + ) + self.assertIn('User already exists!', output_text) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_new_user_page_no_remote_user(self): + """ Can't submit without remote_user """ + csrf_token = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foo'}, + ).get_data(as_text=True).split( + 'name="csrf_token" type="hidden" value="' + )[1].split('">')[0] + items = pagure.lib.query.search_user(self.session) + self.assertEqual(2, len(items)) + output = self.app.post( + '/user/remote/new', + data={ + 'fullname': 'foo m. atic', + 'email_address': 'foobar@example.com', + 'csrf_token': csrf_token + }, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('Server malconfiguration', output_text) + self.assertIn('Home - Pagure', output_text) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_new_user(self): + """ Test the /user/remote/new endpoint with valid data """ + csrf_token = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foobar'}, + ).get_data(as_text=True).split( + 'name="csrf_token" type="hidden" value="' + )[1].split('">')[0] + items = pagure.lib.query.search_user(self.session) + self.assertEqual(2, len(items)) + output = self.app.post( + '/user/remote/new', + environ_base={'REMOTE_USER': 'foobar'}, + data={ + 'fullname': 'foo m. atic', + 'email_address': 'foobar@example.com', + 'csrf_token': csrf_token + }, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('Home - Pagure', output_text) + self.assertIn('Welcome', output_text) + items = pagure.lib.query.search_user(self.session) + self.assertEqual(3, len(items)) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict('pagure.config.config', {'CHECK_SESSION_IP': False}) + def test_login_valid(self): + """ Test to use /login/ with valid user """ + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + output = self.app.get( + '/login/', + environ_base={'REMOTE_USER': 'pingou'}, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('Home - Pagure', output_text) + self.assertNotIn('>Log In', output_text) + self.assertNotIn('>Log Out', output_text) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_login_no_remoteuser(self): + """ Test to use /login/ without remote_user """ + output = self.app.post( + '/login', + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'New user from REMOTE_USER - Pagure', + output.get_data(as_text=True) + ) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + def test_login_new_remoteuser(self): + """ Test to use /login/ with yet-unknown remote_user """ + output = self.app.post( + '/login', + environ_base={'REMOTE_USER': 'foobarbaz'}, + follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'New user from REMOTE_USER - Pagure', + output.get_data(as_text=True) + ) + + @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict('pagure.config.config', {'CHECK_SESSION_IP': False}) + def test_login_createproj_settings(self): + """ Test that user can login, create project and see + Settings button of own project """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos')) + output = self.app.get( + '/test', + environ_base={'REMOTE_USER': 'pingou'}) + output_text = output.get_data(as_text=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Overview - test - Pagure', output_text) + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + output = self.app.get( + '/test', + environ_base={'REMOTE_USER': 'pingou'} + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Overview - test - Pagure', output_text) + self.assertIn( + 'Settings', + output_text) + + +if __name__ == '__main__': + unittest.main(verbosity=2) From d2faaf7d7ee161f0cbfe832d8644047436b56bcb Mon Sep 17 00:00:00 2001 From: Ansgar Tasler Date: Jul 10 2019 17:32:37 +0000 Subject: [PATCH 2/7] style fixes, thanks for all the work 'black' --- diff --git a/pagure/ui/remoteuser_login.py b/pagure/ui/remoteuser_login.py index 3ca1751..53e1b21 100644 --- a/pagure/ui/remoteuser_login.py +++ b/pagure/ui/remoteuser_login.py @@ -58,7 +58,9 @@ def new_remote_user(): "Please inform administrator." ) return flask.redirect(flask.url_for("ui_ns.index")) - dbuser = pagure.lib.query.search_user(flask.g.session, username=username) + dbuser = pagure.lib.query.search_user( + flask.g.session, username=username + ) if dbuser: flask.flash("User already exists!", "error") return flask.redirect(flask.url_for("auth_login")) diff --git a/tests/test_pagure_flask_ui_remoteuser_login.py b/tests/test_pagure_flask_ui_remoteuser_login.py index b6be270..9f5bb16 100644 --- a/tests/test_pagure_flask_ui_remoteuser_login.py +++ b/tests/test_pagure_flask_ui_remoteuser_login.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals -__requires__ = ['SQLAlchemy >= 0.8'] +__requires__ = ["SQLAlchemy >= 0.8"] import pkg_resources import unittest @@ -13,8 +13,9 @@ import pagure.lib import pagure.ui.remoteuser_login from mock import patch, MagicMock -sys.path.insert(0, os.path.join(os.path.dirname( - os.path.abspath(__file__)), '..')) +sys.path.insert( + 0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") +) class PagureFlaskRemoteUserLogintests(tests.SimplePagureTest): @@ -24,268 +25,250 @@ class PagureFlaskRemoteUserLogintests(tests.SimplePagureTest): """ Create the application with PAGURE_AUTH set to remoteuser. """ super(PagureFlaskRemoteUserLogintests, self).setUp() - app = pagure.flask_app.create_app({ - 'DB_URL': self.dbpath, - 'PAGURE_AUTH': 'remoteuser' - }) + app = pagure.flask_app.create_app( + {"DB_URL": self.dbpath, "PAGURE_AUTH": "remoteuser"} + ) # Remove the log handlers for the tests app.logger.handlers = [] self.app = app.test_client() - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_front_page_without_remote_user(self): """ Test the front page without remote_user set """ - output1 = self.app.get('/') + output1 = self.app.get("/") o1data = output1.get_data(as_text=True) self.assertEqual(output1.status_code, 200) - self.assertIn( - 'Home - Pagure', - o1data - ) + self.assertIn("Home - Pagure", o1data) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_front_page_with_remote_user(self): """ test the front page with remote_user set """ - o1data = self.app.get('/').get_data(as_text=True) - output2 = self.app.get('/', environ_base={'REMOTE_USER': 'foo'}) + o1data = self.app.get("/").get_data(as_text=True) + output2 = self.app.get("/", environ_base={"REMOTE_USER": "foo"}) o2data = output2.get_data(as_text=True) self.assertEqual(output2.status_code, 200) - self.assertIn( - 'Home - Pagure', - o2data - ) + self.assertIn("Home - Pagure", o2data) self.assertEqual(o1data, o2data) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_new_user_page(self): """ Check new user page """ output = self.app.get( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, - follow_redirects=True + "/user/remote/new", + environ_base={"REMOTE_USER": "foo"}, + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( - 'New user from REMOTE_USER - Pagure', - output_text - ) - self.assertIn( - 'Create new account for "foo"', - output_text + "New user from REMOTE_USER - Pagure", output_text ) + self.assertIn('Create new account for "foo"', output_text) self.assertIn( - '', - output_text + '', output_text ) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_new_user_no_csrf(self): """ don't accept without csrf token """ output = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, - data={ - 'fullname': 'foo m. atic', - 'email_address': 'foo@bar.com', - }, - follow_redirects=True + "/user/remote/new", + environ_base={"REMOTE_USER": "foo"}, + data={"fullname": "foo m. atic", "email_address": "foo@bar.com"}, + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( - 'New user from REMOTE_USER - Pagure', - output_text + "New user from REMOTE_USER - Pagure", output_text ) self.assertIn( - '', - output_text + '', output_text ) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_new_user_no_email(self): """ don't accept without email """ - csrf_token = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, - ).get_data(as_text=True).split( - 'name="csrf_token" type="hidden" value="' - )[1].split('">')[0] + csrf_token = ( + self.app.post( + "/user/remote/new", environ_base={"REMOTE_USER": "foo"} + ) + .get_data(as_text=True) + .split('name="csrf_token" type="hidden" value="')[1] + .split('">')[0] + ) output = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, + "/user/remote/new", + environ_base={"REMOTE_USER": "foo"}, data={ - 'fullname': 'foo m. atic', - 'email_address': None, - 'csrf_token': csrf_token + "fullname": "foo m. atic", + "email_address": None, + "csrf_token": csrf_token, }, - follow_redirects=True + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( - 'New user from REMOTE_USER - Pagure', - output_text + "New user from REMOTE_USER - Pagure", output_text ) self.assertIn( - '', - output_text + '', output_text ) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_new_user_exists(self): """ submit form with proper data but existing user """ - csrf_token = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, - ).get_data(as_text=True).split( - 'name="csrf_token" type="hidden" value="' - )[1].split('">')[0] + csrf_token = ( + self.app.post( + "/user/remote/new", environ_base={"REMOTE_USER": "foo"} + ) + .get_data(as_text=True) + .split('name="csrf_token" type="hidden" value="')[1] + .split('">')[0] + ) output = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, + "/user/remote/new", + environ_base={"REMOTE_USER": "foo"}, data={ - 'fullname': 'foo m. atic', - 'email_address': 'bar@baz.test', - 'csrf_token': csrf_token + "fullname": "foo m. atic", + "email_address": "bar@baz.test", + "csrf_token": csrf_token, }, - follow_redirects=True + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( - 'New user from REMOTE_USER - Pagure', - output_text + "New user from REMOTE_USER - Pagure", output_text ) - self.assertIn('User already exists!', output_text) + self.assertIn("User already exists!", output_text) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_new_user_page_no_remote_user(self): """ Can't submit without remote_user """ - csrf_token = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foo'}, - ).get_data(as_text=True).split( - 'name="csrf_token" type="hidden" value="' - )[1].split('">')[0] + csrf_token = ( + self.app.post( + "/user/remote/new", environ_base={"REMOTE_USER": "foo"} + ) + .get_data(as_text=True) + .split('name="csrf_token" type="hidden" value="')[1] + .split('">')[0] + ) items = pagure.lib.query.search_user(self.session) self.assertEqual(2, len(items)) output = self.app.post( - '/user/remote/new', + "/user/remote/new", data={ - 'fullname': 'foo m. atic', - 'email_address': 'foobar@example.com', - 'csrf_token': csrf_token + "fullname": "foo m. atic", + "email_address": "foobar@example.com", + "csrf_token": csrf_token, }, - follow_redirects=True + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('Server malconfiguration', output_text) - self.assertIn('Home - Pagure', output_text) + self.assertIn("Server malconfiguration", output_text) + self.assertIn("Home - Pagure", output_text) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) - @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) + @patch("pagure.lib.notify.send_email", MagicMock(return_value=True)) def test_new_user(self): """ Test the /user/remote/new endpoint with valid data """ - csrf_token = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foobar'}, - ).get_data(as_text=True).split( - 'name="csrf_token" type="hidden" value="' - )[1].split('">')[0] + csrf_token = ( + self.app.post( + "/user/remote/new", environ_base={"REMOTE_USER": "foobar"} + ) + .get_data(as_text=True) + .split('name="csrf_token" type="hidden" value="')[1] + .split('">')[0] + ) items = pagure.lib.query.search_user(self.session) self.assertEqual(2, len(items)) output = self.app.post( - '/user/remote/new', - environ_base={'REMOTE_USER': 'foobar'}, + "/user/remote/new", + environ_base={"REMOTE_USER": "foobar"}, data={ - 'fullname': 'foo m. atic', - 'email_address': 'foobar@example.com', - 'csrf_token': csrf_token + "fullname": "foo m. atic", + "email_address": "foobar@example.com", + "csrf_token": csrf_token, }, - follow_redirects=True + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('Home - Pagure', output_text) - self.assertIn('Welcome', output_text) + self.assertIn("Home - Pagure", output_text) + self.assertIn("Welcome", output_text) items = pagure.lib.query.search_user(self.session) self.assertEqual(3, len(items)) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) - @patch.dict('pagure.config.config', {'CHECK_SESSION_IP': False}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) + @patch.dict("pagure.config.config", {"CHECK_SESSION_IP": False}) def test_login_valid(self): """ Test to use /login/ with valid user """ - user = tests.FakeUser(username='pingou') + user = tests.FakeUser(username="pingou") with tests.user_set(self.app.application, user): output = self.app.get( - '/login/', - environ_base={'REMOTE_USER': 'pingou'}, - follow_redirects=True + "/login/", + environ_base={"REMOTE_USER": "pingou"}, + follow_redirects=True, ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('Home - Pagure', output_text) - self.assertNotIn('>Log In', output_text) - self.assertNotIn('>Log Out', output_text) + self.assertIn("Home - Pagure", output_text) + self.assertNotIn(">Log In", output_text) + self.assertNotIn(">Log Out", output_text) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_login_no_remoteuser(self): """ Test to use /login/ without remote_user """ - output = self.app.post( - '/login', - follow_redirects=True - ) + output = self.app.post("/login", follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - 'New user from REMOTE_USER - Pagure', - output.get_data(as_text=True) + "New user from REMOTE_USER - Pagure", + output.get_data(as_text=True), ) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) def test_login_new_remoteuser(self): """ Test to use /login/ with yet-unknown remote_user """ output = self.app.post( - '/login', - environ_base={'REMOTE_USER': 'foobarbaz'}, - follow_redirects=True + "/login", + environ_base={"REMOTE_USER": "foobarbaz"}, + follow_redirects=True, ) self.assertEqual(output.status_code, 200) self.assertIn( - 'New user from REMOTE_USER - Pagure', - output.get_data(as_text=True) + "New user from REMOTE_USER - Pagure", + output.get_data(as_text=True), ) - @patch.dict('pagure.config.config', {'PAGURE_AUTH': 'remoteuser'}) - @patch.dict('pagure.config.config', {'CHECK_SESSION_IP': False}) + @patch.dict("pagure.config.config", {"PAGURE_AUTH": "remoteuser"}) + @patch.dict("pagure.config.config", {"CHECK_SESSION_IP": False}) def test_login_createproj_settings(self): """ Test that user can login, create project and see Settings button of own project """ tests.create_projects(self.session) - tests.create_projects_git(os.path.join(self.path, 'repos')) - output = self.app.get( - '/test', - environ_base={'REMOTE_USER': 'pingou'}) + tests.create_projects_git(os.path.join(self.path, "repos")) + output = self.app.get("/test", environ_base={"REMOTE_USER": "pingou"}) output_text = output.get_data(as_text=True) self.assertEqual(output.status_code, 200) - self.assertIn( - 'Overview - test - Pagure', output_text) - user = tests.FakeUser(username='pingou') + self.assertIn("Overview - test - Pagure", output_text) + user = tests.FakeUser(username="pingou") with tests.user_set(self.app.application, user): output = self.app.get( - '/test', - environ_base={'REMOTE_USER': 'pingou'} + "/test", environ_base={"REMOTE_USER": "pingou"} ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( - 'Overview - test - Pagure', output_text) + "Overview - test - Pagure", output_text + ) self.assertIn( - 'Settings', - output_text) + 'Settings', output_text + ) -if __name__ == '__main__': +if __name__ == "__main__": unittest.main(verbosity=2) From 287b70e6aaea5c93515cfae9f42e6942411164cc Mon Sep 17 00:00:00 2001 From: Ansgar Tasler Date: Jul 10 2019 17:32:37 +0000 Subject: [PATCH 3/7] removed unneeded endpoint name --- diff --git a/pagure/ui/remoteuser_login.py b/pagure/ui/remoteuser_login.py index 53e1b21..f585005 100644 --- a/pagure/ui/remoteuser_login.py +++ b/pagure/ui/remoteuser_login.py @@ -39,10 +39,10 @@ _log = logging.getLogger(__name__) @UI_NS.route( - "/user/remote/new/", methods=["GET", "POST"], endpoint="new_remote_user" + "/user/remote/new/", methods=["GET", "POST"] ) @UI_NS.route( - "/user/remote/new", methods=["GET", "POST"], endpoint="new_remote_user" + "/user/remote/new", methods=["GET", "POST"] ) def new_remote_user(): """ Create a new user from REMOTE_USER. From ea0ff5ba6b0b867958116cd59f41c6b7a9122df0 Mon Sep 17 00:00:00 2001 From: Ansgar Tasler Date: Jul 10 2019 17:32:37 +0000 Subject: [PATCH 4/7] moved flush() to db try/except block --- diff --git a/pagure/ui/remoteuser_login.py b/pagure/ui/remoteuser_login.py index f585005..267e602 100644 --- a/pagure/ui/remoteuser_login.py +++ b/pagure/ui/remoteuser_login.py @@ -76,9 +76,9 @@ def new_remote_user(): form.populate_obj(obj=user) user.default_email = form.email_address.data flask.g.session.add(user) - flask.g.session.flush() try: + flask.g.session.flush() pagure.lib.query.add_email_to_user( flask.g.session, user, form.email_address.data ) From 633fab8cd0d7e79303689504e1beaa07d47ca523 Mon Sep 17 00:00:00 2001 From: Ansgar Tasler Date: Jul 10 2019 17:32:37 +0000 Subject: [PATCH 5/7] removed dead code --- diff --git a/pagure/ui/remoteuser_login.py b/pagure/ui/remoteuser_login.py index 267e602..180b4c9 100644 --- a/pagure/ui/remoteuser_login.py +++ b/pagure/ui/remoteuser_login.py @@ -100,10 +100,3 @@ def new_remote_user(): return flask.render_template("login/remote_user_new.html", form=form) - -def logout(): - """ Remove login metadata in case login process failed - """ - flask.g.fas_session_id = None - flask.g.fas_user = None - flask_app_logout() From 98754ae0dce33c13b89ec19bb152cefd767f86ed Mon Sep 17 00:00:00 2001 From: Ansgar Tasler Date: Jul 10 2019 17:32:37 +0000 Subject: [PATCH 6/7] fixed formatting issue --- diff --git a/pagure/ui/remoteuser_login.py b/pagure/ui/remoteuser_login.py index 180b4c9..afdbb67 100644 --- a/pagure/ui/remoteuser_login.py +++ b/pagure/ui/remoteuser_login.py @@ -38,12 +38,8 @@ See also: auth switch in create_app method at flask_app.py _log = logging.getLogger(__name__) -@UI_NS.route( - "/user/remote/new/", methods=["GET", "POST"] -) -@UI_NS.route( - "/user/remote/new", methods=["GET", "POST"] -) +@UI_NS.route("/user/remote/new/", methods=["GET", "POST"]) +@UI_NS.route("/user/remote/new", methods=["GET", "POST"]) def new_remote_user(): """ Create a new user from REMOTE_USER. """ @@ -99,4 +95,3 @@ def new_remote_user(): return flask.redirect(flask.url_for("ui_ns.index")) return flask.render_template("login/remote_user_new.html", form=form) -