Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

addresses server panic when malformed authorization header is sent #573

Merged
merged 4 commits into from
Apr 29, 2022

Conversation

vroldanbet
Copy link
Contributor

馃憢馃徎 Good day good people of the land of authorization, a quick fix!

What

addresses a server panic when a request with a malformed authorization header is received

How

  • added missing tests to the presharedkey.go file, including one that reproduces the problem
  • adjusts the status.Errorf call that panics

@vroldanbet vroldanbet requested a review from a team as a code owner April 28, 2022 18:55
@github-actions github-actions bot added the area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) label Apr 28, 2022
@vroldanbet vroldanbet changed the title addresses server panic when no token is provided addresses server panic when malformed authorization header is sent Apr 28, 2022
"google.golang.org/grpc/metadata"
)

func TestMultiplePresharedKeys(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use a table-driven test here?

{"valid request with the second key", []string{"one", "two"}, true, "two", codes.OK},
{"denied due to unknown key", []string{"one", "two"}, true, "three", codes.PermissionDenied},
{"unauthenticated due to missing key", []string{"one", "two"}, true, "", codes.Unauthenticated},
{"unauthenticated due to missing metadata", []string{"one", "two"}, false, "", codes.Unauthenticated},
Copy link
Member

Choose a reason for hiding this comment

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

Add a test with sending a header, but an empty one?

Copy link
Member

@josephschorr josephschorr left a comment

Choose a reason for hiding this comment

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

LGTM

@josephschorr josephschorr merged commit 64707c3 into authzed:main Apr 29, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants