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.

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: linter issues for aws #77

Open
wants to merge 1 commit into
base: git-actions
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ test: ensure_container_for_test
lint: export APPUID = $(APP_UID)
lint: ensure_container_for_test
@docker exec test python -m pylama --version
@docker exec test python -m pylama Access/access_modules
@docker exec test python -m pylama Access/access_modules/aws_access/helper.py
@if [ "$$?" -ne 0 ]; then \
echo "Linter checks failed"; \
exit 1; \
Expand Down
34 changes: 24 additions & 10 deletions aws_access/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def approve(
"Something when wrong while adding %s to group %s: %s",
user.email,
label["group"],
str(exception)
str(exception),
)
return False

Expand All @@ -90,7 +90,9 @@ def approve(
label_meta,
)
except Exception as ex:
logger.exception("%s Could not send email for error %s", self.tag(), str(ex))
logger.exception(
"%s Could not send email for error %s", self.tag(), str(ex)
)
return False

return True
Expand All @@ -101,11 +103,15 @@ def __send_approve_email(
"""Generates and sends email in access grant."""
if auto_approve_rules:
rules = " ,".join(auto_approve_rules)
email_subject = (f"Access Granted: {request_id}"
f" for access to {label_desc} for user {user.email}. Rules :- {rules}")
email_subject = (
f"Access Granted: {request_id}"
f" for access to {label_desc} for user {user.email}. Rules :- {rules}"
)
else:
email_subject = (f"Access Granted: {request_id}"
f" for access to {label_desc} for user {user.email}.")
email_subject = (
f"Access Granted: {request_id}"
f" for access to {label_desc} for user {user.email}."
)

email_body = self._generate_string_from_template(
"aws_access/approved_email_template.html.j2",
Expand All @@ -121,8 +127,10 @@ def __send_approve_email(
def __send_revoke_email(self, user, request_id, label_desc):
"""Generates and sends email in for access revoke."""
email_targets = self.email_targets(user)
email_subject = (f"Revoke Request: {request_id}"
f"for access to {label_desc} for user {user.email}")
email_subject = (
f"Revoke Request: {request_id}"
f"for access to {label_desc} for user {user.email}"
)
emailSES(email_targets, email_subject, "")

def get_label_desc(self, access_label):
Expand Down Expand Up @@ -217,7 +225,9 @@ def revoke(self, user, user_identity, label, request):
if not is_revoked:
logger.error(
"Something went wrong while removing %s from %s: %s",
user.email, label["group"], str(exception)
user.email,
label["group"],
str(exception),
)
return False

Expand Down Expand Up @@ -313,9 +323,13 @@ def access_types(self):
return {}

def get_identity_template(self):
""" return the path to the identity template path """
return ""

def verify_identity(self, request, email):
def verify_identity(self, request=None, email=None):
""" return aws Identity which is empty as email itself
is used as identity which is already verified
"""
return {}


Expand Down
3 changes: 3 additions & 0 deletions aws_access/constants.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
"""Constants and error messages for AWS access"""
AWS_ACCESS = "aws_access"
IAM_RESOURCE = "iam"
GROUP_ACCESS = "GroupAccess"
Expand All @@ -7,3 +8,5 @@
"valid_account_required": "Valid account name is required for AWS access",
"valid_group_required": "Valid group name is required for AWS access",
}

BAD_REQUEST = "Bad request please review the request made."
6 changes: 3 additions & 3 deletions aws_access/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def aws_group_exists(account, group):


def _get_aws_config():
""" Gets AWS config. """
"""Gets AWS config."""

return ACCESS_MODULES.get("aws_access", {})

Expand Down Expand Up @@ -97,7 +97,7 @@ def grant_aws_access(user, account, group):
client = get_aws_client(account=account, resource=constants.IAM_RESOURCE)
client.add_user_to_group(GroupName=group, UserName=__get_username(user.email))
except Exception as ex:
logger.exception("Exception while adding user to AWS group: " + str(ex))
logger.exception("Exception while adding user to AWS group: %s", str(ex))
return False, str(ex)
return True, ""

Expand All @@ -119,7 +119,7 @@ def revoke_aws_access(user, account, group):
GroupName=group, UserName=__get_username(user.email)
)
except Exception as ex:
logger.error("Exception while removing user from AWS group: " + str(ex))
logger.error("Exception while removing user from AWS group: %s", str(ex))
return False, str(ex)
return True, ""

Expand Down
23 changes: 16 additions & 7 deletions aws_access/test_aws_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,22 @@ def test_aws_access(mocker):
request_mock = mocker.MagicMock()
request_mock.user = user_mock

mocker.patch("Access.access_modules.aws_access.access.AWSAccess._AWSAccess__send_approve_email", return_value="")
mocker.patch("Access.access_modules.aws_access.access.AWSAccess._AWSAccess__send_revoke_email", return_value="")
mocker.patch(
"Access.access_modules.aws_access.helpers.grant_aws_access",
return_value=(True, ""))
"Access.access_modules.aws_access.access.AWSAccess._AWSAccess__send_approve_email",
return_value="",
)
mocker.patch(
"Access.access_modules.aws_access.access.AWSAccess._AWSAccess__send_revoke_email",
return_value="",
)
mocker.patch(
"Access.access_modules.aws_access.helpers.revoke_aws_access",
return_value=(True, ""))
"Access.access_modules.aws_access.helpers.grant_aws_access",
return_value=(True, ""),
)
mocker.patch(
"Access.access_modules.aws_access.helpers.revoke_aws_access",
return_value=(True, ""),
)
aws_access = access.AWSAccess()

label_1 = {
Expand Down Expand Up @@ -195,7 +203,8 @@ def test_aws_access(mocker):
assert return_value is True

return_value = aws_access.revoke(
user_mock, mocker.MagicMock(), label_1, request_mock)
user_mock, mocker.MagicMock(), label_1, request_mock
)
assert return_value is True


Expand Down
7 changes: 5 additions & 2 deletions aws_access/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ def get_aws_accounts(request):
Returns:
JsonResponse: json response with aws account list
"""
response = {"data": helpers.get_aws_accounts()}
return JsonResponse(response)
if request.GET:
response = {"data": helpers.get_aws_accounts()}
return JsonResponse(response)

return JsonResponse({"error": constants.BAD_REQUEST}, status=400)


@login_required
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ google-auth-httplib2==0.1.0
google-auth-oauthlib==0.8.0
slack-sdk==3.20.2
fabric==3.0.0
boto3-stubs==1.26.143