From bf671751f0df51bb546c2143ebe0917eb6265f48 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Tue, 24 Jan 2023 23:41:37 +0000 Subject: [PATCH 01/29] Config Refactor Round 2. --- src/ApiService/ApiService/Functions/Config.cs | 35 +++++ .../ApiService/OneFuzzTypes/Model.cs | 5 +- .../ApiService/OneFuzzTypes/Responses.cs | 6 + .../ApiService/onefuzzlib/ConfigOperations.cs | 2 +- src/cli/onefuzz/api.py | 34 +++++ src/cli/onefuzz/backend.py | 6 +- src/deployment/bicep-templates/function.bicep | 14 +- src/deployment/config.json | 11 +- src/deployment/deploy.py | 120 +++++++++--------- src/deployment/deploylib/configuration.py | 97 +++++++++++++- .../deploylib/tests/test_deploy_config.py | 26 ++-- src/pytypes/onefuzztypes/models.py | 3 + src/pytypes/onefuzztypes/responses.py | 6 + 13 files changed, 282 insertions(+), 83 deletions(-) create mode 100644 src/ApiService/ApiService/Functions/Config.cs diff --git a/src/ApiService/ApiService/Functions/Config.cs b/src/ApiService/ApiService/Functions/Config.cs new file mode 100644 index 0000000000..f7dc67b029 --- /dev/null +++ b/src/ApiService/ApiService/Functions/Config.cs @@ -0,0 +1,35 @@ +using System.Net; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Azure.Functions.Worker.Http; + +namespace Microsoft.OneFuzz.Service.Functions; + +public class Config { + private readonly ILogTracer _log; + private readonly IOnefuzzContext _context; + + public Config(ILogTracer log, IOnefuzzContext context) { + _log = log; + _context = context; + } + + [Function("Config")] + public Async.Task Run( + [HttpTrigger(AuthorizationLevel.Anonymous, "GET", Route = "config")] HttpRequestData req) { + return Get(req); + } + public async Async.Task Get(HttpRequestData req) { + _log.Info($"getting endpoint config parameters"); + var config = await _context.ConfigOperations.Fetch(); + + var endpointParams = new ConfigResponse( + Authority: config.Authority, + ClientId: config.ClientId, + TenantDomain: config.TenantDomain); + + var response = req.CreateResponse(HttpStatusCode.OK); + await response.WriteAsJsonAsync(endpointParams); + + return response; + } +} \ No newline at end of file diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index 0ce97b28d1..e5c68001d9 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -347,7 +347,10 @@ public record InstanceConfig IDictionary? ApiAccessRules = null, IDictionary? GroupMembership = null, IDictionary? VmTags = null, - IDictionary? VmssTags = null + IDictionary? VmssTags = null, + string? Authority = "", + string? ClientId = "", + string? TenantDomain = "" ) : EntityBase() { public InstanceConfig(string instanceName) : this( instanceName, diff --git a/src/ApiService/ApiService/OneFuzzTypes/Responses.cs b/src/ApiService/ApiService/OneFuzzTypes/Responses.cs index b5391f7ad4..39f5f9fd2c 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Responses.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Responses.cs @@ -159,6 +159,12 @@ public static ScalesetResponse ForScaleset(Scaleset s, bool includeAuth) Nodes: null); } +public record ConfigResponse( + string? Authority, + string? ClientId, + string? TenantDomain +) : BaseResponse(); + public class BaseResponseConverter : JsonConverter { public override BaseResponse? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { return null; diff --git a/src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs b/src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs index 39e3f23c1f..f139be1f10 100644 --- a/src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs @@ -26,7 +26,7 @@ private sealed record InstanceConfigCacheKey(); private static readonly InstanceConfigCacheKey _key = new(); // singleton key public Task Fetch() => _cache.GetOrCreateAsync(_key, async entry => { - entry = entry.SetAbsoluteExpiration(TimeSpan.FromMinutes(10)); // cached for 10 minutes + entry = entry.SetAbsoluteExpiration(TimeSpan.FromMinutes(1)); // cached for 1 minute var key = _context.ServiceConfiguration.OneFuzzInstanceName ?? throw new Exception("Environment variable ONEFUZZ_INSTANCE_NAME is not set"); return await GetEntityAsync(key, key); }); diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index a7b579c57f..3a16ef2183 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -122,6 +122,10 @@ def _req_model( as_params: bool = False, alternate_endpoint: Optional[str] = None, ) -> A: + + # Retrieve Auth Parameters + self._req_config_params() + response = self._req_base( method, data=data, @@ -153,6 +157,36 @@ def _req_model_list( return [model.parse_obj(x) for x in response] + def _req_config_params( + self, + ) -> None: + + endpoint_params = responses.Config( + authority="", + client_id="", + tenant_domain="", + ) + + if self.onefuzz._backend.config.endpoint is not None: + + endpoint = self.onefuzz._backend.config.endpoint + + response = self.onefuzz._backend.session.request( + "GET", endpoint + "/api/config" + ) + + endpoint_params = responses.Config.parse_obj(response.json()) + + if self.onefuzz._backend.config.client_id == "": + self.onefuzz._backend.config.client_id = endpoint_params.client_id + + self.onefuzz._backend.config.authority = endpoint_params.authority + self.onefuzz._backend.config.tenant_domain = endpoint_params.tenant_domain + + self.onefuzz._backend.save_config() + else: + raise Exception("Endpoint Not Configured") + def _disambiguate( self, name: str, diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index dd7b3a962c..749df1e34f 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -177,11 +177,15 @@ def headers(self) -> Dict[str, str]: ) return value + # Can this be generic? def get_access_token(self) -> Any: if not self.config.endpoint: raise Exception("endpoint not configured") - if self.config.tenant_domain: + if ( + self.config.tenant_domain + and "azurewebsites" not in self.config.tenant_domain + ): endpoint = urlparse(self.config.endpoint).netloc.split(".")[0] scopes = [ f"api://{self.config.tenant_domain}/{endpoint}/.default", diff --git a/src/deployment/bicep-templates/function.bicep b/src/deployment/bicep-templates/function.bicep index 617e8366ea..a6e695ffbf 100644 --- a/src/deployment/bicep-templates/function.bicep +++ b/src/deployment/bicep-templates/function.bicep @@ -23,7 +23,6 @@ param diagnostics_log_level string param log_retention int param linux_fx_version string - var siteconfig = (use_windows) ? { } : { linuxFxVersion: linux_fx_version @@ -57,17 +56,17 @@ resource function 'Microsoft.Web/sites@2021-03-01' = { type: 'SystemAssigned' } properties: union({ - siteConfig: union(siteconfig, commonSiteConfig) - httpsOnly: true - serverFarmId: server_farm_id - clientAffinityEnabled: true - }, extraProperties) + siteConfig: union(siteconfig, commonSiteConfig) + httpsOnly: true + serverFarmId: server_farm_id + clientAffinityEnabled: true + }, extraProperties) } resource funcAuthSettings 'Microsoft.Web/sites/config@2021-03-01' = { name: 'authsettingsV2' properties: { - login:{ + login: { tokenStore: { enabled: true } @@ -75,6 +74,7 @@ resource funcAuthSettings 'Microsoft.Web/sites/config@2021-03-01' = { globalValidation: { unauthenticatedClientAction: 'RedirectToLoginPage' requireAuthentication: true + excludedPaths: [ '/api/config' ] } httpSettings: { requireHttps: true diff --git a/src/deployment/config.json b/src/deployment/config.json index 3b138fa5f5..c02ccc67d8 100644 --- a/src/deployment/config.json +++ b/src/deployment/config.json @@ -1,6 +1,15 @@ { + "tenant_id": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "tenant_domain": "azurewebsites.net", + "multi_tenant_domain": "", + "cli_client_id": "72f1562a-8c0c-41ea-beb9-fa2b71c80134", "proxy_nsg_config": { - "allowed_ips": ["*"], + "allowed_ips": [ + "*" + ], + "allowed_ips": [ + "*" + ], "allowed_service_tags": [] } } \ No newline at end of file diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index cbeef8762c..6205e8b092 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -44,11 +44,13 @@ from deploylib.configuration import ( InstanceConfigClient, - NetworkSecurityConfig, + Config, parse_rules, update_admins, update_allowed_aad_tenants, + update_endpoint_params, update_nsg, + NsgRule, ) from deploylib.data_migration import migrate from deploylib.registration import ( @@ -74,10 +76,10 @@ USER_READ_PERMISSION = "e1fe6dd8-ba31-4d61-89e7-88639da4683d" MICROSOFT_GRAPH_APP_ID = "00000003-0000-0000-c000-000000000000" -ONEFUZZ_CLI_AUTHORITY = ( - "https://login.microsoftonline.com/72f988bf-86f1-41af-91ab-2d7cd011db47" -) -COMMON_AUTHORITY = "https://login.microsoftonline.com/common" +# ONEFUZZ_CLI_AUTHORITY = ( +# "https://login.microsoftonline.com/72f988bf-86f1-41af-91ab-2d7cd011db47" +# ) +# COMMON_AUTHORITY = "https://login.microsoftonline.com/common" TELEMETRY_NOTICE = ( "Telemetry collection on stats and OneFuzz failures are sent to Microsoft. " "To disable, delete the ONEFUZZ_TELEMETRY application setting in the " @@ -139,7 +141,7 @@ def __init__( location: str, application_name: str, owner: str, - nsg_config: str, + config: str, client_id: Optional[str], client_secret: Optional[str], app_zip: str, @@ -167,7 +169,7 @@ def __init__( self.location = location self.application_name = application_name self.owner = owner - self.nsg_config = nsg_config + self.config = config self.app_zip = app_zip self.tools = tools self.instance_specific = instance_specific @@ -180,10 +182,7 @@ def __init__( "client_id": client_id, "client_secret": client_secret, } - if self.multi_tenant_domain: - authority = COMMON_AUTHORITY - else: - authority = ONEFUZZ_CLI_AUTHORITY + self.authority = "https://login.microsoftonline.com/" + self.tenant_id self.migrations = migrations self.export_appinsights = export_appinsights self.admins = admins @@ -196,9 +195,15 @@ def __init__( self.host_dotnet_on_windows = host_dotnet_on_windows self.enable_profiler = enable_profiler + self.rules: List[NsgRule] = [] + + self.tenant_id = "" + self.tenant_domain = "" + self.authority = "" + self.cli_config: Dict[str, Union[str, UUID]] = { - "client_id": self.cli_app_id, - "authority": authority, + "client_id": "", + "authority": "", } machine = platform.machine() @@ -305,7 +310,7 @@ def get_instance_url(self) -> str: # The url to access the instance # This also represents the legacy identifier_uris of the application # registration - if self.multi_tenant_domain: + if self.multi_tenant_domain != "": return "https://%s/%s" % (self.multi_tenant_domain, self.application_name) else: return "https://%s.azurewebsites.net" % self.application_name @@ -316,14 +321,14 @@ def get_identifier_url(self) -> str: # to be from an approved domain The format of this value is derived # from the default value proposed by azure when creating an application # registration api://{guid}/... - if self.multi_tenant_domain: + if self.multi_tenant_domain != "": return "api://%s/%s" % (self.multi_tenant_domain, self.application_name) else: return "api://%s.azurewebsites.net" % self.application_name def get_signin_audience(self) -> str: # https://docs.microsoft.com/en-us/azure/active-directory/develop/supported-accounts-validation - if self.multi_tenant_domain: + if self.multi_tenant_domain != "": return "AzureADMultipleOrgs" else: return "AzureADMyOrg" @@ -382,7 +387,7 @@ def setup_rbac(self) -> None: else: self.update_existing_app_registration(app, app_roles) - if self.multi_tenant_domain and app["signInAudience"] == "AzureADMyOrg": + if self.multi_tenant_domain != "" and app["signInAudience"] == "AzureADMyOrg": set_app_audience( app["id"], "AzureADMultipleOrgs", @@ -419,13 +424,10 @@ def setup_rbac(self) -> None: OnefuzzAppRole.CliClient, self.get_subscription_id(), ) - if self.multi_tenant_domain: - authority = COMMON_AUTHORITY - else: - authority = app_info.authority + self.cli_config = { "client_id": app_info.client_id, - "authority": authority, + "authority": self.authority, } else: logger.error( @@ -437,14 +439,10 @@ def setup_rbac(self) -> None: else: onefuzz_cli_app = cli_app authorize_application(uuid.UUID(onefuzz_cli_app["appId"]), app["appId"]) - if self.multi_tenant_domain: - authority = COMMON_AUTHORITY - else: - tenant_id = get_tenant_id(self.get_subscription_id()) - authority = "https://login.microsoftonline.com/%s" % tenant_id + self.cli_config = { "client_id": onefuzz_cli_app["appId"], - "authority": authority, + "authority": self.authority, } # ensure replyURLs is set properly @@ -641,7 +639,7 @@ def deploy_template(self) -> None: # Add --custom_domain value to Allowed token audiences setting if self.custom_domain: - if self.multi_tenant_domain: + if self.multi_tenant_domain != "": root_domain = self.multi_tenant_domain else: root_domain = "%s.azurewebsites.net" % self.application_name @@ -653,7 +651,7 @@ def deploy_template(self) -> None: app_func_audiences.extend(custom_domains) - if self.multi_tenant_domain: + if self.multi_tenant_domain != "": # clear the value in the Issuer Url field: # https://docs.microsoft.com/en-us/sharepoint/dev/spfx/use-aadhttpclient-enterpriseapi-multitenant app_func_issuer = "" @@ -778,35 +776,49 @@ def apply_migrations(self) -> None: table_service = TableService(account_name=name, account_key=key) migrate(table_service, self.migrations) - def set_instance_config(self) -> None: - logger.info("setting instance config") - name = self.results["deploy"]["func_name"]["value"] - key = self.results["deploy"]["func_key"]["value"] - tenant = UUID(self.results["deploy"]["tenant_id"]["value"]) - table_service = TableService(account_name=name, account_key=key) - - config_client = InstanceConfigClient(table_service, self.application_name) + def parse_config(self) -> None: + logger.info("parsing config: %s", self.config) - if self.nsg_config: - logger.info("deploying arm template: %s", self.nsg_config) + if self.config: - with open(self.nsg_config, "r") as template_handle: + with open(self.config, "r") as template_handle: config_template = json.load(template_handle) try: - config = NetworkSecurityConfig(config_template) - rules = parse_rules(config) + config = Config(config_template) + self.rules = parse_rules(config) + + ## Override any input values in favor of config values + self.authority = "https://login.microsoftonline.com/" + config.tenant_id + self.tenant_domain = config.tenant_domain + self.multi_tenant_domain = config.multi_tenant_domain + self.cli_app_id = config.cli_client_id + except Exception as ex: logging.info( - "An Exception was encountered while parsing nsg_config file: %s", ex + "An Exception was encountered while parsing config file: %s", ex ) raise Exception( - "proxy_nsg_config and sub-values were not properly included in config." - + "Please submit a configuration resembling" - + " { 'proxy_nsg_config': { 'allowed_ips': [], 'allowed_service_tags': [] } }" + "config and sub-values were not properly included in config." ) - update_nsg(config_client, rules) + def set_instance_config(self) -> None: + logger.info("setting instance config") + name = self.results["deploy"]["func_name"]["value"] + key = self.results["deploy"]["func_key"]["value"] + tenant = UUID(self.results["deploy"]["tenant_id"]["value"]) + table_service = TableService(account_name=name, account_key=key) + + config_client = InstanceConfigClient(table_service, self.application_name) + + update_nsg(config_client, self.rules) + + update_endpoint_params( + config_client, + self.authority, + self.cli_app_id, + self.tenant_domain, + ) if self.admins: update_admins(config_client, self.admins) @@ -1136,18 +1148,11 @@ def done(self) -> None: "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: - cmd += ["--tenant_domain", str(self.multi_tenant_domain)] - as_str = " ".join(cmd) logger.info(f"Update your CLI config via: {as_str}") @@ -1174,6 +1179,7 @@ def lower_case(arg: str) -> str: def main() -> None: rbac_only_states = [ + ("parse_config", Client.parse_config), ("check_region", Client.check_region), ("rbac", Client.setup_rbac), ("eventgrid", Client.remove_eventgrid), @@ -1200,7 +1206,7 @@ def main() -> None: parser.add_argument("resource_group") parser.add_argument("application_name", type=lower_case) parser.add_argument("owner") - parser.add_argument("nsg_config") + parser.add_argument("config") parser.add_argument( "--bicep-template", type=arg_file, @@ -1337,7 +1343,7 @@ def main() -> None: location=args.location, application_name=args.application_name, owner=args.owner, - nsg_config=args.nsg_config, + config=args.config, client_id=args.client_id, client_secret=args.client_secret, app_zip=args.app_zip, diff --git a/src/deployment/deploylib/configuration.py b/src/deployment/deploylib/configuration.py index dd1bcacd20..93f3163efb 100644 --- a/src/deployment/deploylib/configuration.py +++ b/src/deployment/deploylib/configuration.py @@ -48,12 +48,17 @@ def create_if_missing(self, table_service: TableService) -> None: self.enable_storage_client_logging() -class NetworkSecurityConfig: +class Config: + cli_client_id: str + tenant_id: str + tenant_domain: str + multi_tenant_domain: str allowed_ips: List[str] allowed_service_tags: List[str] def __init__(self, config: Any): self.parse_nsg_json(config) + self.parse_endpoint_json(config) def parse_nsg_json(self, config: Any) -> None: if not isinstance(config, Dict): @@ -107,6 +112,59 @@ def parse_nsg_json(self, config: Any) -> None: self.allowed_ips = proxy_config["allowed_ips"] self.allowed_service_tags = proxy_config["allowed_service_tags"] + def parse_endpoint_json(self, config: Any) -> None: + + if "cli_client_id" not in config: + raise Exception( + "CLI client_id not provided as valid key. Please Provide Valid Config." + ) + + if ( + not isinstance(config["cli_client_id"], str) + or config["cli_client_id"] == "" + ): + raise Exception( + "client_id is not a string. Please Provide Valid client_id." + ) + + if "tenant_id" not in config: + raise Exception( + "tenant_id not provided as valid key. Please Provide Valid Config." + ) + + if not isinstance(config["tenant_id"], str) or config["tenant_id"] == "": + raise Exception( + "tenant_id is not a string. Please Provide Valid tenant_id." + ) + + if "tenant_domain" not in config: + raise Exception( + "tenant_domain not provided as valid key. Please Provide Valid Config." + ) + + if ( + not isinstance(config["tenant_domain"], str) + or config["tenant_domain"] == "" + ): + raise Exception( + "tenant_domain is not a string. Please Provide Valid tenant_domain." + ) + + if "multi_tenant_domain" not in config: + raise Exception( + "multi_tenant_domain not provided as valid key. Please Provide Valid Config." + ) + + if not isinstance(config["multi_tenant_domain"], str): + raise Exception( + "multi_tenant_domain is not a string. Please Provide Valid multi_tenant_domain. If the instance is not multi-tenant, please provide an empty string." + ) + + self.cli_client_id = config["cli_client_id"] + self.tenant_id = config["tenant_id"] + self.tenant_domain = config["tenant_domain"] + self.multi_tenant_domain = config["multi_tenant_domain"] + class NsgRule: rule: str @@ -175,7 +233,7 @@ def update_admins(config_client: InstanceConfigClient, admins: List[UUID]) -> No ) -def parse_rules(proxy_config: NetworkSecurityConfig) -> List[NsgRule]: +def parse_rules(proxy_config: Config) -> List[NsgRule]: allowed_ips = proxy_config.allowed_ips allowed_service_tags = proxy_config.allowed_service_tags @@ -225,3 +283,38 @@ def update_nsg( "proxy_nsg_config": json.dumps(nsg_config), }, ) + + +def update_endpoint_params( + config_client: InstanceConfigClient, + authority: str, + client_id: str, + tenant_domain: str, +) -> None: + + config_client.table_service.insert_or_merge_entity( + TABLE_NAME, + { + "PartitionKey": config_client.resource_group, + "RowKey": config_client.resource_group, + "authority": authority, + }, + ) + + config_client.table_service.insert_or_merge_entity( + TABLE_NAME, + { + "PartitionKey": config_client.resource_group, + "RowKey": config_client.resource_group, + "client_id": client_id, + }, + ) + + config_client.table_service.insert_or_merge_entity( + TABLE_NAME, + { + "PartitionKey": config_client.resource_group, + "RowKey": config_client.resource_group, + "tenant_domain": tenant_domain, + }, + ) diff --git a/src/deployment/deploylib/tests/test_deploy_config.py b/src/deployment/deploylib/tests/test_deploy_config.py index dc0bdf8521..aba328ef54 100644 --- a/src/deployment/deploylib/tests/test_deploy_config.py +++ b/src/deployment/deploylib/tests/test_deploy_config.py @@ -6,7 +6,7 @@ import unittest from typing import Any -from deploylib.configuration import NetworkSecurityConfig +from deploylib.configuration import Config class DeployTests(unittest.TestCase): @@ -15,33 +15,33 @@ def test_config(self) -> None: # Test Dictionary invalid_config: Any = "" with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test Empty Dic invalid_config = {} with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test Invalid Outer Keys invalid_config = {"": ""} with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test Inner Dictionary invalid_config = {"proxy_nsg_config": ""} with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test Inner Keys invalid_config = {"proxy_nsg_config": {}} with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test Inner Keys invalid_config = {"proxy_nsg_config": {"allowed_ips": ""}} with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test Inner Dict Values (lists) invalid_config = { "proxy_nsg_config": {"allowed_ips": [], "allowed_service_tags": ""} } with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) # Test List Values invalid_config = { "proxy_nsg_config": { @@ -50,19 +50,19 @@ def test_config(self) -> None: } } with self.assertRaises(Exception): - NetworkSecurityConfig(invalid_config) + Config(invalid_config) ## Test Valid Configs # Test Empty Lists valid_config: Any = { "proxy_nsg_config": {"allowed_ips": [], "allowed_service_tags": []} } - NetworkSecurityConfig(valid_config) + Config(valid_config) # Test Wild Card Lists valid_config = { "proxy_nsg_config": {"allowed_ips": ["*"], "allowed_service_tags": []} } - NetworkSecurityConfig(valid_config) + Config(valid_config) # Test IPs Lists valid_config = { "proxy_nsg_config": { @@ -70,7 +70,7 @@ def test_config(self) -> None: "allowed_service_tags": [], } } - NetworkSecurityConfig(valid_config) + Config(valid_config) # Test Tags Lists valid_config = { "proxy_nsg_config": { @@ -78,7 +78,7 @@ def test_config(self) -> None: "allowed_service_tags": ["Internet"], } } - NetworkSecurityConfig(valid_config) + Config(valid_config) if __name__ == "__main__": diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index 60fa68cc12..a450afa222 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -901,6 +901,9 @@ class InstanceConfig(BaseModel): group_membership: Optional[Dict[PrincipalID, List[GroupId]]] = None vm_tags: Optional[Dict[str, str]] = None vmss_tags: Optional[Dict[str, str]] = None + authority: Optional[str] = Field(default="") + client_id: Optional[str] = Field(default="") + tenant_domain: Optional[str] = Field(default="") def update(self, config: "InstanceConfig") -> None: for field in config.__fields__: diff --git a/src/pytypes/onefuzztypes/responses.py b/src/pytypes/onefuzztypes/responses.py index 24f62151b1..7926deb2dc 100644 --- a/src/pytypes/onefuzztypes/responses.py +++ b/src/pytypes/onefuzztypes/responses.py @@ -52,6 +52,12 @@ class Info(BaseResponse): insights_instrumentation_key: Optional[str] +class Config(BaseResponse): + authority: str + client_id: str + tenant_domain: str + + class ContainerInfoBase(BaseResponse): name: str metadata: Optional[Dict[str, str]] From 4b1a64e2099e9beaf49914f1874784deb7684ba2 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Tue, 24 Jan 2023 23:56:17 +0000 Subject: [PATCH 02/29] Adding docs. --- docs/webhook_events.md | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/webhook_events.md b/docs/webhook_events.md index cdb7b631db..0ce5dadefc 100644 --- a/docs/webhook_events.md +++ b/docs/webhook_events.md @@ -696,6 +696,8 @@ If webhook is set to have Event Grid message format then the payload will look a "allowed_aad_tenants": [ "00000000-0000-0000-0000-000000000000" ], + "authority": "", + "client_id": "", "default_linux_vm_image": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest", "default_windows_vm_image": "MicrosoftWindowsDesktop:Windows-10:win10-21h2-pro:latest", "network_config": { @@ -707,7 +709,8 @@ If webhook is set to have Event Grid message format then the payload will look a "allowed_service_tags": [] }, "proxy_vm_sku": "Standard_B2s", - "require_admin_privileges": false + "require_admin_privileges": false, + "tenant_domain": "" } } ``` @@ -838,6 +841,16 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Api Access Rules", "type": "object" }, + "authority": { + "default": "", + "title": "Authority", + "type": "string" + }, + "client_id": { + "default": "", + "title": "Client Id", + "type": "string" + }, "default_linux_vm_image": { "default": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest", "title": "Default Linux Vm Image", @@ -878,6 +891,11 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Require Admin Privileges", "type": "boolean" }, + "tenant_domain": { + "default": "", + "title": "Tenant Domain", + "type": "string" + }, "vm_tags": { "additionalProperties": { "type": "string" @@ -6158,6 +6176,16 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Api Access Rules", "type": "object" }, + "authority": { + "default": "", + "title": "Authority", + "type": "string" + }, + "client_id": { + "default": "", + "title": "Client Id", + "type": "string" + }, "default_linux_vm_image": { "default": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest", "title": "Default Linux Vm Image", @@ -6198,6 +6226,11 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Require Admin Privileges", "type": "boolean" }, + "tenant_domain": { + "default": "", + "title": "Tenant Domain", + "type": "string" + }, "vm_tags": { "additionalProperties": { "type": "string" From fc1a86ce6f23b6d427e9f6dad7b83c267573f373 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 00:04:17 +0000 Subject: [PATCH 03/29] Fix file formatting. --- src/ApiService/ApiService/Functions/Config.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ApiService/ApiService/Functions/Config.cs b/src/ApiService/ApiService/Functions/Config.cs index f7dc67b029..416dcc0d5d 100644 --- a/src/ApiService/ApiService/Functions/Config.cs +++ b/src/ApiService/ApiService/Functions/Config.cs @@ -1,4 +1,4 @@ -using System.Net; +using System.Net; using Microsoft.Azure.Functions.Worker; using Microsoft.Azure.Functions.Worker.Http; @@ -32,4 +32,4 @@ public async Async.Task Get(HttpRequestData req) { return response; } -} \ No newline at end of file +} From b8705fc800bcb8ee9df892d7fe40e404e898437c Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 00:16:54 +0000 Subject: [PATCH 04/29] Removing. --- src/deployment/deploy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 6205e8b092..a22384e548 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -182,7 +182,6 @@ def __init__( "client_id": client_id, "client_secret": client_secret, } - self.authority = "https://login.microsoftonline.com/" + self.tenant_id self.migrations = migrations self.export_appinsights = export_appinsights self.admins = admins From 153f0b90a4902b72fe1d734d7e9b6c504382b70a Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 00:32:52 +0000 Subject: [PATCH 05/29] fixing imports. --- src/deployment/deploy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index a22384e548..9d76e9c222 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -43,14 +43,14 @@ from msrest.serialization import TZ_UTC from deploylib.configuration import ( - InstanceConfigClient, Config, + InstanceConfigClient, + NsgRule, parse_rules, update_admins, update_allowed_aad_tenants, update_endpoint_params, update_nsg, - NsgRule, ) from deploylib.data_migration import migrate from deploylib.registration import ( From ec4a2fc1700b74ef98cfa57625dc94b01dbad44c Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 00:45:04 +0000 Subject: [PATCH 06/29] Removing. --- src/deployment/deploy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 9d76e9c222..599352e8d0 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -63,7 +63,6 @@ get_application, get_service_principal, get_signed_in_user, - get_tenant_id, query_microsoft_graph, register_application, set_app_audience, From 16b82e838a2047d892d5ba738680026434816f1c Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 18:08:40 +0000 Subject: [PATCH 07/29] Fixing cli access token retrieval. --- src/cli/onefuzz/api.py | 2 +- src/cli/onefuzz/backend.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index 3a16ef2183..02415ab936 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -125,7 +125,7 @@ def _req_model( # Retrieve Auth Parameters self._req_config_params() - + logging.debug("in req model") response = self._req_base( method, data=data, diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 749df1e34f..28dd2a60e8 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -182,10 +182,7 @@ def get_access_token(self) -> Any: if not self.config.endpoint: raise Exception("endpoint not configured") - if ( - self.config.tenant_domain - and "azurewebsites" not in self.config.tenant_domain - ): + if self.config.tenant_domain == "common": endpoint = urlparse(self.config.endpoint).netloc.split(".")[0] scopes = [ f"api://{self.config.tenant_domain}/{endpoint}/.default", From ae9acaa1cbe000cf5bc6c2d5271404481f08d233 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 18:46:52 +0000 Subject: [PATCH 08/29] Fixing authority check. --- src/cli/onefuzz/api.py | 2 ++ src/cli/onefuzz/backend.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index 02415ab936..eaf331cc35 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -177,9 +177,11 @@ def _req_config_params( endpoint_params = responses.Config.parse_obj(response.json()) + # Will override client id in storage w/ provided client_id for SP use if self.onefuzz._backend.config.client_id == "": self.onefuzz._backend.config.client_id = endpoint_params.client_id + # Ignores provided auth and tenant_domain in favor of what's in storage self.onefuzz._backend.config.authority = endpoint_params.authority self.onefuzz._backend.config.tenant_domain = endpoint_params.tenant_domain diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 28dd2a60e8..8858f18301 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -182,7 +182,7 @@ def get_access_token(self) -> Any: if not self.config.endpoint: raise Exception("endpoint not configured") - if self.config.tenant_domain == "common": + if "common" in self.config.authority: endpoint = urlparse(self.config.endpoint).netloc.split(".")[0] scopes = [ f"api://{self.config.tenant_domain}/{endpoint}/.default", From ae9b57f79cb9550babca0b39f87a8be975c84bb8 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 19:46:15 +0000 Subject: [PATCH 09/29] Small edits. --- src/cli/onefuzz/backend.py | 1 - src/deployment/deploy.py | 4 ---- src/deployment/deploylib/configuration.py | 14 +++++++------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 8858f18301..ab204fa820 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -177,7 +177,6 @@ def headers(self) -> Dict[str, str]: ) return value - # Can this be generic? def get_access_token(self) -> Any: if not self.config.endpoint: raise Exception("endpoint not configured") diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 599352e8d0..f22dafefad 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -75,10 +75,6 @@ USER_READ_PERMISSION = "e1fe6dd8-ba31-4d61-89e7-88639da4683d" MICROSOFT_GRAPH_APP_ID = "00000003-0000-0000-c000-000000000000" -# ONEFUZZ_CLI_AUTHORITY = ( -# "https://login.microsoftonline.com/72f988bf-86f1-41af-91ab-2d7cd011db47" -# ) -# COMMON_AUTHORITY = "https://login.microsoftonline.com/common" TELEMETRY_NOTICE = ( "Telemetry collection on stats and OneFuzz failures are sent to Microsoft. " "To disable, delete the ONEFUZZ_TELEMETRY application setting in the " diff --git a/src/deployment/deploylib/configuration.py b/src/deployment/deploylib/configuration.py index 93f3163efb..d8a8bfab6b 100644 --- a/src/deployment/deploylib/configuration.py +++ b/src/deployment/deploylib/configuration.py @@ -124,22 +124,22 @@ def parse_endpoint_json(self, config: Any) -> None: or config["cli_client_id"] == "" ): raise Exception( - "client_id is not a string. Please Provide Valid client_id." + "client_id is not a string. Please provide valid client_id." ) if "tenant_id" not in config: raise Exception( - "tenant_id not provided as valid key. Please Provide Valid Config." + "tenant_id not provided as valid key. Please provide valid config." ) if not isinstance(config["tenant_id"], str) or config["tenant_id"] == "": raise Exception( - "tenant_id is not a string. Please Provide Valid tenant_id." + "tenant_id is not a string. Please provide valid tenant_id." ) if "tenant_domain" not in config: raise Exception( - "tenant_domain not provided as valid key. Please Provide Valid Config." + "tenant_domain not provided as valid key. Please provide valid config." ) if ( @@ -147,17 +147,17 @@ def parse_endpoint_json(self, config: Any) -> None: or config["tenant_domain"] == "" ): raise Exception( - "tenant_domain is not a string. Please Provide Valid tenant_domain." + "tenant_domain is not a string. Please provide valid tenant_domain." ) if "multi_tenant_domain" not in config: raise Exception( - "multi_tenant_domain not provided as valid key. Please Provide Valid Config." + "multi_tenant_domain not provided as valid key. Please provide valid config. If the instance is not multi-tenant, please provide an empty string." ) if not isinstance(config["multi_tenant_domain"], str): raise Exception( - "multi_tenant_domain is not a string. Please Provide Valid multi_tenant_domain. If the instance is not multi-tenant, please provide an empty string." + "multi_tenant_domain is not a string. Please provide valid multi_tenant_domain. If the instance is not multi-tenant, please provide an empty string." ) self.cli_client_id = config["cli_client_id"] From e7b9c5a9ef03f7a557bfb57a2cb17a96d8915179 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 20:26:46 +0000 Subject: [PATCH 10/29] Removing duplicate. --- src/deployment/config.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/deployment/config.json b/src/deployment/config.json index c02ccc67d8..c041d2b8d6 100644 --- a/src/deployment/config.json +++ b/src/deployment/config.json @@ -4,9 +4,6 @@ "multi_tenant_domain": "", "cli_client_id": "72f1562a-8c0c-41ea-beb9-fa2b71c80134", "proxy_nsg_config": { - "allowed_ips": [ - "*" - ], "allowed_ips": [ "*" ], From 60736897e87c47bb29855abcf80d2e330759df3b Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 20:33:50 +0000 Subject: [PATCH 11/29] Adding uuid check. --- src/deployment/deploylib/configuration.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/deployment/deploylib/configuration.py b/src/deployment/deploylib/configuration.py index d8a8bfab6b..089af682b2 100644 --- a/src/deployment/deploylib/configuration.py +++ b/src/deployment/deploylib/configuration.py @@ -127,6 +127,13 @@ def parse_endpoint_json(self, config: Any) -> None: "client_id is not a string. Please provide valid client_id." ) + try: + UUID(config["cli_client_id"]) + except ValueError: + raise Exception( + "client_id is not a valid UUID. Please provide valid client_id." + ) + if "tenant_id" not in config: raise Exception( "tenant_id not provided as valid key. Please provide valid config." From 733840c0755e8e5fbb2623cda1e3f7f8451f9403 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Wed, 25 Jan 2023 22:01:06 +0000 Subject: [PATCH 12/29] Possible to override with existing params. --- src/deployment/deploy.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index f22dafefad..3cfa3dfcac 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -783,10 +783,16 @@ def parse_config(self) -> None: self.rules = parse_rules(config) ## Override any input values in favor of config values - self.authority = "https://login.microsoftonline.com/" + config.tenant_id - self.tenant_domain = config.tenant_domain - self.multi_tenant_domain = config.multi_tenant_domain - self.cli_app_id = config.cli_client_id + if self.authority == "": + self.authority = ( + "https://login.microsoftonline.com/" + config.tenant_id + ) + if self.tenant_domain == "": + self.tenant_domain = config.tenant_domain + if self.multi_tenant_domain is None: + self.multi_tenant_domain = config.multi_tenant_domain + if self.cli_app_id is None: + self.cli_app_id = config.cli_client_id except Exception as ex: logging.info( From 72669a68ad8ba016d2d3786b7aa0dd375f6b35f7 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 17:30:01 +0000 Subject: [PATCH 13/29] Allowing flags to override storage. --- src/cli/onefuzz/api.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index eaf331cc35..113762c232 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -125,7 +125,7 @@ def _req_model( # Retrieve Auth Parameters self._req_config_params() - logging.debug("in req model") + response = self._req_base( method, data=data, @@ -177,13 +177,13 @@ def _req_config_params( endpoint_params = responses.Config.parse_obj(response.json()) - # Will override client id in storage w/ provided client_id for SP use + # Will override values in storage w/ provided values for SP use if self.onefuzz._backend.config.client_id == "": self.onefuzz._backend.config.client_id = endpoint_params.client_id - - # Ignores provided auth and tenant_domain in favor of what's in storage - self.onefuzz._backend.config.authority = endpoint_params.authority - self.onefuzz._backend.config.tenant_domain = endpoint_params.tenant_domain + if self.onefuzz._backend.config.authority == "": + self.onefuzz._backend.config.authority = endpoint_params.authority + if self.onefuzz._backend.config.tenant_domain == "": + self.onefuzz._backend.config.tenant_domain = endpoint_params.tenant_domain self.onefuzz._backend.save_config() else: From 22f40fc4846731d062f555826fc7c50a06b7116c Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 17:45:56 +0000 Subject: [PATCH 14/29] Trying to fix config params.? --- src/cli/onefuzz/backend.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index ab204fa820..a9fd867441 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -321,6 +321,7 @@ def request( if not endpoint: raise Exception("endpoint not configured") + LOGGER.info(self.headers) url = endpoint + "/api/" + path headers = self.headers() json_data = serialize(json_data) From a83257ee89da41f829c662527ba929bc59fb9d99 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 18:12:08 +0000 Subject: [PATCH 15/29] Fixing. --- src/cli/onefuzz/api.py | 6 ++++-- src/deployment/deploy.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index 113762c232..c1083bc859 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -182,8 +182,10 @@ def _req_config_params( self.onefuzz._backend.config.client_id = endpoint_params.client_id if self.onefuzz._backend.config.authority == "": self.onefuzz._backend.config.authority = endpoint_params.authority - if self.onefuzz._backend.config.tenant_domain == "": - self.onefuzz._backend.config.tenant_domain = endpoint_params.tenant_domain + if not self.onefuzz._backend.config.tenant_domain: + self.onefuzz._backend.config.tenant_domain = ( + endpoint_params.tenant_domain + ) self.onefuzz._backend.save_config() else: diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 3cfa3dfcac..a8f1c28fb8 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -782,7 +782,7 @@ def parse_config(self) -> None: config = Config(config_template) self.rules = parse_rules(config) - ## Override any input values in favor of config values + ## Values provided via the CLI will override what's in the config.json if self.authority == "": self.authority = ( "https://login.microsoftonline.com/" + config.tenant_id From a4eb26de368c6a129484fb581c7358471990c6ee Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:25:49 +0000 Subject: [PATCH 16/29] Set endpoint params via app function. --- src/ApiService/ApiService/Functions/Config.cs | 7 +- .../ApiService/OneFuzzTypes/Model.cs | 6 +- .../ApiService/ServiceConfiguration.cs | 8 ++- src/cli/onefuzz/api.py | 1 + src/deployment/azuredeploy.bicep | 6 ++ .../bicep-templates/function-settings.bicep | 10 ++- src/deployment/deploy.py | 15 +++-- src/deployment/deploylib/configuration.py | 66 +++++++++---------- src/pytypes/onefuzztypes/models.py | 6 +- 9 files changed, 72 insertions(+), 53 deletions(-) diff --git a/src/ApiService/ApiService/Functions/Config.cs b/src/ApiService/ApiService/Functions/Config.cs index 416dcc0d5d..0b8d43d2b8 100644 --- a/src/ApiService/ApiService/Functions/Config.cs +++ b/src/ApiService/ApiService/Functions/Config.cs @@ -20,12 +20,11 @@ public Async.Task Run( } public async Async.Task Get(HttpRequestData req) { _log.Info($"getting endpoint config parameters"); - var config = await _context.ConfigOperations.Fetch(); var endpointParams = new ConfigResponse( - Authority: config.Authority, - ClientId: config.ClientId, - TenantDomain: config.TenantDomain); + Authority: _context.ServiceConfiguration.Authority, + ClientId: _context.ServiceConfiguration.CliAppId, + TenantDomain: _context.ServiceConfiguration.TenantDomain); var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(endpointParams); diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index e5c68001d9..9b1069cfe9 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -348,9 +348,9 @@ public record InstanceConfig IDictionary? GroupMembership = null, IDictionary? VmTags = null, IDictionary? VmssTags = null, - string? Authority = "", - string? ClientId = "", - string? TenantDomain = "" + // string? Authority = "", + // string? ClientId = "", + // string? TenantDomain = "" ) : EntityBase() { public InstanceConfig(string instanceName) : this( instanceName, diff --git a/src/ApiService/ApiService/ServiceConfiguration.cs b/src/ApiService/ApiService/ServiceConfiguration.cs index fb0a308481..ae06d7ad32 100644 --- a/src/ApiService/ApiService/ServiceConfiguration.cs +++ b/src/ApiService/ApiService/ServiceConfiguration.cs @@ -26,7 +26,9 @@ public interface IServiceConfig { public string? DiagnosticsAzureBlobContainerSasUrl { get; } public string? DiagnosticsAzureBlobRetentionDays { get; } - + public string? CliAppId { get; } + public string? Authority { get; } + public string? TenantDomain { get; } public string? MultiTenantDomain { get; } public ResourceIdentifier? OneFuzzDataStorage { get; } public ResourceIdentifier? OneFuzzFuncStorage { get; } @@ -97,7 +99,9 @@ public ServiceConfiguration() { public string? DiagnosticsAzureBlobContainerSasUrl { get => GetEnv("DIAGNOSTICS_AZUREBLOBCONTAINERSASURL"); } public string? DiagnosticsAzureBlobRetentionDays { get => GetEnv("DIAGNOSTICS_AZUREBLOBRETENTIONINDAYS"); } - + public string? CliAppId { get => GetEnv("CLI_APP_ID"); } + public string? Authority { get => GetEnv("AUTHORITY"); } + public string? TenantDomain { get => GetEnv("TenantDomain"); } public string? MultiTenantDomain { get => GetEnv("MULTI_TENANT_DOMAIN"); } public ResourceIdentifier? OneFuzzDataStorage { diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index c1083bc859..f79641e766 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -177,6 +177,7 @@ def _req_config_params( endpoint_params = responses.Config.parse_obj(response.json()) + logging.debug(self.onefuzz._backend.config.authority) # Will override values in storage w/ provided values for SP use if self.onefuzz._backend.config.client_id == "": self.onefuzz._backend.config.client_id = endpoint_params.client_id diff --git a/src/deployment/azuredeploy.bicep b/src/deployment/azuredeploy.bicep index 14123b8182..dd5c1fe8cf 100644 --- a/src/deployment/azuredeploy.bicep +++ b/src/deployment/azuredeploy.bicep @@ -8,6 +8,9 @@ param clientSecret string param signedExpiry string param app_func_issuer string param app_func_audiences array +param cli_app_id string +param authority string +param tenant_domain string param multi_tenant_domain string param enable_remote_debugging bool = false param enable_profiler bool = false @@ -239,6 +242,9 @@ module functionSettings 'bicep-templates/function-settings.bicep' = { fuzz_storage_resource_id: storage.outputs.FuzzId keyvault_name: keyVaultName monitor_account_name: operationalInsights.outputs.monitorAccountName + cli_app_id: cli_app_id + authority: authority + tenant_domain: tenant_domain multi_tenant_domain: multi_tenant_domain enable_profiler: enable_profiler app_config_endpoint: featureFlags.outputs.AppConfigEndpoint diff --git a/src/deployment/bicep-templates/function-settings.bicep b/src/deployment/bicep-templates/function-settings.bicep index 2f65f8d5f4..2ac9d0b1b4 100644 --- a/src/deployment/bicep-templates/function-settings.bicep +++ b/src/deployment/bicep-templates/function-settings.bicep @@ -8,6 +8,9 @@ param app_insights_key string @secure() param func_sas_url string +param cli_app_id string +param authority string +param tenant_domain string param multi_tenant_domain string @secure() @@ -37,7 +40,7 @@ resource function 'Microsoft.Web/sites@2021-02-01' existing = { } var enable_profilers = enable_profiler ? { - APPINSIGHTS_PROFILERFEATURE_VERSION : '1.0.0' + APPINSIGHTS_PROFILERFEATURE_VERSION: '1.0.0' DiagnosticServices_EXTENSION_VERSION: '~3' } : {} @@ -52,6 +55,9 @@ resource functionSettings 'Microsoft.Web/sites/config@2021-03-01' = { APPINSIGHTS_APPID: app_insights_app_id ONEFUZZ_TELEMETRY: telemetry AzureWebJobsStorage: func_sas_url + CLI_APP_ID: cli_app_id + AUTHORIY: authority + TENANT_DOMAIN: tenant_domain MULTI_TENANT_DOMAIN: multi_tenant_domain AzureWebJobsDisableHomepage: 'true' AzureSignalRConnectionString: signal_r_connection_string @@ -66,5 +72,5 @@ resource functionSettings 'Microsoft.Web/sites/config@2021-03-01' = { ONEFUZZ_KEYVAULT: keyvault_name ONEFUZZ_OWNER: owner ONEFUZZ_CLIENT_SECRET: client_secret - }, enable_profilers) + }, enable_profilers) } diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index a8f1c28fb8..5bfa5a19f0 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -672,6 +672,9 @@ def deploy_template(self) -> None: "clientSecret": {"value": self.results["client_secret"]}, "app_func_issuer": {"value": app_func_issuer}, "signedExpiry": {"value": expiry}, + "cli_app_id": {"value": self.cli_app_id}, + "authority": {"value": self.authority}, + "tenant_domain": {"value": self.tenant_domain}, "multi_tenant_domain": multi_tenant_domain, "workbookData": {"value": self.workbook_data}, "enable_remote_debugging": {"value": self.host_dotnet_on_windows}, @@ -813,12 +816,12 @@ def set_instance_config(self) -> None: update_nsg(config_client, self.rules) - update_endpoint_params( - config_client, - self.authority, - self.cli_app_id, - self.tenant_domain, - ) + # update_endpoint_params( + # config_client, + # self.authority, + # self.cli_app_id, + # self.tenant_domain, + # ) if self.admins: update_admins(config_client, self.admins) diff --git a/src/deployment/deploylib/configuration.py b/src/deployment/deploylib/configuration.py index 089af682b2..9d29fd86f6 100644 --- a/src/deployment/deploylib/configuration.py +++ b/src/deployment/deploylib/configuration.py @@ -292,36 +292,36 @@ def update_nsg( ) -def update_endpoint_params( - config_client: InstanceConfigClient, - authority: str, - client_id: str, - tenant_domain: str, -) -> None: - - config_client.table_service.insert_or_merge_entity( - TABLE_NAME, - { - "PartitionKey": config_client.resource_group, - "RowKey": config_client.resource_group, - "authority": authority, - }, - ) - - config_client.table_service.insert_or_merge_entity( - TABLE_NAME, - { - "PartitionKey": config_client.resource_group, - "RowKey": config_client.resource_group, - "client_id": client_id, - }, - ) - - config_client.table_service.insert_or_merge_entity( - TABLE_NAME, - { - "PartitionKey": config_client.resource_group, - "RowKey": config_client.resource_group, - "tenant_domain": tenant_domain, - }, - ) +# def update_endpoint_params( +# config_client: InstanceConfigClient, +# authority: str, +# client_id: str, +# tenant_domain: str, +# ) -> None: + +# config_client.table_service.insert_or_merge_entity( +# TABLE_NAME, +# { +# "PartitionKey": config_client.resource_group, +# "RowKey": config_client.resource_group, +# "authority": authority, +# }, +# ) + +# config_client.table_service.insert_or_merge_entity( +# TABLE_NAME, +# { +# "PartitionKey": config_client.resource_group, +# "RowKey": config_client.resource_group, +# "client_id": client_id, +# }, +# ) + +# config_client.table_service.insert_or_merge_entity( +# TABLE_NAME, +# { +# "PartitionKey": config_client.resource_group, +# "RowKey": config_client.resource_group, +# "tenant_domain": tenant_domain, +# }, +# ) diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index a450afa222..3ba74fee08 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -901,9 +901,9 @@ class InstanceConfig(BaseModel): group_membership: Optional[Dict[PrincipalID, List[GroupId]]] = None vm_tags: Optional[Dict[str, str]] = None vmss_tags: Optional[Dict[str, str]] = None - authority: Optional[str] = Field(default="") - client_id: Optional[str] = Field(default="") - tenant_domain: Optional[str] = Field(default="") + # authority: Optional[str] = None + # client_id: Optional[str] = None + # tenant_domain: Optional[str] = None def update(self, config: "InstanceConfig") -> None: for field in config.__fields__: From 2e2ecc2b969b32253d227f6c4b16fe5dba14893c Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:30:46 +0000 Subject: [PATCH 17/29] Checking changes to params. --- src/deployment/deploy.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 5bfa5a19f0..646d77dd67 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -792,9 +792,9 @@ def parse_config(self) -> None: ) if self.tenant_domain == "": self.tenant_domain = config.tenant_domain - if self.multi_tenant_domain is None: + if self.multi_tenant_domain == "": self.multi_tenant_domain = config.multi_tenant_domain - if self.cli_app_id is None: + if self.cli_app_id == "": self.cli_app_id = config.cli_client_id except Exception as ex: @@ -1281,7 +1281,7 @@ def main() -> None: parser.add_argument( "--multi_tenant_domain", type=str, - default=None, + default="", help="enable multi-tenant authentication with this tenant domain", ) parser.add_argument( @@ -1308,7 +1308,7 @@ def main() -> None: parser.add_argument( "--cli_app_id", type=str, - default="72f1562a-8c0c-41ea-beb9-fa2b71c80134", + default="", help="CLI App Registration to be used during deployment.", ) parser.add_argument( From cd4f408800eeebc31991996beb939e754afc602d Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:35:12 +0000 Subject: [PATCH 18/29] Make tenant_domain default. --- src/cli/onefuzz/api.py | 6 ++++-- src/cli/onefuzz/backend.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index f79641e766..d5887b9cd4 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -183,7 +183,7 @@ def _req_config_params( self.onefuzz._backend.config.client_id = endpoint_params.client_id if self.onefuzz._backend.config.authority == "": self.onefuzz._backend.config.authority = endpoint_params.authority - if not self.onefuzz._backend.config.tenant_domain: + if self.onefuzz._backend.config.tenant_domain == "": self.onefuzz._backend.config.tenant_domain = ( endpoint_params.tenant_domain ) @@ -1901,7 +1901,9 @@ def config( self.logger.debug("set config") if reset: - self._backend.config = BackendConfig(authority="", client_id="") + self._backend.config = BackendConfig( + authority="", client_id="", tenant_domain="" + ) if endpoint is not None: # The normal path for calling the API always uses the oauth2 workflow, diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index a9fd867441..144e2c4300 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -95,7 +95,7 @@ class BackendConfig(BaseModel): client_id: str endpoint: Optional[str] features: Set[str] = Field(default_factory=set) - tenant_domain: Optional[str] + tenant_domain: str class Backend: From ffe81468193d6b2c1e09f2378e86c79d79d9ea23 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:40:08 +0000 Subject: [PATCH 19/29] Remove endoint params from models. --- src/ApiService/ApiService/OneFuzzTypes/Model.cs | 5 +---- src/pytypes/onefuzztypes/models.py | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index 9b1069cfe9..0ce97b28d1 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -347,10 +347,7 @@ public record InstanceConfig IDictionary? ApiAccessRules = null, IDictionary? GroupMembership = null, IDictionary? VmTags = null, - IDictionary? VmssTags = null, - // string? Authority = "", - // string? ClientId = "", - // string? TenantDomain = "" + IDictionary? VmssTags = null ) : EntityBase() { public InstanceConfig(string instanceName) : this( instanceName, diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index 3ba74fee08..60fa68cc12 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -901,9 +901,6 @@ class InstanceConfig(BaseModel): group_membership: Optional[Dict[PrincipalID, List[GroupId]]] = None vm_tags: Optional[Dict[str, str]] = None vmss_tags: Optional[Dict[str, str]] = None - # authority: Optional[str] = None - # client_id: Optional[str] = None - # tenant_domain: Optional[str] = None def update(self, config: "InstanceConfig") -> None: for field in config.__fields__: From a5a6a5424f2172fcc7a14be08509bb6dd141036e Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:42:37 +0000 Subject: [PATCH 20/29] UPdating docs. --- docs/webhook_events.md | 35 +---------------------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/docs/webhook_events.md b/docs/webhook_events.md index 0ce5dadefc..cdb7b631db 100644 --- a/docs/webhook_events.md +++ b/docs/webhook_events.md @@ -696,8 +696,6 @@ If webhook is set to have Event Grid message format then the payload will look a "allowed_aad_tenants": [ "00000000-0000-0000-0000-000000000000" ], - "authority": "", - "client_id": "", "default_linux_vm_image": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest", "default_windows_vm_image": "MicrosoftWindowsDesktop:Windows-10:win10-21h2-pro:latest", "network_config": { @@ -709,8 +707,7 @@ If webhook is set to have Event Grid message format then the payload will look a "allowed_service_tags": [] }, "proxy_vm_sku": "Standard_B2s", - "require_admin_privileges": false, - "tenant_domain": "" + "require_admin_privileges": false } } ``` @@ -841,16 +838,6 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Api Access Rules", "type": "object" }, - "authority": { - "default": "", - "title": "Authority", - "type": "string" - }, - "client_id": { - "default": "", - "title": "Client Id", - "type": "string" - }, "default_linux_vm_image": { "default": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest", "title": "Default Linux Vm Image", @@ -891,11 +878,6 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Require Admin Privileges", "type": "boolean" }, - "tenant_domain": { - "default": "", - "title": "Tenant Domain", - "type": "string" - }, "vm_tags": { "additionalProperties": { "type": "string" @@ -6176,16 +6158,6 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Api Access Rules", "type": "object" }, - "authority": { - "default": "", - "title": "Authority", - "type": "string" - }, - "client_id": { - "default": "", - "title": "Client Id", - "type": "string" - }, "default_linux_vm_image": { "default": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest", "title": "Default Linux Vm Image", @@ -6226,11 +6198,6 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Require Admin Privileges", "type": "boolean" }, - "tenant_domain": { - "default": "", - "title": "Tenant Domain", - "type": "string" - }, "vm_tags": { "additionalProperties": { "type": "string" From b5c3a746f612d077a1d1ba65462ddfa16d4c297b Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:48:20 +0000 Subject: [PATCH 21/29] Setting --- .../IntegrationTests/Fakes/TestServiceConfiguration.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ApiService/IntegrationTests/Fakes/TestServiceConfiguration.cs b/src/ApiService/IntegrationTests/Fakes/TestServiceConfiguration.cs index 725675b37e..ae84d11eff 100644 --- a/src/ApiService/IntegrationTests/Fakes/TestServiceConfiguration.cs +++ b/src/ApiService/IntegrationTests/Fakes/TestServiceConfiguration.cs @@ -25,6 +25,11 @@ public TestServiceConfiguration(string tablePrefix) { public string? OneFuzzTelemetry => "TestOneFuzzTelemetry"; + public string? CliAppId => "TestGuid"; + + public string? Authority => "TestAuthority"; + + public string? TenantDomain => "TestDomain"; public string? MultiTenantDomain => null; public string? OneFuzzInstanceName => "UnitTestInstance"; From f184bc9a17830108ba4d42adc6d32446df11910f Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 21:55:12 +0000 Subject: [PATCH 22/29] Removing hardcoded values. --- src/cli/onefuzz/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index d5887b9cd4..ff9d5f0d75 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -41,8 +41,9 @@ UUID_EXPANSION = TypeVar("UUID_EXPANSION", UUID, str) DEFAULT = BackendConfig( - authority="https://login.microsoftonline.com/72f988bf-86f1-41af-91ab-2d7cd011db47", - client_id="72f1562a-8c0c-41ea-beb9-fa2b71c80134", + authority="", + client_id="", + tenatn_domain="", ) # This was generated randomly and should be preserved moving forwards From efe3a0614afe870b43416c7e5c446837c47c6680 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 22:00:31 +0000 Subject: [PATCH 23/29] Typo. --- src/cli/onefuzz/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index ff9d5f0d75..8f54da4193 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -43,7 +43,7 @@ DEFAULT = BackendConfig( authority="", client_id="", - tenatn_domain="", + tenant_domain="", ) # This was generated randomly and should be preserved moving forwards From 081b454d60e1e7ebca204b34ec9e426c257d84dc Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 22:16:17 +0000 Subject: [PATCH 24/29] Removing endpoint upload. --- src/deployment/deploy.py | 8 ------ src/deployment/deploylib/configuration.py | 35 ----------------------- 2 files changed, 43 deletions(-) diff --git a/src/deployment/deploy.py b/src/deployment/deploy.py index 646d77dd67..23d06a424d 100644 --- a/src/deployment/deploy.py +++ b/src/deployment/deploy.py @@ -49,7 +49,6 @@ parse_rules, update_admins, update_allowed_aad_tenants, - update_endpoint_params, update_nsg, ) from deploylib.data_migration import migrate @@ -816,13 +815,6 @@ def set_instance_config(self) -> None: update_nsg(config_client, self.rules) - # update_endpoint_params( - # config_client, - # self.authority, - # self.cli_app_id, - # self.tenant_domain, - # ) - if self.admins: update_admins(config_client, self.admins) diff --git a/src/deployment/deploylib/configuration.py b/src/deployment/deploylib/configuration.py index 9d29fd86f6..4c56ee7fe9 100644 --- a/src/deployment/deploylib/configuration.py +++ b/src/deployment/deploylib/configuration.py @@ -290,38 +290,3 @@ def update_nsg( "proxy_nsg_config": json.dumps(nsg_config), }, ) - - -# def update_endpoint_params( -# config_client: InstanceConfigClient, -# authority: str, -# client_id: str, -# tenant_domain: str, -# ) -> None: - -# config_client.table_service.insert_or_merge_entity( -# TABLE_NAME, -# { -# "PartitionKey": config_client.resource_group, -# "RowKey": config_client.resource_group, -# "authority": authority, -# }, -# ) - -# config_client.table_service.insert_or_merge_entity( -# TABLE_NAME, -# { -# "PartitionKey": config_client.resource_group, -# "RowKey": config_client.resource_group, -# "client_id": client_id, -# }, -# ) - -# config_client.table_service.insert_or_merge_entity( -# TABLE_NAME, -# { -# "PartitionKey": config_client.resource_group, -# "RowKey": config_client.resource_group, -# "tenant_domain": tenant_domain, -# }, -# ) From 5fa1abcdf3e7d6a4b2b85d2deaf90e865babc3f4 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 23:02:27 +0000 Subject: [PATCH 25/29] Typo. --- src/deployment/bicep-templates/function-settings.bicep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deployment/bicep-templates/function-settings.bicep b/src/deployment/bicep-templates/function-settings.bicep index 2ac9d0b1b4..742f4f39d5 100644 --- a/src/deployment/bicep-templates/function-settings.bicep +++ b/src/deployment/bicep-templates/function-settings.bicep @@ -56,7 +56,7 @@ resource functionSettings 'Microsoft.Web/sites/config@2021-03-01' = { ONEFUZZ_TELEMETRY: telemetry AzureWebJobsStorage: func_sas_url CLI_APP_ID: cli_app_id - AUTHORIY: authority + AUTHORITY: authority TENANT_DOMAIN: tenant_domain MULTI_TENANT_DOMAIN: multi_tenant_domain AzureWebJobsDisableHomepage: 'true' From dbe3f6701aeeed6203b842b26343ce20eaffe42e Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Thu, 26 Jan 2023 23:46:28 +0000 Subject: [PATCH 26/29] Fixing typos. --- src/ApiService/ApiService/ServiceConfiguration.cs | 2 +- src/cli/onefuzz/api.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/ServiceConfiguration.cs b/src/ApiService/ApiService/ServiceConfiguration.cs index ae06d7ad32..c959a90d51 100644 --- a/src/ApiService/ApiService/ServiceConfiguration.cs +++ b/src/ApiService/ApiService/ServiceConfiguration.cs @@ -101,7 +101,7 @@ public ServiceConfiguration() { public string? DiagnosticsAzureBlobRetentionDays { get => GetEnv("DIAGNOSTICS_AZUREBLOBRETENTIONINDAYS"); } public string? CliAppId { get => GetEnv("CLI_APP_ID"); } public string? Authority { get => GetEnv("AUTHORITY"); } - public string? TenantDomain { get => GetEnv("TenantDomain"); } + public string? TenantDomain { get => GetEnv("TENANT_DOMAIN"); } public string? MultiTenantDomain { get => GetEnv("MULTI_TENANT_DOMAIN"); } public ResourceIdentifier? OneFuzzDataStorage { diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index 8f54da4193..a3db493620 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -176,6 +176,7 @@ def _req_config_params( "GET", endpoint + "/api/config" ) + logging.debug(response.json()) endpoint_params = responses.Config.parse_obj(response.json()) logging.debug(self.onefuzz._backend.config.authority) From 794be7a8ba90e3345b4f45a1a7b2a994f6328030 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Mon, 30 Jan 2023 23:23:15 +0000 Subject: [PATCH 27/29] Fix error message about aad tenant. --- src/ApiService/ApiService/UserCredentials.cs | 2 +- src/cli/onefuzz/backend.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ApiService/ApiService/UserCredentials.cs b/src/ApiService/ApiService/UserCredentials.cs index fb9bf2d7f7..58fb547730 100644 --- a/src/ApiService/ApiService/UserCredentials.cs +++ b/src/ApiService/ApiService/UserCredentials.cs @@ -92,7 +92,7 @@ public virtual async Task> ParseJwtToken(HttpRequest } else { var tenantsStr = allowedTenants.OkV is null ? "null" : String.Join(';', allowedTenants.OkV!); _log.Error($"issuer not from allowed tenant. issuer: {token.Issuer:Tag:Issuer} - tenants: {tenantsStr:Tag:Tenants}"); - return OneFuzzResult.Error(ErrorCode.INVALID_REQUEST, new[] { "unauthorized AAD issuer" }); + return OneFuzzResult.Error(ErrorCode.INVALID_REQUEST, new[] { "unauthorized AAD issuer. if multi-tenant auth is failing, make sure to include all tenant_ids in the `allowed_aad_tenants` list in the instance_config. to see the current instance_config, run `onefuzz instance_config get`. " }); } } else { _log.Error($"Failed to get allowed tenants due to {allowedTenants.ErrorV:Tag:Error}"); diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 144e2c4300..530c2c0c04 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -321,7 +321,6 @@ def request( if not endpoint: raise Exception("endpoint not configured") - LOGGER.info(self.headers) url = endpoint + "/api/" + path headers = self.headers() json_data = serialize(json_data) From d78ac48f74e3df934aa7af459f64063ab1a34f93 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Mon, 30 Jan 2023 23:52:15 +0000 Subject: [PATCH 28/29] Responding to comments. --- src/ApiService/ApiService/Functions/Config.cs | 2 +- src/cli/onefuzz/api.py | 43 ++++++++----------- src/cli/onefuzz/backend.py | 2 +- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/ApiService/ApiService/Functions/Config.cs b/src/ApiService/ApiService/Functions/Config.cs index 0b8d43d2b8..097ed6c205 100644 --- a/src/ApiService/ApiService/Functions/Config.cs +++ b/src/ApiService/ApiService/Functions/Config.cs @@ -15,7 +15,7 @@ public Config(ILogTracer log, IOnefuzzContext context) { [Function("Config")] public Async.Task Run( - [HttpTrigger(AuthorizationLevel.Anonymous, "GET", Route = "config")] HttpRequestData req) { + [HttpTrigger(AuthorizationLevel.Anonymous, "GET")] HttpRequestData req) { return Get(req); } public async Async.Task Get(HttpRequestData req) { diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index a3db493620..04eb3f3d51 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -162,37 +162,28 @@ def _req_config_params( self, ) -> None: - endpoint_params = responses.Config( - authority="", - client_id="", - tenant_domain="", - ) + if self.onefuzz._backend.config.endpoint is None: + raise Exception("Endpoint Not Configured") - if self.onefuzz._backend.config.endpoint is not None: + endpoint = self.onefuzz._backend.config.endpoint - endpoint = self.onefuzz._backend.config.endpoint + response = self.onefuzz._backend.session.request( + "GET", endpoint + "/api/config" + ) - response = self.onefuzz._backend.session.request( - "GET", endpoint + "/api/config" - ) + logging.debug(response.json()) + endpoint_params = responses.Config.parse_obj(response.json()) - logging.debug(response.json()) - endpoint_params = responses.Config.parse_obj(response.json()) - - logging.debug(self.onefuzz._backend.config.authority) - # Will override values in storage w/ provided values for SP use - if self.onefuzz._backend.config.client_id == "": - self.onefuzz._backend.config.client_id = endpoint_params.client_id - if self.onefuzz._backend.config.authority == "": - self.onefuzz._backend.config.authority = endpoint_params.authority - if self.onefuzz._backend.config.tenant_domain == "": - self.onefuzz._backend.config.tenant_domain = ( - endpoint_params.tenant_domain - ) + logging.debug(self.onefuzz._backend.config.authority) + # Will override values in storage w/ provided values for SP use + if self.onefuzz._backend.config.client_id == "": + self.onefuzz._backend.config.client_id = endpoint_params.client_id + if self.onefuzz._backend.config.authority == "": + self.onefuzz._backend.config.authority = endpoint_params.authority + if self.onefuzz._backend.config.tenant_domain == "": + self.onefuzz._backend.config.tenant_domain = endpoint_params.tenant_domain - self.onefuzz._backend.save_config() - else: - raise Exception("Endpoint Not Configured") + self.onefuzz._backend.save_config() def _disambiguate( self, diff --git a/src/cli/onefuzz/backend.py b/src/cli/onefuzz/backend.py index 530c2c0c04..9587ee1068 100644 --- a/src/cli/onefuzz/backend.py +++ b/src/cli/onefuzz/backend.py @@ -181,7 +181,7 @@ def get_access_token(self) -> Any: if not self.config.endpoint: raise Exception("endpoint not configured") - if "common" in self.config.authority: + if "https://login.microsoftonline.com/common" in self.config.authority: endpoint = urlparse(self.config.endpoint).netloc.split(".")[0] scopes = [ f"api://{self.config.tenant_domain}/{endpoint}/.default", From ab844c8b63a641501dc093ce505cce22391e66ef Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Tue, 31 Jan 2023 13:33:23 -0800 Subject: [PATCH 29/29] Update src/ApiService/ApiService/UserCredentials.cs Co-authored-by: Marc Greisen --- src/ApiService/ApiService/UserCredentials.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/UserCredentials.cs b/src/ApiService/ApiService/UserCredentials.cs index 58fb547730..8072471951 100644 --- a/src/ApiService/ApiService/UserCredentials.cs +++ b/src/ApiService/ApiService/UserCredentials.cs @@ -92,7 +92,7 @@ public virtual async Task> ParseJwtToken(HttpRequest } else { var tenantsStr = allowedTenants.OkV is null ? "null" : String.Join(';', allowedTenants.OkV!); _log.Error($"issuer not from allowed tenant. issuer: {token.Issuer:Tag:Issuer} - tenants: {tenantsStr:Tag:Tenants}"); - return OneFuzzResult.Error(ErrorCode.INVALID_REQUEST, new[] { "unauthorized AAD issuer. if multi-tenant auth is failing, make sure to include all tenant_ids in the `allowed_aad_tenants` list in the instance_config. to see the current instance_config, run `onefuzz instance_config get`. " }); + return OneFuzzResult.Error(ErrorCode.INVALID_REQUEST, new[] { "unauthorized AAD issuer. If multi-tenant auth is failing, make sure to include all tenant_ids in the `allowed_aad_tenants` list in the instance_config. To see the current instance_config, run `onefuzz instance_config get`. " }); } } else { _log.Error($"Failed to get allowed tenants due to {allowedTenants.ErrorV:Tag:Error}");