Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: VAT Group Management - Member - OAuth2 Token Refresh #28076

Open
1 task done
bsl-paul opened this issue Feb 7, 2025 · 1 comment
Open
1 task done

[Bug]: VAT Group Management - Member - OAuth2 Token Refresh #28076

bsl-paul opened this issue Feb 7, 2025 · 1 comment

Comments

@bsl-paul
Copy link

bsl-paul commented Feb 7, 2025

Describe the issue

In VAT Group Management - VAT Report Setup, the Refresh OAuth2 Token action no longer works for Group VAT Members.
The code is no longer passing the correct Client Secret to the VATGroupCommunication.GetBearerToken() method, as the preceding function call GetClientSecrets populates the variable with just a *.

Expected behavior

The ClientSecretText variable should contain the correct Client Secret retrieved via the Rec.GetSecretAsSecretText(Rec."Client Secret Key") function call.

Steps to reproduce

Add a Member to the VAT Group Setup with authentication type OAuth back to the Representative. Configure all necessary endpoints, Client ID and Client Secret.
Click Refresh OAuth2 Token action and after providing correct credentials an error is shown stating authentication failed see event log for further details. Event Viewer on the server shows Entra error:
AADSTS7000215: Invalid client secret provided. Ensure the secret being sent in the request is the client secret value, not the client secret ID

Additional context

No response

I will provide a fix for a bug

  • I will provide a fix for a bug
@pri-kise
Copy link
Contributor

@bsl-paul I checked the code and you're right that is a bug.

https://github.com/microsoft/ALAppExtensions/blob/main/Apps/W1/VATGroupManagement/app/src/Pages/VATReportSetupExtension.PageExt.al

            action(RenewToken)
            {
                ApplicationArea = Basic, Suite;
                Caption = 'Renew OAuth2 Token';
                ToolTip = 'Initiates a new OAuth2 authetication flow which will result in getting a new token. This should be used if the the previous token has expired or can no longer be used.';
                Promoted = true;
                PromotedCategory = Process;
                Image = AuthorizeCreditCard;
                Visible = (Rec."VAT Group Role" = Rec."VAT Group Role"::Member);

                trigger OnAction()
                var
                    VATGroupCommunication: Codeunit "VAT Group Communication";
                begin
                    GetClientSecrets();
                    //NOTE: ClientSecretText can be `'*'`at this place.
                    VATGroupCommunication.GetBearerToken(ClientIDText, ClientSecretText, Rec."Authority URL", Rec."Redirect URL", Rec."Resource URL");
                end;
            }

    [NonDebuggable]
    local procedure GetClientSecrets()
    begin
        if not Rec.GetSecretAsSecretText(Rec."Client Secret Key").IsEmpty() then
            ClientSecretText := '*'; //This is correct, since we don't want to have the real value in this variable
        ClientIDText := Rec.GetSecretAsSecretText(Rec."Client ID Key").Unwrap();
    end;

Furhtermore when this is fixed, then the procedure GetBearerToken should be updated to accept a ClientSecret as SecretText and then we should be able to remove the NonDebuggable property

    [Scope('OnPrem')]
    [NonDebuggable]
    internal procedure GetBearerToken(ClientId: Text; ClientSecret: Text; AuthorityURL: Text; RedirectURL: Text; ResourceURL: Text)
    var
        OAuth2: Codeunit OAuth2;
        EnvironmentInformation: Codeunit "Environment Information";
        PromptInteraction: Enum "Prompt Interaction";
        Scopes: List of [Text];
        FirstPartyAppId: Text;
        FirstPartyAppCertificate: SecretText;
        BearerToken: SecretText;
        AuthError: Text;
    begin
      
      ...
    end;

@aholstrup1 could you maybe approve this issue?
(I'm tagging you since you sometimes create the merge PRs for this repository.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants