Add authorizations based on subaccount as part of attributes in BTP roles + additional role with space and page for WorkZone

[lemaiwo]

No description provided.

[willempardaens]

Thank you for your contribution Wouter. The use case / scenario with selective access certainly makes sense. Will have a look at it

[willempardaens]

Hi Wouter,
First of all a big thank you for contributing to this repository and the level of detail provided. I can see it covers a lot of scenarios and use cases. I didn't test the code yet, but read through it in detail.

A few questions/remarks:

1. Viewer role: we have to make sure that any customer who has implemented this solution already, and upgrades to the codeline with your contributions, can still work with the application in the same way. Today the Viewer role can access everything and it would be my expectation that this remains the same going forward. I fully understand the viewer/admin reasoning, but since we need to take care of the current 'viewer' role I think it is best to create a separate role for restricted access (maybe RestrictedViewer?) and leave the Viewer role as is. Makes sense? This role should then also be added in each of the @requires annotations of the service CDS files.
2. Alerts createdBy: the approach makes sense for newly created Alerts, but could be an issue for existing Alerts: the new field will be NULL for existing records. So we might have to add a SQL command in the server.ts to set this new field from modifiedBy in case it is NULL? This would be similar to other 'once off' migration steps required during startup.
3. Performance: My test environment is quite small/mockup, but we have customers running this in accounts with 100+ subaccounts. We have to assess what the impact is on performance, especially the resolveHierarchy function which is triggered often. So this is more of an open question if you had the chance to test behavior in a large account?

I will deploy it in a sanbox to play around with it later this week, but again, thank you for taking the time implementing the restricted access feature.

[lemaiwo]

Hello Willem,

Regarding your points:

1. The viewer role is a new role so existing users are not impacted if I'm not mistaken. If you are referring to the base role and you think it is too close to each other, changing it to RestrictedViewer is fine for me.

2. We needed this to only show the alerts a user created himself and not the once of other users or administrators. As we only start using it now, I don't know if it will impact existing alerts. I might have tested this and I had no errors in the very beginning but I did not fully test it.

3. Fair point and this is also one of my concerns, ideally it would be done using restrictions on the entity but this would require more changes to the data model so it's a bit a trade off, less impact but slower performance. At this point, as we are starting to test it, performance looks good for us.

Kr, Wouter

[willempardaens]

Hi Wouter,

Regarding point 1: I was referring to the backend role 'viewer' defined in xs-security.json. Existing users will have this role, without any assigned IDs, so after the update they will be restricted from viewing any sub account data. My suggestion was to leave this 'viewer' role as-is (treat this as the 'admin/all access' role) and create a separate 'restricted viewer' role which can have sub account IDs assigned.

Thanks
Willem