#3969 Add REMOTE_USER authentication support
Opened 5 years ago by tasansga. Modified 4 years ago

file modified
+28 -2
@@ -29,6 +29,7 @@ 

  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 @@ 

  

          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 @@ 

  

          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 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 @@ 

              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)

  

file modified
+12
@@ -113,3 +113,15 @@ 

          'Confirm password  <span class="error">*</span>',

          [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  <span class="error">*</span>',

+         [wtforms.validators.Required(), wtforms.validators.Email()],

+     )

@@ -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 %}

+ <div class="container">

+   <div class="row">

+     <div class="col-md-6 mx-auto pt-5">

+       <h4 class="text-center font-weight-bold mb-4">Create new account for "{{ request.environ.get('REMOTE_USER') }}"</h4>

+       <form action="{{ url_for('ui_ns.new_remote_user') }}" method="post">

+         {{ render_bootstrap_field(

+             form.fullname) }}

+         {{ render_bootstrap_field(

+             form.email_address) }}

+         <input class="btn btn-primary btn-block mt-4" type="submit" value="Create" title="Update description">

+         {{ form.csrf_token }}

+       </form>

+     </div>

+   </div>

+ </div>

+ {% endblock %}

@@ -82,9 +82,11 @@ 

                <a class="dropdown-item" href="{{

                  url_for('ui_ns.view_user_requests', username=g.fas_user.username)

                  }}">My Pull Requests</a>

+               {% if config.get("PAGURE_AUTH", None) != "remoteuser" %}

                <div class="dropdown-divider"></div>

                <a class="dropdown-item" href="{{ url_for('auth_logout')

                  }}?next={{ request.url }}">Log Out</a>

+               {% endif %}

              </div>

            </li>

            {% else %}

file modified
+2
@@ -28,6 +28,8 @@ 

  

  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()

file modified
+29 -23
@@ -85,6 +85,34 @@ 

      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 @@ 

                  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:

@@ -0,0 +1,97 @@ 

+ # -*- 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):

+ <Location ~ "/(login|user/remote/new)">

+     AuthType Basic

+     AuthName "Pagure Login"

+     AuthBasicProvider file

+     AuthUserFile "/path/to/htpasswd"

+     Require valid-user

+ </Location>

+ 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"])

+ @UI_NS.route("/user/remote/new", methods=["GET", "POST"])

+ 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)

+ 

+         try:

+             flask.g.session.flush()

+             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)

@@ -0,0 +1,274 @@ 

+ # -*- 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("<title>Home - Pagure</title>", 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("<title>Home - Pagure</title>", 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(

+             "<title>New user from REMOTE_USER - Pagure</title>", output_text

+         )

+         self.assertIn('Create new account for "foo"', output_text)

+         self.assertIn(

+             '<form action="/user/remote/new" method="post">', 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(

+             "<title>New user from REMOTE_USER - Pagure</title>", output_text

+         )

+         self.assertIn(

+             '<form action="/user/remote/new" method="post">', 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(

+             "<title>New user from REMOTE_USER - Pagure</title>", output_text

+         )

+         self.assertIn(

+             '<form action="/user/remote/new" method="post">', 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(

+             "<title>New user from REMOTE_USER - Pagure</title>", 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("<title>Home - Pagure</title>", 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("<title>Home - Pagure</title>", 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("<title>Home - Pagure</title>", output_text)

+             self.assertNotIn(">Log In</a>", output_text)

+             self.assertNotIn(">Log Out</a>", 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(

+             "<title>New user from REMOTE_USER - Pagure</title>",

+             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(

+             "<title>New user from REMOTE_USER - Pagure</title>",

+             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("<title>Overview - test - Pagure</title>", 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(

+                 "<title>Overview - test - Pagure</title>", output_text

+             )

+             self.assertIn(

+                 '<span class="d-none d-md-inline">Settings</span>', output_text

+             )

+ 

+ 

+ if __name__ == "__main__":

+     unittest.main(verbosity=2)

This patch adds support for REMOTE_USER authentication, which relies on webserver/reverse proxy-induced authentication. New users are asked to provide full name and email address. /login/ and /user/remote/new/ need to be protected by webserver. Anonymous access is still possible, open sessions are tracked by token similar to local login auth. Set PAGURE_AUTH = 'remoteuser' in pagure.cfg to enable.

Example for a simple file-based Apache httpd config (only what's relevant for auth):

<Location ~ "/(login|user/remote/new)">
    AuthType Basic
    AuthName "Pagure Login"
    AuthBasicProvider file
    AuthUserFile "/path/to/htpasswd"
    Require valid-user
</Location>

This should also work with other mod_auth modules like Kerberos/SAML/OIDC and with any WSGI-compatible webserver or reverse proxy.

It looks like it only failed the style test:

15:26:10 Failed tests:
15:26:10 FAILED test: py-test_style

Our code style is enforced by the black tool, which on Fedora is provided by the python3-black package.

1 new commit added

  • style fixes, thanks for all the work 'black'
5 years ago

I believe the endpoint argument isn't needed since the function name is the same

Let's put the flush in the try/except below since flush can raise an SQLAlchemyError

I'm not sure I see where this is being used, is it?

4 new commits added

  • fixed formatting issue
  • removed dead code
  • moved flush() to db try/except block
  • removed unneeded endpoint name
5 years ago

pretty please pagure-ci rebuild

rebased onto 9c444c7

4 years ago

rebased onto 9c444c7

4 years ago

pretty please pagure-ci rebuild

4 years ago