Skip to content

Allow only root user to read fate and scanref tables by default#6203

Open
dlmarion wants to merge 2 commits intoapache:mainfrom
dlmarion:6137-fix-fate-scanref-scan-perms
Open

Allow only root user to read fate and scanref tables by default#6203
dlmarion wants to merge 2 commits intoapache:mainfrom
dlmarion:6137-fix-fate-scanref-scan-perms

Conversation

@dlmarion
Copy link
Contributor

@dlmarion dlmarion commented Mar 9, 2026

Closes #6137

@dlmarion dlmarion added this to the 4.0.0 milestone Mar 9, 2026
@dlmarion dlmarion self-assigned this Mar 9, 2026
Copy link
Contributor

@keith-turner keith-turner left a comment

Choose a reason for hiding this comment

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

Discussed this w/ @ctubbsii and we think all that needs to be done for this change is remove this code and add the test.

SystemTables.FATE.tableName());
verifyHasOnlyTheseTablePermissions(c, test_user_client.whoami(),
SystemTables.SCAN_REF.tableName());

Copy link
Contributor

@keith-turner keith-turner Mar 9, 2026

Choose a reason for hiding this comment

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

If the test scans the fate table here it will probably succeed because the namespace grants permission here. Would be good to also add a scan attempt here.

Seems like we need to remove this namespace code because there is table code that already grants anyone access to read metadata and root. Was not sure if removing that would impact the system user, but it does not seem it will because SecurityOperation has an explicit check that gives the system user all permissions.

Copy link
Member

Choose a reason for hiding this comment

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

because SecurityOperation has an explicit check that gives the system user all permissions

Even that should probably be locked down. It doesn't need access to most tables, just read/write access to the system tables. But, that's out of scope of this PR and can be done separately.

}

// Allow root user to scan all system tables
if (user.equals(getRootUsername()) && SystemTables.containsTableId(table)
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably do not need to hard code this access for root because the root user can grant itself access by default.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. The only special permissions the root user has is the ability to manage permissions of others. It shouldn't get special treatment with access to data. That is not its role. It can always grant itself that role, if that's how a user wants to do things, but it shouldn't be the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this and the code to give all users read permission in the Accumulo namespace causes the new code in the IT (below) to fail. Is this what you expected?

      verifyHasOnlyTheseTablePermissions(c, c.whoami(), SystemTables.FATE.tableName(),
          TablePermission.READ);
      verifyHasOnlyTheseTablePermissions(c, c.whoami(), SystemTables.SCAN_REF.tableName(),
          TablePermission.READ);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Any user can scan the fate and scanref table

3 participants