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

[Schema Registry] Force authorization when kafkaEnableMultiTenantMetadata is false #1807

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BewareMyPower
Copy link
Collaborator

@BewareMyPower BewareMyPower commented Apr 23, 2023

Motivation

Currently when the schema registry and authorization are both enabled, the authorization is only performed when kafkaEnableMultiTenantMetadata is false. It's not reasonable because the role of the token must have the write permission to the schema registry topic.

Modifications

  • In performAuthorizationValidation, do not check kafkaEnableMultiTenantMetadata.
  • Add the testSchemaWrongAuth to verify that a wrong role cannot be used to create an Avro produce.
  • Separate the default namespace and the default schema namespace in KafkaAuthorizationTestBase so that the permission requirements will be clear.

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@codecov
Copy link

codecov bot commented Apr 23, 2023

Codecov Report

Merging #1807 (fcde386) into master (1dda879) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1807      +/-   ##
============================================
+ Coverage     17.72%   17.74%   +0.01%     
- Complexity      751      752       +1     
============================================
  Files           195      195              
  Lines         14155    14156       +1     
  Branches       1322     1322              
============================================
+ Hits           2509     2512       +3     
+ Misses        11463    11462       -1     
+ Partials        183      182       -1     
Impacted Files Coverage Δ
...ive/pulsar/handlers/kop/SchemaRegistryManager.java 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

Demogorgon314
Demogorgon314 previously approved these changes Apr 23, 2023
@Demogorgon314
Copy link
Member

@BewareMyPower Can you help resolve conflicts?

@BewareMyPower
Copy link
Collaborator Author

Okay, I forgot this PR for some time.

…data is false

### Motivation

Currently when the schema registry and authorization are both enabled,
the authorization is only performed when
`kafkaEnableMultiTenantMetadata` is false. It's not reasonable because
the role of the token must have the write permission to the schema
registry topic.

### Modifications

- In `performAuthorizationValidation`, do not chec
  `kafkaEnableMultiTenantMetadata`.
- Add the `testSchemaWrongAuth` to verify that a wrong role cannot be
  used to create an Avro produce.
- Separate the default namespace and the default schema namespace in
  `KafkaAuthorizationTestBase` so that the permission requirements
  will be clear.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants