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

[Schema] Return 401 error when no HTTP authentication is configured #1802

Conversation

BewareMyPower
Copy link
Collaborator

@BewareMyPower BewareMyPower commented Apr 20, 2023

[Schema] Return 401 error when no HTTP authentication is configured

Motivation

When authentication is enabled, if the Schema REST requests were sent without HTTP authentication header, the Schema Registry will return 404, rather than 401.

Modifications

  • When SchemaStorageException is thrown, build the response with the error code and the exception message.
  • Add testSchemaNoAuth to verify 401 unauthorized will be returned.

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)

@BewareMyPower BewareMyPower self-assigned this Apr 20, 2023
@github-actions github-actions bot added the no-need-doc This pr does not need any document label Apr 20, 2023
Demogorgon314
Demogorgon314 previously approved these changes Apr 21, 2023
@BewareMyPower
Copy link
Collaborator Author

There are some failed tests:

Error:  io.streamnative.pulsar.handlers.kop.KafkaAuthorizationKafkaMultitenantTenantMetadataTest.testAvroProduceAndConsumeWithAuth(io.streamnative.pulsar.handlers.kop.KafkaAuthorizationKafkaMultitenantTenantMetadataTest)
Error:    Run 1: KafkaAuthorizationKafkaMultitenantTenantMetadataTest.testAvroProduceAndConsumeWithAuth » Serialization
Error:    Run 2: KafkaAuthorizationKafkaMultitenantTenantMetadataTest.testAvroProduceAndConsumeWithAuth » Serialization

I will fix them soon.

@BewareMyPower BewareMyPower marked this pull request as draft April 23, 2023 02:20
@BewareMyPower BewareMyPower marked this pull request as ready for review April 23, 2023 03:13
@BewareMyPower BewareMyPower force-pushed the bewaremypower/schema-auth-failure branch 2 times, most recently from caddc0e to 6d3218e Compare April 23, 2023 03:38
@codecov
Copy link

codecov bot commented Apr 23, 2023

Codecov Report

Merging #1802 (1a0a58c) into master (f0eda8e) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1802      +/-   ##
============================================
+ Coverage     16.03%   16.04%   +0.01%     
  Complexity      626      626              
============================================
  Files           165      165              
  Lines         12329    12321       -8     
  Branches       1128     1125       -3     
============================================
  Hits           1977     1977              
+ Misses        10189    10181       -8     
  Partials        163      163              

see 2 files with indirect coverage changes

### Motivation

When authentication is enabled, if the Schema REST requests were sent
without HTTP authentication header, the Schema Registry will return 404,
rather than 401.

### Modifications

- When `SchemaStorageException` is thrown, build the response with the
  error code and the exception message.
- Add `testSchemaNoAuth` to verify 401 unauthorized will be returned.
@BewareMyPower BewareMyPower force-pushed the bewaremypower/schema-auth-failure branch from 6d3218e to 1a0a58c Compare April 23, 2023 04:50
@BewareMyPower
Copy link
Collaborator Author

@Demogorgon314 I split the unrelated logic of this PR into another PR: #1807. PTAL again.

@BewareMyPower BewareMyPower merged commit c41ad06 into streamnative:master Apr 23, 2023
@BewareMyPower BewareMyPower deleted the bewaremypower/schema-auth-failure branch April 23, 2023 11:57
BewareMyPower added a commit that referenced this pull request Apr 24, 2023
…1802)

[Schema] Return 401 error when no HTTP authentication is configured

### Motivation

When authentication is enabled, if the Schema REST requests were sent
without HTTP authentication header, the Schema Registry will return 404,
rather than 401.

### Modifications

- When `SchemaStorageException` is thrown, build the response with the
error code and the exception message.
- Add `testSchemaNoAuth` to verify 401 unauthorized will be returned.

(cherry picked from commit c41ad06)
BewareMyPower added a commit that referenced this pull request Apr 24, 2023
…1802)

[Schema] Return 401 error when no HTTP authentication is configured

### Motivation

When authentication is enabled, if the Schema REST requests were sent
without HTTP authentication header, the Schema Registry will return 404,
rather than 401.

### Modifications

- When `SchemaStorageException` is thrown, build the response with the
error code and the exception message.
- Add `testSchemaNoAuth` to verify 401 unauthorized will be returned.

(cherry picked from commit c41ad06)
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