Skip to content
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

Merged
merged 28 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3fd90eb
feat: Add new queries to create and retrieve api keys
swetabar Nov 7, 2024
84b71a3
fix: Fix comments
swetabar Nov 7, 2024
05335db
fix: Add a couple more unit tests
swetabar Nov 7, 2024
d55c268
Update packages/spacecat-shared-data-access/src/dto/api-key.js
swetabar Nov 8, 2024
d91f203
Update packages/spacecat-shared-data-access/src/models/api-key/api-ke…
swetabar Nov 8, 2024
9b88940
Merge branch 'main' into SITES-25915-generate-api-keys-for-importer
swetabar Nov 8, 2024
35a30c6
fix: Update package-lock.json
swetabar Nov 8, 2024
fd186b6
fix: Change the function name getApiKeysByImsUserIdAndImsOrgId
swetabar Nov 8, 2024
d712d8c
fix: Change the error message for deletedAt
swetabar Nov 8, 2024
f2d39e8
fix: Fix the maxlen issue
swetabar Nov 8, 2024
4594e36
fix: Modify the return type to an array
swetabar Nov 8, 2024
2d30f6d
fix: Remove status from ApiKey
swetabar Nov 8, 2024
39705d0
fix: Remove status from UTs
swetabar Nov 8, 2024
e78553f
fix: remove updateStatus from a unit test
swetabar Nov 8, 2024
a8bf73a
Merge branch 'main' into SITES-25915-generate-api-keys-for-importer
swetabar Nov 8, 2024
77f24ab
fix: update package-lock.json
swetabar Nov 8, 2024
ae834c3
fix: remove status
swetabar Nov 8, 2024
b53564b
Update packages/spacecat-shared-data-access/src/service/api-key/index.js
swetabar Nov 8, 2024
eb36d56
fix: refactor
swetabar Nov 8, 2024
9142b07
fix: add unit tests and function for isValid
swetabar Nov 8, 2024
fd2b122
Merge branch 'main' into SITES-25915-generate-api-keys-for-importer
swetabar Nov 8, 2024
d7334ca
fix: Add more unit tests
swetabar Nov 8, 2024
0bbc7eb
fix: add the latest package-lock.json
swetabar Nov 8, 2024
9df129c
fix: change to Date object
swetabar Nov 8, 2024
69b914b
fix: Fix the date comparison
swetabar Nov 8, 2024
14b1535
fix: Add real iso strings for comparison
swetabar Nov 8, 2024
fdf2f6e
fix: Fix the unit tests
swetabar Nov 8, 2024
2c6e841
fix: Fix the year
swetabar Nov 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3,873 changes: 3,700 additions & 173 deletions package-lock.json

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions packages/spacecat-shared-data-access/docs/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1209,11 +1209,45 @@
"AttributeName": "revokedAt",
"AttributeType": "S"
},
{
"AttributeName": "deletedAt",
"AttributeType": "S"
},
{
"AttributeName": "scopes",
"AttributeType": "M"
}
],
"GlobalSecondaryIndexes": [
{
"IndexName": "spacecat-services-api-key-by-ims-user-id-and-ims-org-id",
"KeyAttributes": {
"PartitionKey": {
"AttributeName": "imsUserId",
"AttributeType": "S"
},
"SortKey": {
"AttributeName": "imsOrgId",
"AttributeType": "S"
}
},
"Projection": {
"ProjectionType": "ALL"
}
},
{
"IndexName": "spacecat-services-api-key-by-hashed-api-key",
"KeyAttributes": {
"PartitionKey": {
"AttributeName": "hashedApiKey",
"AttributeType": "S"
}
},
"Projection": {
"ProjectionType": "ALL"
}
}
],
"DataAccess": {
"MySql": {}
},
Expand Down
9 changes: 6 additions & 3 deletions packages/spacecat-shared-data-access/src/dto/api-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@
* governing permissions and limitations under the License.
*/

import { createApiKey } from '../models/api-key.js';
import { createApiKey } from '../models/api-key/api-key.js';

export const ApiKeyDto = {

/**
* Converts an ApiKey object into a DynamoDB item.
* @param apiKey
* @returns {{createdAt: *, name: *, imsUserId, scopes, revokedAt, key: *,
* imsOrgId: *, expiresAt: *}}
* @returns {{createdAt: string, name: string, imsUserId: string,
* scopes: array<object>, revokedAt: string, deletedAt: string,
* hashedApiKey: string, imsOrgId: string, expiresAt: string}}
*/
toDynamoItem: (apiKey) => ({
id: apiKey.getId(),
Expand All @@ -29,6 +30,7 @@ export const ApiKeyDto = {
createdAt: apiKey.getCreatedAt(),
expiresAt: apiKey.getExpiresAt(),
revokedAt: apiKey.getRevokedAt(),
deletedAt: apiKey.getDeletedAt(),
scopes: apiKey.getScopes(),
}),

Expand All @@ -47,6 +49,7 @@ export const ApiKeyDto = {
createdAt: dynamoItem.createdAt,
expiresAt: dynamoItem.expiresAt,
revokedAt: dynamoItem.revokedAt,
deletedAt: dynamoItem.deletedAt,
scopes: dynamoItem.scopes,
};

Expand Down
26 changes: 26 additions & 0 deletions packages/spacecat-shared-data-access/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,26 @@ export interface ApiKey {
*/
getRevokedAt: () => string;

/**
* Retrieves the deletedAt of the API Key.
*/
getDeletedAt: () => string;

/**
* Retrieves the scopes of the API Key.
*/
getScopes: () => Array<string>;

/**
* Updates the deletedAt attribute of the API Key.
*/
updateDeletedAt: (deletedAt: string) => ApiKey;

/**
* Indicates whether the API Key is valid.
*/
isValid: () => boolean;

}

/**
Expand Down Expand Up @@ -839,6 +854,16 @@ export interface DataAccess {
createNewApiKey: (
apiKeyData: object,
) => Promise<ApiKey>;
updateApiKey: (
apiKey: ApiKey,
) => Promise<ApiKey>;
getApiKeysByImsUserIdAndImsOrgId: (
imsUserId: string,
imsOrgId: string,
) => Promise<ApiKey[] | null>;
getApiKeyById: (
id: string,
) => Promise<ApiKey | null>;

// site candidate functions
getSiteCandidateByBaseURL: (baseURL: string) => Promise<SiteCandidate>;
Expand Down Expand Up @@ -893,6 +918,7 @@ interface DataAccessConfig {
indexNameAllImportJobsByDateRange: string,
indexNameImportUrlsByJobIdAndStatus: string,
indexNameApiKeyByHashedApiKey: string,
indexNameApiKeyByImsUserIdAndImsOrgId: string,
pkAllSites: string;
pkAllLatestAudits: string;
pkAllOrganizations: string;
Expand Down
5 changes: 5 additions & 0 deletions packages/spacecat-shared-data-access/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const INDEX_NAME_ALL_IMPORT_JOBS_BY_STATUS = 'spacecat-services-all-import-jobs-
const INDEX_NAME_ALL_IMPORT_JOBS_BY_DATE_RANGE = 'spacecat-services-all-import-jobs-by-date-range-dev';
const INDEX_NAME_ALL_IMPORT_URLS_BY_JOB_ID_AND_STATUS = 'spacecat-services-all-import-urls-by-job-id-and-status-dev';
const INDEX_NAME_API_KEY_BY_HASHED_API_KEY = 'spacecat-services-api-key-by-hashed-api-key-dev';
const INDEX_NAME_API_KEY_BY_IMS_USER_ID_AND_IMS_ORG_ID = 'spacecat-services-api-key-by-ims-user-id-and-ims-org-id-dev';

const PK_ALL_SITES = 'ALL_SITES';
const PK_ALL_CONFIGURATIONS = 'ALL_CONFIGURATIONS';
Expand Down Expand Up @@ -76,6 +77,8 @@ export default function dataAccessWrapper(fn) {
DYNAMO_INDEX_NAME_ALL_IMPORT_URLS_BY_JOB_ID_AND_STATUS =
INDEX_NAME_ALL_IMPORT_URLS_BY_JOB_ID_AND_STATUS,
DYNAMO_INDEX_NAME_API_KEY_BY_HASHED_API_KEY = INDEX_NAME_API_KEY_BY_HASHED_API_KEY,
DYNAMO_INDEX_NAME_API_KEY_BY_IMS_USER_ID_AND_IMS_ORG_ID =
INDEX_NAME_API_KEY_BY_IMS_USER_ID_AND_IMS_ORG_ID,
} = context.env;

context.dataAccess = createDataAccess({
Expand All @@ -102,6 +105,8 @@ export default function dataAccessWrapper(fn) {
indexNameAllImportJobsByDateRange: DYNAMO_INDEX_NAME_ALL_IMPORT_JOBS_BY_DATE_RANGE,
indexNameImportUrlsByJobIdAndStatus: DYNAMO_INDEX_NAME_ALL_IMPORT_URLS_BY_JOB_ID_AND_STATUS,
indexNameApiKeyByHashedApiKey: DYNAMO_INDEX_NAME_API_KEY_BY_HASHED_API_KEY,
indexNameApiKeyByImsUserIdAndImsOrgId:
DYNAMO_INDEX_NAME_API_KEY_BY_IMS_USER_ID_AND_IMS_ORG_ID,
pkAllSites: PK_ALL_SITES,
pkAllOrganizations: PK_ALL_ORGANIZATIONS,
pkAllLatestAudits: PK_ALL_LATEST_AUDITS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import {
hasText, isIsoDate, isObject, isValidUrl,
} from '@adobe/spacecat-shared-utils';
import { Base } from './base.js';
import { Base } from '../base.js';

// List of known scope names that can be used with scoped API keys
const scopeNames = [
Expand All @@ -40,8 +40,79 @@ const ApiKey = (data) => {
self.getCreatedAt = () => self.state.createdAt;
self.getExpiresAt = () => self.state.expiresAt;
self.getRevokedAt = () => self.state.revokedAt;
self.getDeletedAt = () => self.state.deletedAt;
self.getScopes = () => self.state.scopes;

/**
Copy link
Contributor

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or isActive(), alternatively?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

* Checks if the apiKey is valid.
* @returns {boolean} True if the apiKey is valid, false otherwise
*/
self.isValid = () => {
const now = new Date();

if (self.state.deletedAt && new Date(self.state.deletedAt) < now) {
return false;
}

if (self.state.revokedAt && new Date(self.state.revokedAt) < now) {
return false;
}

if (self.state.expiresAt && new Date(self.state.expiresAt) < now) {
return false;
}

return true;
};

/**
* Updates the state of the ApiKey.
* @param key - The key to update.
* @param value - The new value.
* @param validator - An optional validation function to use before updating the value.
* @returns {ApiKey} The updated ApiKey object.
*/
const updateState = (key, value, validator) => {
if (validator && typeof validator === 'function') {
validator(value);
}

self.state[key] = value;
self.touch();

return self;
};

/**
* Updates the deletedAt attribute of the ApiKey.
* @param {string} deletedAt - The deletedAt timestamp - ISO 8601 date string.
*/
self.updateDeletedAt = (deletedAt) => updateState('deletedAt', deletedAt, (value) => {
if (!isIsoDate(value)) {
throw new Error(`Invalid deletedAt during update: ${value}. Must be a valid ISO 8601 date string.`);
}
});

/**
* Updates the expiresAt attribute of the ApiKey.
* @param {string} expiresAt - The expiresAt timestamp - ISO 8601 date string.
*/
self.updateExpiresAt = (expiresAt) => updateState('expiresAt', expiresAt, (value) => {
if (!isIsoDate(value)) {
throw new Error(`Invalid expiresAt during update: ${value}. Must be a valid ISO 8601 date string.`);
}
});

/**
* Updates the revokedAt attribute of the ApiKey.
* @param {string} revokedAt - The revokedAt timestamp - ISO 8601 date string.
*/
self.updateRevokedAt = (revokedAt) => updateState('revokedAt', revokedAt, (value) => {
if (!isIsoDate(value)) {
throw new Error(`Invalid revokedAt during update: ${value}. Must be a valid ISO 8601 date string.`);
}
});

return Object.freeze(self);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@
* governing permissions and limitations under the License.
*/

import { createNewApiKey, getApiKeyByHashedApiKey } from './accessPatterns.js';
import {
createNewApiKey,
getApiKeyByHashedApiKey,
getApiKeyById,
getApiKeysByImsUserIdAndImsOrgId,
updateApiKey,
} from './accessPatterns.js';

export const apiKeyFunctions = (dynamoClient, config, log) => ({
getApiKeyByHashedApiKey: (hashedApiKey) => getApiKeyByHashedApiKey(
Expand All @@ -20,4 +26,12 @@ export const apiKeyFunctions = (dynamoClient, config, log) => ({
config,
),
createNewApiKey: (apiKey) => createNewApiKey(apiKey, dynamoClient, config, log),
getApiKeysByImsUserIdAndImsOrgId: (imsUserId, imsOrgId) => getApiKeysByImsUserIdAndImsOrgId(
imsUserId,
imsOrgId,
dynamoClient,
config,
),
getApiKeyById: (id) => getApiKeyById(id, dynamoClient, config),
updateApiKey: (apiKey) => updateApiKey(apiKey, dynamoClient, config),
});
Loading
Loading