Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSOE-620: Fix UI test. #36

Merged
merged 28 commits into from
Jul 1, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4fbae74
See if the GH Windows test just takes longer.
sarahelsaig Jun 26, 2023
f4b4808
Validate the actual API response before looking at the Tenants page.
sarahelsaig Jun 26, 2023
e79f029
Merge remote-tracking branch 'origin/dev' into issue/OSOE-620
sarahelsaig Jun 26, 2023
794f596
Do retries.
sarahelsaig Jun 26, 2023
b533ba9
Refactor the code in order to remove this break statement.
sarahelsaig Jun 26, 2023
90d3929
Refactor the code in order to remove this break statement.
sarahelsaig Jun 26, 2023
f9eacbe
More diagnostic data
sarahelsaig Jun 26, 2023
5811c52
Actually reload.
sarahelsaig Jun 26, 2023
df36cc3
Navigate to the API response URL first.
sarahelsaig Jun 26, 2023
5f7f38b
Remove unnecessary check, as we aren't here to test the OC admin UI.
sarahelsaig Jun 26, 2023
0b8ffd0
Is the trailig slash needed maybe?
sarahelsaig Jun 27, 2023
9318e01
Make client ID handling clearer.
sarahelsaig Jun 28, 2023
9ed3fd8
Clean up default init a bit more.
sarahelsaig Jun 28, 2023
1a9f487
Clean up default init a bit more.
sarahelsaig Jun 28, 2023
5b7bbbc
Verify that the recipe has successfully created the application.
sarahelsaig Jun 28, 2023
0444d0d
Log in first.
sarahelsaig Jun 28, 2023
cdb681b
Trim token
sarahelsaig Jun 28, 2023
70b001a
pragma warning disable
sarahelsaig Jun 28, 2023
5e28986
Refactor and DRY ConfigurableCertificateValidatingHttpClientHandler
sarahelsaig Jun 28, 2023
db53064
Simplify HttpClient creation.
sarahelsaig Jun 29, 2023
691c112
Make the body buffered in all JSON-bodied API requests.
sarahelsaig Jun 29, 2023
23df222
Use the auth type provided by the token itself.
sarahelsaig Jun 29, 2023
18051ce
Assert logs first, in case we missed something.
sarahelsaig Jun 29, 2023
7362a95
Temporarily change Create address.
sarahelsaig Jun 29, 2023
2f54c20
Disable access token encryption.
sarahelsaig Jun 30, 2023
d36b8f1
Reconfigure and re-export OpenIdServerSettings.
sarahelsaig Jun 30, 2023
c8207d4
Restore API URL.
sarahelsaig Jun 30, 2023
2dff4f5
Additional comment about the Windows GH problem.
sarahelsaig Jun 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Lombiq.Tests.UI.Services;
using OpenQA.Selenium;
using Shouldly;
using System;
using System.Linq;
using System.Threading.Tasks;

Expand All @@ -25,6 +26,16 @@ public static async Task TestOrchardCoreApiClientBehaviorAsync(
? "SqlConnection"
: "Sqlite";

var isDefaultClient = string.IsNullOrEmpty(clientId);
if (isDefaultClient)
{
// If the client ID is not set, change both ID and secret to the default.
#pragma warning disable S1226 // Introduce a new variable instead of reusing the parameter.
clientId = "UITest";
clientSecret = "Password";
#pragma warning restore S1226 // Introduce a new variable instead of reusing the parameter.
}

var createApiModel = new TenantApiModel
{
Description = "Tenant created by UI test",
Expand Down Expand Up @@ -64,18 +75,27 @@ public static async Task TestOrchardCoreApiClientBehaviorAsync(

using var apiClient = new ApiClient(new ApiClientSettings
{
ClientId = clientId ?? "UITest",
ClientSecret = clientSecret ?? "Password",
ClientId = clientId,
ClientSecret = clientSecret,
DefaultTenantUri = context.Scope.BaseUri,
DisableCertificateValidation = true,
});

if (string.IsNullOrEmpty(clientId))
const string defaultClientRecipe = "Lombiq.OrchardCoreApiClient.Tests.UI.OpenId";
context.Scope.AtataContext.Log.Info("Executing the default client recipe \"{0}\": {1}", defaultClientRecipe, isDefaultClient);
if (isDefaultClient)
{
await context.ExecuteRecipeDirectlyAsync("Lombiq.OrchardCoreApiClient.Tests.UI.OpenId");
}
await context.ExecuteRecipeDirectlyAsync(defaultClientRecipe);

await context.SignInDirectlyAsync();
// Verify that the recipe has successfully created the application.
await context.SignInDirectlyAsync();
await context.GoToAdminRelativeUrlAsync("/OpenId/Application/Edit/1");
context.Get(By.Name("ClientId")).GetAttribute("value").ShouldBe(clientId);
}
else
{
await context.SignInDirectlyAsync();
}

await TestTenantCreateAsync(context, apiClient, createApiModel);
await TestTenantSetupAsync(context, apiClient, createApiModel, setupApiModel);
Expand All @@ -89,10 +109,18 @@ private static async Task TestTenantCreateAsync(
ApiClient apiClient,
TenantApiModel createApiModel)
{
await apiClient.OrchardCoreApi.CreateAsync(createApiModel);

await context.GoToAdminRelativeUrlAsync("/Tenants");
context.Exists(By.LinkText(createApiModel.Name));
using (var response = await apiClient.OrchardCoreApi.CreateAsync(createApiModel))
{
await context.AssertLogsAsync();
response.Error.ShouldBeNull(
$"Tenant creation failed with status code {response.StatusCode}. Content: {response.Error?.Content}\n" +
$"Request: {response.RequestMessage}\nDriver URL: {context.Driver.Url}");

// Check if response URL is valid, and visit it (should be the tenant setup page and not 404 error).
var responseUrl = new Uri(response.Content);
responseUrl.AbsolutePath.ShouldBe($"/{createApiModel.Name}", StringCompareShould.IgnoreCase);
await context.GoToAbsoluteUrlAsync(responseUrl);
}

await GoToTenantEditorAndAssertCommonTenantFieldsAsync(context, createApiModel);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,30 @@
},
{
"name": "OpenIdServerSettings",
"TestingModeEnabled": false,
"AccessTokenFormat": "JsonWebToken",
"Authority": "",
"SigningCertificateStoreLocation": "",
"SigningCertificateStoreName": "",
"SigningCertificateThumbprint": "",
"EncryptionCertificateStoreLocation": "",
"EncryptionCertificateStoreName": "",
"EncryptionCertificateThumbprint": "",
"AccessTokenFormat": 0,
"Authority": null,
"DisableAccessTokenEncryption": false,
"EncryptionCertificateStoreLocation": null,
"EncryptionCertificateStoreName": null,
"EncryptionCertificateThumbprint": null,
"SigningCertificateStoreLocation": null,
"SigningCertificateStoreName": null,
"SigningCertificateThumbprint": null,
"EnableTokenEndpoint": true,
"EnableAuthorizationEndpoint": false,
"EnableLogoutEndpoint": false,
"EnableUserInfoEndpoint": false,
"EnableIntrospectionEndpoint": false,
"EnableRevocationEndpoint": false,
"AllowPasswordFlow": false,
"AllowClientCredentialsFlow": true,
"AllowAuthorizationCodeFlow": false,
"AllowRefreshTokenFlow": false,
"AllowImplicitFlow": false
"AllowHybridFlow": false,
"AllowImplicitFlow": false,
"DisableRollingRefreshTokens": true,
"RequireProofKeyForCodeExchange": false,
"UseReferenceAccessTokens": true
},
Psichorex marked this conversation as resolved.
Show resolved Hide resolved
{
"name": "openidapplication",
Expand Down
21 changes: 2 additions & 19 deletions Lombiq.OrchardCoreApiClient/ApiClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,14 @@ public class ApiClient : IDisposable
{
private readonly Lazy<IOrchardCoreApi> _lazyOrchardCoreApi;

private ConfigurableCertificateValidatingHttpClientHandler _certificateValidatingHandler;
private HttpClient _httpClient;

public IOrchardCoreApi OrchardCoreApi => _lazyOrchardCoreApi.Value;

public ApiClient(ApiClientSettings apiClientSettings) =>
_lazyOrchardCoreApi = new(() =>
{
_certificateValidatingHandler = new ConfigurableCertificateValidatingHttpClientHandler(apiClientSettings);

// It's only disabled optionally, like for local testing.
#pragma warning disable CA5399 // HttpClient is created without enabling CheckCertificateRevocationList
_httpClient = new HttpClient(_certificateValidatingHandler)
{
BaseAddress = apiClientSettings.DefaultTenantUri,
};
#pragma warning restore CA5399
_httpClient = ConfigurableCertificateValidatingHttpClientHandler.CreateClient(apiClientSettings);

// We use Newtonsoft Json.NET because Orchard Core uses it too, so the models will behave the same.
return RefitHelper.WithNewtonsoftJson<IOrchardCoreApi>(_httpClient);
Expand Down Expand Up @@ -75,18 +66,10 @@ public void Dispose()

protected virtual void Dispose(bool disposing)
{
if (!_lazyOrchardCoreApi.IsValueCreated) return;

if (_httpClient != null)
if (_lazyOrchardCoreApi.IsValueCreated && _httpClient != null)
{
_httpClient.Dispose();
_httpClient = null;
}

if (_certificateValidatingHandler != null)
{
_certificateValidatingHandler.Dispose();
_certificateValidatingHandler = null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,24 @@
namespace Lombiq.OrchardCoreApiClient;

[SuppressMessage(
"Security",
"SCS0004: Certificate Validation has been disabled.",
Justification = "It's only disabled optionally, like for local testing.")]
"Security",
"SCS0004: Certificate Validation has been disabled.",
Justification = "It's only disabled optionally, like for local testing.")]
[SuppressMessage(
"Security",
"S4830: Enable server certificate validation on this SSL/TLS connection",
Justification = "It's only disabled optionally, like for local testing.")]
"Security",
"S4830: Enable server certificate validation on this SSL/TLS connection",
Justification = "It's only disabled optionally, like for local testing.")]
internal sealed class ConfigurableCertificateValidatingHttpClientHandler : HttpClientHandler
{
private readonly ApiClientSettings _apiClientSettings;

private DateTime _expirationDateUtc = DateTime.MinValue;
private Token _tokenResponse;

public ConfigurableCertificateValidatingHttpClientHandler(ApiClientSettings apiClientSettings)
private ConfigurableCertificateValidatingHttpClientHandler(ApiClientSettings apiClientSettings)
{
_apiClientSettings = apiClientSettings;

if (_apiClientSettings.DisableCertificateValidation)
{
ServerCertificateCustomValidationCallback = (_, _, _, _) => true;
}
ApplyCertificationValidationSetting(this, _apiClientSettings);
}

protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
Expand All @@ -47,21 +43,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage

if (_expirationDateUtc < DateTime.UtcNow.AddSeconds(60))
{
using var handler = new HttpClientHandler();

if (_apiClientSettings.DisableCertificateValidation)
{
handler.ServerCertificateCustomValidationCallback = (_, _, _, _) => true;
handler.CheckCertificateRevocationList = true;
}

// It's only disabled optionally, like for local testing.
#pragma warning disable CA5400 // Ensure HttpClient certificate revocation list check is not disabled
using var httpClient = new HttpClient(handler)
{
BaseAddress = _apiClientSettings.DefaultTenantUri,
};
#pragma warning restore CA5400
using var httpClient = CreateClient(createBasicHandler: true, _apiClientSettings);

Token tokenResponse;
try
Expand Down Expand Up @@ -96,8 +78,43 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
}

request.Headers.Authorization = new AuthenticationHeaderValue(
request.Headers.Authorization.Scheme, _tokenResponse.AccessToken);
_tokenResponse.TokenType,
_tokenResponse.AccessToken);

return await base.SendAsync(request, cancellationToken);
}

[SuppressMessage(
"Reliability",
"CA2000:Dispose objects before losing scope",
Justification = "Disposed by the HttpClient.")]
[SuppressMessage(
"Security",
"CA5400:HttpClient may be created without enabling CheckCertificateRevocationList",
Justification = "It's only disabled optionally, like for local testing.")]
private static HttpClient CreateClient(bool createBasicHandler, ApiClientSettings settings) =>
new(ApplyCertificationValidationSetting(
createBasicHandler
? new HttpClientHandler()
: new ConfigurableCertificateValidatingHttpClientHandler(settings),
settings))
{
BaseAddress = settings.DefaultTenantUri,
};

public static HttpClient CreateClient(ApiClientSettings settings) =>
CreateClient(createBasicHandler: false, settings);

public static HttpClientHandler ApplyCertificationValidationSetting(
HttpClientHandler handler,
ApiClientSettings settings)
{
if (settings.DisableCertificateValidation)
{
handler.ServerCertificateCustomValidationCallback = (_, _, _, _) => true;
handler.CheckCertificateRevocationList = true;
}

return handler;
}
}
10 changes: 5 additions & 5 deletions Lombiq.OrchardCoreApiClient/Interfaces/IOrchardCoreApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ public interface IOrchardCoreApi
/// The necessary parameters to create a tenant: Name, RequestUrlPrefix etc.
/// </param>
/// <returns>The response of the tenant creation.</returns>
[Post("/api/tenants/create")]
[Post("/api/tenants/create/")]
[Headers(AuthorizationBearer)]
Task<ApiResponse<string>> CreateAsync([Body] TenantApiModel createTenantParameters);
Task<ApiResponse<string>> CreateAsync([Body(buffered: true)] TenantApiModel createTenantParameters);

/// <summary>
/// Setup the previously created tenant in Orchard Core.
Expand All @@ -31,7 +31,7 @@ public interface IOrchardCoreApi
/// <returns>The response of the tenant setup.</returns>
[Post("/api/tenants/setup")]
[Headers(AuthorizationBearer)]
Task<string> SetupAsync([Body] TenantSetupApiModel setupTenantParameters);
Task<string> SetupAsync([Body(buffered: true)] TenantSetupApiModel setupTenantParameters);

/// <summary>
/// Edit a previously created tenant in Orchard Core.
Expand All @@ -40,7 +40,7 @@ public interface IOrchardCoreApi
/// <returns>The response of the tenant edit.</returns>
[Post("/api/tenants/edit")]
[Headers(AuthorizationBearer)]
Task<string> EditAsync([Body] TenantApiModel editTenantParameters);
Task<string> EditAsync([Body(buffered: true)] TenantApiModel editTenantParameters);

/// <summary>
/// Edit a previously created tenant in Orchard Core with additional settings inside
Expand All @@ -52,7 +52,7 @@ public interface IOrchardCoreApi
[Post("/api/tenants/custom-edit")]
[Headers(AuthorizationBearer)]
[Obsolete("This shouldn't be here and will be soon removed, see https://github.com/Lombiq/Orchard-Core-API-Client/issues/30.")]
Task<string> CustomEditAsync([Body] TenantApiModel editTenantParameters);
Task<string> CustomEditAsync([Body(buffered: true)] TenantApiModel editTenantParameters);

/// <summary>
/// Remove a previously created tenant in Orchard Core.
Expand Down