From 8126b1b994bbad288a36ecb8f3118e51232392eb Mon Sep 17 00:00:00 2001 From: Karsten Hopp Date: Jun 26 2019 14:30:05 +0000 Subject: [PATCH 1/2] add service accounts move service functions into a new file update DB with service accounts check if service accounts are enabled add description of service accounts flake8 and black fixes Signed-off-by: Karsten Hopp --- diff --git a/dev/ansible/roles/pagure-dev/files/pagure.cfg b/dev/ansible/roles/pagure-dev/files/pagure.cfg index 60e724f..2453c52 100644 --- a/dev/ansible/roles/pagure-dev/files/pagure.cfg +++ b/dev/ansible/roles/pagure-dev/files/pagure.cfg @@ -166,3 +166,9 @@ APPLICATION_ROOT = '/' # was running since before version 1.3 and if you care about backward # compatibility in your URLs. OLD_VIEW_COMMIT_ENABLED = False + +# Service accounts +# Service accounts are special user accounts whose tokens take longer to expire +# Service account names need to stert with 'service:' so they can be easily identified. +#ENABLE_SERVICE_ACCOUNTS = False + diff --git a/doc/configuration.rst b/doc/configuration.rst index f0d8f1f..f313188 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -1594,6 +1594,16 @@ If turned off, users are managed outside of pagure. Defaults to: ``True`` +ENABLE_SERVICE_ACCOUNTS +~~~~~~~~~~~~~~~~~~~~~~~ + +This configuration key allows to turn on or off service accounts. +Service accounts are special user accounts whose tokens take longer to +expire and who can have more than one admin to manage them + +Defaults to: ``False`` + + SESSION_COOKIE_NAME ~~~~~~~~~~~~~~~~~~~ diff --git a/files/pagure.cfg.sample b/files/pagure.cfg.sample index c9e96ed..40f5ebe 100644 --- a/files/pagure.cfg.sample +++ b/files/pagure.cfg.sample @@ -241,3 +241,9 @@ REPOSPANNER_ADMIN_MIGRATION = False # 'push_cert': {'cert': '', # 'key': ''}} REPOSPANNER_REGIONS = {} + +# Service accounts are special user accounts whose tokens take longer to +# expire and who can have more than one admin to manage them +# This configuration key allows to turn on or off service accounts. +# Service account names need to stert with 'service:' so they can be easily identified. +#ENABLE_SERVICE_ACCOUNTS = False diff --git a/pagure/flask_app.py b/pagure/flask_app.py index 6f16786..76a06ac 100644 --- a/pagure/flask_app.py +++ b/pagure/flask_app.py @@ -509,7 +509,38 @@ def after_request(response): def _get_user(username): """ Check if user exists or not """ + temp = None try: - return pagure.lib.query.get_user(flask.g.session, username) + temp = pagure.lib.query.get_user(flask.g.session, username) + if isinstance(temp, list): + return temp[0] + else: + return temp except pagure.exceptions.PagureException as e: flask.abort(404, description="%s" % e) + + +def get_other_user(username=None): + """ returns obj of an user other than the one that is currently logged in, + if the current user is admin of this pagure instance + or the other user is a service account and the current user is an + admin of that service account. + returns obj of the current user is above conditions are not met + """ + name = None + if username: + if pagure.utils.is_admin(): + name = username + else: + # not pagure admin but might be a service account admin + this_user_name = flask.g.fas_user.username + the_other_user = pagure.lib.query.search_user( + flask.g.session, username=username + ) + if the_other_user.is_service: + if ( + this_user_name + in the_other_user.servicemaintainers.split(",") + ): + name = the_other_user + return _get_user(name) diff --git a/pagure/forms.py b/pagure/forms.py index eb8db07..811618c 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -690,6 +690,22 @@ class UserEmailForm(PagureForm): self.email.validators = [wtforms.validators.DataRequired()] +class ServiceAdminForm(PagureForm): + """ Form to add new service admins to a service acount """ + + admin = wtforms.StringField("admin", [wtforms.validators.DataRequired()]) + + def __init__(self, *args, **kwargs): + super(ServiceAdminForm, self).__init__(*args, **kwargs) + if "admins" in kwargs: + if kwargs["admins"]: + self.admin.validators.append( + wtforms.validators.NoneOf(kwargs["admins"]) + ) + else: + self.admin.validators = [wtforms.validators.DataRequired()] + + class ProjectCommentForm(PagureForm): """ Form to represent project. """ diff --git a/pagure/lib/model.py b/pagure/lib/model.py index fd230a3..f49ccb8 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -197,6 +197,8 @@ class User(BASE): id = sa.Column(sa.Integer, primary_key=True) user = sa.Column(sa.String(255), nullable=False, unique=True, index=True) fullname = sa.Column(sa.String(255), nullable=False, index=True) + is_service = sa.Column(sa.Boolean, nullable=True) + servicemaintainers = sa.Column(sa.Text, nullable=True) default_email = sa.Column(sa.Text, nullable=False) _settings = sa.Column(sa.Text, nullable=True) diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 008e731..5a25add 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -179,7 +179,14 @@ def get_next_id(session, projectid): return nid + 1 -def search_user(session, username=None, email=None, token=None, pattern=None): +def search_user( + session, + username=None, + email=None, + token=None, + pattern=None, + is_service=None, +): """ Searches the database for the user or users matching the given criterias. @@ -194,6 +201,8 @@ def search_user(session, username=None, email=None, token=None, pattern=None): :type pattern: string or None :return: A single User object if any of username, email or token is specified, a list of User objects otherwise. + :type is_service: string or None + :kwarg is_service: A list of User objects that are service accounts :rtype: User or [User] """ @@ -214,6 +223,9 @@ def search_user(session, username=None, email=None, token=None, pattern=None): pattern = pattern.replace("*", "%") query = query.filter(model.User.user.like(pattern)) + if is_service is not None: + query = query.filter(model.User.is_service == is_service) + if any([username, email, token]): output = query.first() else: @@ -4253,12 +4265,17 @@ def add_token_to_user(session, project, acls, username, description=None): user = search_user(session, username=username) + if user.is_service: + expiredays = 180 + else: + expiredays = 60 token = pagure.lib.model.Token( id=pagure.lib.login.id_generator(64), user_id=user.id, project_id=project.id if project else None, description=description, - expiration=datetime.datetime.utcnow() + datetime.timedelta(days=60), + expiration=datetime.datetime.utcnow() + + datetime.timedelta(days=expiredays), ) session.add(token) session.flush() diff --git a/pagure/login_forms.py b/pagure/login_forms.py index 2cd8d5c..9a402b6 100644 --- a/pagure/login_forms.py +++ b/pagure/login_forms.py @@ -98,6 +98,22 @@ class NewUserForm(FlaskForm): ) +class NewServiceForm(FlaskForm): + """ Form to add a new service account to the local database. """ + + user = wtforms.StringField( + 'Service Name *', + [wtforms.validators.DataRequired()], + ) + fullname = wtforms.StringField( + "Description", [wtforms.validators.Optional()] + ) + service_admins = wtforms.StringField( + 'Comma separated list of service admins *', + [wtforms.validators.DataRequired()], + ) + + class ChangePasswordForm(FlaskForm): """ Form to reset one's password in the local database. """ diff --git a/pagure/templates/add_token.html b/pagure/templates/add_token.html index 7ea5bf3..6e59959 100644 --- a/pagure/templates/add_token.html +++ b/pagure/templates/add_token.html @@ -31,7 +31,7 @@ repo=repo.name, namespace=repo.namespace) }}" method="post"> {% else %} -
+ {% endif %} {{ render_bootstrap_field( form.description, field_description="Small description of this API token") }} diff --git a/pagure/templates/login/service_account_new.html b/pagure/templates/login/service_account_new.html new file mode 100644 index 0000000..2ac3288 --- /dev/null +++ b/pagure/templates/login/service_account_new.html @@ -0,0 +1,29 @@ +{% extends "master.html" %} +{% from "_formhelper.html" import render_bootstrap_field %} + +{% block title %}New Service Account{% endblock %} +{% set tag = "home" %} + +{% block content %} +
+
+
+ {% if config.get('ENABLE_SERVICE_ACCOUNTS', False) %} +

Create new service account

+ + {{ render_bootstrap_field( + form.user) }} + {{ render_bootstrap_field( + form.fullname) }} + {{ render_bootstrap_field( + form.service_admins) }} + + {{ form.csrf_token }} + + {% else %} +

You need to set ENABLE_SERVICE_ACCOUNTS to True in the configuration before you'll be able to do anything here

+ {% endif %} +
+
+
+{% endblock %} diff --git a/pagure/templates/master.html b/pagure/templates/master.html index fab6dd8..dff99ce 100644 --- a/pagure/templates/master.html +++ b/pagure/templates/master.html @@ -74,6 +74,12 @@ My Starred Projects + {% if config.get('ENABLE_SERVICE_ACCOUNTS', False) %} + Manage Services + {% endif %} +
{% if config.get('ENABLE_TICKETS', True) %} +
+ {% if config.get('ENABLE_SERVICE_ACCOUNTS', False) %} +
+

List of service accounts

+ {% if service_accounts %} +
+ {% for account in service_accounts %} +

+ {{ account.user }} +

+ {{ account.fullname }} + {% endfor %} +
+ {% else %} +

+ None available yet +

+ {% endif %} + {% if g.admin %} + New Service Account + {% endif %} +
+ {% else %} +
+

You need to set ENABLE_SERVICE_ACCOUNTS to True in the configuration before you'll be able to do anything here

+
+ {% endif %} +
+ +{% endblock %} diff --git a/pagure/templates/service_admins.html b/pagure/templates/service_admins.html new file mode 100644 index 0000000..0a8ea12 --- /dev/null +++ b/pagure/templates/service_admins.html @@ -0,0 +1,37 @@ +{% extends "master.html" %} +{% from "_formhelper.html" import render_bootstrap_field %} + +{% block title %}Add Service Admin{% endblock %} +{% set tag = "home" %} + +{% block content %} +
+
+
+
+ Add new Service Admin +
+
+ {% if config.get('ENABLE_SERVICE_ACCOUNTS', False) %} + {% if username %} +
+ {{ render_bootstrap_field(form.admin) }} + + + {{ form.csrf_token }} +
+ {% else %} +
+

Nothing for you to see here

+
+ {% endif %} + {% else %} +
+

You need to set ENABLE_SERVICE_ACCOUNTS to True in the configuration before you'll be able to do anything here

+
+ {% endif %} +
+
+
+
+{% endblock %} diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 56d400d..7b8d006 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -85,7 +85,7 @@ {% endif %} {% if config.get('ENABLE_GIVE_PROJECTS', True) - and (repo.user.user == g.fas_user.username or pagure_admin) + and (repo.user.user == g.fas_user.username or g.admin) and not repo.is_fork %} Give Project @@ -1035,7 +1035,7 @@ {% if config.get('ENABLE_GIVE_PROJECTS', True) - and (repo.user.user == g.fas_user.username or pagure_admin) + and (repo.user.user == g.fas_user.username or g.admin) and not repo.is_fork %}

diff --git a/pagure/templates/user_settings.html b/pagure/templates/user_settings.html index 6dda073..327ea42 100644 --- a/pagure/templates/user_settings.html +++ b/pagure/templates/user_settings.html @@ -5,12 +5,28 @@ {% block title %}{{ user.user }}'s settings{% endblock %} {% set tag = "users"%} +{% macro render_admin(admin, form) %} +
+  {{ admin }} +
+ + {{ form.csrf_token }} + +
+
+{% endmacro %} + {% macro render_email(email, form, validated=True) %}
 {{ email.email }} {% if validated %}
+ action="{{ url_for('ui_ns.remove_user_email', username=user.user) }}"> {{ form.csrf_token }}

+ {% if user.is_service %} + + {% endif %} - {% if config.get('LOCAL_SSH_KEY', True) %}
- +
@@ -270,7 +313,7 @@
- +
{% for key in user.settings | sort %} {% if user.settings[key] in [True, False, 'y'] %} diff --git a/pagure/templates/userprofile_overview.html b/pagure/templates/userprofile_overview.html index 13783a3..63e7c38 100644 --- a/pagure/templates/userprofile_overview.html +++ b/pagure/templates/userprofile_overview.html @@ -8,6 +8,10 @@ {% block userprofile_content %}
+ {% if pagure_admin %} +

You are logged in with Pagure Admin permissions !

+

+ {% endif %} {% if owned_repos %}

Top {{projectstring(plural=True)}}

diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 697c148..b3bdd65 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -23,8 +23,9 @@ import pagure.lib.git import pagure.lib.query import pagure.forms import pagure.ui.filters +import pagure.ui.services from pagure.config import config as pagure_config -from pagure.flask_app import _get_user, admin_session_timedout +from pagure.flask_app import _get_user, admin_session_timedout, get_other_user from pagure.ui import UI_NS from pagure.utils import ( authenticated, @@ -1119,8 +1120,9 @@ def wait_task(taskid): @UI_NS.route("/settings/") @UI_NS.route("/settings") +@UI_NS.route("/settings/") @login_required -def user_settings(): +def user_settings(username=None): """ Update the user settings. """ if admin_session_timedout(): @@ -1128,14 +1130,15 @@ def user_settings(): flask.url_for("auth_login", next=flask.request.url) ) - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) form = pagure.forms.ConfirmationForm() return flask.render_template("user_settings.html", user=user, form=form) @UI_NS.route("/settings/usersettings", methods=["POST"]) +@UI_NS.route("/settings/usersettings/", methods=["POST"]) @login_required -def update_user_settings(): +def update_user_settings(username=None): """ Update the user's settings set in the settings page. """ if admin_session_timedout(): @@ -1145,7 +1148,7 @@ def update_user_settings(): flask.url_for("auth_login", next=flask.request.url) ) - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) form = pagure.forms.ConfirmationForm() @@ -1169,12 +1172,15 @@ def update_user_settings(): flask.g.session.rollback() flask.flash(str(err), "error") - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) @UI_NS.route("/settings/usersettings/addkey", methods=["POST"]) +@UI_NS.route("/settings/usersettings//addkey", methods=["POST"]) @login_required -def add_user_sshkey(): +def add_user_sshkey(username=None): """ Add the specified SSH key to the user. """ if admin_session_timedout(): @@ -1186,8 +1192,8 @@ def add_user_sshkey(): form = pagure.forms.AddSSHKeyForm() + user = get_other_user(username) if form.validate_on_submit(): - user = _get_user(username=flask.g.fas_user.username) try: msg = pagure.lib.query.add_sshkey_to_project_or_user( flask.g.session, @@ -1203,7 +1209,10 @@ def add_user_sshkey(): pagure.lib.tasks.gitolite_post_compile_only.delay() flask.flash(msg) return flask.redirect( - flask.url_for("ui_ns.user_settings") + "#nav-ssh-tab" + flask.url_for("ui_ns.user_settings") + + "/" + + user.user + + "#nav-ssh-tab" ) except pagure.exceptions.PagureException as msg: flask.g.session.rollback() @@ -1214,14 +1223,21 @@ def add_user_sshkey(): _log.exception(err) flask.flash("SSH key could not be added", "error") + userstring = user.user or "" return flask.redirect( - flask.url_for("ui_ns.user_settings") + "#nav-ssh-tab" + flask.url_for("ui_ns.user_settings") + + "/" + + userstring + + "#nav-ssh-tab" ) @UI_NS.route("/settings/usersettings/removekey/", methods=["POST"]) +@UI_NS.route( + "/settings/usersettings/removekey//", methods=["POST"] +) @login_required -def remove_user_sshkey(keyid): +def remove_user_sshkey(keyid, username=None): """ Removes an SSH key from the user. """ if admin_session_timedout(): @@ -1231,8 +1247,8 @@ def remove_user_sshkey(keyid): flask.url_for("auth_login", next=flask.request.url) ) form = pagure.forms.ConfirmationForm() + user = get_other_user(username) if form.validate_on_submit(): - user = _get_user(username=flask.g.fas_user.username) found = False for key in user.sshkeys: if key.id == keyid: @@ -1243,7 +1259,10 @@ def remove_user_sshkey(keyid): if not found: flask.flash("SSH key does not exist in user.", "error") return flask.redirect( - flask.url_for("ui_ns.user_settings") + "#nav-ssh-tab" + flask.url_for("ui_ns.user_settings") + + "/" + + user.user + + "#nav-ssh-tab" ) try: @@ -1259,7 +1278,7 @@ def remove_user_sshkey(keyid): flask.flash("SSH key could not be removed", "error") return flask.redirect( - flask.url_for("ui_ns.user_settings") + "#nav-ssh-tab" + flask.url_for("ui_ns.user_settings") + "/" + user.user + "#nav-ssh-tab" ) @@ -1277,8 +1296,9 @@ def markdown_preview(): @UI_NS.route("/settings/email/drop", methods=["POST"]) +@UI_NS.route("/settings//email/drop", methods=["POST"]) @login_required -def remove_user_email(): +def remove_user_email(username=None): """ Remove the specified email from the logged in user. """ if admin_session_timedout(): @@ -1286,11 +1306,13 @@ def remove_user_email(): flask.url_for("auth_login", next=flask.request.url) ) - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) if len(user.emails) == 1: flask.flash("You must always have at least one email", "error") - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) form = pagure.forms.UserEmailForm() @@ -1303,7 +1325,9 @@ def remove_user_email(): "You do not have the email: %s, nothing to remove" % email, "error", ) - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) for mail in user.emails: if mail.email == email: @@ -1317,13 +1341,16 @@ def remove_user_email(): _log.exception(err) flask.flash("Email could not be removed", "error") - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) @UI_NS.route("/settings/email/add/", methods=["GET", "POST"]) @UI_NS.route("/settings/email/add", methods=["GET", "POST"]) +@UI_NS.route("/settings/email/add/", methods=["GET", "POST"]) @login_required -def add_user_email(): +def add_user_email(username=None): """ Add a new email for the logged in user. """ if admin_session_timedout(): @@ -1331,7 +1358,7 @@ def add_user_email(): flask.url_for("auth_login", next=flask.request.url) ) - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) form = pagure.forms.UserEmailForm( emails=[mail.email for mail in user.emails] @@ -1357,8 +1384,9 @@ def add_user_email(): @UI_NS.route("/settings/email/default", methods=["POST"]) +@UI_NS.route("/settings//email/default", methods=["POST"]) @login_required -def set_default_email(): +def set_default_email(username=None): """ Set the default email address of the user. """ if admin_session_timedout(): @@ -1366,7 +1394,7 @@ def set_default_email(): flask.url_for("auth_login", next=flask.request.url) ) - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) form = pagure.forms.UserEmailForm() if form.validate_on_submit(): @@ -1379,7 +1407,9 @@ def set_default_email(): "error", ) - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) user.default_email = email @@ -1391,12 +1421,15 @@ def set_default_email(): _log.exception(err) flask.flash("Default email could not be set", "error") - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) @UI_NS.route("/settings/email/resend", methods=["POST"]) +@UI_NS.route("/settings//email/resend", methods=["POST"]) @login_required -def reconfirm_email(): +def reconfirm_email(username=None): """ Re-send the email address of the user. """ if admin_session_timedout(): @@ -1404,7 +1437,7 @@ def reconfirm_email(): flask.url_for("auth_login", next=flask.request.url) ) - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) form = pagure.forms.UserEmailForm() if form.validate_on_submit(): @@ -1421,12 +1454,16 @@ def reconfirm_email(): _log.exception(err) flask.flash("Confirmation email could not be re-sent", "error") - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) @UI_NS.route("/settings/email/confirm//") @UI_NS.route("/settings/email/confirm/") -def confirm_email(token): +@UI_NS.route("/settings//email/confirm//") +@UI_NS.route("/settings//email/confirm/") +def confirm_email(token, username=None): """ Confirm a new email. """ if admin_session_timedout(): @@ -1437,6 +1474,7 @@ def confirm_email(token): email = pagure.lib.query.search_pending_email(flask.g.session, token=token) if not email: flask.flash("No email associated with this token.", "error") + return flask.redirect(flask.url_for("ui_ns.user_settings")) else: try: pagure.lib.query.add_email_to_user( @@ -1456,8 +1494,9 @@ def confirm_email(token): "error", ) _log.exception(err) - - return flask.redirect(flask.url_for("ui_ns.user_settings")) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + email.user.username + ) @UI_NS.route("/ssh_info/") @@ -1469,10 +1508,12 @@ def ssh_hostkey(): return flask.render_template("doc_ssh_keys.html") -@UI_NS.route("/settings/token/new/", methods=("GET", "POST")) +# UI_NS.route("/settings/token/new/", methods=("GET", "POST")) +# UI_NS.route("/settings/token/new", methods=("GET", "POST")) @UI_NS.route("/settings/token/new", methods=("GET", "POST")) +@UI_NS.route("/settings//token/new", methods=("GET", "POST")) @login_required -def add_api_user_token(): +def add_api_user_token(username=None): """ Create an user token (not project specific). """ if admin_session_timedout(): @@ -1483,7 +1524,7 @@ def add_api_user_token(): ) # Ensure the user is in the DB at least - user = _get_user(username=flask.g.fas_user.username) + user = get_other_user(username) acls = pagure.lib.query.get_acls( flask.g.session, restrict=pagure_config.get("CROSS_PROJECT_ACLS") @@ -1502,7 +1543,10 @@ def add_api_user_token(): flask.g.session.commit() flask.flash("Token created") return flask.redirect( - flask.url_for("ui_ns.user_settings") + "#nav-api-tab" + flask.url_for("ui_ns.user_settings") + + "/" + + user.user + + "#nav-api-tab" ) except SQLAlchemyError as err: # pragma: no cover flask.g.session.rollback() @@ -1514,14 +1558,19 @@ def add_api_user_token(): flask.flash("You must select at least one permission.", "error") return flask.render_template( - "add_token.html", select="settings", form=form, acls=acls + "add_token.html", + select="settings", + form=form, + acls=acls, + username=user.username, ) @UI_NS.route("/settings/token/revoke//", methods=["POST"]) @UI_NS.route("/settings/token/revoke/", methods=["POST"]) +@UI_NS.route("/settings/token/revoke//", methods=["POST"]) @login_required -def revoke_api_user_token(token_id): +def revoke_api_user_token(token_id, username=None): """ Revoke a user token (ie: not project specific). """ if admin_session_timedout(): @@ -1531,8 +1580,10 @@ def revoke_api_user_token(token_id): token = pagure.lib.query.get_api_token(flask.g.session, token_id) - if not token or token.user.username != flask.g.fas_user.username: - flask.abort(404, description="Token not found") + user = get_other_user(username) + + if not token or token.user.username != user.username: + flask.abort(404, "Token not found") form = pagure.forms.ConfirmationForm() @@ -1551,7 +1602,10 @@ def revoke_api_user_token(token_id): ) return flask.redirect( - flask.url_for("ui_ns.user_settings") + "#nav-api-token" + flask.url_for("ui_ns.user_settings") + + "/" + + user.user + + "#nav-api-token" ) diff --git a/pagure/ui/services.py b/pagure/ui/services.py new file mode 100644 index 0000000..3f6624a --- /dev/null +++ b/pagure/ui/services.py @@ -0,0 +1,258 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2014-2019 - Copyright Red Hat Inc + + Authors: + Karsten Hopp /serviceadmin/add", methods=["GET", "POST"]) +@login_required +def add_service_admin(username=None): + """add another user as serviceadmin to this service account. + """ + if admin_session_timedout(): + return flask.redirect( + flask.url_for("auth_login", next=flask.request.url) + ) + + user = get_other_user(username) + + form = pagure.forms.ServiceAdminForm( + admins=user.servicemaintainers.split(",") + ) + if form.validate_on_submit(): + newadmin = form.admin.data + serviceadmins = user.servicemaintainers.split(",") + if newadmin in serviceadmins: + flask.flash( + "'%s' is already an admin of the '%s' service account" + % (newadmin, username), + "error", + ) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + user_obj = pagure.lib.query.search_user( + flask.g.session, username=newadmin + ) + if not user_obj: + flask.flash("User " + newadmin + " does not exist.", "error") + _log.exception("User " + newadmin + " does not exist.") + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + serviceadmins.append(newadmin) + user.default_email = user.default_email.join( + ",%s" % user_obj.default_email + ) + user.servicemaintainers = ",".join(serviceadmins) + try: + flask.g.session.commit() + flask.flash("Admin added") + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + flask.flash("Admin could not be added", "error") + + return flask.render_template( + "service_admins.html", username=username, form=form + ) + + +@UI_NS.route( + "/settings//serviceadmin/remove/", methods=["GET", "POST"] +) +@login_required +def remove_service_admin(username=None, admin=None): + """ Remove the specified admin from the service account. + """ + if admin_session_timedout(): + return flask.redirect( + flask.url_for("auth_login", next=flask.request.url) + ) + + user = get_other_user(username) + + if len(user.servicemaintainers.split(",")) == 1: + flask.flash("You must always have at least one admin", "error") + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + + form = pagure.forms.ServiceAdminForm() + + if form.validate_on_submit(): + if not user.is_service: + flask.flash( + "Account '%s' is not a service account" % username, "error" + ) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + admin = form.admin.data + serviceadmins = user.servicemaintainers.split(",") + + if admin not in serviceadmins: + flask.flash( + "'%s' is not an admin of the '%s' service account" + % (admin, username), + "error", + ) + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + + serviceadmins.remove(admin) + user_obj = pagure.lib.query.search_user( + flask.g.session, username=admin + ) + if user_obj: + rmadmin_emails = set(user_obj.default_email.split(",")) + admin_emails = user.default_email.split(",") + admin_emails = [x for x in admin_emails if x not in rmadmin_emails] + user.default_email = ",".join(admin_emails) + user.servicemaintainers = ",".join(serviceadmins) + try: + flask.g.session.commit() + flask.flash("Admin %s removed" % admin) + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + flask.flash("Admin %s could not be removed" % admin, "error") + + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + user.user + ) + + +@UI_NS.route("/user//services/", methods=["GET", "POST"]) +@UI_NS.route("/user//services", methods=["GET", "POST"]) +def manage_services(username=None): + """ List, edit or create service accounts. + """ + if admin_session_timedout(): + return flask.redirect( + flask.url_for("auth_login", next=flask.request.url) + ) + + allowed_services = [] + users = pagure.lib.query.search_user(flask.g.session, is_service=True) + if not pagure.utils.is_admin(): + for service in users: + if username in service.servicemaintainers.split(","): + allowed_services.append(service) + if not allowed_services: + flask.flash("Restricted to pagure admins", "error") + return flask.redirect( + flask.url_for("ui_ns.user_settings") + "/" + username + ) + else: + users = allowed_services + + return flask.render_template( + "service_accounts.html", service_accounts=users + ) diff --git a/tests/__init__.py b/tests/__init__.py index 149ff15..fdcaac8 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -592,8 +592,9 @@ class FakeUser(object): # pylint: disable=too-few-public-methods self.user = username self.username = username self.name = username - self.email = "foo@bar.com" - self.default_email = "foo@bar.com" + self.email = 'foo@bar.com' + self.default_email = 'foo@bar.com' + self.is_service = False self.approved_memberships = [ FakeGroup("packager"), diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index 1778268..b07cc56 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -1152,15 +1152,21 @@ class PagureFlaskApptests(tests.Modeltests): user = tests.FakeUser(username="pingou") with tests.user_set(self.app.application, user): - output = self.app.get("/settings", follow_redirects=True) + output = self.app.get( + "/settings/pingou", data=data, follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + pagure.config.config["ENABLE_SERVICE_ACCOUNTS"] = True + output = self.app.get("/settings/pingou", follow_redirects=True) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn("Add SSH key", output_text) csrf_token = self.get_csrf(output=output) - data = {"ssh_key": "asdf"} + data = {"ssh_key": "asdf", "username": user.user} + pagure.config.config["ENABLE_SERVICE_ACCOUNTS"] = False # No CSRF token output = self.app.post( "/settings/usersettings/addkey", @@ -1172,6 +1178,7 @@ class PagureFlaskApptests(tests.Modeltests): self.assertIn("Add SSH key", output_text) data["csrf_token"] = csrf_token + data["username"] = user.user # First, invalid SSH key output = self.app.post( @@ -1201,6 +1208,7 @@ class PagureFlaskApptests(tests.Modeltests): data[ "ssh_key" ] = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAzBMSIlvPRaEiLOTVInErkRIw9CzQQcnslDekAn1jFnGf+SNa1acvbTiATbCX71AA03giKrPxPH79dxcC7aDXerc6zRcKjJs6MAL9PrCjnbyxCKXRNNZU5U9X/DLaaL1b3caB+WD6OoorhS3LTEtKPX8xyjOzhf3OQSzNjhJp5Q==" + data["username"] = "pingou" output = self.app.post( "/settings/usersettings/addkey", data=data, @@ -1252,7 +1260,7 @@ class PagureFlaskApptests(tests.Modeltests): user.username = "pingou" with tests.user_set(self.app.application, user): - data = {"csrf_token": self.get_csrf()} + data = {"csrf_token": self.get_csrf(), "username": user.user} output = self.app.post( "/settings/usersettings/removekey/1", @@ -1331,8 +1339,9 @@ class PagureFlaskApptests(tests.Modeltests): user = tests.FakeUser() user.username = "foo" + data["username"] = user.user with tests.user_set(self.app.application, user): - output = self.app.get("/settings/") + output = self.app.get("/settings/", data=data) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( @@ -1853,6 +1862,8 @@ class PagureFlaskApptests(tests.Modeltests): @patch("pagure.ui.app.admin_session_timedout") def test_confirm_email(self, ast): """ Test the confirm_email endpoint. """ + output = self.app.get("/settings/pingou/email/confirm/foobar") + self.assertEqual(output.status_code, 302) output = self.app.get("/settings/email/confirm/foobar") self.assertEqual(output.status_code, 302) @@ -1874,6 +1885,16 @@ class PagureFlaskApptests(tests.Modeltests): with tests.user_set(self.app.application, user): # Wrong token output = self.app.get( + "/settings/pingou/email/confirm/foobar", follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "pingou's settings - Pagure", output_text + ) + self.assertIn("No email associated with this token.", output_text) + # Wrong token + output = self.app.get( "/settings/email/confirm/foobar", follow_redirects=True ) self.assertEqual(output.status_code, 200) From 07ad58b4e01963f89d0de1095468440f4fd16ff6 Mon Sep 17 00:00:00 2001 From: Karsten Hopp Date: Jun 26 2019 14:30:12 +0000 Subject: [PATCH 2/2] black fixes --- diff --git a/pagure/flask_app.py b/pagure/flask_app.py index 76a06ac..9fa1bdd 100644 --- a/pagure/flask_app.py +++ b/pagure/flask_app.py @@ -538,9 +538,8 @@ def get_other_user(username=None): flask.g.session, username=username ) if the_other_user.is_service: - if ( - this_user_name - in the_other_user.servicemaintainers.split(",") + if this_user_name in the_other_user.servicemaintainers.split( + "," ): name = the_other_user return _get_user(name) diff --git a/tests/__init__.py b/tests/__init__.py index fdcaac8..26663b0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -592,8 +592,8 @@ class FakeUser(object): # pylint: disable=too-few-public-methods self.user = username self.username = username self.name = username - self.email = 'foo@bar.com' - self.default_email = 'foo@bar.com' + self.email = "foo@bar.com" + self.default_email = "foo@bar.com" self.is_service = False self.approved_memberships = [ diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index b07cc56..70ad92e 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -1153,7 +1153,7 @@ class PagureFlaskApptests(tests.Modeltests): user = tests.FakeUser(username="pingou") with tests.user_set(self.app.application, user): output = self.app.get( - "/settings/pingou", data=data, follow_redirects=True + "/settings/pingou", follow_redirects=True ) self.assertEqual(output.status_code, 200) pagure.config.config["ENABLE_SERVICE_ACCOUNTS"] = True