From 6572b907adae3924877514a8f8a054a304940f9f Mon Sep 17 00:00:00 2001 From: František Zatloukal Date: Apr 01 2021 09:04:21 +0000 Subject: [PATCH 1/3] OIDC improvements --- diff --git a/alembic/versions/8da50b403664_oidc_changes.py b/alembic/versions/8da50b403664_oidc_changes.py new file mode 100644 index 0000000..e2e896d --- /dev/null +++ b/alembic/versions/8da50b403664_oidc_changes.py @@ -0,0 +1,28 @@ +"""OIDC changes + +Revision ID: 8da50b403664 +Revises: 856636f558a1 +Create Date: 2021-03-31 11:52:59.177994 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '8da50b403664' +down_revision = '856636f558a1' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('dashboard_user_data', sa.Column('fas_groups', sa.Text(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('dashboard_user_data', 'fas_groups') + # ### end Alembic commands ### diff --git a/conf/settings.py.example b/conf/settings.py.example index 838f55b..e0efce5 100644 --- a/conf/settings.py.example +++ b/conf/settings.py.example @@ -10,6 +10,7 @@ OIDC_CLIENT_SECRETS = './conf/client_secrets.json.example' OIDC_ID_TOKEN_COOKIE_SECURE = False OIDC_REQUIRE_VERIFIED_EMAIL = False OIDC_SCOPES = ['openid', 'email', 'profile'] +OIDC_ID_TOKEN_COOKIE_TTL = 7 * 86400 # How often would users need to re-log in, in seconds # Celery config CELERY_BROKER_URL = 'redis://localhost:6379' diff --git a/oraculum/config.py b/oraculum/config.py index 57e91bc..3bd675b 100644 --- a/oraculum/config.py +++ b/oraculum/config.py @@ -45,6 +45,7 @@ class Config(object): OIDC_ID_TOKEN_COOKIE_SECURE = False OIDC_REQUIRE_VERIFIED_EMAIL = False OIDC_SCOPES = ['openid', 'email', 'profile'] + OIDC_ID_TOKEN_COOKIE_TTL = 7 * 86400 # How often would users need to re-log in, in seconds CELERY_BROKER_URL = 'redis://localhost:6379' CELERY_RESULT_BACKEND = 'redis://localhost:6379' @@ -222,7 +223,9 @@ def openshift_config(config_object, openshift_production): if os.getenv("BZ_API_KEY"): config_object["BZ_API_KEY"] = os.getenv("BZ_API_KEY") + # Some final touches for oidc + config_object["OIDC_SCOPES"].append("https://id.fedoraproject.org/scope/groups") if os.getenv("OVERWRITE_REDIRECT_URI"): config_object["OVERWRITE_REDIRECT_URI"] = os.getenv("OVERWRITE_REDIRECT_URI") diff --git a/oraculum/controllers/oidc_login.py b/oraculum/controllers/oidc_login.py index 660bd97..b181bc8 100644 --- a/oraculum/controllers/oidc_login.py +++ b/oraculum/controllers/oidc_login.py @@ -20,41 +20,50 @@ import flask -from flask_login import UserMixin, login_user, logout_user, current_user +from flask_login import login_user, logout_user, current_user from flask import request, jsonify, redirect -from oraculum import app, login_manager, oidc - - -class User(UserMixin): - def __init__(self, email): - self.id = email - self.email = email +from oraculum import app, db, login_manager, oidc +from oraculum.models.dashboard_users import DashboardUserData @login_manager.user_loader def load_user(userid): - return User(userid) - + return DashboardUserData.query.get(userid) or None @app.route('/api/v1/oidc_login') @oidc.require_login def api_v1_oidc_login(): - email = oidc.user_getfield('email') - user = User(email) + username = oidc.user_getfield('nickname') + groups = oidc.user_getfield('groups') + + user = DashboardUserData.query.filter_by(username=username).first() + + if not user: + user = DashboardUserData(username, groups) + db.session.add(user) + db.session.commit() + user = DashboardUserData.query.filter_by(username=username).first() + else: + user.fas_groups = groups + db.session.add(user) + db.session.commit() + login_user(user) token = oidc.get_access_token() - resp = { - 'msg': 'login successful', - 'email': email - } - print("*"*100) - print(flask.request.args.get('redirect')) - print(token) - # return redirect(flask.request.args.get('redirect')) - return redirect(flask.request.args.get('redirect') + '/%s' % token) + if not flask.request.args.get('redirect'): + return jsonify({ + 'msg': 'login successful', + 'username': username, + 'token': token + }) + + url = flask.request.args.get('redirect') + if "?" in url: + return redirect(url + '&oidc_token=%s' % token) + return redirect(url + '?oidc_token=%s' % token) @app.route('/api/v1/oidc_logout') @@ -66,8 +75,10 @@ def api_v1_oidc_logout(): resp = { 'msg': 'logout successful' } - print("*"*100) - print(flask.request.args.get('redirect')) + + if not flask.request.args.get('redirect'): + return jsonify(resp) + return redirect(flask.request.args.get('redirect')) @@ -75,12 +86,14 @@ def api_v1_oidc_logout(): def api_v1_get_current_user(): resp = { 'is_authenticated': False, + 'fas_groups': [], 'user': '' } if not current_user.is_authenticated: return jsonify(resp) - resp['user'] = current_user.email + resp['user'] = current_user.username + resp['fas_groups'] = current_user.fas_groups resp['is_authenticated'] = True return jsonify(resp) diff --git a/oraculum/controllers/packager_dashboard.py b/oraculum/controllers/packager_dashboard.py index b3f8fb4..cc69935 100644 --- a/oraculum/controllers/packager_dashboard.py +++ b/oraculum/controllers/packager_dashboard.py @@ -20,9 +20,10 @@ from flask import jsonify +from flask_login import current_user from oraculum import app, CACHE, oidc -from oraculum.utils import cache_utils, orphans, health_check, bodhi, koschei, pagure, dashboard_helpers, calendars, versions +from oraculum.utils import bodhi, cache_utils, calendars, dashboard_helpers, health_check, koschei, orphans, pagure, versions def handle_orphan_user(): orphans_data = list(CACHE.get('orphans_json')['affected_packages'].keys()) @@ -74,7 +75,13 @@ def route_dashboard_user_data(user): dashboard_helpers.update_user_access_time(user) prs = dashboard_user_data_prs(user) - if oidc.user_loggedin: + try: + fas_groups = current_user.fas_groups or [] + except AttributeError: + # Hit when user isn't logged in + fas_groups = [] + + if oidc.user_loggedin and "packager" in fas_groups: bzs = dashboard_user_data_bzs(user, authenticated=True) else: bzs = dashboard_user_data_bzs(user, authenticated=False) diff --git a/oraculum/models/dashboard_users.py b/oraculum/models/dashboard_users.py index 487d7e2..e8e7654 100644 --- a/oraculum/models/dashboard_users.py +++ b/oraculum/models/dashboard_users.py @@ -20,14 +20,18 @@ # Authors: # Frantisek Zatloukal -from oraculum import db +import datetime +from oraculum import db +from flask_login import UserMixin -class DashboardUserData(db.Model): +class DashboardUserData(db.Model, UserMixin): id = db.Column(db.Integer, primary_key=True) username = db.Column(db.Text, unique=True) + fas_groups = db.Column(db.Text, unique=False) last_accessed = db.Column(db.DateTime, unique=False) - def __init__(self, username, last_accessed): + def __init__(self, username, fas_groups, last_accessed=None): self.username = username - self.last_accessed = last_accessed + self.fas_groups = fas_groups + self.last_accessed = last_accessed or datetime.datetime.utcnow() diff --git a/oraculum/utils/dashboard_helpers.py b/oraculum/utils/dashboard_helpers.py index cef8e32..6f7f4ff 100644 --- a/oraculum/utils/dashboard_helpers.py +++ b/oraculum/utils/dashboard_helpers.py @@ -21,10 +21,10 @@ # Frantisek Zatloukal # Josef Skladanka +import datetime import json import yaml import requests -import datetime import fedfind.helpers import re @@ -33,9 +33,21 @@ from requests.adapters import HTTPAdapter from requests.exceptions import ConnectionError, RetryError from json import JSONDecodeError -from oraculum import app, CACHE, db +from oraculum import app, db, CACHE from oraculum.models.dashboard_users import DashboardUserData +def update_user_access_time(user): + """ + Updates user last_accessed with current timestamp + """ + row = DashboardUserData.query.filter_by(username=user).first() + if not row: + row = DashboardUserData(user, None) + db.session.add(row) + else: + row.last_accessed = datetime.datetime.utcnow() + db.session.commit() + def get_resource(url, attempt_retry=True, log_errors=True, raw=False): """ Returns data from provided url @@ -124,19 +136,6 @@ def get_fedora_releases(): return releases - -def update_user_access_time(user): - """ - Updates user last_accessed with current timestamp - """ - row = DashboardUserData.query.filter_by(username=user).first() - if not row: - row = DashboardUserData(user, datetime.datetime.utcnow()) - db.session.add(row) - else: - row.last_accessed = datetime.datetime.utcnow() - db.session.commit() - def clean_fas_username(user): """ Cleans up username to contain only characters allowed in FAS usernames From 6f172dd071cd6dbfb89a36c42df6aa9d88f2ad4f Mon Sep 17 00:00:00 2001 From: František Zatloukal Date: Apr 01 2021 09:04:40 +0000 Subject: [PATCH 2/3] Bugzilla: Fix tests while running without redis available --- diff --git a/oraculum/utils/bugzilla.py b/oraculum/utils/bugzilla.py index 2dbfdc4..4b41e5f 100644 --- a/oraculum/utils/bugzilla.py +++ b/oraculum/utils/bugzilla.py @@ -83,8 +83,11 @@ def get_all_package_bugs(package): # For later us or other oraculum maintainers: # We can't pass this as a function parameter as our caching architecture is saving and getting data # from the database according to provider, which includes also the function arguments - redis_conn = celery_app.broker_connection().default_channel.client - counter = int(redis_conn.get("bz_sync_mode_counter")) or 1 + if not app.config["TESTING"]: + redis_conn = celery_app.broker_connection().default_channel.client + counter = int(redis_conn.get("bz_sync_mode_counter")) or 1 + else: + counter = 1 if counter % app.config["BZ_FULL_EVERY_N_TIMES"] == 0: # # Refresh the whole bugzilla From af8d1734ef49d8a4d9b1296f888888a6b1428794 Mon Sep 17 00:00:00 2001 From: František Zatloukal Date: Apr 01 2021 09:05:14 +0000 Subject: [PATCH 3/3] CI: Use verbose pytest --- diff --git a/ci/pytest.yaml b/ci/pytest.yaml index 8d31604..39c30cf 100644 --- a/ci/pytest.yaml +++ b/ci/pytest.yaml @@ -11,6 +11,6 @@ args: chdir: '{{ zuul.project.src_dir }}' - name: run-pytest - command: "pytest" + command: "pytest -vvv" args: chdir: '{{ zuul.project.src_dir }}'