Skip to content

Commit 3685f14

Browse files
pmachapmanpmachapman
authored
SF-3730 Require authentication for SignalR notifications (#3735)
1 parent 3bb4468 commit 3685f14

7 files changed

Lines changed: 78 additions & 93 deletions

File tree

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/project-notification.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export class ProjectNotificationService {
8585
}
8686

8787
async subscribeToProject(projectId: string): Promise<void> {
88-
await this.connection.send('subscribeToProject', projectId).catch(err => {
88+
await this.connection.invoke('subscribeToProject', projectId).catch(err => {
8989
// This error is thrown when a user navigates away quickly after starting the sync
9090
if (err.message === "Cannot send data if the connection is not in the 'Connected' State.") {
9191
return;

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-notification.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class DraftNotificationService {
7070
}
7171

7272
async subscribeToProject(projectId: string): Promise<void> {
73-
await this.connection.send('subscribeToProject', projectId).catch(err => {
73+
await this.connection.invoke('subscribeToProject', projectId).catch(err => {
7474
// This error is thrown when a user navigates away quickly after starting the sync
7575
if (err.message === "Cannot send data if the connection is not in the 'Connected' State.") {
7676
return;

‎src/SIL.XForge.Scripture/Services/DraftNotificationHub.cs‎

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,10 @@
1-
using System.Threading.Tasks;
2-
using Microsoft.AspNetCore.Authorization;
3-
using Microsoft.AspNetCore.SignalR;
4-
using SIL.XForge.Scripture.Models;
1+
using SIL.XForge.Realtime;
52

63
namespace SIL.XForge.Scripture.Services;
74

8-
[Authorize]
9-
public class DraftNotificationHub : Hub<IDraftNotifier>, IDraftNotifier
10-
{
11-
/// <summary>
12-
/// Notifies subscribers to a project of draft application progress.
13-
/// </summary>
14-
/// <param name="projectId">The Scripture Forge project identifier.</param>
15-
/// <param name="draftApplyState">The state of the draft being applied.</param>
16-
/// <returns>The asynchronous task.</returns>
17-
/// <remarks>
18-
/// This differs from the implementation in <see cref="NotificationHub"/> in that this version
19-
/// does have stateful reconnection, and so there is a guarantee that it is received by clients.
20-
///
21-
/// This is a blocking operation if the stateful reconnection buffer is full, so it should only
22-
/// be subscribed to by the user performing the draft import. Using <see cref="NotificationHub"/>
23-
/// is sufficient for all other users to subscribe to, although they will not receive all draft
24-
/// progress notifications, only the final success message.
25-
/// </remarks>
26-
public async Task NotifyDraftApplyProgress(string projectId, DraftApplyState draftApplyState) =>
27-
await Clients.Group(projectId).NotifyDraftApplyProgress(projectId, draftApplyState);
28-
29-
/// <summary>
30-
/// Subscribe to notifications for a project.
31-
///
32-
/// This is called from the frontend via <c>project-notification.service.ts</c>.
33-
/// </summary>
34-
/// <param name="projectId">The Scripture Forge project identifier.</param>
35-
/// <returns>The asynchronous task.</returns>
36-
public async Task SubscribeToProject(string projectId) =>
37-
await Groups.AddToGroupAsync(Context.ConnectionId, projectId);
38-
}
5+
/// <summary>
6+
/// The SignalR notification hub for apply draft notifications.
7+
/// </summary>
8+
/// <param name="realtimeService">The realtime service.</param>
9+
public class DraftNotificationHub(IRealtimeService realtimeService)
10+
: NotificationHubBase<IDraftNotifier>(realtimeService) { }

‎src/SIL.XForge.Scripture/Services/IDraftNotifier.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,4 @@ namespace SIL.XForge.Scripture.Services;
66
public interface IDraftNotifier
77
{
88
Task NotifyDraftApplyProgress(string sfProjectId, DraftApplyState draftApplyState);
9-
Task SubscribeToProject(string projectId);
109
}

‎src/SIL.XForge.Scripture/Services/INotifier.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,4 @@ public interface INotifier
88
Task NotifyBuildProgress(string sfProjectId, ServalBuildState buildState);
99
Task NotifyDraftApplyProgress(string sfProjectId, DraftApplyState draftApplyState);
1010
Task NotifySyncProgress(string sfProjectId, ProgressState progressState);
11-
Task SubscribeToProject(string projectId);
1211
}

‎src/SIL.XForge.Scripture/Services/NotificationHub.cs‎

Lines changed: 6 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,9 @@
1-
using System.Threading.Tasks;
2-
using Microsoft.AspNetCore.Authorization;
3-
using Microsoft.AspNetCore.SignalR;
4-
using SIL.XForge.Scripture.Models;
1+
using SIL.XForge.Realtime;
52

63
namespace SIL.XForge.Scripture.Services;
74

8-
[Authorize]
9-
public class NotificationHub : Hub<INotifier>, INotifier
10-
{
11-
/// <summary>
12-
/// Notifies subscribers to a project of draft build progress.
13-
/// </summary>
14-
/// <param name="projectId">The Scripture Forge project identifier.</param>
15-
/// <param name="buildState">The build state from Serval.</param>
16-
/// <returns>The asynchronous task.</returns>
17-
/// <remarks>
18-
/// This will currently be emitted on the TranslationBuildStarted and TranslationBuildFinished webhooks,
19-
/// and when the draft pre-translations have been retrieved.
20-
/// </remarks>
21-
public async Task NotifyBuildProgress(string projectId, ServalBuildState buildState) =>
22-
await Clients.Group(projectId).NotifyBuildProgress(projectId, buildState);
23-
24-
/// <summary>
25-
/// Notifies subscribers to a project of draft application progress.
26-
/// </summary>
27-
/// <param name="projectId">The Scripture Forge project identifier.</param>
28-
/// <param name="draftApplyState">The state of the draft being applied.</param>
29-
/// <returns>The asynchronous task.</returns>
30-
/// <remarks>
31-
/// This differs from the implementation in <see cref="DraftNotificationHub"/> in that this version
32-
/// does not have stateful reconnection, and so there is no guarantee that the message is received.
33-
/// </remarks>
34-
public async Task NotifyDraftApplyProgress(string projectId, DraftApplyState draftApplyState) =>
35-
await Clients.Group(projectId).NotifyDraftApplyProgress(projectId, draftApplyState);
36-
37-
/// <summary>
38-
/// Notifies subscribers to a project of sync progress.
39-
/// </summary>
40-
/// <param name="projectId">The Scripture Forge project identifier.</param>
41-
/// <param name="progressState">
42-
/// The progress state, including a string value (Paratext only - not used in SF), or percentage value.
43-
/// </param>
44-
/// <returns>The asynchronous task.</returns>
45-
public async Task NotifySyncProgress(string projectId, ProgressState progressState) =>
46-
await Clients.Group(projectId).NotifySyncProgress(projectId, progressState);
47-
48-
/// <summary>
49-
/// Subscribe to notifications for a project.
50-
///
51-
/// This is called from the frontend via <c>project-notification.service.ts</c>.
52-
/// </summary>
53-
/// <param name="projectId">The Scripture Forge project identifier.</param>
54-
/// <returns>The asynchronous task.</returns>
55-
public async Task SubscribeToProject(string projectId) =>
56-
await Groups.AddToGroupAsync(Context.ConnectionId, projectId);
57-
}
5+
/// <summary>
6+
/// The SignalR notification hub for sync and draft notifications.
7+
/// </summary>
8+
/// <param name="realtimeService">The realtime service.</param>
9+
public class NotificationHub(IRealtimeService realtimeService) : NotificationHubBase<INotifier>(realtimeService) { }

‎src/SIL.XForge.Scripture/Services/NotificationHubBase.cs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using Microsoft.AspNetCore.Authorization;
4+
using Microsoft.AspNetCore.SignalR;
5+
using SIL.XForge.Realtime;
6+
using SIL.XForge.Scripture.Models;
7+
using SIL.XForge.Services;
8+
using SIL.XForge.Utils;
9+
10+
namespace SIL.XForge.Scripture.Services;
11+
12+
/// <summary>
13+
/// Base class for SignalR notification hubs, providing shared project subscription and
14+
/// permission checking to ensure only authorized users with Paratext roles receive notifications.
15+
/// </summary>
16+
[Authorize]
17+
public abstract class NotificationHubBase<T>(IRealtimeService realtimeService) : Hub<T>
18+
where T : class
19+
{
20+
/// <summary>
21+
/// Subscribe to notifications for a project.
22+
///
23+
/// This is called from the frontend via <c>project-notification.service.ts</c>.
24+
/// </summary>
25+
/// <param name="projectId">The Scripture Forge project identifier.</param>
26+
/// <returns>The asynchronous task.</returns>
27+
public async Task SubscribeToProject(string projectId)
28+
{
29+
await EnsurePermissionAsync(projectId);
30+
await Groups.AddToGroupAsync(Context.ConnectionId, projectId);
31+
}
32+
33+
/// <summary>
34+
/// Ensures that the user has permission to access the project for SignalR notifications.
35+
/// </summary>
36+
/// <param name="projectId">The Scripture Forge project identifier.</param>
37+
/// <exception cref="DataNotFoundException">The project does not exist.</exception>
38+
/// <exception cref="ForbiddenException">
39+
/// The user does not have permission to access the project.
40+
/// </exception>
41+
protected async Task EnsurePermissionAsync(string projectId)
42+
{
43+
// Load the project from the realtime service
44+
Attempt<SFProject> attempt = await realtimeService.TryGetSnapshotAsync<SFProject>(projectId);
45+
if (!attempt.TryResult(out SFProject project))
46+
{
47+
throw new DataNotFoundException("The project does not exist.");
48+
}
49+
50+
// Retrieve the user identifier
51+
string? userId = Context.GetHttpContext()?.User.FindFirst(XFClaimTypes.UserId)?.Value;
52+
if (string.IsNullOrWhiteSpace(userId))
53+
{
54+
throw new UnauthorizedAccessException();
55+
}
56+
57+
// Ensure the user is on the project, and has a Paratext role
58+
if (!project.UserRoles.TryGetValue(userId, out string? role) || !SFProjectRole.IsParatextRole(role))
59+
{
60+
throw new ForbiddenException();
61+
}
62+
}
63+
}

0 commit comments

Comments
 (0)