feat: RBAC | ACLs based access management

[ravverma]

Please go through wiki for more details :
https://wiki.corp.adobe.com/display/AEMSites/Project+Spacecat+%7C+RBAC

Please ensure your pull request adheres to the following guidelines:

• make sure to link the related issues in this description
• when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

Thanks for contributing!

[bosschaert]

Why does anonymous get full access to everything here?

[ravverma]

this is not actually anonymous, these are slack end points.
and we have slack command for create org , site entities , so with the intent of not breaking any existing flow giving access.
@solaris007 Can we restrict it without any break in current flow ?

[bosschaert]

Ok - I'll take your word for it but it does say user_id: 'anonymous' ...

[solaris007]

i suggest just feeding in the context itself and let the method handle the resolution. this is public API and users should not have to deal with that line. while we're at it, we can probably reduce the signature to context only, as the table name originates from context.env usually.

[ravverma]

So If I am understanding you correctly here we should only feed the whole context in config i.e.

context.dataAccess = createDataAccess({
       context,
      }, log);

and in service layer resolve it to aclCtx, i.e.

export const createDataAccess = (config, log = console, client = undefined) => {
  registerLogger(log);

 // resolve here
 config.aclCtx = config?.context?.attributes?.authInfo?.rbac || {};
 config.tableNameData: config?.context?.env?.DYNAMO_TABLE_NAME_DATA;

  const rawClient = createRawClient(client);
  const electroService = createElectroService(rawClient, config, log);
  const entityRegistry = new EntityRegistry(electroService, config, log);

  return entityRegistry.getCollections();
};

[ravverma]

one more thing to keep it like that way as it is, that the API createDataAccess is used to create dataAccess for RBAC table as well with different ACls ( allow to read role role-member entities) as per need , for example in ACLs fetching while authenticate.
So I think we can keep it like this way to have flexibility

[solaris007]

why was this moved before the validated items size check?

[ravverma]

AFAIU
after the size check , in the next line validatedItems got saved in DB
const response = await this.entity.put(validatedItems).go();
so using #createInstances after it just for get object in-memory and call onCreateMany which actually do nothing.
so it moving up before saving and check permission make sense to me.
@bosschaert Correct me I am wrong here ?

[solaris007]

it is more that we can save the computational effort (as before) if none of the items are valid.

[ravverma]

yup, so we can move permission check within the valid check block like

const createdItems = [];
if (validatedItems.length > 0) {
        createdItems = this.#createInstances(validatedItems);
        // Check that the current user has permission to create each entity
        createdItems.forEach((item) => item.ensurePermission('C'));
        const response = await this.entity.put(validatedItems).go();

        if (isNonEmptyArray(response?.unprocessed)) {
          this.log.error(`Failed to process all items in batch write for [${this.entityName}]: ${JSON.stringify(response.unprocessed)}`);
        }
      }

[ravverma]

moved it after validation, permission check happening before creation

[solaris007]

please complete this todo - also, projection via electrodb might be considered to reduce payload size. only project props that are needed by ensurePermission.

[ravverma]

will look into it 👍

[ravverma]

considered the projection to reduce entity payload and batched the processing, please check

[github-actions]

This PR will trigger a minor release when merged.

[bosschaert]

@ravverma in general it looks good to me, but I'll leave the final review to @solaris007

[ravverma]

@ravverma in general it looks good to me, but I'll leave the final review to @solaris007

Thanks @bosschaert
Adding TODO before merge here

1. Remove unnecessary logs
2. Put back entities in exception list for zero impact
3. Need released version here, may be one more PR after it need to add released version
4. convert requestedResourceUrl to singular to be match with ACLs path