Skip to content

Commit b5d5dcb

Browse files
Merge commit from fork
SCA-138: validate migration source identity
2 parents d8f6a54 + 8c1576c commit b5d5dcb

13 files changed

Lines changed: 382 additions & 40 deletions

File tree

.github/scripts/product_file_line_count_ratchet_baseline/backend-routers.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
{
22
"files": {
3-
"backend/routers/apps.py": 2335,
3+
"backend/routers/apps.py": 2373,
44
"backend/routers/chat.py": 1588,
55
"backend/routers/developer.py": 2263,
66
"backend/routers/mcp_sse.py": 1858,
77
"backend/routers/sync.py": 2058,
88
"backend/routers/users.py": 2046
99
},
1010
"raise_justifications": {
11+
"backend/routers/apps.py": "The owner-migration route validates a source-bound Firebase anonymous-token proof before its existing database write and tracked background work, keeping the authorization decision at the HTTP mutation boundary.",
1112
"backend/routers/chat.py": "Merging the Windows-port chat surface with main combined the stream-error fallback (answered guard) with mains journey/attempt observability in the same SSE generators; +11 lines of combined guard flow, no new route.",
1213
"backend/routers/developer.py": "GET /v1/dev/user/memories and /vector/search both serve the authoritative legacy memories collection for legacy-cohort accounts with no rollout state (#9892/#10203), keeping the narrow deny/legacy-fallback decision in the route that owns the read contract.",
1314
"backend/routers/mcp_sse.py": "MCP SSE memory reads route their legacy-fallback decision through the shared mcp_legacy_read_authorized helper (#9892); one import line.",

app/lib/backend/http/api/apps.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -686,11 +686,11 @@ Future<bool> deleteApiKeyServer(String appId, String keyId) async {
686686
}
687687
}
688688

689-
Future<bool> migrateAppOwnerId(String oldId) async {
689+
Future<bool> migrateAppOwnerId(String oldId, String sourceToken) async {
690690
var response = await makeApiCall(
691691
url: '${Env.apiBaseUrl}v1/apps/migrate-owner?old_id=$oldId',
692-
headers: {},
693-
body: '',
692+
headers: {'Content-Type': 'application/json'},
693+
body: jsonEncode({'source_token': sourceToken}),
694694
method: 'POST',
695695
);
696696
try {

app/lib/providers/auth_provider.dart

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ import 'package:omi/utils/logger.dart';
1919
import 'package:omi/utils/platform/platform_manager.dart';
2020
import 'package:omi/utils/platform/platform_service.dart';
2121

22+
/// Runs a provider-link helper, then migrates only the anonymous source proof
23+
/// it returned after the destination account has been established.
24+
Future<ProviderLinkResult?> completeProviderLinkAndMigrate({
25+
required Future<ProviderLinkResult?> Function() linkProvider,
26+
required Future<bool> Function(String sourceUid, String sourceToken) migrate,
27+
}) async {
28+
final result = await linkProvider();
29+
final source = result?.anonymousSourceMigration;
30+
final destinationUid = result?.destinationUid;
31+
if (source != null && destinationUid != null && destinationUid != source.uid) {
32+
await migrate(source.uid, source.token);
33+
}
34+
return result;
35+
}
36+
2237
class AuthenticationProvider extends BaseProvider {
2338
FirebaseAuth get _auth => FirebaseAuth.instance;
2439

@@ -255,22 +270,15 @@ class AuthenticationProvider extends BaseProvider {
255270
Future<void> linkWithGoogle() async {
256271
setLoading(true);
257272
try {
258-
final result = await AuthService.instance.linkWithGoogle();
273+
final result = await completeProviderLinkAndMigrate(
274+
linkProvider: AuthService.instance.linkWithGoogle,
275+
migrate: migrateAppOwnerId,
276+
);
259277
if (result == null) {
260278
setLoading(false);
261279
return;
262280
}
263281
} catch (e) {
264-
if (e is FirebaseAuthException && e.code == 'credential-already-in-use') {
265-
final oldUserId = FirebaseAuth.instance.currentUser?.uid;
266-
if (oldUserId != null) {
267-
final newUserId = FirebaseAuth.instance.currentUser?.uid;
268-
if (newUserId != null) {
269-
await migrateAppOwnerId(oldUserId);
270-
}
271-
}
272-
return;
273-
}
274282
AppSnackbar.showSnackbarError(
275283
globalNavigatorKey.currentContext?.l10n.authFailedToLinkGoogle ??
276284
'Failed to link with Google, please try again.',
@@ -294,6 +302,7 @@ class AuthenticationProvider extends BaseProvider {
294302
// Get existing user credentials
295303
final existingCred = e.credential;
296304
final oldUserId = FirebaseAuth.instance.currentUser?.uid;
305+
final sourceToken = await FirebaseAuth.instance.currentUser?.getIdToken();
297306

298307
// Sign out current anonymous user
299308
AuthService.instance.handleAuthUserChanged(null);
@@ -309,8 +318,8 @@ class AuthenticationProvider extends BaseProvider {
309318
SharedPreferencesUtil().uid = newUserId ?? '';
310319
SharedPreferencesUtil().email = FirebaseAuth.instance.currentUser?.email ?? '';
311320
SharedPreferencesUtil().givenName = FirebaseAuth.instance.currentUser?.displayName?.split(' ')[0] ?? '';
312-
if (oldUserId != null && newUserId != null) {
313-
await migrateAppOwnerId(oldUserId);
321+
if (oldUserId != null && newUserId != null && sourceToken != null) {
322+
await migrateAppOwnerId(oldUserId, sourceToken);
314323
}
315324
return;
316325
}
@@ -331,7 +340,7 @@ class AuthenticationProvider extends BaseProvider {
331340
}
332341
}
333342

334-
Future<bool> migrateAppOwnerId(String oldId) async {
335-
return await apps_api.migrateAppOwnerId(oldId);
343+
Future<bool> migrateAppOwnerId(String oldId, String sourceToken) async {
344+
return await apps_api.migrateAppOwnerId(oldId, sourceToken);
336345
}
337346
}

app/lib/services/auth_service.dart

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,47 @@ final class _FirebaseAuthTokenGateway implements AuthTokenGateway {
4040
Future<void> signOut() => FirebaseAuth.instance.signOut();
4141
}
4242

43+
/// Source-bound proof captured before a credential-collision sign-in replaces
44+
/// the anonymous Firebase user.
45+
final class AnonymousSourceMigration {
46+
const AnonymousSourceMigration({required this.uid, required this.token});
47+
48+
final String uid;
49+
final String token;
50+
}
51+
52+
/// The outcome of linking an external provider.
53+
///
54+
/// A credential collision signs in to the already-linked destination account
55+
/// inside [AuthService]. The caller must use [anonymousSourceMigration], rather
56+
/// than inspecting FirebaseAuth.currentUser after that switch, to migrate the
57+
/// anonymous source data.
58+
final class ProviderLinkResult {
59+
const ProviderLinkResult({required this.destinationUid, this.anonymousSourceMigration});
60+
61+
final String? destinationUid;
62+
final AnonymousSourceMigration? anonymousSourceMigration;
63+
}
64+
65+
/// Captures an anonymous source proof before [establishDestination] can replace
66+
/// FirebaseAuth.currentUser, then returns both sides of the completed collision.
67+
@visibleForTesting
68+
Future<ProviderLinkResult> resolveProviderCredentialCollision({
69+
required String sourceUid,
70+
required bool sourceIsAnonymous,
71+
required Future<String?> Function() captureSourceToken,
72+
required Future<String?> Function() establishDestination,
73+
}) async {
74+
final sourceToken = sourceIsAnonymous ? await captureSourceToken() : null;
75+
final anonymousSourceMigration =
76+
sourceToken == null ? null : AnonymousSourceMigration(uid: sourceUid, token: sourceToken);
77+
final destinationUid = await establishDestination();
78+
return ProviderLinkResult(
79+
destinationUid: destinationUid,
80+
anonymousSourceMigration: anonymousSourceMigration,
81+
);
82+
}
83+
4384
class AuthService {
4485
static final AuthService _instance = AuthService._internal();
4586
static AuthService get instance => _instance;
@@ -767,7 +808,7 @@ class AuthService {
767808
return base64Url.encode(digest.bytes).replaceAll('=', '');
768809
}
769810

770-
Future<UserCredential?> linkWithProvider(String provider) async {
811+
Future<ProviderLinkResult?> linkWithProvider(String provider) async {
771812
try {
772813
final currentUser = FirebaseAuth.instance.currentUser;
773814
if (currentUser == null) {
@@ -857,11 +898,15 @@ class AuthService {
857898
await _updateUserPreferences(result, provider);
858899

859900
Logger.debug('Firebase account linking successful');
860-
return result;
901+
return ProviderLinkResult(destinationUid: result.user?.uid);
861902
} catch (e) {
862903
if (e is FirebaseAuthException && e.code == 'credential-already-in-use') {
863-
// Handle existing credential case
864-
return await _handleExistingCredential(e);
904+
return await resolveProviderCredentialCollision(
905+
sourceUid: currentUser.uid,
906+
sourceIsAnonymous: currentUser.isAnonymous,
907+
captureSourceToken: currentUser.getIdToken,
908+
establishDestination: () async => (await _handleExistingCredential(e)).user?.uid,
909+
);
865910
}
866911
rethrow;
867912
}
@@ -887,7 +932,7 @@ class AuthService {
887932
}
888933

889934
/// Handle the case when credential is already in use
890-
Future<UserCredential?> _handleExistingCredential(FirebaseAuthException e) async {
935+
Future<UserCredential> _handleExistingCredential(FirebaseAuthException e) async {
891936
// Get existing user credentials
892937
final existingCred = e.credential;
893938

@@ -909,11 +954,11 @@ class AuthService {
909954
return result;
910955
}
911956

912-
Future<UserCredential?> linkWithGoogle() async {
957+
Future<ProviderLinkResult?> linkWithGoogle() async {
913958
return await linkWithProvider('google');
914959
}
915960

916-
Future<UserCredential?> linkWithApple() async {
961+
Future<ProviderLinkResult?> linkWithApple() async {
917962
return await linkWithProvider('apple');
918963
}
919964
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import 'package:flutter_test/flutter_test.dart';
2+
import 'package:omi/providers/auth_provider.dart';
3+
import 'package:omi/services/auth_service.dart';
4+
5+
void main() {
6+
test('Google credential collision propagates the pre-switch source proof to migration after destination sign-in',
7+
() async {
8+
var destinationEstablished = false;
9+
final migrations = <(String, String)>[];
10+
11+
Future<ProviderLinkResult?> googleCollisionHelper() async {
12+
return resolveProviderCredentialCollision(
13+
sourceUid: 'anonymous-source',
14+
sourceIsAnonymous: true,
15+
captureSourceToken: () async {
16+
expect(destinationEstablished, isFalse);
17+
return 'source-id-token';
18+
},
19+
establishDestination: () async {
20+
destinationEstablished = true;
21+
return 'linked-destination';
22+
},
23+
);
24+
}
25+
26+
await completeProviderLinkAndMigrate(
27+
linkProvider: googleCollisionHelper,
28+
migrate: (sourceUid, sourceToken) async {
29+
expect(destinationEstablished, isTrue);
30+
migrations.add((sourceUid, sourceToken));
31+
return true;
32+
},
33+
);
34+
35+
expect(migrations, [('anonymous-source', 'source-id-token')]);
36+
});
37+
}

backend/routers/apps.py

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,19 @@
1010
from urllib.parse import urlparse
1111
from pydantic import BaseModel as PydanticBaseModel, ConfigDict, Field, ValidationError
1212
from ulid import ULID
13-
from fastapi import APIRouter, Depends, Form, UploadFile, File, HTTPException, Header, Query
13+
from fastapi import APIRouter, Body, Depends, Form, UploadFile, File, HTTPException, Header, Query
1414
from fastapi.responses import HTMLResponse
1515

1616
from langchain_core.messages import SystemMessage, HumanMessage
1717
from utils.apps import fetch_app_chat_tools_from_manifest
18-
from utils.executors import db_executor, llm_executor, storage_executor, run_blocking, start_background_task
18+
from utils.executors import (
19+
critical_executor,
20+
db_executor,
21+
llm_executor,
22+
storage_executor,
23+
run_blocking,
24+
start_background_task,
25+
)
1926
from utils.http_client import get_webhook_client
2027
from utils.multipart import APP_IMAGE_MAX_PART_SIZE, MultipartMaxPartSizeRoute, max_part_size
2128
from utils.mcp_client import (
@@ -1680,7 +1687,38 @@ def get_twitter_initial_message(username: str, uid: str = Depends(auth.get_curre
16801687

16811688

16821689
@router.post('/v1/apps/migrate-owner', tags=['v1'], response_model=AppMigrationResponse)
1683-
async def migrate_app_owner(old_id, uid: str = Depends(auth.get_current_user_uid)):
1690+
async def migrate_app_owner(
1691+
old_id,
1692+
source_token: Optional[str] = Body(default=None, embed=True),
1693+
uid: str = Depends(auth.get_current_user_uid),
1694+
):
1695+
# The client captures this token while it still owns the anonymous Firebase
1696+
# session, then calls this route after signing into the destination account.
1697+
# A providerless UserRecord is not affirmative proof of anonymity (for example,
1698+
# custom-token accounts can be providerless), so a bare ``old_id`` never grants
1699+
# access. The source token must be a currently valid Firebase credential for
1700+
# that exact uid and attest to the anonymous sign-in provider.
1701+
if old_id == uid:
1702+
raise HTTPException(status_code=400, detail='Source identity must differ from the authenticated identity')
1703+
if not source_token:
1704+
raise HTTPException(status_code=403, detail='Source identity is not eligible for migration')
1705+
1706+
try:
1707+
source_claims = await run_blocking(
1708+
critical_executor, auth.auth.verify_id_token, source_token, check_revoked=True
1709+
)
1710+
source_user = await run_blocking(critical_executor, auth.get_user, old_id)
1711+
except Exception:
1712+
# Invalid/revoked tokens, missing/deleted users, and Admin lookup failures
1713+
# are deliberately indistinguishable to callers. Neither may mutate state.
1714+
raise HTTPException(status_code=403, detail='Source identity is not eligible for migration')
1715+
1716+
source_uid = source_claims.get('uid')
1717+
firebase_claims = source_claims.get('firebase')
1718+
source_provider = firebase_claims.get('sign_in_provider') if isinstance(firebase_claims, dict) else None
1719+
if source_uid != old_id or source_provider != 'anonymous' or source_user.disabled or source_user.provider_data:
1720+
raise HTTPException(status_code=403, detail='Source identity is not eligible for migration')
1721+
16841722
await run_blocking(db_executor, migrate_app_owner_id_db, uid, old_id)
16851723

16861724
# Tracked background tasks (not bare asyncio.create_task): keeps a live reference

0 commit comments

Comments
 (0)