Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for google sign in issue #1600

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix for google sign in issue #1600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d8d5432
fix for google sign in issue
GMayank0310 Nov 20, 2024
012e949
fix for google sign in issue
GMayank0310 Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions kairon/shared/account/processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,8 @@ def delete_account(account_id: int, email: str = None):
UserActivityLogger.add_log(
a_type=UserActivityType.delete_account.value
, account=account_id)
UserEmailConfirmation.objects().get(email=email).delete()

Comment on lines +1231 to +1232
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling and fix indentation for UserEmailConfirmation deletion

The deletion of UserEmailConfirmation records needs improvement:

  1. Add error handling for cases where the record doesn't exist
  2. Consider moving this operation earlier in the method for better cleanup order
  3. Fix the indentation to match the method's scope

Apply this diff to implement the suggested changes:

-        UserEmailConfirmation.objects().get(email=email).delete()
+        try:
+            UserEmailConfirmation.objects().get(email=email).delete()
+        except DoesNotExist:
+            logging.warning(f"No email confirmation record found for {email}")

Also, consider moving this block right after the account validation and before bot deletion for better cleanup order.

Committable suggestion skipped: line range outside the PR's diff.


@staticmethod
def get_location_and_add_trusted_device(
Expand Down
2 changes: 2 additions & 0 deletions tests/integration_test/services_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from rasa.shared.utils.io import read_config_file
from slack_sdk.web.slack_response import SlackResponse

from kairon.shared.account.data_objects import UserEmailConfirmation
from kairon.shared.actions.models import ActionParameterType, DbActionOperationType, DbQueryValueType
from kairon.shared.admin.data_objects import LLMSecret
from kairon.shared.callback.data_objects import CallbackLog, CallbackRecordStatusType
Expand Down Expand Up @@ -28591,6 +28592,7 @@ def _password_reset(*args, **kwargs):
data={"username": "[email protected]", "password": "Welcome@10"},
)
actual = response_log.json()
UserEmailConfirmation(email="[email protected]").save()

assert actual["success"]
assert actual["error_code"] == 0
Expand Down
41 changes: 33 additions & 8 deletions tests/unit_test/api/api_processor_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
from kairon.exceptions import AppException
from kairon.idp.data_objects import IdpConfig
from kairon.idp.processor import IDPProcessor
from kairon.shared.account.data_objects import Feedback, BotAccess, User, Bot, Account, Organization, TrustedDevice
from kairon.shared.account.data_objects import Feedback, BotAccess, User, Bot, Account, Organization, TrustedDevice, \
UserEmailConfirmation
from kairon.shared.account.processor import AccountProcessor
from kairon.shared.admin.data_objects import BotSecrets
from kairon.shared.auth import Authentication, LoginSSOFactory
Expand Down Expand Up @@ -479,6 +480,8 @@ def test_delete_account_for_account_bots(self):
"last_name": "Test_Delete_Last",
"password": SecretStr("Welcome@1"),
}
email=account.get('email')
UserEmailConfirmation(email=email).save()

loop = asyncio.new_event_loop()
user_detail, mail, link = loop.run_until_complete(AccountProcessor.account_setup(account_setup=account))
Expand All @@ -489,11 +492,13 @@ def test_delete_account_for_account_bots(self):
account_bots_before_delete = list(AccountProcessor.list_bots(pytest.deleted_account))

assert len(account_bots_before_delete) == 2
AccountProcessor.delete_account(pytest.deleted_account)
AccountProcessor.delete_account(pytest.deleted_account, email)

for bot in account_bots_before_delete:
with pytest.raises(DoesNotExist):
Bot.objects(id=bot['_id'], account=pytest.deleted_account, status=True).get()
with pytest.raises(DoesNotExist):
UserEmailConfirmation.objects(email=email).get()

def test_delete_account_for_shared_bot(self):
account = {
Expand All @@ -508,6 +513,8 @@ def test_delete_account_for_shared_bot(self):
user_detail, mail, link = loop.run_until_complete(
AccountProcessor.account_setup(account_setup=account))

email=account.get('email')
UserEmailConfirmation(email=email).save()
# Add shared bot
bot_response = AccountProcessor.add_bot("delete_account_shared_bot", user_detail['account'], "[email protected]", False)
bot_id = bot_response['_id'].__str__()
Expand All @@ -519,11 +526,15 @@ def test_delete_account_for_shared_bot(self):
assert len(accessors_before_delete) == 2
assert accessors_before_delete[0]['accessor_email'] == '[email protected]'
assert accessors_before_delete[1]['accessor_email'] == '[email protected]'
AccountProcessor.delete_account(pytest.deleted_account)
AccountProcessor.delete_account(pytest.deleted_account, email)
accessors_after_delete = list(AccountProcessor.list_bot_accessors(bot_id))
assert len(accessors_after_delete) == 0
assert len(list(Bot.objects(id=bot_id, account=user_detail['account'], status=True))) == 0

with pytest.raises(DoesNotExist):
UserEmailConfirmation.objects(email=email).get()


def test_delete_account_for_account(self):
account = {
"account": "Test_Delete_Account",
Expand All @@ -532,18 +543,23 @@ def test_delete_account_for_account(self):
"last_name": "Test_Delete_Last",
"password": SecretStr("Welcome@1")
}
email=account.get('email')
UserEmailConfirmation(email=email).save()

loop = asyncio.new_event_loop()
user_detail, mail, link = loop.run_until_complete(
AccountProcessor.account_setup(account_setup=account))
pytest.deleted_account = user_detail['account'].__str__()

AccountProcessor.delete_account(pytest.deleted_account)
AccountProcessor.delete_account(pytest.deleted_account, email)
assert AccountProcessor.get_account(pytest.deleted_account)
assert not AccountProcessor.get_account(pytest.deleted_account).get('status')

with pytest.raises(AppException, match="Account does not exist!"):
AccountProcessor.delete_account(pytest.deleted_account)
AccountProcessor.delete_account(pytest.deleted_account, email)

with pytest.raises(DoesNotExist):
UserEmailConfirmation.objects(email=email).get()

def test_delete_account_for_user(self):
account = {
Expand All @@ -553,6 +569,8 @@ def test_delete_account_for_user(self):
"last_name": "Test_Delete_Last",
"password": SecretStr("Welcome@1")
}
email=account.get('email')
UserEmailConfirmation(email=email).save()

loop = asyncio.new_event_loop()
user_detail, mail, link = loop.run_until_complete(
Expand All @@ -573,11 +591,14 @@ def test_delete_account_for_user(self):
assert User.objects(email__iexact="[email protected]", status=True).get()
assert User.objects(email__iexact="[email protected]", status=True).get()

AccountProcessor.delete_account(pytest.deleted_account)
AccountProcessor.delete_account(pytest.deleted_account, email)

assert User.objects(email__iexact="[email protected]", status=False)
assert User.objects(email__iexact="[email protected]", status=False)

with pytest.raises(DoesNotExist):
UserEmailConfirmation.objects(email=email).get()

def test_delete_account_again_add(self):
account = {
"account": "Test_Delete_Account",
Expand All @@ -586,19 +607,23 @@ def test_delete_account_again_add(self):
"last_name": "Test_Delete_Last",
"password": SecretStr("Welcome@1"),
}

email=account.get('email')
UserEmailConfirmation(email=email).save()
loop = asyncio.new_event_loop()
user_detail, mail, link = loop.run_until_complete(
AccountProcessor.account_setup(account_setup=account))
pytest.deleted_account = user_detail['account'].__str__()

AccountProcessor.delete_account(pytest.deleted_account)
AccountProcessor.delete_account(pytest.deleted_account, email)

loop = asyncio.new_event_loop()
user_detail, mail, link = loop.run_until_complete(
AccountProcessor.account_setup(account_setup=account))
new_account_id = user_detail['account'].__str__()

with pytest.raises(DoesNotExist):
UserEmailConfirmation.objects(email=email).get()

assert new_account_id
assert AccountProcessor.get_account(new_account_id).get('status')
assert len(list(AccountProcessor.list_bots(new_account_id))) == 0
Expand Down
Loading