Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

Use pulsar AuthorizationService to authorize - #989

Merged
BewareMyPower merged 5 commits into
streamnative:masterfrom
Demogorgon314:fix/authorize-with-superusers
Dec 29, 2021
Merged

Use pulsar AuthorizationService to authorize#989
BewareMyPower merged 5 commits into
streamnative:masterfrom
Demogorgon314:fix/authorize-with-superusers

Conversation

@Demogorgon314

@Demogorgon314 Demogorgon314 commented Dec 24, 2021

Copy link
Copy Markdown
Member

Motivation

The current implementation of authorizing is using Pulsar's default authorization provider. For future maintenance, we better use the AuthorizationService. because not all users want to use Pulsar's default authorization provider.

Modifications

  1. Split CREATE, DELETE and ALTER.
  2. Use AuthorizationService instead of Self-implementation authorization.

@Demogorgon314 Demogorgon314 self-assigned this Dec 24, 2021
@Demogorgon314 Demogorgon314 changed the title [WIP] Use pulsar authorization provider to authorize [WIP] Use pulsar AuthorizationService to authorize Dec 24, 2021
@Demogorgon314 Demogorgon314 changed the title [WIP] Use pulsar AuthorizationService to authorize Use pulsar AuthorizationService to authorize Dec 24, 2021
@Demogorgon314

Copy link
Copy Markdown
Member Author

I split some of these changes to the #991, when another PR is merged, I'll mark this PR ready for review.

@Demogorgon314
Demogorgon314 force-pushed the fix/authorize-with-superusers branch from a47599a to 3880bba Compare December 28, 2021 14:55
@Demogorgon314
Demogorgon314 marked this pull request as ready for review December 29, 2021 00:09

@BewareMyPower BewareMyPower left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a test to verify a super user that is not configured in superUserRoles has the permission to produce or consume?

@Demogorgon314

Copy link
Copy Markdown
Member Author

@BewareMyPower Test has been added. Please take a look again.

@BewareMyPower

BewareMyPower commented Dec 29, 2021

Copy link
Copy Markdown
Contributor

When I cherry-picked this PR into branch-2.8.2, a conflict happened. Please help resolve the conflict.

After cherry-picking successfully and pushing to branch-2.8.2, please add the cherry-picked/branch-2.8.2 label.

BewareMyPower pushed a commit that referenced this pull request Dec 29, 2021
### Motivation
The current implementation of authorizing is using Pulsar's default authorization provider. For future maintenance, we better use the `AuthorizationService`. because not all users want to use Pulsar's default authorization provider.

### Modifications
1. Split CREATE, DELETE and ALTER.
2. Use `AuthorizationService` instead of Self-implementation authorization.
@Demogorgon314
Demogorgon314 deleted the fix/authorize-with-superusers branch December 30, 2021 00:15
Demogorgon314 added a commit that referenced this pull request Dec 30, 2021
### Motivation
The current implementation of authorizing is using Pulsar's default authorization provider. For future maintenance, we better use the `AuthorizationService`. because not all users want to use Pulsar's default authorization provider.

### Modifications
1. Split CREATE, DELETE and ALTER.
2. Use `AuthorizationService` instead of Self-implementation authorization.

(cherry picked from commit c0c1e48)
michaeljmarshall pushed a commit to michaeljmarshall/kop that referenced this pull request Dec 13, 2022
The current implementation of authorizing is using Pulsar's default authorization provider. For future maintenance, we better use the `AuthorizationService`. because not all users want to use Pulsar's default authorization provider.

1. Split CREATE, DELETE and ALTER.
2. Use `AuthorizationService` instead of Self-implementation authorization.

(cherry picked from commit c0c1e48)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants