docs icon indicating copy to clipboard operation
docs copied to clipboard

[SDK] Authentication arguments inconsistency

Open sandros94 opened this issue 1 year ago • 2 comments

Describe the Bug

Currently the Authentication via SDK has some inconsistencies. To be more precise the issue is with both operation arguments as well as defaults used for each one (for now I can confirm with rest, but also auth isn't clear):

Description

I'll separate the Rest description form the Auth questions

Rest

login:

Code defaults to cookie (as it should) https://github.com/directus/directus/blob/75c03bdd997714fde85148c39ad14bfa975e19c7/sdk/src/rest/commands/auth/login.ts#L24 While docs say json

Screenshot

logout and refresh

Both correctly defaults to cookie, no docs info (assuming json as login), but also the order of the arguments is inconsistent. Which comes first, the chicken or the egg? Also the body: JSON.stringify(....) is inconsistent, causing potential developer error in making the refresh work, but not the logout (and vice versa). https://github.com/directus/directus/blob/75c03bdd997714fde85148c39ad14bfa975e19c7/sdk/src/rest/commands/auth/logout.ts#L12-L18 https://github.com/directus/directus/blob/75c03bdd997714fde85148c39ad14bfa975e19c7/sdk/src/rest/commands/auth/refresh.ts#L12-L21

Auth

login's mode vs logout and refresh

In the auth's login the mode can be manually overwritten, but what would be the advantage if it cannot be also overwritten for logout and refresh? Wouldn't this cause inability for neither refreshing nor invalidating user's sessions if the developer erroneously edited the mode during login?

  • login: https://github.com/directus/directus/blob/75c03bdd997714fde85148c39ad14bfa975e19c7/sdk/src/auth/composable.ts#L131
  • logout: https://github.com/directus/directus/blob/75c03bdd997714fde85148c39ad14bfa975e19c7/sdk/src/auth/composable.ts#L167
  • refresh: https://github.com/directus/directus/blob/75c03bdd997714fde85148c39ad14bfa975e19c7/sdk/src/auth/composable.ts#L103

To Reproduce

These issues were original discussed inside the nuxt-directus#254 issue. But only now confirmed as I finally have noticed my error in the order of arguments for refresh and logout, and documentation declaring a different default.

Directus Version

v10.10.5

Hosting Strategy

Self-Hosted (Docker Image)

sandros94 avatar Apr 14 '24 22:04 sandros94

Some good points of inconsistencies that have cropped up over time. A small disclaimer the linked docs pages under "API reference" were written for the API not the SDK and while they have a lot of overlap if there are conflicting defaults we have no good place to document that currently.

image The default being cookie in the SDK is actually the error here as it is correctly documented to be json for the API but a different decision was made for the SDK early on which requires a breaking change to fix and misses a place to properly document that beyond the auto-generated reference in the documentation.

br41nslug avatar Apr 15 '24 10:04 br41nslug