Greenfield: Rename API key redirect params; switch to POST body (#1898)

* Rename user param to userId in API key redirect

This way it is clearer what to expect and it also make the parameteer easier to consume.

* Post redirect: Allow form url and prettify page

- Form URL as alternative to controller/action for external URLs
- Making it look nice and add explanation for non-JS case

* APIKeys: Minor view updates

fix

* APIKeys: Use POST redirect for confirmation

fix

* UI: Minor update to confirm view

Tidies it up and adapts to the newly added ConfirmAPIKeys view.

* APIKeys: Update delete view

Structures the information in title and description better.

* APIKeys: Distinguish authorize and confirm (reuse)

* Upgrade ChromeDriver

* Test fixes

* Clean up PostRedirect view

By adding missing forgery token

* Re-add tests for callback post values

* Rename key param to apiKey in API key redirect

* Update BTCPayServer/wwwroot/swagger/v1/swagger.template.authorization.json

Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>

* Use DEBUG conditional for postredirect-callback-test route

* Remove unnecessary ChromeDriver references

* Add debug flag

* Remove debug flags

Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>
This commit is contained in:
Dennis Reimann 2020-09-17 11:37:49 +02:00 committed by GitHub
parent 8ba852084e
commit 7e60328cff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 181 additions and 111 deletions

View File

@ -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.Security.GreenField;
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 @@ namespace BTCPayServer.Tests
tester.PayTester.HttpClient);
});
//let's test the authorized screen now
//options for authorize are:
//applicationName
@ -120,28 +119,27 @@ namespace BTCPayServer.Tests
//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 @@ namespace BTCPayServer.Tests
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 @@ namespace BTCPayServer.Tests
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>();
}
}
}

View File

@ -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>

View File

@ -308,8 +308,9 @@ namespace BTCPayServer.Tests
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];

View File

@ -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" />

View File

@ -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.Security;
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 @@ namespace BTCPayServer.Controllers
return View();
}
[HttpPost]
[Route("translate")]
public async Task<IActionResult> BitpayTranslator(BitpayTranslatorViewModel vm)
@ -199,6 +200,18 @@ namespace BTCPayServer.Controllers
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 });

View File

@ -7,9 +7,7 @@ using BTCPayServer.Client;
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 @@ namespace BTCPayServer.Controllers
}
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 @@ namespace BTCPayServer.Controllers
}
[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 @@ namespace BTCPayServer.Controllers
}
//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,16 +255,37 @@ namespace BTCPayServer.Controllers
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()
@ -272,21 +293,14 @@ namespace BTCPayServer.Controllers
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 @@ namespace BTCPayServer.Controllers
});
return RedirectToAction("APIKeys");
}
private IActionResult HandleCommands(AddApiKeyViewModel viewModel)
{
if (string.IsNullOrEmpty(viewModel.Command))
@ -333,7 +348,6 @@ namespace BTCPayServer.Controllers
switch (command)
{
case "change-store-mode":
permissionValueItem.StoreMode = permissionValueItem.StoreMode == AddApiKeyViewModel.ApiKeyStoreMode.Specific
? AddApiKeyViewModel.ApiKeyStoreMode.AllStores
: AddApiKeyViewModel.ApiKeyStoreMode.Specific;
@ -344,6 +358,7 @@ namespace BTCPayServer.Controllers
permissionValueItem.SpecificStores.Add(null);
}
return View(viewModel);
case "add-store":
permissionValueItem.SpecificStores.Add(null);
return View(viewModel);
@ -501,9 +516,9 @@ namespace BTCPayServer.Controllers
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; }

View File

@ -6,6 +6,8 @@ namespace BTCPayServer.Models
{
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>>();
}
}

View File

@ -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>
}

View File

@ -18,16 +18,15 @@
<input type="hidden" asp-for="SelectiveStores" value="@Model.SelectiveStores"/>
<input type="hidden" asp-for="ApplicationIdentifier" value="@Model.ApplicationIdentifier"/>
<section>
<div class="card container">
<div class="container">
<div class="row">
<div class="col-lg-12 section-heading">
<h2>Authorization Request</h2>
<hr class="primary">
<p class="mb-1">@(Model.ApplicationName ?? "An application") is requesting access to your account.</p>
<p class="my-3">@(Model.ApplicationName ?? "An application") is requesting access to your account.</p>
@if (Model.RedirectUrl != null)
{
<p class="mb-1 alert alert-info">
If authorized, the generated API key will be provided to <strong>@Model.RedirectUrl.AbsoluteUri</strong>
If authorized, the generated API key will be provided to <strong>@Model.RedirectUrl.AbsoluteUri</strong>
</p>
}
</div>
@ -174,13 +173,10 @@
}
</div>
</div>
<div class="row my-2">
<div class="row my-4">
<div class="col-lg-12 text-center">
<div class="btn-group">
<button class="btn btn-primary" name="command" id="consent-yes" type="submit" value="Yes">Authorize app</button>
</div>
<button class="btn btn-secondary" id="consent-no" name="command" type="submit" value="No">Cancel</button>
<button class="btn btn-primary mx-2" name="command" id="consent-yes" type="submit" value="Authorize">Authorize app</button>
<button class="btn btn-secondary mx-2" name="command" id="consent-no" type="submit" value="Cancel">Cancel</button>
</div>
</div>
</div>

View File

@ -0,0 +1,41 @@
@model BTCPayServer.Controllers.ManageController.AuthorizeApiKeysViewModel
@{
var displayName = Model.ApplicationName ?? Model.ApplicationIdentifier;
ViewData["Title"] = $"Are you sure about exposing your API Key to {displayName}?";
Layout = null;
}
<!DOCTYPE html>
<html lang="en">
<head>
<partial name="Header" />
</head>
<body class="bg-light">
<div class="modal-dialog modal-dialog-centered">
<div class="modal-content">
<div class="modal-header">
<h4 class="modal-title w-100 text-center">@ViewData["Title"]</h4>
</div>
<div class="modal-body text-center">
You've previously generated the API Key <code>@Model.ApiKey</code> specifically for
<strong>@displayName</strong> with the URL <code>@Model.RedirectUrl</code>.
</div>
<form method="post" class="modal-footer justify-content-center" asp-controller="Manage" asp-action="AuthorizeAPIKey">
<input type="hidden" asp-for="ApplicationName" value="@Model.ApplicationName"/>
<input type="hidden" asp-for="ApplicationIdentifier" value="@Model.ApplicationIdentifier"/>
<input type="hidden" asp-for="ApiKey" value="@Model.ApiKey"/>
<input type="hidden" asp-for="Strict" value="@Model.Strict"/>
<input type="hidden" asp-for="RedirectUrl" value="@Model.RedirectUrl"/>
<input type="hidden" asp-for="Permissions" value="@Model.Permissions"/>
<input type="hidden" asp-for="SelectiveStores" value="@Model.SelectiveStores"/>
<button type="submit" class="btn btn-primary w-25 mx-2" id="continue" name="command" value="Confirm">Confirm</button>
<button type="submit" class="btn btn-secondary w-25 mx-2" onclick="history.back(); return false;">Go back</button>
</form>
</div>
</div>
</body>
</html>

View File

@ -16,33 +16,25 @@
<div class="modal-header">
<h4 class="modal-title w-100 text-center">@Model.Title</h4>
</div>
<div class="modal-body">
<div class="row">
<div class="col-lg-12 text-center">
<p>
@if (Model.DescriptionHtml)
{
@Safe.Raw(Model.Description)
}
else
{
@Model.Description
}
</p>
</div>
</div>
@if (!String.IsNullOrEmpty(Model.Action))
<div class="modal-body text-center">
@if (Model.DescriptionHtml)
{
<div class="row">
<div class="col-lg-12 text-center">
<form method="post" action="@Model.ActionUrl">
<button id="continue" type="submit" class="btn @Model.ButtonClass w-25">@Model.Action</button>
<button type="submit" class="btn btn-secondary w-25" onclick="history.back(); return false;">Go back</button>
</form>
</div>
</div>
@Safe.Raw(Model.Description)
}
else
{
@Model.Description
}
</div>
@if (!String.IsNullOrEmpty(Model.Action))
{
<form method="post" class="modal-footer justify-content-center" action="@Model.ActionUrl">
<button type="submit" class="btn @Model.ButtonClass w-25 mx-2" id="continue">@Model.Action</button>
<button type="submit" class="btn btn-secondary w-25 mx-2" onclick="history.back(); return false;">Go back</button>
</form>
}
</div>
</div>
</body>

View File

@ -8,22 +8,34 @@
{
routeParams["walletId"] = routeData.Values["walletId"]?.ToString();
}
var action = Model.FormUrl ?? Url.Action(Model.AspAction, Model.AspController, routeParams);
}
<html>
<head></head>
<html lang="en">
<head>
<partial name="Header" />
<title>Post Redirect</title>
</head>
<body>
<form
method="post"
id="postform"
asp-action="@Model.AspAction"
asp-controller="@Model.AspController"
asp-all-route-data="@routeParams"
>
@foreach(var o in Model.Parameters) {
<input type="hidden" name="@o.Key" value="@o.Value" />
}
</form>
<div class="modal-dialog modal-dialog-centered">
<div class="modal-content">
<form method="post" id="postform" action="@action" class="modal-body text-center my-3">
@Html.AntiForgeryToken()
@foreach (var o in Model.Parameters)
{
<input type="hidden" name="@o.Key" value="@o.Value"/>
}
<noscript>
<p>
This redirection page is supposed to be submitted automatically.
<br>
Since you have not enabled JavaScript, please submit manually.
</p>
<button class="btn btn-primary" type="submit">Submit</button>
</noscript>
</form>
</div>
</div>
<script type="text/javascript">
document.forms.item(0).submit();
</script>

View File

@ -86,7 +86,7 @@
}
},
"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"
"description": "Makes browser do an HTTP POST request to the specified url in `redirect` with a JSON body consisting of `apiKey` (the api key created or matched), `permissions` (the permissions the user consented to), and `userId` (the id of the user that consented) upon consent"
}
},
"security": []