Skip to content

Conversation

@collado-mike
Copy link
Contributor

Addresses comments from #623

public SecurityContext createSecurityContext(
SecurityContext ctx, AuthenticatedPolarisPrincipal principal) {
Set<String> validRoleNames = activeRolesProvider.getActiveRoles(principal);
if (validRoleNames.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is making the tests fail, and I think we do have a problem. DefaultActiveRolesProvider expects the principal to have the role PRINCIPAL_ROLE_ALL when all roles are activated:

boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);

But BasePolarisAuhtenticator creates a principal with an empty roles set in that case. You need to change BasePolarisAuthenticator.getPrincipal():

  if (tokenInfo.getScope() != null) {
      if (tokenInfo.getScope().equals(PRINCIPAL_ROLE_ALL)) {
        activatedPrincipalRoles.add(PRINCIPAL_ROLE_ALL); // this is missing
      } else {
        activatedPrincipalRoles.addAll(...);
      }
    }

@adutra
Copy link
Contributor

adutra commented Jan 14, 2025

@collado-mike if you want to rebase this PR, the two filters were moved/renamed to:

org.apache.polaris.service.auth.PolarisPrincipalAuthenticatorFilter
org.apache.polaris.service.auth.PolarisPrincipalRolesProviderFilter

Both are in polaris-service-common.

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Feb 14, 2025
@github-actions github-actions bot closed this Feb 27, 2025
@github-project-automation github-project-automation bot moved this from PRs In Progress to Done in Basic Kanban Board Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants