diff --git a/frontend/coprs_frontend/coprs/auth.py b/frontend/coprs_frontend/coprs/auth.py index 64a574036..11a6da5b2 100644 --- a/frontend/coprs_frontend/coprs/auth.py +++ b/frontend/coprs_frontend/coprs/auth.py @@ -92,37 +92,26 @@ class GroupAuth: `app.config["FAS_LOGIN"]` and `app.config["KRB5_LOGIN"]` should be encapsulated within this class. """ - @classmethod - def update_user_groups(cls, user, oid_resp=None): + def update_user_groups(cls, user, groups=None): """ Upon a successful login, try to (a) load the list of groups from authoritative source, and (b) (re)set the user.openid_groups. """ - def _do_update(user, grouplist): user.openid_groups = { "fas_groups": grouplist, } + if not groups: + groups = [] - if oid_resp: - _do_update(user, OpenIDGroups.group_names(oid_resp)) - return - - # If we have a LDAP pre-configured, now is the right time to load the - # data, or fail. - keys = ["LDAP_URL", "LDAP_SEARCH_STRING"] - if all(app.config[k] for k in keys): - _do_update(user, LDAPGroups.group_names(user.username)) + if not isinstance(groups, list): + app.logger.error("groups should be a list object") return - # We only ever call update_user_groups() with oid_resp!= None with FAS - assert not app.config["FAS_LOGIN"] - - app.logger.warning("Nowhere to get groups from") - # This copr doesn't support groups. - _do_update(user, []) - + app.logger.info(f"groups add: {groups}") + _do_update(user, groups) + return class FedoraAccounts: """ @@ -193,7 +182,7 @@ def user_from_response(cls, oid_resp): # Update user attributes from FAS user.mail = oid_resp.email user.timezone = oid_resp.timezone - GroupAuth.update_user_groups(user, oid_resp) + GroupAuth.update_user_groups(user, OpenIDGroups.group_names(oid_resp)) return user @@ -239,7 +228,9 @@ def user_from_username(username, load_metadata=False): # Create a new user object krb_config = app.config['KRB5_LOGIN'] user.mail = username + "@" + krb_config['email_domain'] - GroupAuth.update_user_groups(user) + keys = ["LDAP_URL", "LDAP_SEARCH_STRING"] + if all(app.config[k] for k in keys): + GroupAuth.update_user_groups(user, LDAPGroups.group_names(user.username)) return user @staticmethod @@ -402,4 +393,15 @@ def user_from_userinfo(userinfo): and userinfo['zoneinfo'] else None user = UserAuth.get_or_create_user(userinfo['username'], userinfo['email'], zoneinfo) + GroupAuth.update_user_groups(user, OpenIDConnect.groups_from_userinfo(userinfo)) return user + + @staticmethod + def groups_from_userinfo(userinfo): + """ + Create a `models.User` object from oidc user info + """ + if not userinfo: + return None + + return userinfo.get("groups") diff --git a/frontend/coprs_frontend/coprs/context_processors.py b/frontend/coprs_frontend/coprs/context_processors.py index 9b66928de..2a91f5f20 100644 --- a/frontend/coprs_frontend/coprs/context_processors.py +++ b/frontend/coprs_frontend/coprs/context_processors.py @@ -4,6 +4,7 @@ from coprs import app from coprs.constants import BANNER_LOCATION from coprs.helpers import current_url +from coprs.oidc import oidc_enabled @app.context_processor @@ -67,7 +68,7 @@ def login_menu(): 'desc': 'sign up', }) - if config['OIDC_LOGIN'] and config['OIDC_PROVIDER_NAME']: + if oidc_enabled(config): menu.append({ 'link': flask.url_for("misc.oidc_login"), 'desc': '{} login'.format(app.config['OIDC_PROVIDER_NAME']), diff --git a/frontend/coprs_frontend/coprs/oidc.py b/frontend/coprs_frontend/coprs/oidc.py index 667c7bc93..f16428f3b 100644 --- a/frontend/coprs_frontend/coprs/oidc.py +++ b/frontend/coprs_frontend/coprs/oidc.py @@ -11,13 +11,18 @@ def is_config_valid(config): """ If OpenID Connect is enabled """ - return "OIDC_LOGIN" in config and config["OIDC_LOGIN"] is True + return "OIDC_LOGIN" in config and config["OIDC_LOGIN"] is True and \ + "OIDC_PROVIDER_NAME" in config and config["OIDC_PROVIDER_NAME"] def oidc_enabled(config): """ Check whether the config is valid """ + if not is_config_valid(config): + logger.error("OIDC_LOGIN or OIDC_PROVIDER_NAME is empty") + return False + if not config.get("OIDC_CLIENT"): logger.error("OIDC_CLIENT is empty") return False @@ -47,7 +52,7 @@ def init_oidc_app(app): When configs check failed, a invalid client object is returned """ oidc = OAuth(app) - if oidc_enabled(app.config) and is_config_valid(app.config): + if oidc_enabled(app.config): client_id = app.config.get("OIDC_CLIENT") secret = app.config.get("OIDC_SECRET") client_kwargs = { diff --git a/frontend/coprs_frontend/tests/test_auth.py b/frontend/coprs_frontend/tests/test_auth.py index 81ea39612..cdb75661f 100644 --- a/frontend/coprs_frontend/tests/test_auth.py +++ b/frontend/coprs_frontend/tests/test_auth.py @@ -3,7 +3,7 @@ from unittest import mock from tests.coprs_test_case import CoprsTestCase from coprs import app -from coprs.auth import GroupAuth +from coprs.auth import GroupAuth, LDAPGroups class TestGroupAuth(CoprsTestCase): @@ -29,7 +29,7 @@ def test_group_names_ldap(self, get_user_groups): b'cn=another-group-2,ou=foo,ou=bar,dc=company,dc=com' ] user = mock.MagicMock() - GroupAuth.update_user_groups(user) + GroupAuth.update_user_groups(user, LDAPGroups.group_names(user.username)) assert user.openid_groups == { "fas_groups": ["group1", "group2", "another-group", "another-group-2"]}