[discuss] accounts-server token creator

[ozsay]

Hi,

In our project, we use a 3rd party authenticator service. This service manages access tokens itself. We wrote an AuthenticationService to support it, but it has duplicate code with accounts-server (and specific graphql endpoints) since we can't use its loginWithService and refreshTokens functionality.

The reason why we can't use loginWithService is the way we deliver our 3rd party token to the client. The only way for us to do that with accountsjs is to use it as a session token. then we create access token and refresh token using accounts and deliver them. since accounts-server generates the session token itself, loginWithService is not capable of supporting this scenario.

The way we thought of solving it, is to provide accounts-server with a custom token creator which will request the access token from the authenticator service and return it to be used as a session token.

The other problem is with refreshTokens. When the authenticator service tells us to refresh its tokens, we currently invalidate the current session and create a new one with the new access token. we want to be able to use accountsServer functionality to do that. that's why we want to add createSessionTokenOnRefresh option to create a new session token when refreshing tokens.

These two changes allow us to use more of accounts-server and client functionality without implementing it ourselves.

@davidyaha @elie222 FYI

[codecov]

Codecov Report

Merging #774 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #774      +/-   ##
==========================================
+ Coverage   95.15%   95.17%   +0.02%     
==========================================
  Files          80       80              
  Lines        1774     1783       +9     
  Branches      348      352       +4     
==========================================
+ Hits         1688     1697       +9     
  Misses         78       78              
  Partials        8        8

Impacted Files	Coverage Δ
packages/server/src/accounts-server.ts	90.81% <100%> (+0.3%)	⬆️
packages/database-mongo/src/mongo.ts	99.31% <100%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dca8132...31f6a21. Read the comment docs.

[codecov]

Codecov Report

Merging #774 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #774      +/-   ##
==========================================
+ Coverage   95.15%   95.17%   +0.02%     
==========================================
  Files          80       80              
  Lines        1774     1783       +9     
  Branches      348      352       +4     
==========================================
+ Hits         1688     1697       +9     
  Misses         78       78              
  Partials        8        8

Impacted Files	Coverage Δ
packages/server/src/accounts-server.ts	90.81% <100%> (+0.3%)	⬆️
packages/database-mongo/src/mongo.ts	99.31% <100%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dca8132...cf4554f. Read the comment docs.

[pradel]

These options make sense, just left some minor comments but otherwise, the pr looks good!

[pradel]

Does it needs to be a separate interface or can it be just a function?

[ozsay]

as I think about our use-case, the creator should receive "static" options. so I think it will be better to implement it as an interface that you can instantiate ratter than passing the options in the function

[TimMikeladze]

Looks good.