MutliTenancy Middleware returns 400 when call is not authenticated instead 401

[ayuksekkaya]

Is there a reason why the MutliTenancy Middleware returns 400 "The ID of the organization is missing from this request" instead of 401 when the call is not authenticated?

Since we are saying that org id doesn't need to be passed in for requests on the default org, I think it would make more sense to return 401 when the call requiring multitenancy is not authenticated.

Currently in MultiTenancy Middleware, we have:

    private async Task<Result<string?, Error>> VerifyDefaultOrganizationIdForCallerAsync(ICallerContext caller,
        IEndUsersService endUsersService, List<Membership>? memberships, CancellationToken cancellationToken)
    {
        if (!caller.IsAuthenticated)
        {
            return Error.Validation(Resources.MultiTenancyMiddleware_MissingDefaultOrganization);
        }

        if (memberships.NotExists())
        {
            var retrievedMemberships = await GetMembershipsForCallerAsync(caller, endUsersService, cancellationToken);
            if (retrievedMemberships.IsFailure)
            {
                return retrievedMemberships.Error;
            }

            memberships = retrievedMemberships.Value;
        }

        var defaultOrganizationId = GetDefaultOrganizationId(memberships);
        if (defaultOrganizationId.HasValue())
        {
            return defaultOrganizationId;
        }

        return Error.Validation(Resources.MultiTenancyMiddleware_MissingDefaultOrganization);
    }

But unless I am missing something, I think it makes more sense to have it as:

    private async Task<Result<string?, Error>> VerifyDefaultOrganizationIdForCallerAsync(ICallerContext caller,
        IEndUsersService endUsersService, List<Membership>? memberships, CancellationToken cancellationToken)
    {
        if (!caller.IsAuthenticated)
        {
             return Error.NotAuthenticated(Resources.AuthenticationHandler_Failed);
        }
...

Why this change is needed?

Usually front end clients like react will be using Axios to inject a bearer token with every request, and will call refresh endpoint when the server returns 401. In this case, for an expired token, the server returns 400, instead of 401, so the client will have to rely on the specific error message to distinguish this 400 from other 400 type errors which I don't think is ideal.

I know the supported model for saastack is BEFFE architecture where the token is not directly sent from the front end client, but I think there is a high chance that startups might want to start with only one front end and one back end in order to deploy the front end client statically to a CDN and just have one back end, in which case they will have to do the token injection to Axios or fetch. So, because of the ease this would provide to getting a fresh token, unless I am missing some side effect, maybe we can return 401 there instead of 400?

[jezzsantos]

Hey @ayuksekkaya,

The context here is that:

1. you've made an HTTP request
2. the request is for a tenanted resource
3. You haven't included the TenantId anywhere in the request
4. You are not authenticated (so we can't assume your default organization)

Technically, we should say that it's a HTTP - 400 validation error because your request is not well-formed - you are missing vital data in the request. In this case the OrganizationId The fact that you are not authenticated is kind of irrelevant at this point in time, this check in the code is really just an optimization to save processing of your memberships.

It just so happens that this OrganizationId is supposed to be in the body of the request, but if it were in the route, you would first get a HTTP - 404 before this code ever ran.

So, I'm not convinced this should change, technically.

The fact that the multitenancy middleware runs before the authorization middleware runs might be necessary in order to perform the authorization of the caller. Or maybe not. The authorization middleware never gets a chance to run in this scenario.

To your point about ajax clients, they should never cause a 400. They can cause 401, 402, 403, etc, of course, becuase those are more dynamic.

Let me consider the order of the middleware. In theory the authorization middleware should throw HTTP - 401 before we process the data in the request. please stand by