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

Add Flow for passwordless authentication #3678

Conversation

pascalwei
Copy link
Contributor

The access token method is inferior to password login for a non-password account because the default fallback for an invalid/expired token is the require sign in screen.

Should fix #3567.

The access token method is inferior to password login for a non-password account because the default fallback for an invalid/expired token is the require sign in screen.
@nielsvanvelzen
Copy link
Member

This would cause the app to constantly create new sessions which is not a desired behavior. If there is an access token we must use that.

- Fix forced sign in on invalid session token
- Exclude no password accounts from always authenticate
@pascalwei
Copy link
Contributor Author

I didn't see any problem in the server dashboard when logging in using the credentials instead of the token. But I agree that a cleaner solution is to use the token if we have one.

In an effort to fix the problem, I created a no password flow that tries the session token first and then a blank password for accounts marked as passwordless. If both don't work, it falls back to requiring a login.

While working with the login mechanism, I noticed that the Always authenticate setting also applies to passwordless accounts. I would like to change this to always authenticate password protected accounts.

I think this is more user friendly, as a parent might always want to log in with their own password-protected account, but a child's passwordless account shouldn't have to confirm a blank password every time they log in. This could also be the reason for the linked issue, not the fact that a session has been deleted / expired.

@pascalwei pascalwei changed the title Prefer login without password over access token Add Flow for passwordless authentication Jun 20, 2024
@pascalwei
Copy link
Contributor Author

Is this a better approach to fix the linked issue @nielsvanvelzen?

@nielsvanvelzen
Copy link
Member

The "automatic sign in" option is slightly different then the name implies. What it actually does is skip the "Who's watching?" screen when you start the app (e.g. auto-select a user) as long as there is a valid access token.

So this is 100% meant to work for password protected users. Currently, the server exposes an boolean called "HasPassword", which we do use in some places to skip the password prompt. We'll remove this in the future (could be 10.10, could be later) so we shouldn't add additional behavior that relies on it.

@pascalwei
Copy link
Contributor Author

Oh, I think there has been a misunderstanding. In the Authentication tab there are two options, the "automatic sign in" you wrote about and the "always ask for credentials" setting. I'd like to ignore "always ask for credentials" when we have a user marked as having no password in the local authenticationStore.

Screenshot_20240620_214435

Another thing is that when we use the (default) token-based method in authenticateAutomatic and the token is no longer working, for example because a session got cleared in the server, we always prompt for a password and don't remember we could just try to get a fresh token with an empty password. That's all the new flow does.

The change doesn't rely on a variable provided by the server. I imagine that once the variable is removed from the server, it is only stored locally after the first login? Or do we remove no password accounts altogether?

@nielsvanvelzen
Copy link
Member

Sorry my bad, thanks for clarifying. The answer is still mostly the same. The local user model is mostly cached data from the server. The value for "requirePassword" is set from "hasPassword". So once the server support for this property is removed it will always be false (we don't remove properties to prevent API breakage, so server would always return false).

So in all cases, this would prompt for a password when authentication is required. We don't know there is no password. We technically could set it to true when a user manually signs in with a password, but this comes with other caveats. We always prefer QuickConnect, in this case we still don't know if there is no password. This would cause strange UX where only sometimes we know there is no password required, and other times we still think we do.

Or do we remove no password accounts altogether?

Once it's removed server-side we'll remove that support locally too. In fact, if we know 10.10 will remove it we may already remove it from the client before 10.10 is released.

@pascalwei pascalwei closed this Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

skip login for accounts with no password
2 participants