SAP / cloud-sdk-java

Use the SAP Cloud SDK for Java to reduce development effort when building applications on SAP Business Technology Platform that communicate with SAP solutions and services such as SAP S/4HANA Cloud, SAP SuccessFactors, and many others.
Apache License 2.0
22 stars 13 forks source link

OAuth token not retrieved when getting destination #428

Closed ErwanAirone closed 5 months ago

ErwanAirone commented 6 months ago

Issue Description

When getting the destination, a change of behavior is to be seen when using Authentication OAuth2Password.

Expected: Token was retrieved and fetched with the destination as a property cloudsdk.authTokens.

Actual: Token is not part of the destination.

Code sample:

public Destination getDestination(String destinationName) {
        try {
            DestinationOptions options = DestinationOptions
                    .builder()
                    .augmentBuilder(
                            ScpCfDestinationOptionsAugmenter
                                    .augmenter()
                                    .retrievalStrategy(ScpCfDestinationRetrievalStrategy.ONLY_SUBSCRIBER))
                    .build();
            return DestinationAccessor.getLoader().tryGetDestination(destinationName, options).get();
        } catch (Exception e) {
            //Not getting there
        }
    }

Impact / Priority

Impacting our production landscapes and customers.

Project Details

Johannes-Schneider commented 6 months ago

Hi @ErwanAirone,

we didn't do any changes in our logic with regards to handling auth tokens. Therefore, I'd like to run a little sanity check:

Would you be able to

Please share your findings in this issue. (Make sure to mask any sensitive information before attaching logs, though)

Best regards, Johannes

ErwanAirone commented 6 months ago

Here is what we get from the rest endpoint:

{
    "owner": {
        "SubaccountId": "HIDDEN",
        "InstanceId": null
    },
    "destinationConfiguration": {
        "mail.transport.protocol": "smtp",
        "scope": "SMTP.Send",
        "Password": "HIDDEN",
        "mail.smtp.ssl.checkserveridentity": "false",
        "mail.smtp.ssl.enable": "false",
        "mail.smtp.from": "HIDDEN",
        "mail.smtp.starttls.enable": "true",
        "mail.smtp.auth.mechanisms": "XOAUTH2",
        "User": "hidden",
        "Name": "sap_process_automation_mail",
        "Type": "MAIL",
        "ProxyType": "Internet",
        "tokenServiceURL": "
[https://login.microsoftonline.com/organizations/oauth2/v2.0/token"](https://login.microsoftonline.com/organizations/oauth2/v2.0/token%22)
,
        "mail.smtp.starttls.required": "true",
        "mail.smtp.port": "587",
        "clientSecret": "HIDDEN",
        "mail.smtp.auth": "true",
        "Authentication": "OAuth2Password",
        "clientId": "HIDDEN",
        "mail.smtp.host": "smtp.office365.com",
        "tokenServiceURLType": "Dedicated"
    },
    "authTokens": [
        {
            "type": "Bearer",
            "value": "HIDDEN",
            "http_header": {
                "key": "Authorization",
                "value": "HIDDEN"
            },
            "expires_in": "HIDDEN",
            "scope": "Mail.Send openid SMTP.Send profile email"
        }
    ]
}

Here is the object we get from the sdk: image

Johannes-Schneider commented 5 months ago

Hi @ErwanAirone,

as it seems, you are fetching a destination of type MAIL, which is not supported by SAP Cloud SDK for Java. Unfortunately, due to other priorities, we are also not planning to support mail destinations in the foreseeable future.

Best regards, Johannes

ErwanAirone commented 5 months ago

Hello @Johannes-Schneider ,

Thank you for your reply. However, it was working in the previous version, we used it for years. We are not handling it as mail destination but as a common destination.

Best regards, Erwan

Johannes-Schneider commented 5 months ago

@ErwanAirone wrote:

However, it was working in the previous version

Which version did you use previously?

ErwanAirone commented 5 months ago

5.4.0

Johannes-Schneider commented 5 months ago

That's surprising. As you can see, this is how we handled auth tokens from the destination service in Cloud SDK version 5.4.0. And this is how we are handling them in 5.8.0.

There are two things I'd like to point out:

  1. There is no difference between the two Cloud SDK versions.
  2. We always handled auth tokens for HTTP destinations only.

Would you mind reverting the Cloud SDK update (i.e. switch back to version 5.4.0) and take the same screenshot as above (i.e. the properties of the fetched destination), as a quick sanity check?

ErwanAirone commented 5 months ago

The version was also updated from 4.29.0 to 5.4.0 a month before. If there are no changes between 5.4.0 and 5.8.0, then it could be because the issue was not seen and introduced when updating the first time.

Johannes-Schneider commented 5 months ago

from 4.29.0 to 5.4.0 a month before

Okay, that makes more sense. Indeed, in Cloud SDK major version 4, we added the auth tokens (if any) to any kind of Destination. This has been changed with version 5.

I can reach out to our product owner and the team to see whether it makes sense to re-add this behavior.

Best regards, Johannes

ErwanAirone commented 5 months ago

Thank you Johannes.

This has now very high priority because of 7 affected customers

Johannes-Schneider commented 5 months ago

Hey @ErwanAirone,

as you might have seen, we just merged a fix for the regression. We will need to run our internal test suite and, if everything goes well, can ship the fix with our next planned version (5.9.0).

I'll let you know once we have released the change.

ErwanAirone commented 5 months ago

Thank you @Johannes-Schneider .

Any idea of when that would be ?

Johannes-Schneider commented 5 months ago

If everything does go according to plan, I'm planning to publish the release still today. So considering a bit of synchronization effort on common maven repos, I'd expect this fix to be consumable tomorrow. (As I said: Iff everything works out as expected, I can't make any promises, though)

Johannes-Schneider commented 5 months ago

@ErwanAirone FYI: Version 5.9.0 has just been released. So after all of the common maven repos have updated their caches, you should be able to test the included fix.

Please let me know whether everything is indeed working as expected.

Best regards, Johannes

Johannes-Schneider commented 5 months ago

Hi @ErwanAirone,

as version 5.9.0 should be available by now, would you mind trying it out and letting me know whether the issue has been resolved?

Thanks and best regards, Johannes

ErwanAirone commented 5 months ago

Hello @Johannes-Schneider ,

Yes, it's indeed working again. Thank you very much.