-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support queries to generate self-service api-keys #428
Changes from all commits
3fd90eb
84b71a3
05335db
d55c268
d91f203
9b88940
35a30c6
fd186b6
d712d8c
f2d39e8
4594e36
2d30f6d
39705d0
e78553f
a8bf73a
77f24ab
ae834c3
b53564b
eb36d56
9142b07
fd2b122
d7334ca
0bbc7eb
9df129c
69b914b
14b1535
fdf2f6e
2c6e841
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,14 +11,15 @@ | |
*/ | ||
|
||
import { ApiKeyDto } from '../../dto/api-key.js'; | ||
import { createApiKey } from '../../models/api-key/api-key.js'; | ||
|
||
/** | ||
* Get ApiKey by Hashed Key | ||
* @param {string} hashedApiKey | ||
* @param {DynamoClient} dynamoClient | ||
* @param {Object} config | ||
* @param {Logger} log | ||
* @returns {Promise<ApiKeyDto>} | ||
* @returns {Promise<ApiKey>} | ||
*/ | ||
export const getApiKeyByHashedApiKey = async (hashedApiKey, dynamoClient, log, config) => { | ||
const items = await dynamoClient.query({ | ||
|
@@ -42,10 +43,68 @@ export const getApiKeyByHashedApiKey = async (hashedApiKey, dynamoClient, log, c | |
* @param {DynamoClient} dynamoClient | ||
* @param {Object} config | ||
* @param {Logger} log | ||
* @returns {Promise<ApiKeyDto>} | ||
* @returns {Promise<ApiKey>} | ||
*/ | ||
export const createNewApiKey = async (apiKey, dynamoClient, config, log) => { | ||
const item = ApiKeyDto.toDynamoItem(apiKey); | ||
await dynamoClient.putItem(config.tableNameApiKeys, item, log); | ||
const item = createApiKey(apiKey); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this function was previously invoked with an API key object. Has it changed to accept a POJO instead? If so, the JSDoc should be updated to reflect that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we were not using this method before. I refactored it to work similar to how import-job works now for consistency. |
||
await dynamoClient.putItem(config.tableNameApiKeys, ApiKeyDto.toDynamoItem(item), log); | ||
return item; | ||
}; | ||
|
||
/** | ||
* Get ApiKeys by IMS User ID and IMS Org ID | ||
* @param imsUserId | ||
* @param imsOrgId | ||
* @param dynamoClient | ||
* @param config | ||
* @returns {Promise<ApiKey[]>} | ||
*/ | ||
export const getApiKeysByImsUserIdAndImsOrgId = async ( | ||
imsUserId, | ||
imsOrgId, | ||
dynamoClient, | ||
config, | ||
) => { | ||
const items = await dynamoClient.query({ | ||
TableName: config.tableNameApiKeys, | ||
IndexName: config.indexNameApiKeyByImsUserIdAndImsOrgId, | ||
KeyConditionExpression: '#imsUserId = :imsUserId AND #imsOrgId = :imsOrgId', | ||
ExpressionAttributeNames: { | ||
'#imsUserId': 'imsUserId', | ||
'#imsOrgId': 'imsOrgId', | ||
}, | ||
ExpressionAttributeValues: { | ||
':imsUserId': imsUserId, | ||
':imsOrgId': imsOrgId, | ||
}, | ||
}); | ||
return items.map((item) => ApiKeyDto.fromDynamoItem(item)); | ||
}; | ||
|
||
/** | ||
* Get ApiKey by ID | ||
* @param id | ||
* @param dynamoClient | ||
* @param config | ||
* @returns {Promise<ApiKey|null>} | ||
*/ | ||
export const getApiKeyById = async (id, dynamoClient, config) => { | ||
const item = await dynamoClient.getItem(config.tableNameApiKeys, { id }); | ||
return item ? ApiKeyDto.fromDynamoItem(item) : null; | ||
}; | ||
|
||
/** | ||
* Update ApiKey | ||
* @param apiKey | ||
* @param dynamoClient | ||
* @param config | ||
* @returns {Promise<ApiKey>} | ||
*/ | ||
export const updateApiKey = async (apiKey, dynamoClient, config) => { | ||
const existingApiKey = await getApiKeyById(apiKey.getId(), dynamoClient, config); | ||
if (!existingApiKey) { | ||
throw new Error(`API Key with id ${apiKey.getId()} not found`); | ||
} | ||
await dynamoClient.putItem(config.tableNameApiKeys, ApiKeyDto.toDynamoItem(apiKey)); | ||
return apiKey; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this model be the right place for an
isValid()
method, which could check all the internal properties to ensure that a key is still valid?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or
isActive()
, alternatively?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do that in the service. Do we need it in spacecat-shared?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do so in the service than anywhere that re-uses this model will need to implement something similar. I see it as a direct concern of the model itself, "am I still valid", and think that it makes sense to have that logic live alongside the model.
I also like the pattern of "asking" the entity itself, are you still valid:
if (apiKeyInstance.isValid()) {
— nice and clean.