diff --git a/auth/oauth.py b/auth/oauth.py index 6c1171dafc..4b70e39fa4 100644 --- a/auth/oauth.py +++ b/auth/oauth.py @@ -65,7 +65,7 @@ def validate_sso_oauth_token(token): options["verify_signature"] = False decoded_id_token = service.decode_user_jwt(token, options=options) - sub, lusername, lemail = get_sub_username_email_from_token( + sub, lusername, lemail, additional_info = get_sub_username_email_from_token( decoded_id_token, None, service.config, False ) diff --git a/data/migrations/versions/135fd3e94615_insert_oidc_in_loginservice.py b/data/migrations/versions/135fd3e94615_insert_oidc_in_loginservice.py new file mode 100644 index 0000000000..5317b12989 --- /dev/null +++ b/data/migrations/versions/135fd3e94615_insert_oidc_in_loginservice.py @@ -0,0 +1,28 @@ +"""insert oidc in loginservice + +Revision ID: 135fd3e94615 +Revises: b4da5b09c8df +Create Date: 2024-02-21 09:48:54.463860 + +""" + +# revision identifiers, used by Alembic. +revision = "135fd3e94615" +down_revision = "b4da5b09c8df" + +import sqlalchemy as sa + + +def upgrade(op, tables, tester): + op.bulk_insert( + tables.loginservice, + [ + {"name": "oidc"}, + ], + ) + + +def downgrade(op, tables, tester): + op.execute( + tables.loginservice.delete().where(tables.loginservice.name == op.inline_literal("oidc")) + ) diff --git a/data/model/team.py b/data/model/team.py index b181be8b29..f9eabe82f4 100644 --- a/data/model/team.py +++ b/data/model/team.py @@ -569,3 +569,49 @@ def delete_members_not_present(team, member_id_set): return query.execute() return 0 + + +def get_federated_user_teams(user_obj, login_service_name): + """ + Return all user's teams that are synced with the login service + """ + query = ( + Team.select(Team, TeamSync) + .join(TeamMember) + .switch(Team) + .join(TeamSync) + .join(LoginService) + .where(TeamMember.user == user_obj, LoginService.name == login_service_name) + ) + return query + + +def delete_all_team_members(team): + """ + Delete all users that are a member of the given team + """ + with db_transaction(): + user_ids = set([u.id for u in list_team_users(team)]) + if user_ids: + query = TeamMember.delete().where(TeamMember.team == team, TeamMember.user << user_ids) + return query.execute() + + return 0 + + +def get_oidc_team_from_groupname(group_name, login_service_name): + """ + Fetch TeamSync row synced with login_service_name from `group_name` in TeamSync.config + """ + response = [] + with db_transaction(): + query_result = ( + TeamSync.select() + .join(LoginService) + .where(TeamSync.config.contains(group_name), LoginService.name == login_service_name) + ) + for row in query_result: + if json.loads(row.config).get("group_name", None) == group_name: + response.append(row) + + return response diff --git a/data/model/test/test_team.py b/data/model/test/test_team.py index 1f2bb21f11..cffc1db52d 100644 --- a/data/model/test/test_team.py +++ b/data/model/test/test_team.py @@ -1,7 +1,6 @@ -from test.fixtures import * - import pytest +from data.database import TeamMember from data.model import DataModelException from data.model.organization import create_organization from data.model.team import ( @@ -10,12 +9,16 @@ add_user_to_team, confirm_team_invite, create_team, + delete_all_team_members, + get_federated_user_teams, list_team_users, remove_team, remove_user_from_team, + set_team_syncing, validate_team_name, ) from data.model.user import create_user_noverify, get_user +from test.fixtures import * @pytest.mark.parametrize( @@ -115,3 +118,51 @@ def test_remove_user_from_team(initialized_db): # Another admin should be able to remove_user_from_team("testorg", "testteam", "randomuser", "devtable") + + +def test_delete_all_team_members(initialized_db): + dev_user = get_user("devtable") + random_user = get_user("randomuser") + public_user = get_user("public") + fresh_user = get_user("freshuser") + reader_user = get_user("reader") + + new_org = create_organization("testorg", "testorg" + "@example.com", dev_user) + + team_1 = create_team("team_1", new_org, "member") + assert add_user_to_team(dev_user, team_1) + assert add_user_to_team(random_user, team_1) + assert add_user_to_team(public_user, team_1) + assert add_user_to_team(fresh_user, team_1) + assert add_user_to_team(reader_user, team_1) + + before_deletion_count = TeamMember.select().where(TeamMember.team == team_1).count() + assert before_deletion_count == 5 + delete_all_team_members(team_1) + + after_deletion_count = TeamMember.select().where(TeamMember.team == team_1).count() + assert after_deletion_count == 0 + + +@pytest.mark.parametrize("login_service_name", ["oidc", "ldap"]) +def test_get_federated_user_teams(login_service_name, initialized_db): + dev_user = get_user("devtable") + new_org = create_organization("testorg", "testorg" + "@example.com", dev_user) + + team_1 = create_team("team_1", new_org, "member") + assert add_user_to_team(dev_user, team_1) + assert set_team_syncing(team_1, "oidc", None) + + team_2 = create_team("team_2", new_org, "member") + assert add_user_to_team(dev_user, team_2) + assert set_team_syncing(team_2, "oidc", None) + + team_3 = create_team("team_3", new_org, "member") + assert add_user_to_team(dev_user, team_3) + assert set_team_syncing(team_3, "ldap", None) + + user_teams = get_federated_user_teams(dev_user, login_service_name) + if login_service_name == "oidc": + assert len(user_teams) == 2 + elif login_service_name == "ldap": + assert len(user_teams) == 1 diff --git a/data/users/externaloidc.py b/data/users/externaloidc.py index 6202d53d3b..651ab68306 100644 --- a/data/users/externaloidc.py +++ b/data/users/externaloidc.py @@ -1,5 +1,11 @@ +import json +import logging + +from data.model import InvalidTeamException, UserAlreadyInTeam, team from data.users.federated import FederatedUsers +logger = logging.getLogger(__name__) + class OIDCUsers(FederatedUsers): def __init__( @@ -50,3 +56,77 @@ def query_users(self, query, limit): No way to query users so returning empty list """ return ([], self.federated_service, None) + + def sync_oidc_groups(self, user_groups, user_obj, service_name): + """ + Adds user to quay teams that have team sync enabled with an OIDC group + """ + if user_groups is None: + return + + for oidc_group in user_groups: + # fetch TeamSync row if exists, for the oidc_group synced with the login service + synced_teams = team.get_oidc_team_from_groupname(oidc_group, service_name) + if len(synced_teams) == 0: + logger.debug( + f"OIDC group: {oidc_group} is either not synced with a team in quay or is not synced with the {service_name} service" + ) + continue + + # fetch team name and organization name for the Teamsync row + for team_synced in synced_teams: + team_name = team_synced.team.name + org_name = team_synced.team.organization.username + if not team_name or not org_name: + logger.debug(f"Cannot retrieve team for the oidc group: {oidc_group}") + + # add user to team + try: + # team_obj = team.get_organization_team(org_name, team_name) + team.add_user_to_team(user_obj, team_synced.team) + except InvalidTeamException as err: + logger.exception(err) + except UserAlreadyInTeam: + # Ignore + pass + return + + def ping(self): + """ + TODO: get the OIDC connection here + """ + return (True, None) + + def resync_quay_teams(self, user_groups, user_obj, login_service_name): + """ + Fetch quay teams that user is a member of. + Remove user from teams that are synced with an OIDC group but group does not exist in "user_groups" + """ + # fetch user's quay teams that have team sync enabled + existing_user_teams = team.get_federated_user_teams(user_obj, login_service_name) + user_groups = user_groups or [] + for user_team in existing_user_teams: + try: + sync_group_info = json.loads(user_team.teamsync.config) + # remove user's membership from teams that were not returned from users OIDC groups + if ( + sync_group_info.get("group_name", None) + and sync_group_info["group_name"] not in user_groups + ): + org_name = user_team.teamsync.team.organization.username + team.remove_user_from_team(org_name, user_team.name, user_obj.username, None) + logger.debug( + f"Successfully removed user: {user_obj.username} from team: {user_team.name} in organization: {org_name}" + ) + except Exception as err: + logger.exception(err) + return + + def sync_user_groups(self, user_groups, user_obj, login_service): + if not user_obj: + return + + service_name = login_service.service_id() + self.sync_oidc_groups(user_groups, user_obj, service_name) + self.resync_quay_teams(user_groups, user_obj, service_name) + return diff --git a/endpoints/api/team.py b/endpoints/api/team.py index 9c643c3291..b690243b69 100644 --- a/endpoints/api/team.py +++ b/endpoints/api/team.py @@ -8,7 +8,7 @@ from flask import request import features -from app import authentication, avatar +from app import app, authentication, avatar from auth import scopes from auth.auth_context import get_authenticated_user from auth.permissions import ( @@ -308,6 +308,10 @@ def post(self, orgname, teamname): # Set the team's syncing config. model.team.set_team_syncing(team, authentication.federated_service, config) + if app.config["AUTHENTICATION_TYPE"] == "OIDC": + # delete existing team members, team membership will be synced with OIDC group + model.team.delete_all_team_members(team) + return team_view(orgname, team) raise Unauthorized() @@ -384,7 +388,7 @@ def get(self, orgname, teamname, parsed_args): "service": sync_info.service.name, } - if SuperUserPermission().can(): + if features.NONSUPERUSER_TEAM_SYNCING_SETUP or SuperUserPermission().can(): data["synced"].update( { "last_updated": format_date(sync_info.last_updated), diff --git a/endpoints/oauth/login.py b/endpoints/oauth/login.py index ce80f8e833..4e1cd717b4 100644 --- a/endpoints/oauth/login.py +++ b/endpoints/oauth/login.py @@ -118,7 +118,7 @@ def callback_func(): # Exchange the OAuth code for login information. code = request.values.get("code") try: - lid, lusername, lemail = login_service.exchange_code_for_login( + lid, lusername, lemail, additional_info = login_service.exchange_code_for_login( app.config, client, code, "" ) except OAuthLoginException as ole: @@ -149,6 +149,7 @@ def callback_func(): lemail, metadata=metadata, captcha_verified=captcha_verified, + additional_login_info=additional_info, ) if result.requires_verification: return render_page_template_with_routedata( @@ -170,7 +171,7 @@ def attach_func(): # Exchange the OAuth code for login information. code = request.values.get("code") try: - lid, lusername, _ = login_service.exchange_code_for_login( + lid, lusername, _, _ = login_service.exchange_code_for_login( app.config, client, code, "/attach" ) except OAuthLoginException as ole: diff --git a/oauth/login.py b/oauth/login.py index 65e1e4f103..a28e8b409d 100644 --- a/oauth/login.py +++ b/oauth/login.py @@ -99,7 +99,7 @@ def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix login or attach their account. Raises a OAuthLoginService exception on failure. Returns a tuple consisting of (service_id, - service_username, email) + service_username, email, additional_info) """ # Retrieve the token for the OAuth code. @@ -136,6 +136,7 @@ def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix service_user_id = self.get_login_service_id(user_info) service_username = self.get_login_service_username(user_info) + additional_info = {} logger.debug( "Completed successful exchange for service %s: %s, %s, %s", @@ -144,4 +145,4 @@ def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix service_username, email_address, ) - return (service_user_id, service_username, email_address) + return (service_user_id, service_username, email_address, additional_info) diff --git a/oauth/login_utils.py b/oauth/login_utils.py index e9c38f2931..199d7367ab 100644 --- a/oauth/login_utils.py +++ b/oauth/login_utils.py @@ -40,10 +40,13 @@ def get_jwt_issuer(token): return decoded.get("iss", None) -def get_sub_username_email_from_token(decoded_id_token, user_info=None, config={}, mailing=False): +def get_sub_username_email_from_token( + decoded_id_token, user_info=None, config={}, mailing=False, fetch_groups=False +): if not user_info: user_info = decoded_id_token + additional_info = {} # Verify for impersonation if user_info.get("impersonated", False): logger.debug("Requests from impersonated principals are not supported") @@ -88,7 +91,17 @@ def get_sub_username_email_from_token(decoded_id_token, user_info=None, config={ if lusername.find("@") >= 0: lusername = lusername[0 : lusername.find("@")] - return decoded_id_token["sub"], lusername, email_address + if fetch_groups: + if config.get("PREFERRED_GROUP_CLAIM_NAME", None) is None: + logger.exception( + "PREFERRED_GROUP_CLAIM_NAME needs to be added in the config for teamsync via OIDC" + ) + else: + additional_info["groups"] = user_info.get( + config.get("PREFERRED_GROUP_CLAIM_NAME", None) + ) + + return decoded_id_token["sub"], lusername, email_address, additional_info def _oauthresult( @@ -126,6 +139,16 @@ def _attach_service(config, login_service, user_obj, lid, lusername): return _oauthresult(service_name=login_service.service_name(), error_message=err) +def sync_oidc_groups(additional_login_info, user_obj, auth_system, login_service, config): + if ( + config.get("AUTHENTICATION_TYPE", "oidc") + and config.get("FEATURE_TEAM_SYNCING", False) + and additional_login_info + ): + auth_system.sync_user_groups(additional_login_info.get("groups"), user_obj, login_service) + return + + def _conduct_oauth_login( config, analytics, @@ -136,6 +159,7 @@ def _conduct_oauth_login( lemail, metadata=None, captcha_verified=False, + additional_login_info=None, ): """ Conducts login from the result of an OAuth service's login flow and returns the status of the @@ -148,6 +172,7 @@ def _conduct_oauth_login( # and redirect. user_obj = model.user.verify_federated_login(service_id, lid) if user_obj is not None: + sync_oidc_groups(additional_login_info, user_obj, auth_system, login_service, config) return _oauthresult(user_obj=user_obj, service_name=service_name) # If the login service has a bound field name, and we have a defined internal auth type that is @@ -182,6 +207,7 @@ def _conduct_oauth_login( if result.error_message is not None: return result + sync_oidc_groups(additional_login_info, user_obj, auth_system, login_service, config) return _oauthresult(user_obj=user_obj, service_name=service_name) # Otherwise, we need to create a new user account. @@ -220,6 +246,7 @@ def _conduct_oauth_login( # Success, tell analytics analytics.track(user_obj.username, "register", {"service": service_name.lower()}) + sync_oidc_groups(additional_login_info, user_obj, auth_system, login_service, config) return _oauthresult(user_obj=user_obj, service_name=service_name) except model.InvalidEmailAddressException: diff --git a/oauth/oidc.py b/oauth/oidc.py index a5fd4cbea1..02788b8e36 100644 --- a/oauth/oidc.py +++ b/oauth/oidc.py @@ -183,7 +183,7 @@ def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix user_info = decoded_id_token return get_sub_username_email_from_token( - decoded_id_token, user_info, self.config, self._mailing + decoded_id_token, user_info, self.config, self._mailing, fetch_groups=True ) @property diff --git a/oauth/services/rhsso.py b/oauth/services/rhsso.py index 32dc653627..0435f9b943 100644 --- a/oauth/services/rhsso.py +++ b/oauth/services/rhsso.py @@ -15,7 +15,7 @@ class RHSSOOAuthService(OIDCLoginService): def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix): - sub, lusername, email_address = super().exchange_code_for_login( + sub, lusername, email_address, additional_info = super().exchange_code_for_login( app_config, http_client, code, redirect_suffix ) @@ -73,4 +73,4 @@ def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix exc_info=True, ) - return sub, lusername, email_address + return sub, lusername, email_address, additional_info diff --git a/oauth/test/test_oidc.py b/oauth/test/test_oidc.py index dcf503ec7e..a19a242806 100644 --- a/oauth/test/test_oidc.py +++ b/oauth/test/test_oidc.py @@ -388,7 +388,7 @@ def test_exchange_code_validcode( oidc_service.exchange_code_for_login(app_config, http_client, valid_code, "") else: # Should succeed. - lid, lusername, lemail = oidc_service.exchange_code_for_login( + lid, lusername, lemail, additional_info = oidc_service.exchange_code_for_login( app_config, http_client, valid_code, "" ) diff --git a/test/test_external_oidc.py b/test/test_external_oidc.py new file mode 100644 index 0000000000..0e338db6d5 --- /dev/null +++ b/test/test_external_oidc.py @@ -0,0 +1,207 @@ +import unittest + +import pytest + +from data import model +from data.database import TeamMember +from data.users.externaloidc import OIDCUsers +from initdb import finished_database_for_testing, setup_database_for_testing +from oauth.oidc import OIDCLoginService +from test.fixtures import * + + +class OIDCAuthTests(unittest.TestCase): + def fake_oidc(self): + """ + Instantiates a fake OIDC instance to use in the test cases + """ + client_id = "quay-test" + client_secret = "secret" + oidc_server = "http://test-server/realms/myrealm" + service_name = "test" + login_scopes = ["openid"] + preferred_group_claim_name = "groups" + + oidc_instance = OIDCUsers( + client_id, + client_secret, + oidc_server, + service_name, + login_scopes, + preferred_group_claim_name, + ) + return oidc_instance + + def fake_oidc_login_service(self): + config = { + "CLIENT_ID": "foo", + "CLIENT_SECRET": "bar", + "SERVICE_NAME": "Some Cool Service", + "SERVICE_ICON": "http://some/icon", + "OIDC_SERVER": "http://fakeoidc", + "DEBUGGING": True, + } + return OIDCLoginService(config, "OIDC_LOGIN_CONFIG") + + def setUp(self): + setup_database_for_testing(self) + self.oidc_instance = self.fake_oidc() + self.oidc_login_service = self.fake_oidc_login_service() + + def tearDown(self): + finished_database_for_testing(self) + + def test_sync_for_empty_oidc_groups(self): + user_obj = model.user.get_user("devtable") + + test_org = model.organization.create_organization( + "testorg", "testorg@example.com", user_obj + ) + team_1 = model.team.create_team("team_1", test_org, "member") + assert model.team.add_user_to_team(user_obj, team_1) + + team_2 = model.team.create_team("team_2", test_org, "member") + assert model.team.add_user_to_team(user_obj, team_2) + + user_teams_before_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + self.oidc_instance.sync_oidc_groups([], user_obj, "oidc") + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + assert user_teams_before_sync == user_teams_after_sync + + self.oidc_instance.sync_oidc_groups(None, user_obj, "oidc") + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + assert user_teams_before_sync == user_teams_after_sync + + def test_sync_for_non_empty_oidc_groups(self): + user_obj = model.user.get_user("devtable") + fresh_user = model.user.get_user("freshuser") + random_user = model.user.get_user("randomuser") + + test_org_1 = model.organization.create_organization( + "test_org_1", "testorg1@example.com", user_obj + ) + test_org_2 = model.organization.create_organization( + "test_org_2", "testorg2@example.com", fresh_user + ) + test_org_3 = model.organization.create_organization( + "test_org_3", "testorg3@example.com", random_user + ) + + team_1 = model.team.create_team("team_1", test_org_1, "member") + assert model.team.set_team_syncing(team_1, "oidc", {"group_name": "test_org_1_team_1"}) + + team_2 = model.team.create_team("team_2", test_org_2, "member") + assert model.team.set_team_syncing(team_2, "oidc", {"group_name": "test_org_2_team_2"}) + + team_3 = model.team.create_team("team_3", test_org_3, "member") + assert model.team.set_team_syncing(team_3, "ldap", {"group_name": "test_org_3_team_3"}) + + user_groups = [ + "test_org_1_team_1", + "test_org_2_team_2", + "test_org_3_team_3", + "wrong_group_name", + ] + user_teams_before_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + self.oidc_instance.sync_oidc_groups(user_groups, user_obj, "oidc") + + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + + assert user_teams_before_sync + 2 == user_teams_after_sync + + def test_resync_for_empty_quay_teams(self): + user_obj = model.user.get_user("devtable") + + user_teams_before_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + self.oidc_instance.resync_quay_teams([], user_obj, "oidc") + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + assert user_teams_before_sync == user_teams_after_sync + + # add user to team that doesn't have team sync enabled + test_org_1 = model.organization.create_organization( + "test_org_1", "testorg1@example.com", user_obj + ) + team_1 = model.team.create_team("team_1", test_org_1, "member") + assert model.team.add_user_to_team(user_obj, team_1) + + # add user to team that has team sync enabled with a different login service + team_2 = model.team.create_team("team_2", test_org_1, "member") + assert model.team.set_team_syncing(team_2, "ldap", None) + assert model.team.add_user_to_team(user_obj, team_2) + + user_teams_before_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + self.oidc_instance.resync_quay_teams([], user_obj, "oidc") + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + assert user_teams_before_sync == user_teams_after_sync + + def test_resync_for_non_empty_quay_teams(self): + user_obj = model.user.get_user("devtable") + fresh_user = model.user.get_user("freshuser") + + test_org_1 = model.organization.create_organization( + "test_org_1", "testorg1@example.com", user_obj + ) + test_org_2 = model.organization.create_organization( + "test_org_2", "testorg2@example.com", fresh_user + ) + + # add user to team that has team sync enabled with oidc login service + team_1 = model.team.create_team("team_1", test_org_1, "member") + assert model.team.set_team_syncing(team_1, "oidc", {"group_name": "test_org_1_team_1"}) + assert model.team.add_user_to_team(user_obj, team_1) + + # add user to another team that has team sync enabled with oidc login service + team_2 = model.team.create_team("team_1", test_org_2, "member") + assert model.team.set_team_syncing(team_2, "oidc", {"group_name": "test_org_2_team_1"}) + assert model.team.add_user_to_team(user_obj, team_2) + + user_groups = ["test_org_1_team_1", "another_group"] + # user should be removed from team_2 + self.oidc_instance.resync_quay_teams(user_groups, user_obj, "oidc") + assert ( + TeamMember.select() + .where(TeamMember.user == user_obj, TeamMember.team == team_2) + .count() + == 0 + ) + + def test_sync_user_groups_for_empty_user_obj(self): + assert self.oidc_instance.sync_user_groups([], None, self.oidc_login_service) is None + assert self.oidc_instance.sync_user_groups(None, None, self.oidc_login_service) is None + + user_groups = ["test_org_1_team_1", "another_group"] + assert ( + self.oidc_instance.sync_user_groups(user_groups, None, self.oidc_login_service) is None + ) + + user_obj = model.user.get_user("devtable") + fresh_user = model.user.get_user("freshuser") + test_org_1 = model.organization.create_organization( + "test_org_1", "testorg1@example.com", user_obj + ) + # team set to sync but user is not added to team + team_1 = model.team.create_team("team_1", test_org_1, "member") + assert model.team.set_team_syncing( + team_1, self.oidc_login_service.service_id(), {"group_name": "test_org_1_team_1"} + ) + + test_org_2 = model.organization.create_organization( + "test_org_2", "testorg2@example.com", fresh_user + ) + # team set to sync and user is added to team + team_2 = model.team.create_team("team_1", test_org_2, "member") + assert model.team.set_team_syncing( + team_2, self.oidc_login_service.service_id(), {"group_name": "test_org_2_team_1"} + ) + assert model.team.add_user_to_team(user_obj, team_2) + + user_teams_before_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + # user will be removed from team_2 + self.oidc_instance.sync_user_groups([], user_obj, self.oidc_login_service) + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + assert user_teams_before_sync == user_teams_after_sync + 1 + + # user will be removed from team_1 + self.oidc_instance.sync_user_groups(user_groups, user_obj, self.oidc_login_service) + user_teams_after_sync = TeamMember.select().where(TeamMember.user == user_obj).count() + assert user_teams_before_sync == user_teams_after_sync diff --git a/web/cypress/e2e/team-sync.cy.ts b/web/cypress/e2e/team-sync.cy.ts index 69949a97ad..4f17005c89 100644 --- a/web/cypress/e2e/team-sync.cy.ts +++ b/web/cypress/e2e/team-sync.cy.ts @@ -43,20 +43,19 @@ describe('OIDC Team Sync', () => { cy.get('#team-members-toolbar').within(() => cy.get('button:contains("Enable Directory Sync")').click(), ); - cy.get('#oidc-team-sync-helper-text').contains( - 'The expected OIDC group name format is - org_name:team_name. Must match ^[a-z0-9][a-z0-9]+:[a-z0-9]+$', - ); + cy.get('button:contains("Enable Sync")').should('be.disabled'); cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') .type('random'); - cy.get('button:contains("Enable Sync")').should('be.disabled'); + cy.get('button:contains("Enable Sync")').should('not.be.disabled'); + cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') .clear(); cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') - .type('1:1'); + .type(' '); cy.get('button:contains("Enable Sync")').should('be.disabled'); cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') @@ -64,7 +63,7 @@ describe('OIDC Team Sync', () => { cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') - .type('org:team'); + .type('team_name'); cy.get('button:contains("Enable Sync")').should('not.be.disabled'); }); @@ -82,7 +81,7 @@ describe('OIDC Team Sync', () => { ); cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') - .type('org:team'); + .type('org_team_group'); cy.get('button:contains("Enable Sync")').click(); cy.wait('@getTeamSyncSuccess'); cy.contains('Successfully updated team sync config'); @@ -104,7 +103,7 @@ describe('OIDC Team Sync', () => { ).should('exist'); cy.contains('Directory Synchronization Config').should('exist'); cy.contains('Bound to group').should('exist'); - cy.contains('teamsyncorg:groupname').should('exist'); + cy.contains('testteam_teamsync_group').should('exist'); cy.contains('Last Updated').should('exist'); cy.contains('Never').should('exist'); cy.contains('tr', 'teamsyncorg+robotacct'); @@ -186,7 +185,7 @@ describe('OIDC Team Sync', () => { cy.contains('button', 'Enable Directory Sync').click(); cy.get('#directory-sync-modal') .find('input[id="team-sync-group-name"]') - .type('org:team'); + .type('org_team_group'); cy.get('button:contains("Enable Sync")').click(); cy.wait('@getTeamSyncSuccess'); cy.contains('Successfully updated team sync config'); diff --git a/web/cypress/fixtures/teamsynced-members-superuser.json b/web/cypress/fixtures/teamsynced-members-superuser.json index 1fae697c7b..264fb5bc62 100644 --- a/web/cypress/fixtures/teamsynced-members-superuser.json +++ b/web/cypress/fixtures/teamsynced-members-superuser.json @@ -21,7 +21,7 @@ "synced": { "service": "oidc", "config": { - "group_name": "teamsyncorg:groupname" + "group_name": "testteam_teamsync_group" }, "last_updated": "" } diff --git a/web/src/components/modals/OIDCTeamSyncModal.tsx b/web/src/components/modals/OIDCTeamSyncModal.tsx index 4fe0dfdd31..28d389b1e2 100644 --- a/web/src/components/modals/OIDCTeamSyncModal.tsx +++ b/web/src/components/modals/OIDCTeamSyncModal.tsx @@ -2,47 +2,18 @@ import {useState} from 'react'; import { Alert, Button, - HelperText, - HelperTextItem, Modal, Text, TextInput, TextVariants, } from '@patternfly/react-core'; import Conditional from 'src/components/empty/Conditional'; -import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon'; - -interface Validation { - message: string; - isValid: boolean; - type: 'default' | 'error'; -} - -const defaultMessage: Validation = { - message: - 'The expected OIDC group name format is - org_name:team_name. Must match ^[a-z0-9][a-z0-9]+:[a-z0-9]+$', - isValid: false, - type: 'default', -}; export default function OIDCTeamSyncModal(props: OIDCTeamSyncModalProps) { const [groupName, setGroupName] = useState(''); - const [validation, setValidation] = useState(defaultMessage); const handleInputChange = (value: string) => { - const regex = /^[a-z0-9][a-z0-9]+:[a-z0-9]+$/; - if (!regex.test(value)) { - setValidation({ - message: - 'The expected OIDC group name format is - org_name:team_name. Must match ^[a-z0-9][a-z0-9]+:[a-z0-9]+$', - isValid: false, - type: 'error', - }); - } else { - defaultMessage['isValid'] = true; - setValidation(defaultMessage); - } - setGroupName(value); + setGroupName(value.trim()); }; return ( @@ -57,7 +28,7 @@ export default function OIDCTeamSyncModal(props: OIDCTeamSyncModalProps) { key="confirm" variant="primary" onClick={() => props.onConfirmSync(groupName)} - isDisabled={!validation.isValid} + isDisabled={groupName == ''} > Enable Sync , @@ -77,18 +48,7 @@ export default function OIDCTeamSyncModal(props: OIDCTeamSyncModalProps) { type="text" onChange={(_event, value) => handleInputChange(value)} id="team-sync-group-name" - validated={validation.type} /> - - , - })} - > - {validation.message} - -