Skip to content
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.

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

Logout Button Allows to Skip Account Setup #21

Closed
1 task done
Supereg opened this issue Jun 11, 2023 · 4 comments
Closed
1 task done

Logout Button Allows to Skip Account Setup #21

Supereg opened this issue Jun 11, 2023 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Supereg
Copy link
Member

Supereg commented Jun 11, 2023

Description

The AccountSetup can fundamentally display two different states

  • No account logged in
  • Account logged in

In the second state, the UI displays an logout button (e.g. allowing you to change your account again). This can be either reached when navigating back from the "HealthKit Access" view or when uninstalling and reinstalling the app.
The problem is that when pressing the Logout button the view will jump to the next onboarding step allowing to skip Account onboarding.

Reproduction

  1. Go throng the onboarding steps till you reach Account onbaording
  2. Sign into your account
  3. Once arriving at the "HealthKit Access" view navigate Back.
  4. Press the Logout button (this will trigger the will change of the Account instance where signedIn is still set to true, resulting in our onReceive handler getting called and navigating us to the next onboarding step).
  5. Continue with HealthKit Access
  6. You now finished the onboarding flow and entered the App without being signed into an Account

Expected behavior

I would expect the AccountSetup view to just re-render when pressing the Logout button, requiring me to create a new account or sign into a different account.
More specifically, as a programmer, I would expect that once the user completed the onboarding flow, I can be pretty sure that they are signed into an account.

Additional context

A possible workaround is to set signedIn to false before calling the Logout button action.

Button("Logout", role: .destructive) {
    account.signedIn = false // workaround

    try? Auth.auth().signOut()
}

I haven't gone into the specifics on how SpeziAccount/SpeziFirebase specifics work. It may be sensible to provide a first party API to handle sign out. Or is there already?

Code of Conduct

  • I agree to follow this project's Code of Conduct and Contributing Guidelines
@Supereg Supereg added the bug Something isn't working label Jun 11, 2023
@PSchmiedmayer PSchmiedmayer changed the title Bug report: Logout Button allows to skip Account Setup ogout Button Allows to Skip Account Setup Jun 13, 2023
@PSchmiedmayer PSchmiedmayer changed the title ogout Button Allows to Skip Account Setup Logout Button Allows to Skip Account Setup Jun 13, 2023
@PSchmiedmayer PSchmiedmayer moved this to Backlog in Project Planning Jun 13, 2023
@PSchmiedmayer
Copy link
Member

PSchmiedmayer commented Jun 13, 2023

This is an interesting issue; thank you for sharing this @Supereg!

SpeziAccount/SpeziFirebase does actually include the functionality to set the signedIn property to false when a user is logged out: StanfordSpezi/SpeziFirebase - Sources/SpeziFirebaseAccount/FirebaseAccountConfiguration.swift#L102

Can you reproduce that this method is called? Maybe we don't properly refresh the view or have some other errors in the handling logic to observe the signed in or signed out state?

@Supereg Supereg self-assigned this Jul 22, 2023
@Supereg
Copy link
Member Author

Supereg commented Jul 22, 2023

The problem is that we are navigating to the next onboarding step on the receive handler of the objectWillChange Publisher of the Account object. When signedIn is set to false, it will still be true in the willSet handler.

https://github.com/StanfordSpezi/SpeziTemplateApplication/blob/160887df60056dca8fed102cdd52e844d92f5b67/TemplateApplication/Onboarding/AccountSetup/AccountSetup.swift#L44C13-L50

I will address this issue once I'm upgrading the template app to the new SpeziAccount version.

@PSchmiedmayer PSchmiedmayer moved this from Backlog to In Progress in Project Planning Jul 23, 2023
@PSchmiedmayer
Copy link
Member

@Supereg Is this issue still relevant with the latest chances that we made in the template app?

@Supereg
Copy link
Member Author

Supereg commented Nov 6, 2023

You are right, this was resolved with #35

@Supereg Supereg closed this as completed Nov 6, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Project Planning Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants