Greenfield: Rename API key redirect params; switch to POST body

BTCPayServer.Tests/ApiKeysTests.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using System.Net.Http;
 using System.Net.Http.Headers;
@@ -11,6 +10,7 @@
 using BTCPayServer.Tests.Logging;
 using BTCPayServer.Views.Manage;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 using OpenQA.Selenium;
 using Xunit;
 using Xunit.Abstractions;
@@ -109,7 +109,6 @@ public async Task CanCreateApiKeys()
                         tester.PayTester.HttpClient);
                 });
 
-
                 //let's test the authorized screen now
                 //options for authorize are:
                 //applicationName
@@ -120,28 +119,27 @@ public async Task CanCreateApiKeys()
                 //redirect
                 //appidentifier
                 var appidentifier = "testapp";
+                var callbackUrl = tester.PayTester.ServerUri + "postredirect-callback-test";
                 var authUrl = BTCPayServerClient.GenerateAuthorizeUri(tester.PayTester.ServerUri,
-                    new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, applicationDetails: (appidentifier, new Uri("https://local.local/callback"))).ToString();
+                    new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, applicationDetails: (appidentifier, new Uri(callbackUrl))).ToString();
                 s.Driver.Navigate().GoToUrl(authUrl);
-                Assert.True(s.Driver.PageSource.Contains(appidentifier));
+                Assert.Contains(appidentifier, s.Driver.PageSource);
                 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());
                 Assert.Equal("true", s.Driver.FindElement(By.Id("btcpay.server.canmodifyserversettings")).GetAttribute("value").ToLowerInvariant());
                 Assert.DoesNotContain("change-store-mode", s.Driver.PageSource);
                 s.Driver.FindElement(By.Id("consent-yes")).Click();
-                var url = s.Driver.Url;
-                Assert.StartsWith("https://local.local/callback", url);
-                IEnumerable<KeyValuePair<string, string>> results = url.Split("?").Last().Split("&")
-                    .Select(s1 => new KeyValuePair<string, string>(s1.Split("=")[0], s1.Split("=")[1]));
+                Assert.Equal(callbackUrl, s.Driver.Url);
 
                 var apiKeyRepo = s.Server.PayTester.GetService<APIKeyRepository>();
+                var accessToken = GetAccessTokenFromCallbackResult(s.Driver);
 
-                await TestApiAgainstAccessToken(results.Single(pair => pair.Key == "key").Value, tester, user,
-                    (await apiKeyRepo.GetKey(results.Single(pair => pair.Key == "key").Value)).GetBlob().Permissions);
+                await TestApiAgainstAccessToken(accessToken, tester, user,
+                    (await apiKeyRepo.GetKey(accessToken)).GetBlob().Permissions);
 
                 authUrl = BTCPayServerClient.GenerateAuthorizeUri(tester.PayTester.ServerUri,
-                    new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true,  applicationDetails: (null, new Uri("https://local.local/callback"))).ToString();
+                    new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true,  applicationDetails: (null, new Uri(callbackUrl))).ToString();
 
                 s.Driver.Navigate().GoToUrl(authUrl);
                 Assert.DoesNotContain("kukksappname", s.Driver.PageSource);
@@ -154,34 +152,27 @@ public async Task CanCreateApiKeys()
                 s.SetCheckbox(s, "btcpay.server.canmodifyserversettings", false);
                 Assert.Contains("change-store-mode", s.Driver.PageSource);
                 s.Driver.FindElement(By.Id("consent-yes")).Click();
-                url = s.Driver.Url;
-                Assert.StartsWith("https://local.local/callback", url);
-                results = url.Split("?").Last().Split("&")
-                    .Select(s1 => new KeyValuePair<string, string>(s1.Split("=")[0], s1.Split("=")[1]));
+                Assert.Equal(callbackUrl, s.Driver.Url);
+
+                accessToken = GetAccessTokenFromCallbackResult(s.Driver);
+                await TestApiAgainstAccessToken(accessToken, tester, user,
+                    (await apiKeyRepo.GetKey(accessToken)).GetBlob().Permissions);
 
-                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();
-
+                    new[] { Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings }, false, true, (appidentifier, new Uri(callbackUrl))).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);
+                Assert.Equal(callbackUrl, s.Driver.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"));
-
+                Assert.False(s.Driver.Url.StartsWith("https://international.com/callback"));
             }
         }
 
@@ -339,5 +330,12 @@ public async Task<T> TestApiAgainstAccessToken<T>(string apikey, string url, Htt
 
             return JsonConvert.DeserializeObject<T>(rawJson);
         }
+
+        private string GetAccessTokenFromCallbackResult(IWebDriver driver)
+        {
+            var source = driver.FindElement(By.TagName("body")).Text;
+            var json = JObject.Parse(source);
+            return json.GetValue("apiKey")!.Value<string>();
+        }
     }
 }

BTCPayServer.Tests/BTCPayServer.Tests.csproj

@@ -22,7 +22,7 @@
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.6.1" />
     <PackageReference Include="Newtonsoft.Json.Schema" Version="3.0.13" />
     <PackageReference Include="Selenium.WebDriver" Version="3.141.0" />
-    <PackageReference Include="Selenium.WebDriver.ChromeDriver" Version="83.0.4103.3900" />
+    <PackageReference Include="Selenium.WebDriver.ChromeDriver" Version="85.0.4183.8700" />
     <PackageReference Include="xunit" Version="2.4.1" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.2">
       <PrivateAssets>all</PrivateAssets>

BTCPayServer.Tests/SeleniumTester.cs

@@ -308,8 +308,9 @@ public string CreateInvoice(string storeName, decimal amount = 100, string curre
             currencyEl.Clear();
             currencyEl.SendKeys(currency);
             Driver.FindElement(By.Id("BuyerEmail")).SendKeys(refundEmail);
-            Driver.FindElement(By.Name("StoreId")).SendKeys(storeName + Keys.Enter);
-            Driver.FindElement(By.Id("Create")).ForceClick();
+            Driver.FindElement(By.Name("StoreId")).SendKeys(storeName);
+            Driver.FindElement(By.Id("Create")).Click();
+
             Assert.True(Driver.PageSource.Contains("just created!"), "Unable to create Invoice");
             var statusElement = Driver.FindElement(By.ClassName("alert-success"));
             var id = statusElement.Text.Split(" ")[1];

BTCPayServer/BTCPayServer.csproj

@@ -43,7 +43,7 @@
     <Content Remove="Views\Shared\Ethereum\**\*" />
     <Content Remove="Views\Shared\Monero\**\*" />
   </ItemGroup>
-  
+
   <ItemGroup>
     <PackageReference Include="BTCPayServer.Hwi" Version="1.1.3" />
     <PackageReference Include="BTCPayServer.Lightning.All" Version="1.2.4" />

BTCPayServer/Controllers/HomeController.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
@@ -12,6 +13,7 @@
 using BTCPayServer.Services.Apps;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.FileProviders;
@@ -139,7 +141,6 @@ public IActionResult SwaggerDocs()
             return View();
         }
 
-
         [HttpPost]
         [Route("translate")]
         public async Task<IActionResult> BitpayTranslator(BitpayTranslatorViewModel vm)
@@ -199,6 +200,18 @@ public IActionResult RecoverySeedBackup(RecoverySeedBackupViewModel vm)
             return View("RecoverySeedBackup", vm);
         }
 
+        [HttpPost]
+        [Route("postredirect-callback-test")]
+        public ActionResult PostRedirectCallbackTestpage(IFormCollection data)
+        {
+            var list = data.Keys.Aggregate(new Dictionary<string, string>(), (res, key) =>
+            {
+                res.Add(key, data[key]);
+                return res;
+            });
+            return Json(list);
+        }
+
         public IActionResult Error()
         {
             return View(new ErrorViewModel { RequestId = Activity.Current?.Id ?? HttpContext.TraceIdentifier });

BTCPayServer/Controllers/ManageController.APIKeys.cs

@@ -7,9 +7,7 @@
 using BTCPayServer.Data;
 using BTCPayServer.Models;
 using BTCPayServer.Security.GreenField;
-using ExchangeSharp;
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using NBitcoin;
 using NBitcoin.DataEncoders;
@@ -40,10 +38,11 @@ public async Task<IActionResult> RemoveAPIKey(string id)
             }
             return View("Confirm", new ConfirmModel()
             {
-                Title = "Delete API Key " + (string.IsNullOrEmpty(key.Label) ? string.Empty : key.Label) + "(" + key.Id + ")",
-                Description = "Any application using this api key will immediately lose access",
+                Title = $"Delete API Key {(string.IsNullOrEmpty(key.Label) ? string.Empty : key.Label)}",
+                DescriptionHtml = true,
+                Description = $"Any application using this API key will immediately lose access: <code>{key.Id}</code>",
                 Action = "Delete",
-                ActionUrl = this.Url.ActionLink(nameof(RemoveAPIKeyPost), values: new { id = id })
+                ActionUrl = Url.ActionLink(nameof(RemoveAPIKeyPost), values: new { id })
             });
         }
 
@@ -81,7 +80,7 @@ public async Task<IActionResult> AddApiKey()
         }
 
         [HttpGet("~/api-keys/authorize")]
-        public async Task<IActionResult> AuthorizeAPIKey( string[] permissions, string applicationName = null, Uri redirect = null,
+        public async Task<IActionResult> AuthorizeAPIKey(string[] permissions, string applicationName = null, Uri redirect = null,
             bool strict = true, bool selectiveStores = false, string applicationIdentifier = null)
         {
             if (!_btcPayServerEnvironment.IsSecure)
@@ -157,16 +156,17 @@ public async Task<IActionResult> AddApiKey()
                     }
 
                     //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()
+                    return View("ConfirmAPIKey",
+                        new AuthorizeApiKeysViewModel()
                         {
-                            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"
+                            ApiKey = key.Id,
+                            RedirectUrl = redirect,
+                            Label = applicationName,
+                            ApplicationName = applicationName,
+                            SelectiveStores = selectiveStores,
+                            Strict = strict,
+                            Permissions = string.Join(';', permissions),
+                            ApplicationIdentifier = applicationIdentifier
                         });
                 }
             }
@@ -255,38 +255,52 @@ public async Task<IActionResult> AuthorizeAPIKey([FromForm] AuthorizeApiKeysView
                 return View(viewModel);
             }
 
-            switch (viewModel.Command.ToLowerInvariant())
+            var command = viewModel.Command.ToLowerInvariant();
+            switch (command)
             {
-                case "no":
+                case "cancel":
                     return RedirectToAction("APIKeys");
-                case "yes":
-                    var key = await CreateKey(viewModel, (viewModel.ApplicationIdentifier, viewModel.RedirectUrl?.Authority));
+
+                case "authorize":
+                case "confirm":
+                    var key = command == "authorize"
+                        ? await CreateKey(viewModel, (viewModel.ApplicationIdentifier, viewModel.RedirectUrl?.Authority))
+                        : await _apiKeyRepository.GetKey(viewModel.ApiKey);
 
                     if (viewModel.RedirectUrl != null)
                     {
-                        return Redirect(GetRedirectToApplicationUrl(viewModel.RedirectUrl, key));
+                        var permissions = key.GetBlob().Permissions;
+                        var redirectVm = new PostRedirectViewModel()
+                        {
+                            FormUrl = viewModel.RedirectUrl.ToString(),
+                            Parameters =
+                            {
+                                new KeyValuePair<string, string>("apiKey", key.Id),
+                                new KeyValuePair<string, string>("userId", key.UserId)
+                            }
+                        };
+                        foreach (var permission in permissions)
+                        {
+                            redirectVm.Parameters.Add(
+                                new KeyValuePair<string, string>("permissions[]", permission));
+                        }
+
+                        return View("PostRedirect", redirectVm);
                     }
 
                     TempData.SetStatusMessageModel(new StatusMessageModel()
                     {
                         Severity = StatusMessageModel.StatusSeverity.Success,
                         Html = $"API key generated! <code class='alert-link'>{key.Id}</code>"
                     });
+
                     return RedirectToAction("APIKeys", new { key = key.Id });
+
                 default:
                     return View(viewModel);
             }
         }
 
-        private string GetRedirectToApplicationUrl(Uri redirect, APIKeyData key)
-        {
-            var uri = new UriBuilder(redirect);
-            var permissions = key.GetBlob().Permissions;
-            BTCPayServerClient.AppendPayloadToQuery(uri,
-                new Dictionary<string, object>() {{"key", key.Id}, {"permissions", permissions}, {"user", key.UserId}});
-            return uri.Uri.AbsoluteUri;
-        }
-
         [HttpPost]
         public async Task<IActionResult> AddApiKey(AddApiKeyViewModel viewModel)
         {
@@ -313,6 +327,7 @@ public async Task<IActionResult> AddApiKey(AddApiKeyViewModel viewModel)
             });
             return RedirectToAction("APIKeys");
         }
+
         private IActionResult HandleCommands(AddApiKeyViewModel viewModel)
         {
             if (string.IsNullOrEmpty(viewModel.Command))
@@ -333,7 +348,6 @@ private IActionResult HandleCommands(AddApiKeyViewModel viewModel)
             switch (command)
             {
                 case "change-store-mode":
-
                     permissionValueItem.StoreMode = permissionValueItem.StoreMode == AddApiKeyViewModel.ApiKeyStoreMode.Specific
                         ? AddApiKeyViewModel.ApiKeyStoreMode.AllStores
                         : AddApiKeyViewModel.ApiKeyStoreMode.Specific;
@@ -344,6 +358,7 @@ private IActionResult HandleCommands(AddApiKeyViewModel viewModel)
                         permissionValueItem.SpecificStores.Add(null);
                     }
                     return View(viewModel);
+
                 case "add-store":
                     permissionValueItem.SpecificStores.Add(null);
                     return View(viewModel);
@@ -501,9 +516,9 @@ public class AuthorizeApiKeysViewModel : AddApiKeyViewModel
             public bool Strict { get; set; }
             public bool SelectiveStores { get; set; }
             public string Permissions { get; set; }
+            public string ApiKey { get; set; }
         }
 
-
         public class ApiKeysViewModel
         {
             public List<APIKeyData> ApiKeyDatas { get; set; }

BTCPayServer/Models/PostRedictViewModel.cs

@@ -6,6 +6,8 @@ public class PostRedirectViewModel
     {
         public string AspAction { get; set; }
         public string AspController { get; set; }
+        public string FormUrl { get; set; }
+
         public List<KeyValuePair<string, string>> Parameters { get; set; } = new List<KeyValuePair<string, string>>();
     }
 }

BTCPayServer/Views/Manage/APIKeys.cshtml

@@ -34,7 +34,7 @@
                     <ul>
                         @foreach (var permission in Permission.ToPermissions(permissions).Select(c => c.ToString()).Distinct().ToArray())
                         {
-                            <li>@permission</li>
+                            <li><code>@permission</code></li>
                         }
                     </ul>
                 }