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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Path and method parsed before auth check #1328

Closed
poslegm opened this issue Jun 19, 2021 · 2 comments
Closed

[BUG] Path and method parsed before auth check #1328

poslegm opened this issue Jun 19, 2021 · 2 comments

Comments

@poslegm
Copy link

poslegm commented Jun 19, 2021

Tapir version: 0.17.19

Scala version: 2.13.3

Describe the bug

Here authentication header decodes after path and method decoding.

How to reproduce?

  1. add bearer auth for endpoint
  2. add path segment codec with some logic
  3. send request without auth header
  4. see 400 instead of 401 and error message from codec

Additional information

Authentication way from docs was not satisfied me, because serverLogicForCurrent runs after request body parsing. I found hack how to avoid it:

Auth "middleware" for base header:

auth
  .bearer[String](WWWAuthenticate.bearer())
  .mapDecode { token =>
    if (token == trustedToken) DecodeResult.Value(())
    else DecodeResult.Error("", new AuthorizationError)
  }(_ => "")

And decodeFailureHandler for 400 to 401 fix:

ServerDefaults.decodeFailureHandler.copy(
  response = { (resp, msg) =>
    if (resp.status == StatusCode.BadRequest && msg == "Invalid value for: header Authorization") {
      ServerDefaults.decodeFailureHandler.response(resp.copy(status = StatusCode.Unauthorized), msg)
    } else {
      ServerDefaults.decodeFailureHandler.response(resp, msg)
    }
  }
)

It fixes body parsing but does not work for path codecs.

Anyway, my hacks is my problems. But in simple case with completely missing auth header tapir doesn't return 401 before path segments parsing!

@adamw
Copy link
Member

adamw commented Jun 28, 2021

Thanks for the feedback!

Not sure if authentication header parsing should run before path parsing. Let's say we have two endpoints:

  1. /a with authentication
  2. /b

in that order. If a /b request comes in, without authentication, shouldn't the first endpoint be skipped, and only the second attempted? This is in-line with request <-> endpoint matching as defined by OpenAPI, that is that the endpoint is uniquely defined by the path + method combination.

@poslegm
Copy link
Author

poslegm commented Jun 28, 2021

Thank you for response. Ok, I seen the problem.

In my case I can move segment validations into handler logic.

Body parsing problem solved currently with my hack and I hope that an elegant solution will be achieved in #1167

@poslegm poslegm closed this as completed Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants