Skip to content

Commit 9a041e5

Browse files
dnys1dnys1
committed
chore(cloud_auth): Auth improvements
- Register viewer/editor roles in DB - Restrict users service to authenticated role - Shared policy cache (ensures can be read/written by multiple processes)
1 parent f5a6061 commit 9a041e5

File tree

7 files changed

+293
-35
lines changed

7 files changed

+293
-35
lines changed

‎services/celest_cloud_auth/lib/src/authorization/celest_role.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ extension type const CelestRole._(EntityUid _) implements EntityUid {
99
EntityUid.of('Celest::Role', 'authenticated'),
1010
);
1111

12+
static const CelestRole viewer = CelestRole._(
13+
EntityUid.of('Celest::Role', 'viewer'),
14+
);
15+
16+
static const CelestRole editor = CelestRole._(
17+
EntityUid.of('Celest::Role', 'editor'),
18+
);
19+
1220
static const CelestRole admin = CelestRole._(
1321
EntityUid.of('Celest::Role', 'admin'),
1422
);

‎services/celest_cloud_auth/lib/src/database/auth_database_accessors.dart‎

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import 'package:async/async.dart';
21
import 'package:cedar/ast.dart';
32
import 'package:cedar/cedar.dart';
43
import 'package:celest_ast/celest_ast.dart';
@@ -177,9 +176,17 @@ class CloudAuthDatabaseAccessors extends DatabaseAccessor<GeneratedDatabase>
177176
uid: CelestRole.authenticated,
178177
parents: [CelestRole.anonymous],
179178
),
179+
CelestRole.viewer: const Entity(
180+
uid: CelestRole.viewer,
181+
parents: [CelestRole.authenticated],
182+
),
183+
CelestRole.editor: const Entity(
184+
uid: CelestRole.editor,
185+
parents: [CelestRole.viewer],
186+
),
180187
CelestRole.admin: const Entity(
181188
uid: CelestRole.admin,
182-
parents: [CelestRole.authenticated],
189+
parents: [CelestRole.editor],
183190
),
184191
CelestRole.owner: const Entity(
185192
uid: CelestRole.owner,
@@ -454,13 +461,11 @@ class CloudAuthDatabaseAccessors extends DatabaseAccessor<GeneratedDatabase>
454461
return result;
455462
}
456463

457-
final _effectivePolicySetCache = AsyncCache<PolicySet>(
458-
const Duration(hours: 1),
459-
);
464+
late final _PolicySetCache _policySetCache = _PolicySetCache(db);
460465

461466
/// The effective [PolicySet] for the project.
462467
Future<PolicySet> get effectivePolicySet {
463-
return _effectivePolicySetCache.fetch(loadEffectivePolicySet);
468+
return _policySetCache.fetch(loadEffectivePolicySet);
464469
}
465470

466471
/// Loads the effective policy set from the database.
@@ -722,7 +727,7 @@ class CloudAuthDatabaseAccessors extends DatabaseAccessor<GeneratedDatabase>
722727
);
723728
}
724729
});
725-
_effectivePolicySetCache.invalidate();
730+
_policySetCache.invalidate();
726731
}
727732

728733
/// Creates a new [entity] in the database.
@@ -901,6 +906,55 @@ class CloudAuthDatabaseAccessors extends DatabaseAccessor<GeneratedDatabase>
901906
}
902907
}
903908

909+
final class _PolicySetCache {
910+
_PolicySetCache(this._db);
911+
912+
final GeneratedDatabase _db;
913+
914+
PolicySet? _cached;
915+
int? _dataVersion;
916+
Future<PolicySet>? _pending;
917+
918+
Future<PolicySet> fetch(Future<PolicySet> Function() loader) async {
919+
final version = await _readDataVersion();
920+
final cached = _cached;
921+
if (cached != null && _dataVersion == version) {
922+
return cached;
923+
}
924+
925+
_pending ??= _reload(loader);
926+
return _pending!;
927+
}
928+
929+
Future<PolicySet> _reload(Future<PolicySet> Function() loader) async {
930+
try {
931+
while (true) {
932+
final before = await _readDataVersion();
933+
final loaded = await loader();
934+
final after = await _readDataVersion();
935+
if (before == after) {
936+
_cached = loaded;
937+
_dataVersion = after;
938+
return loaded;
939+
}
940+
}
941+
} finally {
942+
_pending = null;
943+
}
944+
}
945+
946+
void invalidate() {
947+
_cached = null;
948+
_dataVersion = null;
949+
_pending = null;
950+
}
951+
952+
Future<int> _readDataVersion() async {
953+
final row = await _db.customSelect('PRAGMA data_version').getSingle();
954+
return row.data.values.single as int;
955+
}
956+
}
957+
904958
/// Creates a diff of a project's authorization config between two versions.
905959
final class _ProjectAuthDiff extends ResolvedAstVisitorWithArg<void, Node?> {
906960
_ProjectAuthDiff();

‎services/celest_cloud_auth/lib/src/users/users_service.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ extension type UsersService._(_Deps _deps) implements Object {
127127
policySet: PolicySet(
128128
templateLinks: [
129129
TemplateLink(
130-
templateId: 'cloud.functions.anonymous',
130+
templateId: 'cloud.functions.authenticated',
131131
newId: apiId,
132132
values: {SlotId.resource: apiUid},
133133
),
@@ -254,6 +254,7 @@ extension type UsersService._(_Deps _deps) implements Object {
254254
final principal = context.get(ContextKey.principal);
255255
await _authorizer.expectAuthorized(
256256
principal: principal?.uid,
257+
resource: UsersService.apiUid,
257258
action: CelestAction.list,
258259
);
259260
final response = await listUsers(

‎services/celest_cloud_auth/test/authorization/authorizer_test.dart‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'package:cedar/cedar.dart';
33
import 'package:celest_ast/celest_ast.dart';
44
import 'package:celest_cloud_auth/src/authorization/authorizer.dart';
55
import 'package:celest_cloud_auth/src/database/auth_database.dart';
6+
import 'package:celest_cloud_auth/src/users/users_repository.dart';
67
import 'package:logging/logging.dart';
78
import 'package:pub_semver/pub_semver.dart';
89
import 'package:test/test.dart';
@@ -13,11 +14,14 @@ import '../tester.dart';
1314
const roleAdmin = EntityUid.of('Celest::Role', 'admin');
1415
const roleAuthenticated = EntityUid.of('Celest::Role', 'authenticated');
1516
const roleAnonymous = EntityUid.of('Celest::Role', 'anonymous');
17+
const roleEditor = EntityUid.of('Celest::Role', 'editor');
18+
const roleViewer = EntityUid.of('Celest::Role', 'viewer');
1619

1720
// Users
1821
const userAlice = EntityUid.of('Celest::User', 'alice');
1922
const userBob = EntityUid.of('Celest::User', 'bob');
2023
const userCharlie = EntityUid.of('Celest::User', 'charlie');
24+
const userDiana = EntityUid.of('Celest::User', 'diana');
2125

2226
// Actions
2327
const actionCreate = EntityUid.of('Celest::Action', 'create');
@@ -54,7 +58,7 @@ Policy forbidUnless(
5458
);
5559

5660
void main() {
57-
Logger.root.level = Level.ALL;
61+
Logger.root.level = Level.WARNING;
5862
Logger.root.onRecord.listen((record) {
5963
print('${record.level.name}: ${record.message}');
6064
});
@@ -63,6 +67,39 @@ void main() {
6367
late CloudAuthDatabase db;
6468
late Authorizer authorizer;
6569

70+
group('core roles', () {
71+
late CloudAuthDatabase roleDb;
72+
late UsersRepository users;
73+
74+
setUp(() async {
75+
roleDb = CloudAuthDatabase.memory(project: defaultProject);
76+
await roleDb.ping();
77+
users = UsersRepository(db: roleDb);
78+
});
79+
80+
tearDown(() async {
81+
await roleDb.close();
82+
});
83+
84+
test('viewer and editor roles are assignable', () async {
85+
final anonymous = await users.createAnonymousUser();
86+
87+
await roleDb.cloudAuth.setUserRoles(
88+
userId: anonymous.userId,
89+
roles: const [roleViewer],
90+
);
91+
var updated = await roleDb.cloudAuth.getUser(userId: anonymous.userId);
92+
expect(updated!.roles, contains(roleViewer));
93+
94+
await roleDb.cloudAuth.setUserRoles(
95+
userId: anonymous.userId,
96+
roles: const [roleEditor],
97+
);
98+
updated = await roleDb.cloudAuth.getUser(userId: anonymous.userId);
99+
expect(updated!.roles, contains(roleEditor));
100+
});
101+
});
102+
66103
Future<void> createEntities(List<Entity> entities) async {
67104
for (final entity in entities) {
68105
await db.cloudAuth.createEntity(entity);
@@ -116,6 +153,7 @@ void main() {
116153
Entity(uid: userAlice, parents: [roleAdmin]),
117154
Entity(uid: userBob, parents: [roleAuthenticated]),
118155
Entity(uid: userCharlie, parents: [roleAnonymous]),
156+
Entity(uid: userDiana, parents: [roleViewer]),
119157
Entity(uid: functionAuthenticated, parents: [apiTest]),
120158
Entity(uid: functionAdmin, parents: [apiTest]),
121159
Entity(uid: functionPublic, parents: [apiTest]),
@@ -201,6 +239,33 @@ void main() {
201239
),
202240
expected: Decision.allow,
203241
),
242+
(
243+
description: 'viewer can invoke admin function via viewer policy',
244+
request: AuthorizationRequest(
245+
action: actionInvoke,
246+
resource: functionAdmin,
247+
principal: userDiana,
248+
),
249+
expected: Decision.allow,
250+
),
251+
(
252+
description: 'viewer can invoke authenticated function',
253+
request: AuthorizationRequest(
254+
action: actionInvoke,
255+
resource: functionAuthenticated,
256+
principal: userDiana,
257+
),
258+
expected: Decision.allow,
259+
),
260+
(
261+
description: 'viewer can invoke public function',
262+
request: AuthorizationRequest(
263+
action: actionInvoke,
264+
resource: functionPublic,
265+
principal: userDiana,
266+
),
267+
expected: Decision.allow,
268+
),
204269
(
205270
description: 'admin user can invoke admin function',
206271
request: AuthorizationRequest(

‎services/celest_cloud_auth/test/database/auth_database/policy_cache_test.dart‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import 'package:cedar/cedar.dart';
2+
import 'package:celest_cloud_auth/celest_cloud_auth.dart';
3+
import 'package:drift/drift.dart';
4+
import 'package:file/file.dart';
5+
import 'package:file/local.dart';
6+
import 'package:test/test.dart';
7+
8+
import '../../tester.dart' show defaultProject;
9+
10+
void main() {
11+
driftRuntimeOptions.dontWarnAboutMultipleDatabases = true;
12+
13+
group('CloudAuthDatabaseAccessors policy cache', () {
14+
test('reuses cached policy set when data_version is unchanged', () async {
15+
final fs = const LocalFileSystem();
16+
final Directory directory = await fs.systemTempDirectory.createTemp(
17+
'celest_policy_cache_test_',
18+
);
19+
addTearDown(() async {
20+
if (await directory.exists()) {
21+
await directory.delete(recursive: true);
22+
}
23+
});
24+
25+
final db = CloudAuthDatabase.localDir(directory, project: defaultProject);
26+
addTearDown(db.close);
27+
28+
final first = await db.cloudAuth.effectivePolicySet;
29+
final second = await db.cloudAuth.effectivePolicySet;
30+
31+
expect(identical(first, second), isTrue);
32+
});
33+
34+
test('reloads policies after external updates via data_version', () async {
35+
final fs = const LocalFileSystem();
36+
final Directory directory = await fs.systemTempDirectory.createTemp(
37+
'celest_policy_cache_test_',
38+
);
39+
addTearDown(() async {
40+
if (await directory.exists()) {
41+
await directory.delete(recursive: true);
42+
}
43+
});
44+
45+
final dbA = CloudAuthDatabase.localDir(
46+
directory,
47+
project: defaultProject,
48+
);
49+
final dbB = CloudAuthDatabase.localDir(
50+
directory,
51+
project: defaultProject,
52+
);
53+
addTearDown(() async {
54+
await dbA.close();
55+
await dbB.close();
56+
});
57+
58+
final initial = await dbA.cloudAuth.effectivePolicySet;
59+
final initialPolicyIds = initial.policies.keys.toSet();
60+
const newLinkId = 'policy.cache.test';
61+
62+
final policyResource = EntityUid.of(
63+
'Celest::Function',
64+
'policy-cache-test',
65+
);
66+
await dbB.cloudAuth.createEntity(
67+
Entity(
68+
uid: policyResource,
69+
parents: [EntityUid.of('Celest::Api', 'test')],
70+
),
71+
);
72+
73+
final newLink = TemplateLink(
74+
templateId: 'cloud.functions.authenticated',
75+
newId: newLinkId,
76+
values: {SlotId.resource: policyResource},
77+
);
78+
79+
await dbB.cloudAuth.upsertPolicySet(PolicySet(templateLinks: [newLink]));
80+
81+
final updated = await dbA.cloudAuth.effectivePolicySet;
82+
expect(identical(initial, updated), isFalse);
83+
final updatedPolicyIds = updated.policies.keys.toSet();
84+
final newPolicyIds = updatedPolicyIds.difference(initialPolicyIds);
85+
expect(newPolicyIds, hasLength(1));
86+
87+
final cached = await dbA.cloudAuth.effectivePolicySet;
88+
expect(identical(updated, cached), isTrue);
89+
});
90+
});
91+
}

0 commit comments

Comments
 (0)