fedora-qa / oraculum

Clone
Source Code
GIT

#144 OIDC improvements
Merged 3 years ago by frantisekz. Opened 3 years ago by frantisekz.
oidc_improvements  into  master

file added
+28 
@@ -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 ###

file modified
+1 -1 
@@ -11,6 +11,6 @@ 
 

        args:
 

          chdir: '{{ zuul.project.src_dir }}'
 

      - name: run-pytest
 

-       command: "pytest"
 

+       command: "pytest -vvv"
 

        args:
 

          chdir: '{{ zuul.project.src_dir }}'

file modified
+1 
@@ -10,6 +10,7 @@ 
 

  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'

file modified
+3 
@@ -45,6 +45,7 @@ 
 

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

  

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

file modified
+37 -24 
@@ -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 @@ 
 

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

file modified
+9 -2 
@@ -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 @@ 
 

              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)

file modified
+8 -4 
@@ -20,14 +20,18 @@ 
 

  # Authors:
 

  #   Frantisek Zatloukal <fzatlouk@redhat.com>
 

  

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

file modified
+5 -2 
@@ -83,8 +83,11 @@ 
 

      # 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

file modified
+14 -15 
@@ -21,10 +21,10 @@ 
 

  #   Frantisek Zatloukal <fzatlouk@redhat.com>
 

  #   Josef Skladanka <jskladan@redhat.com>
 

  

+ import datetime
 

  import json
 

  import yaml
 

  import requests
 

- import datetime
 

  import fedfind.helpers
 

  import re
 

  
@@ -33,9 +33,21 @@ 
 

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

  

      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

no initial comment

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

I'm not really stoked about this. If nothing else, than for the fact that this now becomes "heavily tied" to the packager dashboard, since we won't allow a non-packager to login via this method. That might be fine ATM, but kind of dismisses the purpose of having the OIDC login in oraculum, no?

I'm not sure what the best solution is, as I'm not really versed in the OIDC related details. Let's have a call and sort things out in person.

rebased onto 61b83b8
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 7a5f688
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 7e8dd6a
3 years ago

rebased onto e947894
3 years ago

1 new commit added

  • Better
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

I'd rather make the timeout shorter then the default (7 days) than longer, if we decide to change it.

I'd rather make the timeout shorter then the default (7 days) than longer, if we decide to change it.

other than the token timeout (and the zuul job failure, which is possibly a zuul error, but should be looked at anyway) LGTM

1 new commit added

  • Token valid for 7 days
3 years ago

1 new commit added

  • Try bodhi-client from repos
3 years ago

1 new commit added

  • [DEBUG] Verbose ci pytest
3 years ago

1 new commit added

  • Fix CI
3 years ago

Build succeeded.

rebased onto 3e255d9
3 years ago

Build succeeded.

rebased onto 6572b90
3 years ago

Build succeeded.

Pull-Request has been merged by frantisekz
3 years ago
Metadata
None
No Tags
Changes Summary 9
+28
file added
alembic/versions/8da50b403664_oidc_changes.py
+1 -1
file changed
ci/pytest.yaml
+1 -0
file changed
conf/settings.py.example
+3 -0
file changed
oraculum/config.py
+37 -24
file changed
oraculum/controllers/oidc_login.py
+9 -2
file changed
oraculum/controllers/packager_dashboard.py
+8 -4
file changed
oraculum/models/dashboard_users.py
+5 -2
file changed
oraculum/utils/bugzilla.py
+14 -15
file changed
oraculum/utils/dashboard_helpers.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.