ReadOnly mode takes precedence over Normal/Edit Mode if a user has perms for both

[newOnahtaN]

Currently in Auth.cs, the following logic exists:

// Also validating App Roles, but only if any of relevant setting is set
var allowedAppRoles = settings.AllowedAppRoles;
var allowedReadOnlyAppRoles = settings.AllowedReadOnlyAppRoles;

if (allowedAppRoles != null || allowedReadOnlyAppRoles != null)
{
    var roleClaims = principal.FindAll(settings.RolesClaimName);

    bool userIsInAppRole = roleClaims.Any(claim => allowedAppRoles != null && allowedAppRoles.Contains(claim.Value));
    bool userIsInReadonlyRole = roleClaims.Any(claim => allowedReadOnlyAppRoles != null && allowedReadOnlyAppRoles.Contains(claim.Value));

    // If user belongs _neither_ to AllowedAppRoles _nor_ to AllowedReadOnlyAppRoles
    if (!(userIsInAppRole || userIsInReadonlyRole))
    {
        throw new DfmUnauthorizedException($"User {userNameClaim.Value} doesn't have any of roles mentioned in {EnvVariableNames.DFM_ALLOWED_APP_ROLES} or {EnvVariableNames.DFM_ALLOWED_READ_ONLY_APP_ROLES} config setting. Call is rejected");
    }

    // If current operation modifies any data, then validating that user is _not_ in ReadOnly mode
    if (operationKind != OperationKind.Read && userIsInReadonlyRole)
    {
        throw new DfmAccessViolationException($"User {userNameClaim.Value} is in read-only mode");
    }

    return userIsInReadonlyRole ? DfmMode.ReadOnly : settings.Mode;
}

As a result of the final ternary expression in the return statement of this block, if a given user possesses both Read permissions and Normal permissions (Write/Edit permissions?) as I did in my testing, then this function returns DfmMode.ReadOnly.

Is this the intended behavior of the tool? My assumption based on similar situations in other systems would be that if a user has Write/Edit and Read permissions for a given resource, they would be able to write/edit to that resource without needing to revoke their own read permissions on that resource first.

If it is the intended behavior, then perhaps a callout in the configuration documentation would be appropriate.

Thank you very much for this tool, by the way. Cool as heck 🦾

[scale-tone]

Hi @newOnahtaN , and thanks for your feedback!

Is this the intended behavior of the tool?

I'd say - yes. More restrictive policies should always take precedence - that's the philosophy behind.

As a bit of a context, DFM_ALLOWED_READ_ONLY_APP_ROLES was added long after DFM_ALLOWED_APP_ROLES, and the goal was not to introduce any breaking changes.

But I agree, it might appear contra-intuitive. Will clarify the docs.

[newOnahtaN]

Interesting. Okay, I will take effort not to assign read only access to anyone who also has write access then.

[scale-tone]

Done:

Closing this, but please open more issues, if anything else is not right.