Skip to content

Commit

Permalink
[SG-419] Fix problems with push notifications on self-host (#2338)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@trmartin4 @kspearrin
trmartin4 and kspearrin authored Nov 1, 2022
1 parent 14074e1 commit e277b9e
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 23 deletions.
50 changes: 49 additions & 1 deletion .vscode/launch.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
"Identity",
"Sso",
"Icons",
"Billing"
"Billing",
"Notifications"
],
"presentation": {
"hidden": false,
Expand All @@ -57,6 +58,7 @@
"EventsProcessor-SelfHost",
"Identity-SelfHost",
"Sso-SelfHost",
"Notifications-SelfHost"
],
"presentation": {
"hidden": false,
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
16 changes: 16 additions & 0 deletions .vscode/tasks.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 7 additions & 6 deletions ...izationSponsorships/FamiliesForEnterprise/SelfHosted/SelfHostedSyncSponsorshipsCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,13 @@ public async Task SyncOrganization(Guid organizationId, Guid cloudOrganizationId

foreach (var orgSponsorshipsBatch in organizationSponsorshipsDict.Values.Chunk(1000))
{
var response = await SendAsync<OrganizationSponsorshipSyncRequestModel, OrganizationSponsorshipSyncResponseModel>(HttpMethod.Post, "organization/sponsorship/sync", new OrganizationSponsorshipSyncRequestModel
{
BillingSyncKey = billingSyncConfig.BillingSyncKey,
SponsoringOrganizationCloudId = cloudOrganizationId,
SponsorshipsBatch = orgSponsorshipsBatch.Select(s => new OrganizationSponsorshipRequestModel(s))
});
var response = await SendAsync<OrganizationSponsorshipSyncRequestModel, OrganizationSponsorshipSyncResponseModel>(
HttpMethod.Post, "organization/sponsorship/sync", new OrganizationSponsorshipSyncRequestModel
{
BillingSyncKey = billingSyncConfig.BillingSyncKey,
SponsoringOrganizationCloudId = cloudOrganizationId,
SponsorshipsBatch = orgSponsorshipsBatch.Select(s => new OrganizationSponsorshipRequestModel(s))
}, true);

if (response == null)
{
Expand Down
35 changes: 26 additions & 9 deletions src/Core/Services/Implementations/BaseIdentityClientService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,21 @@ public BaseIdentityClientService(
protected string AccessToken { get; private set; }

protected Task SendAsync(HttpMethod method, string path) =>
SendAsync<object, object>(method, path, null);
SendAsync<object>(method, path, null);

protected Task SendAsync<TRequest>(HttpMethod method, string path, TRequest body) =>
SendAsync<TRequest, object>(method, path, body);
protected Task SendAsync<TRequest>(HttpMethod method, string path, TRequest requestModel) =>
SendAsync<TRequest, object>(method, path, requestModel, false);

protected async Task<TResult> SendAsync<TRequest, TResult>(HttpMethod method, string path, TRequest requestModel)
protected async Task<TResult> SendAsync<TRequest, TResult>(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;
}

Expand All @@ -71,7 +73,19 @@ protected async Task<TResult> SendAsync<TRequest, TResult>(HttpMethod method, st
try
{
var response = await Client.SendAsync(message);
return await response.Content.ReadFromJsonAsync<TResult>();
if (response.IsSuccessStatusCode)
{
if (hasJsonResult)
{
return await response.Content.ReadFromJsonAsync<TResult>();
}
}
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)
{
Expand All @@ -82,8 +96,9 @@ protected async Task<TResult> SendAsync<TRequest, TResult>(HttpMethod method, st

protected async Task<bool> 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;
Expand Down Expand Up @@ -118,12 +133,13 @@ protected async Task<bool> HandleTokenStateAsync()

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)
{
Expand All @@ -139,7 +155,8 @@ protected async Task<bool> HandleTokenStateAsync()
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;
Expand Down
3 changes: 2 additions & 1 deletion src/Identity/IdentityServer/CustomTokenRequestValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public async Task ValidateAsync(CustomTokenRequestValidationContext context)
string[] allowedGrantTypes = { "authorization_code", "client_credentials" };
if (!allowedGrantTypes.Contains(context.Result.ValidatedRequest.GrantType)
|| context.Result.ValidatedRequest.ClientId.StartsWith("organization")
|| context.Result.ValidatedRequest.ClientId.StartsWith("installation"))
|| context.Result.ValidatedRequest.ClientId.StartsWith("installation")
|| context.Result.ValidatedRequest.ClientId.StartsWith("internal"))
{
return;
}
Expand Down
15 changes: 9 additions & 6 deletions src/Notifications/HubHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ namespace Bit.Notifications;

public static class HubHelpers
{
private static JsonSerializerOptions _deserializerOptions =
new JsonSerializerOptions { PropertyNameCaseInsensitive = true };

public static async Task SendNotificationToHubAsync(
string notificationJson,
IHubContext<NotificationsHub> hubContext,
Expand All @@ -23,7 +26,7 @@ public static async Task SendNotificationToHubAsync(
case PushType.SyncLoginDelete:
var cipherNotification =
JsonSerializer.Deserialize<PushNotificationData<SyncCipherPushNotification>>(
notificationJson);
notificationJson, _deserializerOptions);
if (cipherNotification.Payload.UserId.HasValue)
{
await hubContext.Clients.User(cipherNotification.Payload.UserId.ToString())
Expand All @@ -41,7 +44,7 @@ await hubContext.Clients.Group(
case PushType.SyncFolderDelete:
var folderNotification =
JsonSerializer.Deserialize<PushNotificationData<SyncFolderPushNotification>>(
notificationJson);
notificationJson, _deserializerOptions);
await hubContext.Clients.User(folderNotification.Payload.UserId.ToString())
.SendAsync("ReceiveMessage", folderNotification, cancellationToken);
break;
Expand All @@ -52,7 +55,7 @@ await hubContext.Clients.User(folderNotification.Payload.UserId.ToString())
case PushType.LogOut:
var userNotification =
JsonSerializer.Deserialize<PushNotificationData<UserPushNotification>>(
notificationJson);
notificationJson, _deserializerOptions);
await hubContext.Clients.User(userNotification.Payload.UserId.ToString())
.SendAsync("ReceiveMessage", userNotification, cancellationToken);
break;
Expand All @@ -61,21 +64,21 @@ await hubContext.Clients.User(userNotification.Payload.UserId.ToString())
case PushType.SyncSendDelete:
var sendNotification =
JsonSerializer.Deserialize<PushNotificationData<SyncSendPushNotification>>(
notificationJson);
notificationJson, _deserializerOptions);
await hubContext.Clients.User(sendNotification.Payload.UserId.ToString())
.SendAsync("ReceiveMessage", sendNotification, cancellationToken);
break;
case PushType.AuthRequestResponse:
var authRequestResponseNotification =
JsonSerializer.Deserialize<PushNotificationData<AuthRequestPushNotification>>(
notificationJson);
notificationJson, _deserializerOptions);
await anonymousHubContext.Clients.Group(authRequestResponseNotification.Payload.Id.ToString())
.SendAsync("AuthRequestResponseRecieved", authRequestResponseNotification, cancellationToken);
break;
case PushType.AuthRequest:
var authRequestNotification =
JsonSerializer.Deserialize<PushNotificationData<AuthRequestPushNotification>>(
notificationJson);
notificationJson, _deserializerOptions);
await hubContext.Clients.User(authRequestNotification.Payload.UserId.ToString())
.SendAsync("ReceiveMessage", authRequestNotification, cancellationToken);
break;
Expand Down

0 comments on commit e277b9e

Please sign in to comment.