Skip to content

fix: prevent unauthorized account.get() call on app init (#746) #785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Arunodoy18 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: prevent unauthorized account.get() call on app init (#746) #785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b614a05
feat: refactor profile screen to support public view mode
Arunodoy18 Mar 1, 2026
d472396
fix: guard account.get() in onInit() to prevent 401 for guest users
Arunodoy18 Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/controllers/auth_state_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ class AuthStateController extends GetxController {
@override
void onInit() async {
super.onInit();
await setUserProfileData();
if (await getLoginState) {
await setUserProfileData();
}
Comment on lines +108 to +110
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes AuthStateController.onInit() behavior, but tests only cover getLoginState directly. Please add a unit test that exercises onInit() for a guest user and asserts that profile initialization is skipped (i.e., setUserProfileData() is not invoked / no user DB reads happen).

Copilot uses AI. Check for mistakes.
Comment on lines +108 to +110
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onInit now guards setUserProfileData() with getLoginState, but getLoginState itself calls account.get() (and setUserProfileData() calls account.get() again). This means the app still makes an unauthorized account.get() request for guest users (just swallowed), and authenticated users pay for two account.get() calls. Consider switching the guard to a session check that does not call account.get(), or refactor so setUserProfileData() can reuse the already-fetched appwriteUser instead of fetching again.

Suggested change
if (await getLoginState) {
await setUserProfileData();
}
await setUserProfileData();

Copilot uses AI. Check for mistakes.
Comment on lines +108 to +110
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard still triggers account.get() for guest init path.

At Line 108, getLoginState is used as a guard, but getLoginState itself calls account.get() (Line 190). So unauthenticated startup still performs the unauthorized account request (and authenticated startup does two account.get() calls: Line 190 and Line 200). This does not fully meet the stated objective of preventing unauthorized account.get() on init.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/controllers/auth_state_controller.dart` around lines 108 - 110, The guard
uses getLoginState which itself calls account.get(), causing unauthorized or
duplicate account.get() calls; change the flow so the init guard does not call
account.get(): either (A) refactor getLoginState to return a local cached
boolean (e.g. _isLoggedIn) or check a non-network auth token/session flag
instead of calling account.get(), or (B) change the init sequence to call
account.get() exactly once and pass its result into setUserProfileData; update
symbols getLoginState, setUserProfileData, and any use of account.get() so
unauthenticated startup never triggers account.get() and authenticated startup
calls account.get() only once.


// ask for settings permissions
await messaging.requestPermission(
Expand Down
43 changes: 23 additions & 20 deletions lib/views/screens/profile_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ class ProfileScreen extends StatefulWidget {
}

class _ProfileScreenState extends State<ProfileScreen> {
/// Returns true if the current user is viewing their own profile (owner mode)
/// Returns false if viewing another user's profile (public view mode)
bool get isOwner => widget.isCreatorProfile == null;

Comment on lines +44 to +47
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

isOwner predicate can desync with init/data assumptions.

isOwner treats any non-null isCreatorProfile as public mode (Line 46), but initState only runs public-profile initialization when isCreatorProfile == true (Line 51). If isCreatorProfile is false, !isOwner branches can still dereference widget.creator! without aligned initialization/invariants.

Proposed fix (unify owner/public predicate and constructor invariant) 
 class ProfileScreen extends StatefulWidget {
   final ResonateUser? creator;
 
   final bool? isCreatorProfile;
 
   ProfileScreen({super.key, this.creator, this.isCreatorProfile})
     : assert(
-        isCreatorProfile != true || (creator != null && creator.uid != null),
-        'creator and creator.uid are required when isCreatorProfile is true',
+        creator == null || creator.uid != null,
+        'creator.uid is required when viewing another user profile',
       );
@@
 class _ProfileScreenState extends State<ProfileScreen> {
-  bool get isOwner => widget.isCreatorProfile == null;
+  bool get isOwner => widget.creator == null;
@@
   void initState() {
     super.initState();
-    if (widget.isCreatorProfile == true) {
+    if (!isOwner) {
       WidgetsBinding.instance.addPostFrameCallback((_) async {
         await userProfileController.initializeProfile(widget.creator!.uid!);
       });
     }
   }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/views/screens/profile_screen.dart` around lines 44 - 47, The isOwner
predicate and initState public/owner branching are inconsistent: change the
getter to align with initState by making owner be anything not explicitly true
(bool get isOwner => widget.isCreatorProfile != true), and add a
constructor/assert invariant to guarantee widget.creator is non-null when
isCreatorProfile == true (e.g. assert(widget.isCreatorProfile != true ||
widget.creator != null)) so initState and any dereferences of widget.creator are
safe and consistent with the new predicate.

@override
void initState() {
super.initState();
Expand Down Expand Up @@ -74,7 +78,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
return Scaffold(
appBar: AppBar(
title: Text(AppLocalizations.of(context)!.profile),
actions: widget.isCreatorProfile == null
actions: isOwner
? [
IconButton(
onPressed: () {
Expand Down Expand Up @@ -143,7 +147,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
SizedBox(width: UiSizes.width_20),
CircleAvatar(
backgroundColor: Theme.of(Get.context!).colorScheme.secondary,
backgroundImage: widget.isCreatorProfile != null
backgroundImage: !isOwner
? NetworkImage(widget.creator!.profileImageUrl ?? '')
: controller.profileImageUrl == null ||
controller.profileImageUrl!.isEmpty
Expand All @@ -157,8 +161,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
if (widget.isCreatorProfile == null &&
controller.isEmailVerified!)
if (isOwner && controller.isEmailVerified!)
Padding(
padding: EdgeInsets.only(top: 10),
child: Row(
Expand All @@ -176,7 +179,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
),
),
Text(
widget.isCreatorProfile != null
!isOwner
? widget.creator!.name ?? ''
: controller.displayName.toString(),
style: TextStyle(
Expand All @@ -187,7 +190,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
),
Chip(
label: Text(
"@${widget.isCreatorProfile != null ? widget.creator!.userName : controller.userName}",
"@${!isOwner ? widget.creator!.userName : controller.userName}",
style: TextStyle(
fontSize: UiSizes.size_14,
overflow: TextOverflow.ellipsis,
Expand All @@ -204,7 +207,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
Padding(
padding: const EdgeInsets.only(left: 5),
child: Text(
widget.isCreatorProfile == null
isOwner
? (authController.ratingTotal /
authController.ratingCount)
.toStringAsFixed(1)
Expand All @@ -221,7 +224,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
.length ==
1 &&
userProfileController.isFollowingUser.value) &&
widget.isCreatorProfile != null) {
!isOwner) {
//Remove current user from followers list
final sanitizedFollowersList = userProfileController
.searchedUserFollowers
Expand All @@ -239,7 +242,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
Icon(Icons.people),
Padding(
padding: const EdgeInsets.only(left: 5),
child: widget.isCreatorProfile == null
child: isOwner
? Text(
authController.followerDocuments.length
.toString(),
Expand Down Expand Up @@ -271,7 +274,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
EmailVerifyController emailVerifyController,
AuthStateController controller,
) {
if (widget.isCreatorProfile != null || controller.isEmailVerified!) {
if (!isOwner || controller.isEmailVerified!) {
return const SizedBox.shrink();
}

Expand Down Expand Up @@ -305,7 +308,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
child: Obx(() {
return ElevatedButton(
onPressed: () {
if (widget.isCreatorProfile != null) {
if (!isOwner) {
if (userProfileController.isFollowingUser.value) {
userProfileController.unfollowCreator();
} else {
Expand All @@ -331,7 +334,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
mainAxisAlignment: MainAxisAlignment.center,
children: [
Icon(
widget.isCreatorProfile != null
!isOwner
? userProfileController.isFollowingUser.value
? Icons.done
: Icons.add
Expand All @@ -340,7 +343,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
),
const SizedBox(width: 8),
Text(
widget.isCreatorProfile != null
!isOwner
? userProfileController.isFollowingUser.value
? AppLocalizations.of(context)!.following
: AppLocalizations.of(context)!.follow
Expand All @@ -354,7 +357,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
),
const SizedBox(width: 10),

widget.isCreatorProfile == null
isOwner
? SizedBox(
height: 50,
width: 50,
Expand Down Expand Up @@ -494,7 +497,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
Align(
alignment: Alignment.centerLeft,
child: Text(
widget.isCreatorProfile != null
!isOwner
? AppLocalizations.of(context)!.userCreatedStories
: AppLocalizations.of(context)!.yourStories,
style: TextStyle(
Expand All @@ -508,10 +511,10 @@ class _ProfileScreenState extends State<ProfileScreen> {
Obx(
() => _buildStoriesList(
context,
widget.isCreatorProfile != null
!isOwner
? userProfileController.searchedUserStories
: exploreStoryController.userCreatedStories,
widget.isCreatorProfile != null
!isOwner
? AppLocalizations.of(context)!.userNoStories
: AppLocalizations.of(context)!.youNoStories,
),
Expand All @@ -520,7 +523,7 @@ class _ProfileScreenState extends State<ProfileScreen> {
Align(
alignment: Alignment.centerLeft,
child: Text(
widget.isCreatorProfile != null
!isOwner
? AppLocalizations.of(context)!.userLikedStories
: AppLocalizations.of(context)!.yourLikedStories,
style: TextStyle(
Expand All @@ -534,10 +537,10 @@ class _ProfileScreenState extends State<ProfileScreen> {
Obx(
() => _buildStoriesList(
context,
widget.isCreatorProfile != null
!isOwner
? userProfileController.searchedUserLikedStories
: exploreStoryController.userLikedStories,
widget.isCreatorProfile != null
!isOwner
? AppLocalizations.of(context)!.userNoLikedStories
: AppLocalizations.of(context)!.youNoLikedStories,
),
Expand Down
8 changes: 8 additions & 0 deletions test/controllers/auth_state_controller_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,14 @@ void main() {
expect(loginState, true);
});

test('test getLoginState returns false for guest user', () async {
when(mockAccount.get()).thenThrow(AppwriteException('Unauthorized', 401));
final loginState = await authStateController.getLoginState;
expect(loginState, false);
// Restore stub for subsequent tests
when(mockAccount.get()).thenAnswer((_) => Future.value(mockUser));
Comment on lines +251 to +252
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stub reset at the end of this test is redundant because setUp() re-stubs mockAccount.get() before every test. Keeping it here makes the test order-dependent if future changes add setUpAll or modify setup behavior; prefer relying on setUp() (or add a tearDown()/reset() if needed).

Suggested change
// Restore stub for subsequent tests
when(mockAccount.get()).thenAnswer((_) => Future.value(mockUser));

Copilot uses AI. Check for mistakes.
});

test(
'test addRegistrationTokentoSubscribedandCreatedUpcomingRooms',
() async {
Expand Down
Loading