-
Notifications
You must be signed in to change notification settings - Fork 190
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
Add IAM based auth for S3 policy repo #691
Open
kiesenverseist
wants to merge
11
commits into
permitio:master
Choose a base branch
from
kiesenverseist:iam-based-auth-for-s3-policy
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
28fb39b
Add time cache decorator
kiesenverseist 3350181
Add extra config required for iam auth
kiesenverseist 7e4cd88
Make build_auth_headers async
kiesenverseist bad572e
Add method to get temp credentials via iam auth
kiesenverseist 1278ca8
remove prefix from aws provided env vars
kiesenverseist c11b03c
more logs, and set content type
kiesenverseist cdf669f
Force direct read of env vars
kiesenverseist be75932
Change ttl cache to work for async
kiesenverseist c7dedb7
Allow aws header builder to take session token
kiesenverseist 160ba20
Fix XML, async file handling, use session token
kiesenverseist d6b71c1
Docs, typos and naming.
kiesenverseist File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,10 @@ | |
from pathlib import Path | ||
from typing import Optional, Tuple | ||
from urllib.parse import urlparse | ||
from xml.etree import ElementTree | ||
|
||
import aiohttp | ||
import aiofiles | ||
from fastapi import status | ||
from fastapi.exceptions import HTTPException | ||
from opal_common.git_utils.tar_file_to_local_git_extractor import ( | ||
|
@@ -17,6 +19,7 @@ | |
hash_file, | ||
throw_if_bad_status_code, | ||
tuple_to_dict, | ||
async_time_cache, | ||
) | ||
from opal_server.config import PolicyBundleServerType | ||
from tenacity import AsyncRetrying | ||
|
@@ -43,6 +46,9 @@ class ApiPolicySource(BasePolicySource): | |
token (str, optional): auth token to include in connections to bundle server. Defaults to POLICY_BUNDLE_SERVER_TOKEN. | ||
token_id (str, optional): auth token ID to include in connections to bundle server. Defaults to POLICY_BUNDLE_SERVER_TOKEN_ID. | ||
bundle_server_type (PolicyBundleServerType, optional): the type of bundle server | ||
region (str, optional): the aws region of s3 bucket containing the bundle | ||
aws_role_arn (str, optional): the aws iam role to assume when accessing the s3 bucket. Only required when using temporary sts credentials. | ||
aws_web_id_token_file (str, optional): the file containing a web id token for the target aws iam role. Only required when using temporary sts credentials. | ||
""" | ||
|
||
def __init__( | ||
|
@@ -53,6 +59,8 @@ def __init__( | |
token: Optional[str] = None, | ||
token_id: Optional[str] = None, | ||
region: Optional[str] = None, | ||
aws_role_arn: Optional[str] = None, | ||
aws_web_id_token_file: Optional[str] = None, | ||
bundle_server_type: Optional[PolicyBundleServerType] = None, | ||
policy_bundle_path=".", | ||
policy_bundle_git_add_pattern="*", | ||
|
@@ -66,6 +74,8 @@ def __init__( | |
self.token_id = token_id | ||
self.server_type = bundle_server_type | ||
self.region = region | ||
self.aws_role_arn = aws_role_arn | ||
self.aws_web_id_token_file = aws_web_id_token_file | ||
self.bundle_hash = None | ||
self.etag = None | ||
self.tmp_bundle_path = Path(policy_bundle_path) | ||
|
@@ -126,7 +136,84 @@ async def api_update_policy(self) -> Tuple[bool, str, str]: | |
) | ||
raise | ||
|
||
def build_auth_headers(self, token=None, path=None): | ||
@async_time_cache(ttl=3000) | ||
async def get_temporary_sts_credentials(self) -> tuple[str, str, str]: | ||
""" | ||
This function will fetch a set of temporary credentials for a IAM role | ||
from Amazon STS. It requires an aws region, the arn for the target role | ||
and the file containing the web token. | ||
|
||
This function will return the id and secret key required for login. | ||
When using temporary credentials, AWS also requires a session token | ||
which this function also provides. | ||
|
||
This result of this funciton is cached to avoid being rate limited by | ||
STS. | ||
""" | ||
assert self.aws_web_id_token_file | ||
assert self.aws_role_arn | ||
assert self.region | ||
|
||
async with aiofiles.open(self.aws_web_id_token_file) as token_file: | ||
token = await token_file.read() | ||
|
||
sts_url = f"sts.{self.region}.amazonaws.com" | ||
params: dict[str, str] = { | ||
"Action": "AssumeRoleWithWebIdentity", | ||
"DurationSeconds": "3600", | ||
"RoleSessionName": "Opal", | ||
"RoleArn": self.aws_role_arn, | ||
"WebIdentityToken": token, | ||
"Version": "2011-06-15", | ||
} | ||
|
||
async with aiohttp.ClientSession() as session: | ||
try: | ||
async with session.get( | ||
f"https://{sts_url}", | ||
params=params, | ||
headers={"Content-Type": "application/xml"}, | ||
) as response: | ||
if response.status == status.HTTP_404_NOT_FOUND: | ||
logger.warning( | ||
"requested url not found: {sts_url}", | ||
sts_url=sts_url, | ||
) | ||
raise HTTPException( | ||
status_code=status.HTTP_404_NOT_FOUND, | ||
detail=f"requested url not found: {sts_url}", | ||
) | ||
|
||
body = await response.read() | ||
|
||
# the default aws xml namespace | ||
ns = {"": "https://sts.amazonaws.com/doc/2011-06-15/"} | ||
|
||
et = ElementTree.fromstring(body) | ||
credentials = et.find( | ||
"AssumeRoleWithWebIdentityResult/Credentials", ns | ||
) | ||
assert credentials | ||
|
||
id = credentials.findtext("AccessKeyId", namespaces=ns) | ||
key = credentials.findtext("SecretAccessKey", namespaces=ns) | ||
session_token = credentials.findtext("SessionToken", namespaces=ns) | ||
|
||
assert id | ||
assert key | ||
assert session_token | ||
|
||
except (aiohttp.ClientError, HTTPException) as e: | ||
logger.warning("server connection error: {err}", err=repr(e)) | ||
raise | ||
except Exception as e: | ||
logger.error("unexpected server connection error: {err}", err=repr(e)) | ||
raise | ||
|
||
logger.info("Successfully generated temporary AWS credentials") | ||
return id, key, session_token | ||
|
||
async def build_auth_headers(self, token=None, path=None): | ||
# if it's a simple HTTP server with a bearer token | ||
if self.server_type == PolicyBundleServerType.HTTP and token is not None: | ||
return tuple_to_dict(get_authorization_header(token)) | ||
|
@@ -136,14 +223,34 @@ def build_auth_headers(self, token=None, path=None): | |
and token is not None | ||
and self.token_id is not None | ||
): | ||
logger.info("Using provided token to log in to AWS_S3") | ||
|
||
split_url = urlparse(self.remote_source_url) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kiesenverseist this is duplicate with the code below, I would refactor the section of both cases of S3 to use common code. right? |
||
host = split_url.netloc | ||
path = split_url.path + "/" + path | ||
|
||
return build_aws_rest_auth_headers( | ||
self.token_id, token, host, path, self.region | ||
) | ||
elif ( | ||
self.server_type == PolicyBundleServerType.AWS_S3 | ||
and self.aws_role_arn is not None | ||
and self.aws_web_id_token_file is not None | ||
and self.region is not None | ||
): | ||
logger.info("Using IAM Web auth to log in to AWS_S3") | ||
|
||
split_url = urlparse(self.remote_source_url) | ||
host = split_url.netloc | ||
path = split_url.path + "/" + path | ||
|
||
id, key, session_token = await self.get_temporary_sts_credentials() | ||
|
||
return build_aws_rest_auth_headers( | ||
id, key, host, path, self.region, session_token | ||
) | ||
else: | ||
logger.info("Not authenticating on bundle endpoint") | ||
return {} | ||
|
||
async def fetch_policy_bundle_from_api_source( | ||
|
@@ -166,7 +273,7 @@ async def fetch_policy_bundle_from_api_source( | |
""" | ||
path = "bundle.tar.gz" | ||
|
||
auth_headers = self.build_auth_headers(token=token, path=path) | ||
auth_headers = await self.build_auth_headers(token=token, path=path) | ||
etag_headers = ( | ||
{"ETag": self.etag, "If-None-Match": self.etag} if self.etag else {} | ||
) | ||
|
@@ -278,4 +385,4 @@ async def check_for_changes(self): | |
prev_head=prev, | ||
new_head=latest, | ||
) | ||
await self._on_new_policy(old=prev_commit, new=new_commit) | ||
await self._on_new_policy(old=prev_commit, new=new_commit) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kiesenverseist hi, you don't really need elif here, right as there is a return from each case before moving to the next. so you could just have the if's one after the other starting from line 221.