Skip to content

Commit

Permalink
Merge branch 'main' into Updating-Trivy
Browse files Browse the repository at this point in the history
Updating working branch with changes in main branch.
  • Loading branch information
faizan12123 committed Jan 9, 2024
2 parents d32ed8a + 8df0faa commit d0a70d1
Show file tree
Hide file tree
Showing 15 changed files with 325 additions and 114 deletions.
1 change: 1 addition & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation change

## Checklist
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test_backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # actions/checkout@v3.5.2
- uses: actions/setup-python@41b7212b1668f5de9d65e9c82aa777e6bbedb3a8 # actions/setup-python@v2.1.4
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # actions/checkout@v4.1.1
- uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # actions/setup-python@v5.0.0
with:
python-version: "3.9"
- name: Run backend tests
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test_orchestrator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # actions/checkout@v3.5.2
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # actions/checkout@v4.1.1
- name: setup python environment
uses: actions/setup-python@41b7212b1668f5de9d65e9c82aa777e6bbedb3a8 # actions/setup-python@v2.1.4
uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # actions/setup-python@v5.0.0
with:
python-version: "3.9"
architecture: "x64"
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test_ui.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ jobs:
env:
UI_PATH: ./ui
HADOLINT_URL: https://github.com/hadolint/hadolint/releases/download/v2.10.0/hadolint-Linux-x86_64
NPM_VERSION: 9
NPM_VERSION: 10

steps:
- name: Checkout the code
uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # actions/checkout@v3.5.2
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # actions/checkout@v4.1.1

- name: Setup node from node version file
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # actions/setup-node@v3.6.0
uses: actions/setup-node@b39b52d1213e96004bfcb1c61a8a6fa8ab84f3e8 # actions/setup-node@v4.0.1
with:
node-version-file: "${{ env.UI_PATH }}/.nvmrc"

Expand Down
55 changes: 51 additions & 4 deletions backend/lambdas/api/authorizer/authorizer/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ def process_user_auth(event):
_verify_claims(claims)

# Get the user
user = _get_update_or_create_user(email=claims["email"].lower())
user = _get_update_or_create_user(
email=claims["email"].lower(), source_ip=event["requestContext"]["identity"]["sourceIp"]
)

if not user:
raise Exception("Unauthorized")
Expand Down Expand Up @@ -172,7 +174,7 @@ def _verify_claims(claims: dict):


@transaction.atomic
def _get_update_or_create_user(email: str) -> User:
def _get_update_or_create_user(email: str, source_ip: str) -> User:
"""
Attempt to get a user based on email.
If user does not exist, try to match based on EMAIL_DOMAIN_ALIASES, and update email if successful.
Expand All @@ -183,15 +185,21 @@ def _get_update_or_create_user(email: str) -> User:
# If user is soft-deleted, return None so that auth fails
# this should never occur in practice because user email is modified with a suffix of "_DELETED_{timestamp}" at deletion, but it is ok to retain this check
if user and user.deleted:
LOG.debug(f"User found by email {email}, but is deleted")
return None

# if user is not found by email, check for aliases
if not user and EMAIL_DOMAIN_ALIASES:
LOG.debug(f"User not found by email {email}, checking for aliases")
email_local_part = email.split("@")[0]
email_domain = email.split("@")[1]

# Check if any aliases exist for domain, and attempt to find a match
for alias in EMAIL_DOMAIN_ALIASES:
# if user was found in a previous iteration, break the loop
if user and not user.deleted:
break

if alias["new_domain"] == email_domain:
for old_domain in alias["old_domains"]:
if alias.get("email_transformation"):
Expand All @@ -204,23 +212,51 @@ def _get_update_or_create_user(email: str) -> User:
else:
old_email = f"{email_local_part}@{old_domain}"

LOG.debug(f"Attempting to get user with possible alias {old_email}")
user = _get_user(old_email)

# If a user is found with an old email (and was not soft-deleted), update the email and return user
if user and not user.deleted:
LOG.debug(f"User account discovered with alias {old_email}")
LOG.debug(f"Attempting to update user email from {old_email} to {email}")
user.email = email
user.save()

audit_log = AuditLogger(principal=old_email, source_ip=source_ip)
audit_log.user_modified(
user=user.email, scope=user.scope, features=user.features, admin=user.admin
)
LOG.debug(f"User account email updated to {user.email}")

# since user was successfully found and updated, break out of inner loop
break

# if user is found directly or via alias (and was not soft-deleted), ensure that user's self group is named correctly, then return user
if user and not user.deleted:
LOG.debug("Checking that email and self group name match")
if user.self_group.name != user.email:
LOG.debug(f"Self group {user.self_group.name} does not match email {user.email}")
LOG.debug(f"Attempting to update self group name to {user.email}")
user.self_group.name = user.email
user.self_group.save()

audit_log = AuditLogger(principal=user.email, source_ip=source_ip)
audit_log.group_modified(
group_id=str(user.self_group.group_id),
name=user.self_group.name,
scope=user.self_group.scope,
features=user.self_group.features,
admin=user.self_group.admin,
allowlist=user.self_group.allowlist,
)
LOG.debug(f"Self group name updated to {user.self_group.name}")

LOG.debug(f"Returning user with email {user.email} and self group {user.self_group.name}")
return user

# Create the user since no match has been found at this point
return _create_user(email)
LOG.debug(f"No matching user discovered. Creating a new user with email {email}")
return _create_user(email, source_ip)


def _update_login_timestamp(user: User) -> None:
Expand All @@ -231,7 +267,7 @@ def _update_login_timestamp(user: User) -> None:
user.save()


def _create_user(email: str) -> User:
def _create_user(email: str, source_ip: str) -> User:
"""
Create a new user
"""
Expand All @@ -240,6 +276,17 @@ def _create_user(email: str) -> User:
# User was created so create their self group as well
Group.create_self_group(user)

audit_log = AuditLogger(principal=email, source_ip=source_ip)
audit_log.user_created(user=user.email, scope=user.scope, features=user.features, admin=user.admin)
audit_log.group_created(
group_id=str(user.self_group.group_id),
name=user.self_group.name,
scope=user.self_group.scope,
features=user.self_group.features,
admin=user.self_group.admin,
allowlist=user.self_group.allowlist,
)

return user


Expand Down
81 changes: 72 additions & 9 deletions backend/lambdas/api/authorizer/tests/test_get_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
"old_domains": ["company.com"],
"email_transformation": {"new_email_regex": "_", "old_email_expr": "."},
},
{
"new_domain": "company5.com",
"old_domains": ["company6.com", "company6andsuffix.com"],
"email_transformation": {"new_email_regex": "[.]", "old_email_expr": "_"},
},
{"new_domain": "newcompany.com", "old_domains": ["company.com"]},
]

Expand Down Expand Up @@ -60,12 +65,33 @@
"last_login": "2023-01-01 00:00:00.000000+00:00",
"self_group": {"name": "[email protected]"},
},
{
"id": 6,
"email": "[email protected]",
"deleted": False,
"last_login": "2023-01-01 00:00:00.000000+00:00",
"self_group": {"name": "[email protected]"},
},
{
"id": 7,
"email": "[email protected]",
"deleted": False,
"last_login": "2023-01-01 00:00:00.000000+00:00",
"self_group": {"name": "[email protected]"},
},
]

EVENT_IP = "0.0.0.0"


class MockGroup(object):
def __init__(self, **kwargs):
self.name = kwargs.get("name") or ""
self.group_id = kwargs.get("group_id") or ""
self.scope = kwargs.get("scope") or []
self.features = kwargs.get("features") or {}
self.admin = kwargs.get("admin") or False
self.allowlist = kwargs.get("allowlist") or False

def save(self):
pass
Expand All @@ -88,6 +114,9 @@ def __init__(self, **kwargs):
self_group = kwargs.get("self_group") or None
if self_group:
self.self_group = MockGroup(name=self_group.get("name"))
self.scope = kwargs.get("scope") or []
self.features = kwargs.get("features") or {}
self.admin = kwargs.get("admin") or False

def save(self):
for user in MockUser.users:
Expand Down Expand Up @@ -118,6 +147,10 @@ def get(email: str, **kwargs):
_get_update_or_create_user = authorizer.handlers._get_update_or_create_user.__wrapped__


@patch("authorizer.handlers.AuditLogger.group_created", lambda *x, **y: None)
@patch("authorizer.handlers.AuditLogger.group_modified", lambda *x, **y: None)
@patch("authorizer.handlers.AuditLogger.user_created", lambda *x, **y: None)
@patch("authorizer.handlers.AuditLogger.user_modified", lambda *x, **y: None)
@patch("authorizer.handlers.EMAIL_DOMAIN_ALIASES", EMAIL_DOMAIN_ALIASES)
@patch("authorizer.handlers.User", MockUser)
@patch("authorizer.handlers.Group", MockGroup)
Expand All @@ -128,7 +161,7 @@ def test_get_existing_user(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 1
and user.__dict__.get("email") == email
Expand All @@ -141,7 +174,7 @@ def test_get_deleted_user(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(user == None)

def test_get_nonexistent_user(self):
Expand All @@ -151,7 +184,7 @@ def test_get_nonexistent_user(self):
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
expected_userid = MockUser.users[-1].get("id") + 1
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == expected_userid
and user.__dict__.get("email") == email
Expand All @@ -165,7 +198,7 @@ def test_get_user_with_new_email(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 1
and user.__dict__.get("email") == email
Expand All @@ -179,7 +212,7 @@ def test_get_user_with_new_email_with_transformation(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 2
and user.__dict__.get("email") == email
Expand All @@ -193,7 +226,7 @@ def test_get_user_with_new_email_with_transformation2(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 4
and user.__dict__.get("email") == email
Expand All @@ -207,7 +240,7 @@ def test_get_user_with_new_email_with_transformation3(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 1
and user.__dict__.get("email") == email
Expand All @@ -221,7 +254,7 @@ def test_get_user_with_email_and_self_group_mismatch(self):
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 5
and user.__dict__.get("email") == email
Expand All @@ -236,9 +269,39 @@ def test_get_user_with_new_email_and_deleted_old_user(self):
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
expected_userid = MockUser.users[-1].get("id") + 1
user = _get_update_or_create_user(email=email)
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == expected_userid
and user.__dict__.get("email") == email
and user.__dict__.get("self_group").name == email
)

def test_get_user_with_new_email_and_multiple_old_domains_first_domain(self):
"""
User logs in with email "[email protected]" and has an existing acount with email "[email protected]"
Existing account is found, and email and self group name are updated to the new email "[email protected]"
This is distinct from other tests because there are multiple old domains mapped to new domain company5.com
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 6
and user.__dict__.get("email") == email
and user.__dict__.get("self_group").name == email
)

def test_get_user_with_new_email_and_multiple_old_domains_second_domain(self):
"""
User logs in with email "[email protected]" and has an existing acount with email "[email protected]"
Existing account is found, and email and self group name are updated to the new email "[email protected]"
This is distinct from other tests because there are multiple old domains mapped to new domain company5.com
"""
MockUser.users = copy.deepcopy(USERS)
email = "[email protected]"
user = _get_update_or_create_user(email=email, source_ip=EVENT_IP)
self.assertTrue(
user.__dict__.get("id") == 7
and user.__dict__.get("email") == email
and user.__dict__.get("self_group").name == email
)
3 changes: 3 additions & 0 deletions backend/lambdas/api/users/users/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ def delete(event, email=None, admin: bool = False):
# Hard delete all of the user's self group API keys
user.self_group.apikey_set.all().delete()

# Hard delete user's services
user.userservice_set.all().delete()

# This is outside the transaction so that if the transaction rolled back we don't log the audit event
if user.deleted:
audit_log = AuditLogger(principal=email, source_ip=event["requestContext"]["identity"]["sourceIp"])
Expand Down
Loading

0 comments on commit d0a70d1

Please sign in to comment.