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

Remove client_secret from config, add env var #1918

Merged
merged 4 commits into from
May 9, 2022
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
1 change: 0 additions & 1 deletion src/cli/examples/azure-functions-example/info/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ def main(req: func.HttpRequest) -> func.HttpResponse:
endpoint=os.environ.get("ONEFUZZ_ENDPOINT"),
authority=os.environ.get("ONEFUZZ_AUTHORITY"),
client_id=os.environ.get("ONEFUZZ_CLIENT_ID"),
client_secret=os.environ.get("ONEFUZZ_CLIENT_SECRET"),
ranweiler marked this conversation as resolved.
Show resolved Hide resolved
)
info = o.info.get()
return func.HttpResponse(info.json())
32 changes: 23 additions & 9 deletions src/cli/onefuzz/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@

UUID_RE = r"^[a-f0-9]{8}-?[a-f0-9]{4}-?[a-f0-9]{4}-?[a-f0-9]{4}-?[a-f0-9]{12}\Z"

# Environment variable optionally used for setting an application client secret.
CLIENT_SECRET_ENV_VAR = "ONEFUZZ_CLIENT_SECRET" # nosec


class PreviewFeature(Enum):
job_templates = "job_templates"
Expand Down Expand Up @@ -1639,11 +1642,22 @@ def build_container_name(

class Onefuzz:
def __init__(
self, config_path: Optional[str] = None, token_path: Optional[str] = None
self,
config_path: Optional[str] = None,
token_path: Optional[str] = None,
client_secret: Optional[str] = None,
) -> None:
self.logger = logging.getLogger("onefuzz")

if client_secret is None:
# If not explicitly provided, check the environment for a user-provided client secret.
client_secret = self._client_secret_from_env()

self._backend = Backend(
config=DEFAULT, config_path=config_path, token_path=token_path
config=DEFAULT,
config_path=config_path,
token_path=token_path,
client_secret=client_secret,
)
self.containers = Containers(self)
self.repro = Repro(self)
Expand All @@ -1670,6 +1684,12 @@ def __init__(

self.__setup__()

# Try to obtain a confidential client secret from the environment.
#
# If not set, return `None`.
def _client_secret_from_env(self) -> Optional[str]:
return os.environ.get(CLIENT_SECRET_ENV_VAR)

def __setup__(
self,
endpoint: Optional[str] = None,
Expand All @@ -1686,7 +1706,7 @@ def __setup__(
if client_id is not None:
self._backend.config.client_id = client_id
if client_secret is not None:
self._backend.config.client_secret = client_secret
self._backend.client_secret = client_secret
if tenant_domain is not None:
self._backend.config.tenant_domain = tenant_domain

Expand Down Expand Up @@ -1730,7 +1750,6 @@ def config(
endpoint: Optional[str] = None,
authority: Optional[str] = None,
client_id: Optional[str] = None,
client_secret: Optional[str] = None,
enable_feature: Optional[PreviewFeature] = None,
tenant_domain: Optional[str] = None,
reset: Optional[bool] = None,
Expand Down Expand Up @@ -1759,8 +1778,6 @@ def config(
self._backend.config.authority = authority
if client_id is not None:
self._backend.config.client_id = client_id
if client_secret is not None:
self._backend.config.client_secret = client_secret
if enable_feature:
self._backend.enable_feature(enable_feature.name)
if tenant_domain is not None:
Expand All @@ -1769,9 +1786,6 @@ def config(
self._backend.save_config()

data = self._backend.config.copy(deep=True)
if data.client_secret is not None:
# replace existing secrets with "*** for user display
data.client_secret = "***" # nosec

if not data.endpoint:
self.logger.warning("endpoint not configured yet")
Expand Down
12 changes: 7 additions & 5 deletions src/cli/onefuzz/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def check_application_error(response: requests.Response) -> None:
class BackendConfig(BaseModel):
authority: str
client_id: str
client_secret: Optional[str]
endpoint: Optional[str]
features: Set[str] = Field(default_factory=set)
tenant_domain: Optional[str]
Expand All @@ -100,9 +99,11 @@ def __init__(
config: BackendConfig,
config_path: Optional[str] = None,
token_path: Optional[str] = None,
client_secret: Optional[str] = None,
):
self.config_path = os.path.expanduser(config_path or DEFAULT_CONFIG_PATH)
self.token_path = os.path.expanduser(token_path or DEFAULT_TOKEN_PATH)
self.client_secret = client_secret
self.config = config
self.token_cache: Optional[msal.SerializableTokenCache] = None
self.init_cache()
Expand Down Expand Up @@ -187,16 +188,17 @@ def get_access_token(self) -> Any:
f"https://{netloc}/.default", # before 3.0.0 release
]

if self.config.client_secret:
return self.client_secret(scopes)
if self.client_secret:
return self.access_token_from_client_secret(scopes)

return self.device_login(scopes)

def client_secret(self, scopes: List[str]) -> Any:
def access_token_from_client_secret(self, scopes: List[str]) -> Any:
if not self.app:
self.app = msal.ConfidentialClientApplication(
self.config.client_id,
authority=self.config.authority,
client_credential=self.config.client_secret,
client_credential=self.client_secret,
token_cache=self.token_cache,
)

Expand Down
1 change: 1 addition & 0 deletions src/cli/onefuzz/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
)


# Call `Onefuzz.setup()`, which enables overriding configuration and authentication parameters.
def call_setup(api: Any, args: argparse.Namespace) -> None:
setup = getattr(api, "__setup__", None)
if setup is None:
Expand Down