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

Test unknown named application privileges #104863

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

n1v0lg
Copy link
Contributor

@n1v0lg n1v0lg commented Jan 29, 2024

POST /_security/privilege
{
    "kibana": {
        "read": {
            "actions": [
                "data:read/*"
            ]
        }    
    }
}

PUT /_security/role/test
{
    "cluster": [
        "manage_security"
    ],
    "applications": [
        {
            "application": "kibana",
            "privileges": [
                "create"
            ],
            "resources": [
                "*"
            ]
        }
    ]
}

PUT /_security/user/test-admin
{
    "password": "elastic-password",
    "roles": [
        "test"
    ]
}

// run by `test-admin` 
GET /_security/user/_has_privileges
{
    "application": [
        {
            "application": "kibana",
            "privileges": [
                "create"
            ],
            "resources": [
                "*"
            ]
        }
    ]
}
// returns `true` for `create` as expected

PUT /_security/role/test
{
    "cluster": [
        "manage_security"
    ],
    "applications": [
        {
            "application": "kibana",
            "privileges": [
                "create",
                "update"
            ],
            "resources": [
                "*"
            ]
        }
    ]
}

// same has privileges call again
GET /_security/user/_has_privileges
{
    "application": [
        {
            "application": "kibana",
            "privileges": [
                "create"
            ],
            "resources": [
                "*"
            ]
        }
    ]
}
// returns `false` for `create` -> unexpected

@n1v0lg n1v0lg added >test Issues or PRs that are addressing/adding tests :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Jan 29, 2024
@n1v0lg n1v0lg self-assigned this Jan 29, 2024
public void testNamed() throws IOException {
createApplicationPrivilege("app", "write", new String[] { "action:write/*" });
createRole("correct", "app", new String[] { "read" }, new String[] { "*" });
createRole("wrong", "app", new String[] { "read", randomFrom("create", "write") }, new String[] { "*" });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only difference between the roles: "single" unknown privilege or unknown privilege + plus another (known or unknown, doesn't matter).

.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue("wrong", password))
.build();
var actual = hasPrivilege(reqOptions, "app", new String[] { "read" }, new String[] { "resource" });
assertSinglePrivilege(actual, "resource", "read", true);
Copy link
Contributor Author

@n1v0lg n1v0lg Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails but shouldn't.

Copy link
Contributor Author

@n1v0lg n1v0lg Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the problem:

We are checking for equality, but when there's more than one privilege on the Role, we fail this check since name is Set.of("create", "read") for the privilege on the role, and it does not equal Set.of("read") -- the privilege from the request (i.e., other)

.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue("correct", password))
.build();
var actual = hasPrivilege(reqOptions, "app", new String[] { "read" }, new String[] { "resource" });
assertSinglePrivilege(actual, "resource", "read", true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works and is expected behavior based on: #33928 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC >test Issues or PRs that are addressing/adding tests v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants