serverlessworkflow / synapse

Serverless Workflow Management System (WFMS)
https://serverlessworkflow.io
Apache License 2.0
226 stars 35 forks source link

Authentication Support with Azure AD with B2C #342

Closed jaliyaudagedara closed 1 month ago

jaliyaudagedara commented 1 year ago

I couldn't get authentication to work with Azure AD B2C.

Upon looking at the code, we are using IdentityModel in the OAuth2TokenManager.

Unfortunately following line doesn't work with Azure AD B2C. discoveryDocument is going to contain an error.

var discoveryDocument = 
    await this.HttpClient.GetDiscoveryDocumentAsync(oauthProperties.Authority.ToString(), cancellationToken);

The error is, Endpoint belongs to different authority which is getting thrown from IdentityModel library.

And because of that, discoveryDocument.TokenEndpoint is going to be null here.

using var request = new HttpRequestMessage(HttpMethod.Post, new Uri(oauthProperties.Authority, discoveryDocument.TokenEndpoint))
{
    Content = new FormUrlEncodedContent(properties)
};

We can get past that error when using IdentityModel by doing something like below,

DiscoveryDocumentResponse discoveryDocument
    = await httpClient.GetDiscoveryDocumentAsync(new DiscoveryDocumentRequest
    {
        Address = "https://login.microsoftonline.com/<TenantId>/v2.0",
        Policy = new DiscoveryPolicy
        {
            AdditionalEndpointBaseAddresses =
            {
                "https://login.microsoftonline.com/<TenantId>",
        "https://graph.microsoft.com/oidc/userinfo"
            }
        }
    });

But OAuth2TokenManager isn't extendable.

cdavernas commented 1 year ago

We can get past that error when using IdentityModel by doing something like below

Suggested fix is easy enough, but the problem is where to get those additional base addresses from? We could potentially store said addresses in a custom property of the OAuth2AuthenticationProperties metadata to make it work for now.

I think, however, that it would make sense to add it to the ServerlessWorkflow's OAuth2AuthenticationProperties well known and well described properties. Could you take care of opening a new feature request about it on https://github.com/serverlessworkflow/specification?

But OAuth2TokenManager isn't extendable.

It is extendable, no service class is sealed, and all their protected/public methods are virtual. I understand it is not, however, practical to have to rebuild a custom worker just for said use case.

jaliyaudagedara commented 1 year ago

I think, however, that it would make sense to add it to the ServerlessWorkflow's OAuth2AuthenticationProperties well known and well described properties. Could you take care of opening a new feature request about it on

Completely agree, the suggestion was to get it to work, we of course need to come up with a better approach. Will create a feature request in the spec.

It is extendable, no service class is sealed, and all their protected/public methods are virtual. I understand it is not, however, practical to have to rebuild a custom worker just for said use case.

Of course, it's extendable, but then we will have to create a custom worker (as you mentioned) which we shouldn't be doing.

jaliyaudagedara commented 1 year ago

@cdavernas, found another issue:

OAuth2 Properties Definition in Serverless Workflow Specification doesn't follow their OAuth2AuthenticationProperties.

Basically if I specify auth as follows,

"auth": [
    {
        "scheme": "oauth2",
        "name": "authentication-1",
        "properties": {
            "authority": "https://informasoftwaredev.b2clogin.com/informasoftwaredev.onmicrosoft.com/B2C_1A_AD_SIGNUP_SIGNIN/oauth2/v2.0/authorize",
            "grantType": "clientCredentials"
            "audiences": [
                "<audience-1>"
            ],
            "clientId": "<clientId>",
            "clientSecret": "<secret>",
            "scopes": [
                "<scope-1>"
            ]
        }
    }
],

Not all the properties are getting serialized into OAuth2AuthenticationProperties

And even values for OAuth2GrantType, isn't getting serialized.

I had to put auth as follows in order for FE to happy and values to be serialized.

"auth": [
    {
        "scheme": "oauth2",
        "name": "authentication-1",
        "properties": {
            "authority": "<authority>",
            "grant_type": "client_credentials",
            "grantType": "clientCredentials", // just to satisfy the FE
            "audience": "<audience-1>",
            "clientId": "<clientId>",  // just to satisfy the FE
            "client_id": "<clientId>",
            "clientSecret": "<secret>",  // just to satisfy the FE
            "client_secret": "<secret>",
            "scope": "<scope-1>"
        }
    }
],

FE validations are based on: https://github.com/serverlessworkflow/synapse/blob/main/src/dashboard/Synapse.Dashboard/wwwroot/schemas/0.8/auth.json

Another possible bug: https://github.com/serverlessworkflow/synapse/blob/cba235fcfe665c6ec9712c5ce592e829870fba38/src/dashboard/Synapse.Dashboard/Features/Workflows/WorkflowEditor/AuthenticationEditor.razor#L169

Believe this should be, image

cdavernas commented 1 year ago

That well known issue is due to casing, which I did not properly document in SW spec. OAuth2 indeed mandates snake case instead of camel's. As you realized, using snake cased properties does the trick.

Your last comment is I believe correct, this looks like a bug. Strange however we did not face it so far, though we heavily rely on that auth scheme in our use cases

cdavernas commented 1 month ago

Closed as completed by https://github.com/serverlessworkflow/specification/issues/912