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

Accommodate new SSO aws_account_name #15

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Accommodate new SSO aws_account_name #15

wants to merge 3 commits into from

Conversation

chrowe
Copy link
Collaborator

@chrowe chrowe commented May 23, 2023

WRI is starting to use SSO to provision new users on AWS. For SSO users the Arn has a different structure.
This change should accommodate both formats but for SSO users their aws_account_name will now be their email address.

I also switched over to using boto3.Session to accommodate people with multiple user accounts. This is set to default by default so it should continue to function as is for existing users.

@chrowe
Copy link
Collaborator Author

chrowe commented May 23, 2023

Leaving this as a draft until I can get someone to test this branch.

@chrowe chrowe requested review from jterry64 and loganbyers May 23, 2023 17:34
@@ -39,7 +40,7 @@ class TreeCoverLossAnalysis(object):
self.description = descript_1 + descript_2 + descript_3
self.canRunInBackground = False
self.aws_account_name = (
boto3.client("sts").get_caller_identity().get("Arn").split("/")[1]
aws_session.client("sts").get_caller_identity().get("Arn").split("/")[-1]
Copy link
Member

@loganbyers loganbyers May 23, 2023

Choose a reason for hiding this comment

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

Could/should the @wri.org part of the user label be removed?

aws_session.client("sts").get_caller_identity().get("Arn").split("/")[-1].split("@")[0]

@@ -5,6 +5,7 @@ from datetime import datetime
import itertools
import os

aws_session = boto3.Session(profile_name='default')
Copy link
Member

@loganbyers loganbyers May 23, 2023

Choose a reason for hiding this comment

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

I suspect this would be a little more appropriate as a class-level variable in TreeCoverLossAnalysis.

Is the idea that SSO users will likely override which profile is being used? I worry that hard-coding the string 'default' might not be better than passing None, which would signal that boto should find whatever the fallback profile is. Can the choice of profile be provided as a parameter to the execution of the tool? That way people don't have to edit the code if they have a less common environment setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants