search

magento / architecture

A place where Magento architectural discussions happen
275 stars 153 forks source link

MC-34395: auth annotation #379

Closed cpartica closed 4 years ago

cpartica commented 4 years ago

Problem

PWA client needs to know what queries require authentication

Solution

Add @auth annotation to schema

Requested Reviewers

@paliarush @zetlen

DrewML commented 4 years ago

PWA client needs to know what queries require authentication

Can we include an example use-case?

DrewML commented 4 years ago

Would love to see prior art in this proposal, i.e examples of how other applications are handling a mix of authenticated and anonymous queries

zetlen commented 4 years ago

We originally asked for this because the PWA UX guidelines require that login state behave much like traditional sessions:

  1. Session timeout resets on every request, so it never expires while the user is actively using the site
  2. An expired session still connects to a user (warm auth) and allows segmentation

Since the PWA's thin backend doesn't create a session, it uses customer auth tokens alone to manage login. Those tokens always expire after a certain TTL, whether they're still being used or not. This has the unfortunate effect of unexpectedly logging a customer out, after an hour of using the site.

Our first idea was to let sessions expire and solve (2) above by using cached user data to "simulate" a logged-in state. However, that meant removing the expired token and popping a login only when the user tries to access a privileged value in the graph. Therefore, we wanted to optimize our fake-warm-auth experience by knowing ahead of time which queries would require login. That's why we proposed the @auth schema directive.

However, after speaking with @cpartica and @nrkapoor a bit more, we really need to solve (1) above by adding refresh tokens and readable TTLs. Right now, users get unexpectedly logged out; it's a high-priority, high-severity issue, and once it's solved, we don't really need the auth directive anymore.

DrewML commented 4 years ago

However, after speaking with @cpartica and @nrkapoor a bit more, we really need to solve (1) above by adding refresh tokens and readable TTLs. Right now, users get unexpectedly logged out; it's a high-priority, high-severity issue, and once it's solved, we don't really need the auth directive anymore.

Can we get clarification about what we're looking to proceed with? Not totally clear whether the ask is for @auth until token refresh exists, or if the ask if to build the token refresh functionality now and skip this auth proposal

DrewML commented 4 years ago

Any updates @zetlen @cpartica?

cpartica commented 4 years ago

yes, we close this out, it's not needed for the time beng