Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

unless the user is using registration.py directly, no longer log credentials #1172

Merged
Merged
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
50 changes: 20 additions & 30 deletions src/deployment/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ def __init__(
create_registration: bool,
migrations: List[str],
export_appinsights: bool,
log_service_principal: bool,
multi_tenant_domain: str,
upgrade: bool,
subscription_id: Optional[str],
Expand Down Expand Up @@ -160,7 +159,6 @@ def __init__(
}
self.migrations = migrations
self.export_appinsights = export_appinsights
self.log_service_principal = log_service_principal
self.admins = admins
self.allowed_aad_tenants = allowed_aad_tenants

Expand Down Expand Up @@ -455,12 +453,6 @@ def try_sp_create() -> None:
self.results["client_id"] = app.app_id
self.results["client_secret"] = password

# Log `client_secret` for consumption by CI.
if self.log_service_principal:
logger.info("client_id: %s client_secret: %s", app.app_id, password)
else:
logger.debug("client_id: %s client_secret: %s", app.app_id, password)

def deploy_template(self) -> None:
logger.info("deploying arm template: %s", self.arm_template)

Expand Down Expand Up @@ -894,23 +886,27 @@ def update_registration(self) -> None:

def done(self) -> None:
logger.info(TELEMETRY_NOTICE)
client_secret_arg = (
("--client_secret %s" % self.cli_config["client_secret"])
if "client_secret" in self.cli_config
else ""
)
multi_tenant_domain = ""

cmd: List[str] = [
"onefuzz",
"config",
"--endpoint",
f"https://{self.application_name}.azurewebsites.net",
"--authority",
str(self.cli_config["authority"]),
"--client_id",
str(self.cli_config["client_id"]),
]

if "client_secret" in self.cli_config:
cmd += ["--client_secret", "YOUR_CLIENT_SECRET_HERE"]

if self.multi_tenant_domain:
multi_tenant_domain = "--tenant_domain %s" % self.multi_tenant_domain
logger.info(
"Update your CLI config via: onefuzz config --endpoint "
"https://%s.azurewebsites.net --authority %s --client_id %s %s %s",
self.application_name,
self.cli_config["authority"],
self.cli_config["client_id"],
client_secret_arg,
multi_tenant_domain,
)
cmd += ["--tenant_domain", str(self.multi_tenant_domain)]
ranweiler marked this conversation as resolved.
Show resolved Hide resolved

as_str = " ".join(cmd)

logger.info(f"Update your CLI config via: {as_str}")


def arg_dir(arg: str) -> str:
Expand Down Expand Up @@ -1021,11 +1017,6 @@ def main() -> None:
action="store_true",
help="enable appinsight log export",
)
parser.add_argument(
"--log_service_principal",
action="store_true",
help="display service prinipal with info log level",
)
parser.add_argument(
"--multi_tenant_domain",
type=str,
Expand Down Expand Up @@ -1076,7 +1067,6 @@ def main() -> None:
create_registration=args.create_pool_registration,
migrations=args.apply_migrations,
export_appinsights=args.export_appinsights,
log_service_principal=args.log_service_principal,
multi_tenant_domain=args.multi_tenant_domain,
upgrade=args.upgrade,
subscription_id=args.subscription_id,
Expand Down
12 changes: 8 additions & 4 deletions src/deployment/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,8 @@ def create_and_display_registration(
registration_name: str,
approle: OnefuzzAppRole,
subscription_id: str,
*,
display_secret: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

We should document this.

Copy link
Member

Choose a reason for hiding this comment

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

And consider giving it a scarier name.

) -> None:
logger.info("Updating application registration")
application_info = register_application(
Expand All @@ -414,10 +416,11 @@ def create_and_display_registration(
approle=approle,
subscription_id=subscription_id,
)
logger.info("Registration complete")
logger.info("These generated credentials are valid for a year")
logger.info("client_id: %s" % application_info.client_id)
logger.info("client_secret: %s" % application_info.client_secret)
if display_secret:
print("Registration complete")
print("These generated credentials are valid for a year")
print(f"client_id: {application_info.client_id}")
print(f"client_secret: {application_info.client_secret}")


def update_pool_registration(onefuzz_instance_name: str, subscription_id: str) -> None:
Expand Down Expand Up @@ -655,6 +658,7 @@ def main() -> None:
registration_name,
OnefuzzAppRole.CliClient,
args.subscription_id,
display_secret=True,
)
elif args.command == "assign_scaleset_role":
assign_app_role(
Expand Down