From 0e07fcc706a7f7bec3afc87716f4bd3b248cf137 Mon Sep 17 00:00:00 2001 From: Kukks Date: Tue, 4 Aug 2020 13:10:48 +0200 Subject: [PATCH] fixes and adapt --- .../BTCPayServerClient.Authorization.cs | 11 ++++- BTCPayServer.Client/BTCPayServerClient.cs | 40 ++++++++++------- BTCPayServer.Data/Data/APIKeyData.cs | 1 - BTCPayServer.Tests/ApiKeysTests.cs | 29 ++++++++++-- .../Controllers/ManageController.APIKeys.cs | 44 +++++++------------ .../Security/GreenField/APIKeyRepository.cs | 6 --- .../v1/swagger.template.authorization.json | 24 ++++++++++ 7 files changed, 100 insertions(+), 55 deletions(-) diff --git a/BTCPayServer.Client/BTCPayServerClient.Authorization.cs b/BTCPayServer.Client/BTCPayServerClient.Authorization.cs index 9ec8062aa..c59775935 100644 --- a/BTCPayServer.Client/BTCPayServerClient.Authorization.cs +++ b/BTCPayServer.Client/BTCPayServerClient.Authorization.cs @@ -7,7 +7,7 @@ namespace BTCPayServer.Client { public static Uri GenerateAuthorizeUri(Uri btcpayHost, string[] permissions, bool strict = true, - bool selectiveStores = false) + bool selectiveStores = false, (string ApplicationIdentifier, Uri Redirect) applicationDetails = default) { var result = new UriBuilder(btcpayHost); result.Path = "api-keys/authorize"; @@ -18,6 +18,15 @@ namespace BTCPayServer.Client {"strict", strict}, {"selectiveStores", selectiveStores}, {"permissions", permissions} }); + if (applicationDetails.Redirect != null) + { + AppendPayloadToQuery(result, new KeyValuePair("redirect", applicationDetails.Redirect)); + if (!string.IsNullOrEmpty(applicationDetails.ApplicationIdentifier)) + { + AppendPayloadToQuery(result, new KeyValuePair("applicationIdentifier", applicationDetails.ApplicationIdentifier)); + } + } + return result.Uri; } } diff --git a/BTCPayServer.Client/BTCPayServerClient.cs b/BTCPayServer.Client/BTCPayServerClient.cs index 833320c83..d934976cf 100644 --- a/BTCPayServer.Client/BTCPayServerClient.cs +++ b/BTCPayServer.Client/BTCPayServerClient.cs @@ -103,29 +103,37 @@ namespace BTCPayServer.Client return request; } + private static void AppendPayloadToQuery(UriBuilder uri, KeyValuePair keyValuePair) + { + if (uri.Query.Length > 1) + uri.Query += "&"; + + UriBuilder uriBuilder = uri; + if (!(keyValuePair.Value is string) && + keyValuePair.Value.GetType().GetInterfaces().Contains((typeof(IEnumerable)))) + { + foreach (var item in (IEnumerable)keyValuePair.Value) + { + uriBuilder.Query = uriBuilder.Query + Uri.EscapeDataString(keyValuePair.Key) + "=" + + Uri.EscapeDataString(item.ToString()) + "&"; + } + } + else + { + uriBuilder.Query = uriBuilder.Query + Uri.EscapeDataString(keyValuePair.Key) + "=" + + Uri.EscapeDataString(keyValuePair.Value.ToString()) + "&"; + } + uri.Query = uri.Query.Trim('&'); + } + private static void AppendPayloadToQuery(UriBuilder uri, Dictionary payload) { if (uri.Query.Length > 1) uri.Query += "&"; foreach (KeyValuePair keyValuePair in payload) { - UriBuilder uriBuilder = uri; - if (!(keyValuePair.Value is string) && keyValuePair.Value.GetType().GetInterfaces().Contains((typeof(IEnumerable)))) - { - foreach (var item in (IEnumerable)keyValuePair.Value) - { - uriBuilder.Query = uriBuilder.Query + Uri.EscapeDataString(keyValuePair.Key) + "=" + - Uri.EscapeDataString(item.ToString()) + "&"; - } - } - else - { - uriBuilder.Query = uriBuilder.Query + Uri.EscapeDataString(keyValuePair.Key) + "=" + - Uri.EscapeDataString(keyValuePair.Value.ToString()) + "&"; - } + AppendPayloadToQuery(uri, keyValuePair); } - - uri.Query = uri.Query.Trim('&'); } } } diff --git a/BTCPayServer.Data/Data/APIKeyData.cs b/BTCPayServer.Data/Data/APIKeyData.cs index 187d03699..ffde380f9 100644 --- a/BTCPayServer.Data/Data/APIKeyData.cs +++ b/BTCPayServer.Data/Data/APIKeyData.cs @@ -15,7 +15,6 @@ namespace BTCPayServer.Data [MaxLength(50)] public string StoreId { get; set; } [MaxLength(50)] public string UserId { get; set; } - public string ApplicationIdentifier { get; set; } public APIKeyType Type { get; set; } = APIKeyType.Legacy; diff --git a/BTCPayServer.Tests/ApiKeysTests.cs b/BTCPayServer.Tests/ApiKeysTests.cs index 01df4e2cd..1a07cd740 100644 --- a/BTCPayServer.Tests/ApiKeysTests.cs +++ b/BTCPayServer.Tests/ApiKeysTests.cs @@ -117,10 +117,13 @@ namespace BTCPayServer.Tests //permissions //strict //selectiveStores + //redirect + //appidentifier + var appidentifier = "testapp"; var authUrl = BTCPayServerClient.GenerateAuthorizeUri(tester.PayTester.ServerUri, - new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }).ToString(); + new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, applicationDetails: (appidentifier, new Uri("https://local.local/callback"))).ToString(); s.Driver.Navigate().GoToUrl(authUrl); - s.Driver.PageSource.Contains("kukksappname"); + Assert.True(s.Driver.PageSource.Contains(appidentifier)); Assert.Equal("hidden", s.Driver.FindElement(By.Id("btcpay.store.canmodifystoresettings")).GetAttribute("type").ToLowerInvariant()); Assert.Equal("true", s.Driver.FindElement(By.Id("btcpay.store.canmodifystoresettings")).GetAttribute("value").ToLowerInvariant()); Assert.Equal("hidden", s.Driver.FindElement(By.Id("btcpay.server.canmodifyserversettings")).GetAttribute("type").ToLowerInvariant()); @@ -138,7 +141,7 @@ namespace BTCPayServer.Tests (await apiKeyRepo.GetKey(results.Single(pair => pair.Key == "key").Value)).GetBlob().Permissions); authUrl = BTCPayServerClient.GenerateAuthorizeUri(tester.PayTester.ServerUri, - new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true).ToString(); + new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true, applicationDetails: (null, new Uri("https://local.local/callback"))).ToString(); s.Driver.Navigate().GoToUrl(authUrl); Assert.DoesNotContain("kukksappname", s.Driver.PageSource); @@ -158,6 +161,26 @@ namespace BTCPayServer.Tests await TestApiAgainstAccessToken(results.Single(pair => pair.Key == "key").Value, tester, user, (await apiKeyRepo.GetKey(results.Single(pair => pair.Key == "key").Value)).GetBlob().Permissions); + + + //let's test the app identifier system + authUrl = BTCPayServerClient.GenerateAuthorizeUri(tester.PayTester.ServerUri, + new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true, (appidentifier, new Uri("https://local.local/callback"))).ToString(); + + + //if it's the same, go to the confirm page + s.Driver.Navigate().GoToUrl(authUrl); + s.Driver.FindElement(By.Id("continue")).Click(); + url = s.Driver.Url; + Assert.StartsWith("https://local.local/callback", url); + + //same app but different redirect = nono + authUrl = BTCPayServerClient.GenerateAuthorizeUri(tester.PayTester.ServerUri, + new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true, (appidentifier, new Uri("https://international.local/callback"))).ToString(); + + s.Driver.Navigate().GoToUrl(authUrl); + url = s.Driver.Url; + Assert.False(url.StartsWith("https://international.com/callback")); } } diff --git a/BTCPayServer/Controllers/ManageController.APIKeys.cs b/BTCPayServer/Controllers/ManageController.APIKeys.cs index 1cde90ab2..ac8a1ca75 100644 --- a/BTCPayServer/Controllers/ManageController.APIKeys.cs +++ b/BTCPayServer/Controllers/ManageController.APIKeys.cs @@ -80,18 +80,6 @@ namespace BTCPayServer.Controllers return View("AddApiKey", await SetViewModelValues(new AddApiKeyViewModel())); } - /// The permissions to request - /// The name of your application - /// The URl to redirect to after the user consents, with the query paramters appended to it: permissions, user-id, api-key. If not specified, user is redirect to their API Key list. - /// If permissions are specified, and strict is set to false, it will allow the user to reject some of permissions the application is requesting. - /// If the application is requesting the CanModifyStoreSettings permission and selectiveStores is set to true, this allows the user to only grant permissions to selected stores under the user's control. - /// If specified, BTCPay will check if there is an existing API key stored associated with the user that also has this application identifer, redirect host AND the permissions required match(takes selectiveStores and strict into account). applicationIdentifier is ignored if redirect is not specified. - // [OpenApiTags("Authorization")] - // [OpenApiOperation("Authorize User", - // "Redirect the browser to this endpoint to request the user to generate an api-key with specific permissions")] - // [SwaggerResponse(StatusCodes.Status307TemporaryRedirect, null, - // Description = "Redirects to the specified url with query string values for api-key, permissions, and user-id upon consent")] - // [IncludeInOpenApiDocs] [HttpGet("~/api-keys/authorize")] public async Task AuthorizeAPIKey( string[] permissions, string applicationName = null, Uri redirect = null, bool strict = true, bool selectiveStores = false, string applicationIdentifier = null) @@ -165,16 +153,16 @@ namespace BTCPayServer.Controllers continue; } - //we have a key that is sufficient, redirect to a page to confirm that it's ok to provide this key to the app. - return View("Confirm", - new ConfirmModel() - { - Title = $"Are you sure about exposing your API Key to {redirect}?", - Description = $"You've previously generated this API Key ({key.Id}) specifically for {applicationName}", - ActionUrl = GetRedirectToApplicationUrl(redirect, key), - ButtonClass = "btn-secondary", - Action = "Confirm" - }); + //we have a key that is sufficient, redirect to a page to confirm that it's ok to provide this key to the app. + return View("Confirm", + new ConfirmModel() + { + Title = $"Are you sure about exposing your API Key to {applicationName??applicationIdentifier}?", + Description = $"You've previously generated this API Key ({key.Id}) specifically for {applicationName??applicationIdentifier} with the url {redirect}. ", + ActionUrl = GetRedirectToApplicationUrl(redirect, key), + ButtonClass = "btn-secondary", + Action = "Confirm" + }); } } } @@ -258,7 +246,6 @@ namespace BTCPayServer.Controllers } } - if (!ModelState.IsValid) { return View(viewModel); @@ -269,7 +256,7 @@ namespace BTCPayServer.Controllers case "no": return RedirectToAction("APIKeys"); case "yes": - var key = await CreateKey(viewModel, viewModel.ApplicationIdentifier); + var key = await CreateKey(viewModel, (viewModel.ApplicationIdentifier, viewModel.RedirectUrl.Authority)); if (viewModel.RedirectUrl != null) { @@ -293,7 +280,7 @@ namespace BTCPayServer.Controllers var permissions = key.GetBlob().Permissions; uri.AppendPayloadToQuery(new Dictionary() { - {"api-key", key.Id}, {"permissions",permissions}, {"user-id", key.UserId} + {"key", key.Id}, {"permissions",permissions}, {"user", key.UserId} }); //uri builder has bug around string[] params return uri.Uri.ToStringInvariant().Replace("permissions=System.String%5B%5D", @@ -374,7 +361,7 @@ namespace BTCPayServer.Controllers return null; } - private async Task CreateKey(AddApiKeyViewModel viewModel, string appIdentifier = null) + private async Task CreateKey(AddApiKeyViewModel viewModel, (string appIdentifier, string appAuthority) app = default) { var key = new APIKeyData() { @@ -382,11 +369,12 @@ namespace BTCPayServer.Controllers Type = APIKeyType.Permanent, UserId = _userManager.GetUserId(User), Label = viewModel.Label, - ApplicationIdentifier = appIdentifier }; key.SetBlob(new APIKeyBlob() { - Permissions = GetPermissionsFromViewModel(viewModel).Select(p => p.ToString()).Distinct().ToArray() + Permissions = GetPermissionsFromViewModel(viewModel).Select(p => p.ToString()).Distinct().ToArray(), + ApplicationAuthority = app.appAuthority, + ApplicationIdentifier = app.appIdentifier }); await _apiKeyRepository.CreateKey(key); return key; diff --git a/BTCPayServer/Security/GreenField/APIKeyRepository.cs b/BTCPayServer/Security/GreenField/APIKeyRepository.cs index 5ffe4782b..94e48e561 100644 --- a/BTCPayServer/Security/GreenField/APIKeyRepository.cs +++ b/BTCPayServer/Security/GreenField/APIKeyRepository.cs @@ -36,12 +36,6 @@ namespace BTCPayServer.Security.GreenField { queryable = queryable.Where(data => query.UserId.Contains(data.UserId)); } - - if (query.ApplicationIdentifier != null && query.ApplicationIdentifier.Any()) - { - queryable = queryable.Where(data => - query.ApplicationIdentifier.Contains(data.ApplicationIdentifier)); - } } return await queryable.ToListAsync(); diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.authorization.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.authorization.json index 02a24b192..d8712e39b 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.authorization.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.authorization.json @@ -54,6 +54,27 @@ "nullable": true }, "x-position": 4 + }, + { + "name": "redirect", + "description": "The url to redirect to after the user consents, with the query parameters appended to it: permissions, user-id, api-key. If not specified, user is redirected to their API Key list.", + "in": "query", + "schema": { + "type": "string", + "format": "url", + "nullable": true + }, + "x-position": 5 + }, + { + "name": "applicationIdentifier", + "description": "If specified, BTCPay Server will check if there is an existing API key associated with the user that also has this application identifier, redirect host AND the permissions required match(takes selectiveStores and strict into account). `applicationIdentifier` is ignored if redirect is not specified.", + "in": "query", + "schema": { + "type": "string", + "nullable": true + }, + "x-position": 6 } ], "responses": { @@ -63,6 +84,9 @@ "text/html": { } } + }, + "307": { + "description": "Redirects to the specified url in `redirect` with query string values for `key` (the api key created or matched), `permissions` (the permissions the user consented to), and `user` (the id of the user that consented) upon consent" } }, "security": []