Add refreshTokenUpdatedHandler service client constructor option parameter

superfaceai / service-client

Library provides client for superface backend apis.

MIT License

5 stars 1 forks source link

Add refreshTokenUpdatedHandler service client constructor option parameter #52

Closed janhalama closed 3 years ago

janhalama commented 3 years ago

This PR addresses requirement of CLI to store refresh token in .netrc file (persistent storage).

Requirement has been discussed here: https://www.notion.so/superface/CLI-requirements-for-service-client-64f8645cffa74deeb5770004150c5bfa

I propose slightly different solution from the one agreed:

I did not add optional async function to get credentials from persistent storage, the reason is that options already contains refresh token and service url, which can be used to pass refresh token from persistent storage to ServiceClient
refreshTokenUpdatedHandler option parameter should be used to store refresh token in persistent storage once it has been changed
Brain token endpoint do not provide refresh token expiration, refresh token can be revoked any time, CLI should validate that refresh token is valid with each ServiceClient initialisation

Jakub-Vacek commented 3 years ago

Code looks ok 👍But I am not sure how will CLI validate that refresh token is valid with each ServiceClient initialisation 🤷

Jakub-Vacek commented 3 years ago

Also we could maybe call refreshTokenUpdatedHandler in login and logout functions 🤷

janhalama commented 3 years ago

Code looks ok 👍But I am not sure how will CLI validate that refresh token is valid with each ServiceClient initialisation 🤷

I see two options:

CLI can call refreshAccessToken method by itself
CLI can fetch user info

janhalama commented 3 years ago

Also we could maybe call refreshTokenUpdatedHandler in login and logout functions 🤷

You are right 👍

Jakub-Vacek commented 3 years ago

Code looks ok 👍But I am not sure how will CLI validate that refresh token is valid with each ServiceClient initialisation 🤷

I see two options:

CLI can call refreshAccessToken method by itself

CLI can fetch user info

I was thinking about the first option, refreshAccessToken is called in fetch when we have expired accessToken (and we need to auth our call) , maybe this is enough for now - enforcing valid accessToken during ServiceClient initialization (in CLI) will force user to login when he wants to just consume stuff not publish.

janhalama commented 3 years ago

@Jakub-Vacek I moved refreshTokenUpdatedHandler calling in login and logout methods, could you please check it once more

janhalama commented 3 years ago

@kysely this PR will introduce breaking change into service-client interface, login and logout methods are asynchronous now, could you confirm that it will not be big deal for air

kysely commented 3 years ago

@kysely this PR will introduce breaking change into service-client interface, login and logout methods are asynchronous now, could you confirm that it will not be big deal for air

Thanks for pointing this out @janhalama . I did a search through the Air repo & updated all calls to .login (logout method isn't used anywhere in Air).

Prepared in Air PR #189. It is ready to simply have the service-client version bumped and can be merged.

I will do it once this is released, just please ping me when it's available on NPM :)