feat: add Ims token for db commands#36
feat: add Ims token for db commands#36AjazSumaiya wants to merge 5 commits intoepic/abdb-implementationfrom
Conversation
nofuss
left a comment
nofuss
left a comment
There was a problem hiding this comment.
Question about token expiration.
src/utils/authHelper.js
Outdated
| if (currentToken) { | ||
| const validToken = await ims.validateToken(currentToken) | ||
| if (validToken.valid) { | ||
| return currentToken |
There was a problem hiding this comment.
Is there any chance that even if a token is "valid" that it might be expired? In other words, is there any need to generate a new token after some period of time?
There was a problem hiding this comment.
yeah, so token has expiration of 24 hrs, so if the token stored in local context expires, we need to generate new in that case
There was a problem hiding this comment.
aio-lib-ims already takes care of refreshing the token. Here what we should do is similar to https://github.com/adobe/aio-cli-plugin-app/blob/d28deaf0b2f8c973bf67fadea986834cdf143e50/src/lib/auth-helper.js#L36
but instead of using the CLI context (reserved context for user tokens, that triggers a SUSI login) we should set the context to the IMS_OAUTH_S2S env vars. See the quickstart in https://github.com/adobe/aio-lib-ims?tab=readme-ov-file#quickstart
bottom line:
- we should not manipulate the config ourselves, which is manipulated internally by the aio-lib-ims
contextconcept - we don't need to refresh the token
There was a problem hiding this comment.
One more thing, there is a caveat in the aio-lib-ims plugin system which will require us to set also a technical_account_email and technical_account_id in the context, this is needed so that aio-lib-ims recognizes the credential as a client credential for the oauth s2s flow. You should be able to generate the token by setting dummy values for those two fields.
src/utils/authHelper.js
Outdated
| if (tokenResponse.payload?.access_token) { | ||
| return tokenResponse.payload.access_token | ||
| } | ||
|
|
||
| if (tokenResponse.access_token) { | ||
| return tokenResponse.access_token | ||
| } | ||
|
|
||
| if (typeof tokenResponse === 'string') { | ||
| return tokenResponse | ||
| } |
There was a problem hiding this comment.
Do we not know the format of the response?
There was a problem hiding this comment.
checked response format and fixed
src/utils/authHelper.js
Outdated
| const runtimeNamespace = config.get(CONFIG_RUNTIME_NAMESPACE) | ||
|
|
||
| if (!runtimeNamespace) { | ||
| throw new Error('Runtime namespace is required. Please set CONFIG_RUNTIME_NAMESPACE.') |
There was a problem hiding this comment.
this is env AIO_RUNTIME_NAMESPACE
(this happens when the local project is not synced with console)
| @@ -0,0 +1,142 @@ | |||
| /* | |||
There was a problem hiding this comment.
Most of this logic is already handled by aio-lib-ims
please rewrite accordingly to https://github.com/adobe/aio-cli-plugin-app-storage/pull/36/changes#r2869855342
4 participants
Description
This PR changes DBBase Command to rely on IMS-lib to fetch or reuse IMS tokens for Ims authentication for db service
Changes done -
authHelper.getAccessToken()which internally uses Ims libauthHelper.getAccessToken()checksims.contexts.<runtime_namespace>for an existing tokenims.getAccessTokenByClientCredentials()ims.contexts.<namespace>.tokenRelated Issue
Motivation and Context
How Has This Been Tested?
On local
Screenshots (if appropriate):
Types of changes
Checklist: