From e277b9e84e52562e7a702a6c366e310ba3c33ce1 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 1 Nov 2022 09:58:28 -0400 Subject: [PATCH] [SG-419] Fix problems with push notifications on self-host (#2338) * Added "internal" to non-user-based request types to avoid failing validation. * Added handling of unsuccessful response so that JSON parsing eror doesn't occur. * Added logging for token errors. (cherry picked from commit dad143b3e42247bc6b397b60803e25d243bd83a5) * Fixed bug in next auth attempt handling. * Fixed linting. * Added deserialization options to handle case insensitivity. * Added a new method for SendAsync that does not expect a result from the client. * hasJsonResult param to make Send more reusable * some cleanup * fix lint problems * Added launch config for Notifications. * Added Notifications to Full Server config. Co-authored-by: Kyle Spearrin --- .vscode/launch.json | 50 ++++++++++++++++++- .vscode/tasks.json | 16 ++++++ .../SelfHostedSyncSponsorshipsCommand.cs | 13 ++--- .../BaseIdentityClientService.cs | 35 +++++++++---- .../CustomTokenRequestValidator.cs | 3 +- src/Notifications/HubHelpers.cs | 15 +++--- 6 files changed, 109 insertions(+), 23 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index fdbc654f0..7e09ae524 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -40,7 +40,8 @@ "Identity", "Sso", "Icons", - "Billing" + "Billing", + "Notifications" ], "presentation": { "hidden": false, @@ -57,6 +58,7 @@ "EventsProcessor-SelfHost", "Identity-SelfHost", "Sso-SelfHost", + "Notifications-SelfHost" ], "presentation": { "hidden": false, @@ -238,6 +240,28 @@ "/Views": "${workspaceFolder}/Views" } }, + { + "name": "Notifications", + "presentation": { + "hidden": true, + "group": "cloud", + "order": 100 + }, + "requireExactSource": true, + "type": "coreclr", + "request": "launch", + "preLaunchTask": "buildNotifications", + "program": "${workspaceFolder}/src/Notifications/bin/Debug/net6.0/Notifications.dll", + "args": [], + "cwd": "${workspaceFolder}/src/Notifications", + "stopAtEntry": false, + "env": { + "ASPNETCORE_ENVIRONMENT": "Development" + }, + "sourceFileMap": { + "/Views": "${workspaceFolder}/Views" + } + }, { "name": "Identity-SelfHost", "presentation": { @@ -336,6 +360,30 @@ "/Views": "${workspaceFolder}/Views" } }, + { + "name": "Notifications-SelfHost", + "presentation": { + "hidden": true, + "group": "self-host", + "order": 999 + }, + "requireExactSource": true, + "type": "coreclr", + "request": "launch", + "preLaunchTask": "buildNotifications", + "program": "${workspaceFolder}/src/Notifications/bin/Debug/net6.0/Notifications.dll", + "args": [], + "cwd": "${workspaceFolder}/src/Notifications", + "stopAtEntry": false, + "env": { + "ASPNETCORE_ENVIRONMENT": "Development", + "ASPNETCORE_URLS": "http://localhost:61841", + "developSelfHosted": "true" + }, + "sourceFileMap": { + "/Views": "${workspaceFolder}/Views" + } + }, { "name": "EventsProcessor-SelfHost", "presentation": { diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 02e7ddcac..61d1f9240 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -89,6 +89,22 @@ "isDefault": true } }, + { + "label": "buildNotifications", + "command": "dotnet", + "type": "process", + "args": [ + "build", + "${workspaceFolder}/src/Notifications/Notifications.csproj", + "/property:GenerateFullPaths=true", + "/consoleloggerparameters:NoSummary" + ], + "problemMatcher": "$msCompile", + "group": { + "kind": "build", + "isDefault": true + } + }, { "label": "buildBilling", "command": "dotnet", diff --git a/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs b/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs index be588cd4e..c1a3e97e8 100644 --- a/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs @@ -72,12 +72,13 @@ public class SelfHostedSyncSponsorshipsCommand : BaseIdentityClientService, ISel foreach (var orgSponsorshipsBatch in organizationSponsorshipsDict.Values.Chunk(1000)) { - var response = await SendAsync(HttpMethod.Post, "organization/sponsorship/sync", new OrganizationSponsorshipSyncRequestModel - { - BillingSyncKey = billingSyncConfig.BillingSyncKey, - SponsoringOrganizationCloudId = cloudOrganizationId, - SponsorshipsBatch = orgSponsorshipsBatch.Select(s => new OrganizationSponsorshipRequestModel(s)) - }); + var response = await SendAsync( + HttpMethod.Post, "organization/sponsorship/sync", new OrganizationSponsorshipSyncRequestModel + { + BillingSyncKey = billingSyncConfig.BillingSyncKey, + SponsoringOrganizationCloudId = cloudOrganizationId, + SponsorshipsBatch = orgSponsorshipsBatch.Select(s => new OrganizationSponsorshipRequestModel(s)) + }, true); if (response == null) { diff --git a/src/Core/Services/Implementations/BaseIdentityClientService.cs b/src/Core/Services/Implementations/BaseIdentityClientService.cs index 753fc9c0c..2f63a1b7c 100644 --- a/src/Core/Services/Implementations/BaseIdentityClientService.cs +++ b/src/Core/Services/Implementations/BaseIdentityClientService.cs @@ -47,19 +47,21 @@ public abstract class BaseIdentityClientService : IDisposable protected string AccessToken { get; private set; } protected Task SendAsync(HttpMethod method, string path) => - SendAsync(method, path, null); + SendAsync(method, path, null); - protected Task SendAsync(HttpMethod method, string path, TRequest body) => - SendAsync(method, path, body); + protected Task SendAsync(HttpMethod method, string path, TRequest requestModel) => + SendAsync(method, path, requestModel, false); - protected async Task SendAsync(HttpMethod method, string path, TRequest requestModel) + protected async Task SendAsync(HttpMethod method, string path, + TRequest requestModel, bool hasJsonResult) { var fullRequestPath = string.Concat(Client.BaseAddress, path); var tokenStateResponse = await HandleTokenStateAsync(); if (!tokenStateResponse) { - _logger.LogError("Unable to send {method} request to {requestUri} because an access token was unable to be obtained", method.Method, fullRequestPath); + _logger.LogError("Unable to send {method} request to {requestUri} because an access token was unable to be obtained", + method.Method, fullRequestPath); return default; } @@ -71,7 +73,19 @@ public abstract class BaseIdentityClientService : IDisposable try { var response = await Client.SendAsync(message); - return await response.Content.ReadFromJsonAsync(); + if (response.IsSuccessStatusCode) + { + if (hasJsonResult) + { + return await response.Content.ReadFromJsonAsync(); + } + } + else + { + _logger.LogError("Request to {url} is unsuccessful with status of {code}-{reason}", + message.RequestUri.ToString(), response.StatusCode, response.ReasonPhrase); + } + return default; } catch (Exception e) { @@ -82,8 +96,9 @@ public abstract class BaseIdentityClientService : IDisposable protected async Task HandleTokenStateAsync() { - if (_nextAuthAttempt.HasValue && DateTime.UtcNow > _nextAuthAttempt.Value) + if (_nextAuthAttempt.HasValue && DateTime.UtcNow < _nextAuthAttempt.Value) { + _logger.LogInformation("Not requesting a token at {now} because the next request time is {nextAttempt}", DateTime.UtcNow, _nextAuthAttempt.Value); return false; } _nextAuthAttempt = null; @@ -118,12 +133,13 @@ public abstract class BaseIdentityClientService : IDisposable if (response == null) { + _logger.LogError("Empty token response from {identity} for client {clientId} with status {code}-{reason}", IdentityClient.BaseAddress, _identityClientId, response.StatusCode, response.ReasonPhrase); return false; } if (!response.IsSuccessStatusCode) { - _logger.LogInformation("Unsuccessful token response from {identity} for client {clientId} with status code {StatusCode}", IdentityClient.BaseAddress, _identityClientId, response.StatusCode); + _logger.LogError("Unsuccessful token response from {identity} for client {clientId} with status {code}-{reason}", IdentityClient.BaseAddress, _identityClientId, response.StatusCode, response.ReasonPhrase); if (response.StatusCode == HttpStatusCode.BadRequest) { @@ -139,7 +155,8 @@ public abstract class BaseIdentityClientService : IDisposable return false; } - using var jsonDocument = await JsonDocument.ParseAsync(await response.Content.ReadAsStreamAsync()); + var content = await response.Content.ReadAsStreamAsync(); + using var jsonDocument = await JsonDocument.ParseAsync(content); AccessToken = jsonDocument.RootElement.GetProperty("access_token").GetString(); return true; diff --git a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/CustomTokenRequestValidator.cs index b9bf16f58..c817acd68 100644 --- a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/CustomTokenRequestValidator.cs @@ -52,7 +52,8 @@ public class CustomTokenRequestValidator : BaseRequestValidator hubContext, @@ -23,7 +26,7 @@ public static class HubHelpers case PushType.SyncLoginDelete: var cipherNotification = JsonSerializer.Deserialize>( - notificationJson); + notificationJson, _deserializerOptions); if (cipherNotification.Payload.UserId.HasValue) { await hubContext.Clients.User(cipherNotification.Payload.UserId.ToString()) @@ -41,7 +44,7 @@ public static class HubHelpers case PushType.SyncFolderDelete: var folderNotification = JsonSerializer.Deserialize>( - notificationJson); + notificationJson, _deserializerOptions); await hubContext.Clients.User(folderNotification.Payload.UserId.ToString()) .SendAsync("ReceiveMessage", folderNotification, cancellationToken); break; @@ -52,7 +55,7 @@ public static class HubHelpers case PushType.LogOut: var userNotification = JsonSerializer.Deserialize>( - notificationJson); + notificationJson, _deserializerOptions); await hubContext.Clients.User(userNotification.Payload.UserId.ToString()) .SendAsync("ReceiveMessage", userNotification, cancellationToken); break; @@ -61,21 +64,21 @@ public static class HubHelpers case PushType.SyncSendDelete: var sendNotification = JsonSerializer.Deserialize>( - notificationJson); + notificationJson, _deserializerOptions); await hubContext.Clients.User(sendNotification.Payload.UserId.ToString()) .SendAsync("ReceiveMessage", sendNotification, cancellationToken); break; case PushType.AuthRequestResponse: var authRequestResponseNotification = JsonSerializer.Deserialize>( - notificationJson); + notificationJson, _deserializerOptions); await anonymousHubContext.Clients.Group(authRequestResponseNotification.Payload.Id.ToString()) .SendAsync("AuthRequestResponseRecieved", authRequestResponseNotification, cancellationToken); break; case PushType.AuthRequest: var authRequestNotification = JsonSerializer.Deserialize>( - notificationJson); + notificationJson, _deserializerOptions); await hubContext.Clients.User(authRequestNotification.Payload.UserId.ToString()) .SendAsync("ReceiveMessage", authRequestNotification, cancellationToken); break;