Skip to content

Commit 6cd2b77

Browse files
committed
refactor: The API should now consistently enforce authentication and correctly scope data and settings according to the authenticated user: __Key Changes:__
- All data and settings routes now require user authentication. Unauthenticated access will result in a 401 error. - Settings routes have been moved from `/api/v1/users/me/settings/` to `/api/v1/users/{userId}/settings/`. Access is restricted so users can only manage their own settings (requests where `{userId}` in the path does not match the authenticated user's ID will result in a 403 Forbidden error). - Generic data routes (`/api/v1/data/...`) now operate based on the `ModelConfig.ownership` setting: - For global models (e.g., Headlines, Categories), data is fetched globally but requires authentication to access. - For future user-owned models, data operations will be scoped to the authenticated user. - `HtAppSettingsRepository` is now instantiated with the correct user ID within the settings route handlers, ensuring proper data scoping. - The global middleware in `routes/_middleware.dart` has been updated to correctly provide `HtAppSettingsClient`. The API should now consistently enforce authentication and correctly scope data and settings according to the authenticated user. **
1 parent b72661b commit 6cd2b77

File tree

10 files changed

+424
-193
lines changed

10 files changed

+424
-193
lines changed

lib/src/registry/model_registry.dart

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ import 'package:dart_frog/dart_frog.dart';
55
import 'package:ht_data_client/ht_data_client.dart';
66
import 'package:ht_shared/ht_shared.dart';
77

8+
/// Defines the ownership type of a data model.
9+
enum ModelOwnership {
10+
/// Data is global and not specific to any single user.
11+
global,
12+
13+
/// Data is owned by a specific user.
14+
userOwned,
15+
}
16+
817
/// {@template model_config}
918
/// Configuration holder for a specific data model type [T].
1019
///
@@ -20,17 +29,18 @@ class ModelConfig<T> {
2029
/// {@macro model_config}
2130
const ModelConfig({
2231
required this.fromJson,
23-
// toJson removed
2432
required this.getId,
33+
required this.ownership, // New field
2534
});
2635

2736
/// Function to deserialize JSON into an object of type [T].
2837
final FromJson<T> fromJson;
2938

30-
// toJson field removed
31-
3239
/// Function to extract the unique string ID from an item of type [T].
3340
final String Function(T item) getId;
41+
42+
/// The ownership type of this model.
43+
final ModelOwnership ownership;
3444
}
3545

3646
// Repository providers are no longer defined here.
@@ -54,23 +64,23 @@ class ModelConfig<T> {
5464
final modelRegistry = <String, ModelConfig<dynamic>>{
5565
'headline': ModelConfig<Headline>(
5666
fromJson: Headline.fromJson,
57-
// toJson removed
5867
getId: (h) => h.id,
68+
ownership: ModelOwnership.global, // Added ownership
5969
),
6070
'category': ModelConfig<Category>(
6171
fromJson: Category.fromJson,
62-
// toJson removed
6372
getId: (c) => c.id,
73+
ownership: ModelOwnership.global, // Added ownership
6474
),
6575
'source': ModelConfig<Source>(
6676
fromJson: Source.fromJson,
67-
// toJson removed
6877
getId: (s) => s.id,
78+
ownership: ModelOwnership.global, // Added ownership
6979
),
7080
'country': ModelConfig<Country>(
7181
fromJson: Country.fromJson,
72-
// toJson removed
7382
getId: (c) => c.id, // Assuming Country has an 'id' field
83+
ownership: ModelOwnership.global, // Added ownership
7484
),
7585
};
7686

routes/_middleware.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ import 'package:ht_api/src/services/auth_token_service.dart';
1414
import 'package:ht_api/src/services/jwt_auth_token_service.dart';
1515
import 'package:ht_api/src/services/token_blacklist_service.dart';
1616
import 'package:ht_api/src/services/verification_code_storage_service.dart';
17+
// Import HtAppSettingsClient interface
18+
import 'package:ht_app_settings_client/ht_app_settings_client.dart';
1719
import 'package:ht_app_settings_inmemory/ht_app_settings_inmemory.dart';
18-
import 'package:ht_app_settings_repository/ht_app_settings_repository.dart';
1920
import 'package:ht_data_inmemory/ht_data_inmemory.dart';
2021
import 'package:ht_data_repository/ht_data_repository.dart';
2122
import 'package:ht_email_inmemory/ht_email_inmemory.dart';
@@ -167,8 +168,7 @@ Handler middleware(Handler handler) {
167168
final categoryRepository = _createCategoryRepository();
168169
final sourceRepository = _createSourceRepository();
169170
final countryRepository = _createCountryRepository();
170-
final settingsClient = HtAppSettingsInMemory(); // Using in-memory for now
171-
final settingsRepository = HtAppSettingsRepository(client: settingsClient);
171+
final settingsClientImpl = HtAppSettingsInMemory(); // Using in-memory for now
172172
const uuid = Uuid();
173173

174174
// --- Auth Dependencies ---
@@ -258,7 +258,8 @@ Handler middleware(Handler handler) {
258258
(_) => userRepository,
259259
),
260260
) // Used by Auth services
261-
.use(provider<HtAppSettingsRepository>((_) => settingsRepository))
261+
// Provide the HtAppSettingsClient interface globally
262+
.use(provider<HtAppSettingsClient>((_) => settingsClientImpl))
262263
.use(
263264
provider<HtEmailRepository>(
264265
(_) => emailRepository,

routes/api/v1/data/[id].dart

Lines changed: 97 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,40 @@ import '../../../_middleware.dart';
3636
Future<Response> onRequest(RequestContext context, String id) async {
3737
// Read dependencies provided by middleware
3838
final modelName = context.read<String>();
39-
// Read ModelConfig for fromJson (needed for PUT)
4039
final modelConfig = context.read<ModelConfig<dynamic>>();
41-
// Read the unique RequestId provided by the root middleware
4240
final requestId = context.read<RequestId>().id;
41+
// Since requireAuthentication is used, User is guaranteed to be non-null.
42+
final authenticatedUser = context.read<User>();
4343

4444
try {
4545
switch (context.request.method) {
4646
case HttpMethod.get:
47-
// Pass requestId down to the handler
48-
return await _handleGet(context, id, modelName, requestId);
47+
return await _handleGet(
48+
context,
49+
id,
50+
modelName,
51+
modelConfig, // Pass modelConfig
52+
authenticatedUser,
53+
requestId,
54+
);
4955
case HttpMethod.put:
50-
// Pass requestId down to the handler
51-
return await _handlePut(context, id, modelName, modelConfig, requestId);
56+
return await _handlePut(
57+
context,
58+
id,
59+
modelName,
60+
modelConfig,
61+
authenticatedUser,
62+
requestId,
63+
);
5264
case HttpMethod.delete:
53-
// DELETE doesn't return a body, so no metadata needed here
54-
return await _handleDelete(context, id, modelName, requestId);
65+
return await _handleDelete(
66+
context,
67+
id,
68+
modelName,
69+
modelConfig, // Pass modelConfig
70+
authenticatedUser,
71+
requestId,
72+
);
5573
// Add cases for other methods if needed in the future
5674
default:
5775
// Methods not allowed on the item endpoint
@@ -83,24 +101,34 @@ Future<Response> _handleGet(
83101
RequestContext context,
84102
String id,
85103
String modelName,
86-
String requestId, // Receive requestId
104+
ModelConfig<dynamic> modelConfig, // Receive modelConfig
105+
User authenticatedUser, // Receive authenticatedUser
106+
String requestId,
87107
) async {
88108
dynamic item; // Use dynamic
109+
110+
String? userIdForRepoCall;
111+
if (modelConfig.ownership == ModelOwnership.userOwned) {
112+
userIdForRepoCall = authenticatedUser.id;
113+
} else {
114+
userIdForRepoCall = null;
115+
}
116+
89117
// Repository exceptions (like NotFoundException) will propagate up.
90118
try {
91119
switch (modelName) {
92120
case 'headline':
93121
final repo = context.read<HtDataRepository<Headline>>();
94-
item = await repo.read(id);
122+
item = await repo.read(id: id, userId: userIdForRepoCall);
95123
case 'category':
96124
final repo = context.read<HtDataRepository<Category>>();
97-
item = await repo.read(id);
125+
item = await repo.read(id: id, userId: userIdForRepoCall);
98126
case 'source':
99127
final repo = context.read<HtDataRepository<Source>>();
100-
item = await repo.read(id);
128+
item = await repo.read(id: id, userId: userIdForRepoCall);
101129
case 'country':
102130
final repo = context.read<HtDataRepository<Country>>();
103-
item = await repo.read(id);
131+
item = await repo.read(id: id, userId: userIdForRepoCall);
104132
default:
105133
// This case should ideally be caught by middleware, but added for safety
106134
return Response(
@@ -151,7 +179,8 @@ Future<Response> _handlePut(
151179
String id,
152180
String modelName,
153181
ModelConfig<dynamic> modelConfig,
154-
String requestId, // Receive requestId
182+
User authenticatedUser, // Receive authenticatedUser
183+
String requestId,
155184
) async {
156185
final requestBody = await context.request.json() as Map<String, dynamic>?;
157186
if (requestBody == null) {
@@ -185,6 +214,16 @@ Future<Response> _handlePut(
185214
}
186215

187216
dynamic updatedItem; // Use dynamic
217+
218+
String? userIdForRepoCall;
219+
if (modelConfig.ownership == ModelOwnership.userOwned) {
220+
userIdForRepoCall = authenticatedUser.id;
221+
} else {
222+
// For global models, update might imply admin rights.
223+
// For now, pass null, assuming repo handles global updates or has other checks.
224+
userIdForRepoCall = null;
225+
}
226+
188227
// Repository exceptions (like NotFoundException, BadRequestException)
189228
// will propagate up.
190229
try {
@@ -193,57 +232,69 @@ Future<Response> _handlePut(
193232
{
194233
final repo = context.read<HtDataRepository<Headline>>();
195234
final typedItem = itemToUpdate as Headline;
196-
// Validate ID match between path and body
197235
if (typedItem.id != id) {
198236
return Response(
199237
statusCode: HttpStatus.badRequest,
200238
body:
201239
'Bad Request: ID in request body ("${typedItem.id}") does not match ID in path ("$id").',
202240
);
203241
}
204-
updatedItem = await repo.update(id, typedItem);
242+
updatedItem = await repo.update(
243+
id: id,
244+
item: typedItem,
245+
userId: userIdForRepoCall,
246+
);
205247
}
206248
case 'category':
207249
{
208250
final repo = context.read<HtDataRepository<Category>>();
209251
final typedItem = itemToUpdate as Category;
210-
// Validate ID match between path and body
211252
if (typedItem.id != id) {
212253
return Response(
213254
statusCode: HttpStatus.badRequest,
214255
body:
215256
'Bad Request: ID in request body ("${typedItem.id}") does not match ID in path ("$id").',
216257
);
217258
}
218-
updatedItem = await repo.update(id, typedItem);
259+
updatedItem = await repo.update(
260+
id: id,
261+
item: typedItem,
262+
userId: userIdForRepoCall,
263+
);
219264
}
220265
case 'source':
221266
{
222267
final repo = context.read<HtDataRepository<Source>>();
223268
final typedItem = itemToUpdate as Source;
224-
// Validate ID match between path and body
225269
if (typedItem.id != id) {
226270
return Response(
227271
statusCode: HttpStatus.badRequest,
228272
body:
229273
'Bad Request: ID in request body ("${typedItem.id}") does not match ID in path ("$id").',
230274
);
231275
}
232-
updatedItem = await repo.update(id, typedItem);
276+
updatedItem = await repo.update(
277+
id: id,
278+
item: typedItem,
279+
userId: userIdForRepoCall,
280+
);
233281
}
234282
case 'country':
235283
{
236284
final repo = context.read<HtDataRepository<Country>>();
237285
final typedItem = itemToUpdate as Country;
238-
// Validate ID match between path and body
239286
if (typedItem.id != id) {
240287
return Response(
241288
statusCode: HttpStatus.badRequest,
242289
body:
243290
'Bad Request: ID in request body ("${typedItem.id}") does not match ID in path ("$id").',
244291
);
245292
}
246-
updatedItem = await repo.update(id, typedItem);
293+
updatedItem = await repo.update(
294+
id: id,
295+
item: typedItem,
296+
userId: userIdForRepoCall,
297+
);
247298
}
248299
default:
249300
// This case should ideally be caught by middleware, but added for safety
@@ -293,20 +344,38 @@ Future<Response> _handleDelete(
293344
RequestContext context,
294345
String id,
295346
String modelName,
296-
String requestId, // Receive requestId for logging
347+
ModelConfig<dynamic> modelConfig, // Receive modelConfig
348+
User authenticatedUser, // Receive authenticatedUser
349+
String requestId,
297350
) async {
351+
String? userIdForRepoCall;
352+
if (modelConfig.ownership == ModelOwnership.userOwned) {
353+
userIdForRepoCall = authenticatedUser.id;
354+
} else {
355+
// For global models, delete might imply admin rights.
356+
// For now, pass null.
357+
userIdForRepoCall = null;
358+
}
359+
298360
// Allow repository exceptions (e.g., NotFoundException) to propagate
299361
// upwards to be handled by the standard error handling mechanism.
300-
// (Removed the overly broad try-catch block that was previously here).
301362
switch (modelName) {
302363
case 'headline':
303-
await context.read<HtDataRepository<Headline>>().delete(id);
364+
await context
365+
.read<HtDataRepository<Headline>>()
366+
.delete(id: id, userId: userIdForRepoCall);
304367
case 'category':
305-
await context.read<HtDataRepository<Category>>().delete(id);
368+
await context
369+
.read<HtDataRepository<Category>>()
370+
.delete(id: id, userId: userIdForRepoCall);
306371
case 'source':
307-
await context.read<HtDataRepository<Source>>().delete(id);
372+
await context
373+
.read<HtDataRepository<Source>>()
374+
.delete(id: id, userId: userIdForRepoCall);
308375
case 'country':
309-
await context.read<HtDataRepository<Country>>().delete(id);
376+
await context
377+
.read<HtDataRepository<Country>>()
378+
.delete(id: id, userId: userIdForRepoCall);
310379
default:
311380
// This case should ideally be caught by the data/_middleware.dart,
312381
// but added for safety. Consider logging this unexpected state.

0 commit comments

Comments
 (0)