SharePoint Auth - Improved README and use the Token Cache

[jwikman]

After some fighting to get the SharePoint module to work, I want to improve the SharePoint Authorization module with my findings.

1. Improved the README.md file
   • Added information and examples of API Permissions and Scope needed to get the authorization to work.
   • Corrected the syntaxes of the functions
2. Added support for Token Cache when acquiring an Access Token
   • This removes unnecessary popups of the authorization window, and I believe it is faster
   • This makes it possible to schedule jobs that works against SharePoint, as long as the user is authenticated now and then (I believe the default life for a Refresh Token is 3 months)
3. Removed the custom caching of Access Token.
   • "SharePoint Authorization Code" had implemented it's own caching of the Access Token, with a hardcoded lifetime of 1 hour. From what I read, that would probably work for most cases since the lifetime of an Access Token is between 60 and 90 minutes. But as stated here, the lifetime of an Access Token can be configured to a shorter time, that would not work together with this "custom" token cache. A custom caching must read the Access Token lifetime to be water proof, but the easy fix here is to just remove it. :)
   • This change leaves the caching to the OAuth2 module.
4. Reworked the logic around the IsSuccess return variable.
   • This had some issues, where AcquireTokenByAuthorizationCode() would return true (it is a TryFunction, and some issues with acquiring an access token does not result in an Error()), but with an empty Access Token. This made me spend waaay to much time troubleshooting this codeunit... The only thing that really tells us if acquiring an Access Token failed or succeeded is if the Access Token has a value or not.

The tests are unaffected by this change.

[jwikman]

I'm not sure that the workflow failure is because of my changes...

I suspect that it is the changes in #21330 that is causing this... Or what do you think, @aholstrup1?

[pri-kise]

@jwikman your changes lgtm so far.
Have you thought about adding and example code similiar to the storage authorization (https://github.com/microsoft/ALAppExtensions/tree/main/Modules/System/Azure%20Storage%20Services%20Authorization).
Normally I'd check the test codeunits for examples on how to use a module, but for authorization it's somehow useful to have some short examples on how the codeunit can be used.

[jwikman]

Have you thought about adding and example

@pri-kise that is indeed a good idea. I'll try to add something.

[jwikman]

@pri-kise would that do?
I was a bit hesitant about if I should include the GetAadTenantNameFromBaseUrl() function or not, since it is a bit unrelated. But then again, it is a nice function since the AAD Tenant Name is included in the BaseUrl...

[pri-kise]

@jwikman I thought the same

@pri-kise would that do? I was a bit hesitant about if I should include the GetAadTenantNameFromBaseUrl() function or not, since it is a bit unrelated. But then again, it is a nice function since the AAD Tenant Name is included in the BaseUrl...

I thought the same. This function might not be necessary for the most, who will check this out, since at least our customers are having only one tenant for sharepoint and for business central. Therefore the TenantId of the Environment Information would be enough for Cloud Customers.
I would keep the procedure there for the moment, since it might be usefule for anyone else and wait for someone of microsoft to provide feedback on this PR.

[jwikman]

I would keep the procedure there for the moment, since it might be usefule for anyone else and wait for someone of microsoft to provide feedback on this PR.

@pri-kise, Yes I, agree.

Now I'll just wait for the verdict. 😅

[JesperSchulz]

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.

Build ID: 51788.

[JesperSchulz]

Availability update: We will publish a fix for this issue in the next update for release 21.

Build ID to track: 52140.