LC0068 on tableextension object

[Arthurvdv]

To verify this, is there a way to assign permissions in the scenario below?

tableextension 50100 MyCustomer extends Customer
{
    procedure MyProcedure()
    var
        ShiptoAddress: Record "Ship-to Address";
    begin
        ShiptoAddress.SetRange("Customer No.", Rec."No.");
        ShiptoAddress.FindFirst(); // The current object is missing permission "r" for tabledata "Ship-to Address" (LC0068)
    end;
}

I thought of adding the InherentPermissions Attribute, to the method like this: [InherentPermissions(PermissionObjectType::TableData, Database::"Ship-to Address", 'r', InherentPermissionsScope::Permissions)] ... but that raises another error An application object 'Table 222' could not be found in the current extension. Only application objects that belong to the current extensions can be used in this context. AL0720.

[StefanMaron]

No there is not, if you want to comply with the rule you have to move the code to a codeunit.

[Arthurvdv]

I'm a bit doubting here on what is the best approach is here. I initially thought of adding this to the documentation of this rule, where I now doubting on if excluding records that a not part of the App itself and inside a table(extension) object, from this rule makes more sense?

Potentially we could add an extra rule, something like "..move database operation outside table(extension) object for managing (indirect) permissions.." to catch these.

I've created an PR for this, would be great to have your thoughts on this.

[TKapitan]

@Arthurvdv @StefanMaron Yes, I believe excluding the objects that are not part of the App is correct solution. We have now 150+ warnings in most of the apps for base app tables (Customer, Currency, Bank Account - standard tables where we can't add inherentopermissions).

[StefanMaron]

@TKapitan But still, just because the language doesn't support adding inherent permissions in extensions objects, doesn't mean you don't need them.

If you don't care, you can either disable the rule completely, or put a pragma at the top of the extension objects

[TKapitan]

@StefanMaron what should this message for objects not under our control indicate? What should be the steps to fix the code without pragma?

I like the rule for my own objects, I don’t want to disable the whole rule, but also I can’t apply pragma in 300 places (another extension) as it would make code really messy

[StefanMaron]

@TKapitan I am not sure I understand the issue then. In the example Arthur gave at the top he is creating a Table extension which he obviously has control over.

In order to comply with the rule to set permissions for all objects, you can not do the write/read/delete/modify on the extension object itself, you need to delegate this to a codeunit

[TKapitan]

@StefanMaron What’s the difference in this case of having it in CU or in a tableextension?

[StefanMaron]

@TKapitan The difference is that the Codeunit supports the Permissions property

[Arthurvdv]

I would prefer to comply with this rule if it can be addressed using the Permissions property or the InherentPermissions decorator. Initially, I considered excluding objects in a (table)extension that aren’t part of the App itself in the PR, but this seems contrary to the spirit of the rule.

My idea was to introduce a second rule for greater flexibility:

• LC0068 with exclude table(extension)
• LC00xx move database operation outside table(extension) object for managing (indirect) permissions

This would provide more control to handle indirect permissions partially. Handling only part of the indirect permissions scenarios feels incomplete to be honest. And playing devil's advocate here: Would it really benefit users if indirect permissions generally work, but fails in some cases?

With that in mind, I’m considering canceling my PR and moving my code into a codeunit instead.

[StefanMaron]

Thanks @Arthurvdv this explains my point of view as well. I would not mind to split this into two Rules, but like you described (and like I tried to describe before :D) this would still leave code in extension objects with potential issues.

I also want to remind again, what my initial motivation for this rule was:

For some tables and pages the Microsoft license dictates that we (developers) need to use the indirect permission assignment in code. And since we have no way to find out at compile time which objects are affected by this, my solution was to just have the permissions always defined, as its better practice anyways.

But we might update the diagnistic description to give a hint on extension objects, that the code has to be moved? How about that?

[StefanMaron]

Sorry pages is non-sense. Its reading thats sometimes restricted as well

[TKapitan]

@StefanMaron still not sure that this is working completely (sorry for using this issue; it's probably not specifically related to table extensions.

This warning can be fixed by

• Adding Permissions on the object level that add the object permissions (extends user's indirect permission)
• Adding InherentPermissions that changes the user permissions but does not extend the user's indirect permission.

This mean, that adding indirect permission in InherentPermission should not fix the warning as developer must

• Add direct permission in InherentPermissions
• Add indirect permission in InherentPermissions and add the Permission property with indirect permission

[TKapitan]

I was forced by our dev team to disable the rule as they don't agree with moving all code from reportextensions to codeunits.

I'll try to test a bit more examples once I'm back from holiday, but splitting to two rules (for own objects and for external objects) may be helpful for similar cases as ours.

[helenapi]

We fixed the issue by disabling this rule. It does not make the code any more readable to move it away to a codeunit and giving indirect permissions may cause the same kind of random error that not giving them does, according to my interpretation of the Note at the end of the documentation from microsoft... just in the opposite way where a user may lack some permission given at the top of the codeunit but not a permission specific to run a procedure in that codeunit: https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-permissions-property

[lvanvugt]

Wouldn't it be nice to add a code action that does set the Permissions property to cover the permissions? Or is this to be asked for in the AL Code Action extension.

[Arthurvdv]

Wouldn't it be nice to add a code action that does set the Permissions property to cover the permissions? Or is this to be asked for in the AL Code Action extension.

I believe it would be fantastic to have this feature, though unfortunately, it isn't available today. If you'd like to support this idea, we could use more votes on this BC Idea.

For now, the best option is indeed to request this through one of the VS Code extensions.

[Arthurvdv]

I'll try to test a bit more examples once I'm back from holiday, but splitting to two rules (for own objects and for external objects) may be helpful for similar cases as ours.

It would be great if you could share some examples so we can gain further insights on this.

[Arthurvdv]

We fixed the issue by disabling this rule. It does not make the code any more readable to move it away to a codeunit and giving indirect permissions may cause the same kind of random error that not giving them does

I'm not sure if I fully understand your point. The purpose of this rule is that the method or codeunit will function if the user has been granted indirect permissions to the tabledata, rather than full permissions.

With setup and configuration, you can always assign full read, write, insert, and delete permissions. However, assigning indirect read, write, insert, or delete permissions will only work if the developer has explicitly set the permission property in the code. The rule highlights all possible scenarios where the permission property must be set in order to enable this functionality.

[StefanMaron]

I think that note from the Docs is very confusing. Its like Arthur said. The Permissions property is necessary to make the Indirect Permissions of the users permission sets work.

Without the property, indirect permissions are simply useless.

And again, there are objects that are per Microsofts license indirect only. Most commonly: writing to Ledger entry or Posted document tables. But I recently noticed that its also sometimes set for reading data, like Sent Mails table.

Since we can never know during development if a table needs those permissions and since we enable the usage of indirect permissions for all other tables, I figured it would be good practice to always set Permissions for all Database operations.

[fridrichovsky]

Wouldn't it be nice to add a code action that does set the Permissions property to cover the permissions? Or is this to be asked for in the AL Code Action extension.

Hello @lvanvugt and @Arthurvdv may be you now it but function exists. It is part of "AZ AL Dev Tools" extension in VSC. You have to click to the object name in code. Wait for light bulb and there is option for create permissions. It creates property permissions with required tables and their permissions.

[fridrichovsky]

I know that you mentioned here many times and I fully understand that permissions are required without exceptions. I agree with you. But I have same problem as @TKapitan. Developers ask me for disabling this rule because they are working on upgrades and they have to much warnings on all extension objects. I think when you split rules as prepared @Arthurvdv it will still check what do you want. My point of view is that when I use your default rules I will get warning about permission for normal objects and second warning for extensions. It will follow your primary idea. Good think is that it gives me option disable rule for object extensions. If I disable it, it will be my bad (exists option that my code will not work because permissions.), but It gives me chance how to keep rule for normal objects. I don't want disable this rule because I feel that it is right way, but I can't fight with developers. So imagine my ask as looking for smaller evil ("Have one's cake and eat it too"). I hope that you understand what I mean.

[StefanMaron]

On Directions I talked to a few people about this rule as well and asked for feedback. I got mixed opinions but the main point I took was, that this rule might not make sense for all projects and especially for Upgrade projects it might actually make sense to not have it enabled at the very beginning.

My problem with splitting is that I can not merge them back later, in case Microsoft would decide to introduce the Permissions property on Extension Objects, as removing a rule would be a breaking change.

However, I do understand the problem you have.

Did you try already to disable the warning on all extension objects with regex? replace ^((pageextension|tableextension) \d+ [\s\S.\r\n]*?\{) with $1\n#pragma warning disable LC0068 globally across all files.

Maybe this can help you as it would be an instant solution.

Let me know!

[cegekaJG]

I was confused about this for a while since the official documentation is deceptive, but from my understanding, there's no way to request direct permissions using the Permissions property, correct? If that's the case, I don't think we'll use this rule because satisfying the rule would mean allowing indirect permissions as well.

In any case, however, I think splitting the rule for table extensions is a good idea since there should be a degree of flexibility. And at the very least, table extensions should consider permissions set in the original object. There's no reason to either use a pragma or codeunit when reading a Sales Line in a Sales Header.

[fridrichovsky]

Hello @StefanMaron, thank you for your explanation and for replace regex. I discussed this solution with my developers team and they agree with this solution. So I can keep this rule set. For object extensions we are using pragma now. If MS allows permissions for extensions we can easily remove pragma by regex.