Secrets Handling in AL

[StefanMaron]

I am quite surprised this was not brought up yet, so let me start this :)

What would be the best practice way of handling secret values in an App?

Before we got the IsolatedStorage they where just saved in a setup table with Masked=true which is of course not ideal.

I started to implement them with Isolated storage now, with a Codeunit that handles all the getting and setting for me.
I also created a Page to let the user enter the Secret and I show on the Setup page if the Secret is set or not.

I would like to know if I am missing something obvious or if there is still room for improvement.

Once the idea is complete I will happily summarize this in a Docs page here :)

[NKarolak]

I would like to know if I am missing something obvious.

Not to my knowledge, as this is exactly how I have implemented it as well.

[ChrisBlankDe]

Codeunit that handles all the getting and setting

centralized codeunits should not provide a getter method. this would be an security issue.

[ChrisBlankDe]

do not remove internalsvisibleto. just move the GetPassword() Proc to the codeunit wher its been used and change the access modifier to local.
that would resolve my security concern.

GetCallerModuleInfo would just return the faked app info. this function cant be used for security topics.

[StefanMaron]

Of course it would return that, but I can verify that the caller is never another app that my own app.

Having the getter functions makes it way easier to handle the secrets tho

[ChrisBlankDe]

ah ok. didn't get your intention at first.
checking GetCallerModuleInfo AppId against GetCurrentModuleInfo AppId would be secure but imho kind of strange.

i dont see any problem in moving the code.
not to beat around the bush, here is the code we use most of the time:

 [NonDebuggable]
 local procedure GetPassword() sText: SecretText;
 var
     StorageKeyTok: 'AnyGuid', Locked = true; // actually not a label but the retrun value of a centralized global function
 begin
     if IsolatedStorage.Contains(StorageKeyTok, DataScope::Module) then
         IsolatedStorage.Get(StorageKeyTok, DataScope::Module, sText);
 end;

imho this can be duplicated and does not have to be centralized

[StefanMaron]

If it is just that, agree. But sometimes more logic can be involved reading a value

[StefanMaron]

Just to give an example

[ChrisBlankDe]

It should also be mentioned to always use secrettext as data type if possible. this is available since 2023w2 and got usable by 2024w1.

[curateideas]

What is a secure way to transfer screts between systems (i. e. from NAV to BC if it is not a direct migration)?

[ChrisBlankDe]

Looking at the screenshot from you (#251 (reply in thread)) im thinking about that all secrets that belong together should also be stored together.
in the given example its a access token and its temporal validity.

i would sewrialize them to a json object and store it as one secret.
Benefit: less sql transaction. remeber every contains, set, get is a database transaction with no control about locking and readuncommited behaviour.

[StefanMaron]

I think that brings us into an Area of personal preference already. I like to keep my Database at least a bit normalized. I would guess with caching and optimizations the extra SQL queries are not that impact-full.

But the point is very valid. If getter functions are used, GetCallerInfo should be used to make sure the secret can not be read from outside the app. I also created a feature request on Al-Go to be able to remove the InternalsVisibleTo from the appjson before publish.

I agree that this is still not the ideal solution but I saw that AlOps also offers this to at least decrease the likelihood of internals being exposed by accident.
microsoft/AL-Go#1248