Skip to content

[Maintenance] Refactored scopes and auth #66

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

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

Conversation

brokensound77
Copy link
Contributor

@brokensound77 brokensound77 commented Aug 12, 2023

The commands for scopes and auth have been removed.

  • Scopes should be configured in the etc/config.yaml, etc/custom_config.yaml, or emulation configs
  • auth should be called by each emulating command as needed. The mechanism to do so is now within the Cred, simply by using Cred.session() with scopes as an optional parameter
  • custom global configs support

@@ -73,18 +78,22 @@ def to_dict(self):
class Cred:

creds: Optional[CRED_TYPES]
session: Optional[Credentials]

def session(self, scopes: Optional[list[str]] = None) -> Optional[Credentials]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to get a session, just call this method, with optional scopes

self.custom_config = yaml.safe_load(custom_path.read_text()) if custom_path.exists() else {}

@property
def merged(self) -> dict:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when getting items, use this property

when setting, set directly to custom_config - these are all saved on exit

@@ -41,6 +41,7 @@ def __call__(self,
raise argparse.ArgumentError(self, f'invalid filter argument "{value}", expected "key=value"')
setattr(namespace, self.dest, {key: val})


Copy link
Contributor Author

Choose a reason for hiding this comment

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

This command is failing, but I think unrelated?

@@ -25,7 +26,8 @@ class Emulation(BaseEmulation):
def __init__(self, **kwargs) -> None:
super().__init__(**kwargs)
self.folder_id = self.args.folder_id
self.service = build('drive', 'v3', credentials=self.obj.cred_store.store[self.args.session_key].session)
creds = self.obj.cred_store.get('default', validate_type='oauth')
self.service = build('drive', 'v3', credentials=creds.session())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

creds.session()!

def main():
parser = argparse.ArgumentParser(description='SWAT CLI')
parser.add_argument('--debug', action='store_true', help='Debug mode')
parser.add_argument('--custom-config', type=Path, default=DEFAULT_CUSTOM_CONFIG_PATH,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default custom but optional param

@@ -27,6 +34,7 @@ def main():
finally:
if shell.save_on_exit:
shell.obj.cred_store.save()
shell.obj.config.save_custom()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

saved on exit

@brokensound77
Copy link
Contributor Author

SWAT> emulate admin_add_roles_to_users --username test --roles test

gets error

2023-08-11 20:42:10 ERROR Error: Authorized user info was not in the expected format, missing fields client_id, refresh_token, client_secret. (default:shell.py:195)

Looks like refresh_token is missing from creds??

@terrancedejesus
Copy link
Contributor

class OAuthCreds(BaseCreds):
    """Data class for OAuth2.0 application credentials."""

    auth_provider_x509_cert_url: str
    auth_uri: str
    client_id: str
    client_secret: str
    project_id: str
    redirect_uris: list[str]
    token_uri: str
    refresh_token: str = None


def session(self, scopes: Optional[list[str]] = None) -> Optional[Credentials]:
if isinstance(self.creds, OAuthCreds):
session = Credentials.from_authorized_user_info(self.creds.to_dict(), scopes=scopes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
session = Credentials.from_authorized_user_info(self.creds.to_dict(), scopes=scopes)
session = Credentials.from_authorized_user_info(self.creds.to_dict()['installed'], scopes=scopes)

if isinstance(self.creds, OAuthCreds):
session = Credentials.from_authorized_user_info(self.creds.to_dict(), scopes=scopes)
else:
session = ServiceCredentials.from_service_account_info(self.creds.to_dict(), scopes=scopes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
session = ServiceCredentials.from_service_account_info(self.creds.to_dict(), scopes=scopes)
session = ServiceCredentials.from_service_account_info(self.creds.to_dict()['installed'], scopes=scopes)

@terrancedejesus terrancedejesus changed the title Refactored scopes and auth [Maintenance] Refactored scopes and auth Aug 12, 2023
@terrancedejesus
Copy link
Contributor

Some additional thoughts here:

  • Service accounts will require scopes to be defined based on the IAM role assigned to the service account. Scopes cannot be adjusted after statically defined during authN/authZ accounts. Therefore, dynamically adjusting scopes is only compatible with OAuth workflow. Domain-wide delegation has to also be established for the service account by adding the ID of the service account to Google Workspace via the admin console.
  • If we can, let's add API credentials to this as well to cover all GCP-based credentials for API interaction.

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

Successfully merging this pull request may close these issues.

2 participants