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

Interaction between active login, passive login, and programmatically provided access token #55

Open
LeaVerou opened this issue Sep 2, 2023 · 4 comments
Labels
Milestone

Comments

@LeaVerou
Copy link
Contributor

LeaVerou commented Sep 2, 2023

Working on #11 recently, I realized a lot of tricky cases where I'm not sure we have the best DX.

For context, backend.login() supports two options:

  • passive: If true, only try to log in with locally stored credentials, do not show any UI (false by default)
  • accessToken (new): programmatically provided access token

1. Does backend.login() log the user out first?

Maybe it depends on the settings?

IMO passive login should be graceful, we need to be able to call it multiple times with no side effects if a user is already logged in. So it should not log the user out in that case.
I’m less sure about the case where it's an active login and/or an access token is provided (which may or may not be valid).

2. What happens if the access token provided programmatically is invalid?

backend.login({accessToken: invalidAccessToken})

What should Madata do if the access token is invalid?

  1. Nothing
  2. Console warning. The downside of that is that the calling code will have no way of knowing.
  3. Throw error

If we don't go with (3), what should happen next?

  1. Display login UI , unless passive: true is also specified
  2. Try to log in passively
  3. Nothing (return null)

I see a few valid scenarios here:

  1. Throwing, so that calling code can detect this and communicate it to the end-user. That way we also don't need to decide what to do. OTOH, this makes the API more cumbersome to use
  2. Nothing (or Console warning), then nothing else, just return null. This way, calling code can detect this by checking the return value.

3. Does a programmatically provided access token override stored credentials?

backend.login({ accessToken: foo, passive: true })

What should happen, if there's an access token in local storage? (i.e. passive login would succeed)

  1. Stored info takes precedence, ignore access token
  2. Access token takes precedence, ignore stored info

IMO 2: I see programmatically providing an access token as a more "active" form of login than using stored credentials, so if one is provided we should respect it. If calling code wants to use a programmatically provided access token as a fallback, they can try a passive login first, and then one where they provide the access token.

4. Do we always try to log in passively first?

backend.login();

What should happen?

  1. Passive login attempted first, if it fails, we move on to active.
  2. Straight to active login.

I guess that's also related to 1 (do we log out first?).
Right now we seem to try passive login first, then …discard the result, so the only case where it makes a difference is if the user aborts the active login.

Note that there are two use cases here:

  1. We want to access a protected resource: here we'd want to try passive first, then active
  2. Login button is clicked: we definitely want active to happen

If we go with (1) (passive login first), the login button use case would require backend.logout() first, otherwise it would result in weird bugs (though the end-user shouldn't be able to click the login button at all if they're logged in).
If we go with (2), both use cases are still served with a one liner. The second one with the single login() call, and the former like so:

(await backend.login({ passive: true })) ?? backend.login();

So it could go either way here.


Regardless of which way we go, we could also introduce new syntax to do the other thing. E.g. we may decide to have all three as separate: if you provide an access token, you just get authentication via an access token, if you specify that you want an active login, you just get that. But we could also have settings that attempt passive first, or passive as a fallback etc. Or we could introduce them at a later time, as sugar.

@LeaVerou LeaVerou added the DX label Sep 2, 2023
@joyously
Copy link

joyously commented Sep 2, 2023

Is there ever a case where a user might want to be logged in as two different users for the same backend, like two apps on the same page?

@LeaVerou
Copy link
Contributor Author

LeaVerou commented Sep 2, 2023

Is there ever a case where a user might want to be logged in as two different users for the same backend, like two apps on the same page?

Yes, it's not uncommon for people to have multiple accounts.

@LeaVerou LeaVerou added this to the Public launch milestone Jun 24, 2024
@LeaVerou
Copy link
Contributor Author

We should figure this out and document it before we launch, even if we don’t yet provide controls to override the behavior.

@DmitrySharabin
Copy link
Member

1. Does backend.login() log the user out first?

Maybe it depends on the settings?

I would provide an option for this. For example, the access token for Google backends is short-lived (an hour only). So, in most cases, when we try to log the user in passively, it'll fail. It would be nice if we had an option (e.g., forceLogout) that I could pass to the (grand) parent class so that when passive login is called (on class creation), it logs the user out. I might suggest that it can be helpful not only to the Google backends.

2. What happens if the access token provided programmatically is invalid?

2. Nothing (or Console warning), then nothing else, just return null. This way, calling code can detect this by checking the return value.

Seconded. I believe a console warning also makes sense here (additionally to the returned null).

3. Does a programmatically provided access token override stored credentials?

IMO 2: I see programmatically providing an access token as a more "active" form of login than using stored credentials, so if one is provided, we should respect it.

+1

  1. Do we always try to log in passively first?
    Right now, we seem to try passive login first

It seems that not every backend might call it super.login(), so backend authors have a choice of whether to log the user in passively or not. However, if they extend AuthBackend or any of its subclasses, they don't have that choice since AuthBackend calls this.login({passive: true}) on object creation:

this.login({passive: true});

I would advocate for this to be the default behavior. However, I also believe that having an option (one of the standard constructor options, for example) to not call this.login() on object creation might be worth considering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants