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

.NET SDK throws exception when GetCheckAsync returns allowed: false #851

Closed
5 of 6 tasks
christiansurges opened this issue Mar 9, 2022 · 7 comments · Fixed by #853
Closed
5 of 6 tasks

.NET SDK throws exception when GetCheckAsync returns allowed: false #851

christiansurges opened this issue Mar 9, 2022 · 7 comments · Fixed by #853
Labels
bug Something is not working.

Comments

@christiansurges
Copy link

Preflight checklist

Describe the bug

When using GetCheckAsync of the .NET Keto SDK, an exception is thrown for requests that are not allowed.

Reproducing the bug

I have created a repository to reproduce this behavior. Github Repository

  1. clone repository
  2. start docker containers
    docker-compose up -d
  3. start .NET api
    dotnet run
  4. open webbrowser api swagger url (you may need to change the port)
  5. on swagger hit the CreateExampleRelationTuple endpoint to create an example RealationTuple (or call endpoint directly via endpoint)
  6. use GetCheckAsyncSuccess for an working success example(or call endpoint directly via endpoint)
  7. use GetCheckAsyncFailure for an example with an exception(or call endpoint directly via endpoint)
  8. use GetCheckAsyncWorkaround for an workaround (or call endpoint directly via endpoint)

Relevant log output

No response

Relevant configuration

No response

Version

oryd/keto:v0.8.0

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Docker Compose

Additional Context

No response

@christiansurges christiansurges added the bug Something is not working. label Mar 9, 2022
@zepatrik
Copy link
Member

Thanks for this. As pointed out previously, the status code in case of a false check response is set to 401 on purpose to allow easy integration with other services. Maybe @aeneasr has an idea on how this can be improved in the SDKs? Maybe we can add a second simple endpoint that does not set the status code?
This likely also affects other SDKs.

Current workaround: catch the 401 exception and treat it as a not allowed response.

@aeneasr
Copy link
Member

aeneasr commented Mar 11, 2022

I believe that error status codes (400,500,..) will always return an error response in OpenAPI generated clients. You can type those for better assertiveness, but it will still throw an error!

@zepatrik
Copy link
Member

It is not an error message, it is a legit response:

if allowed {
h.d.Writer().WriteCode(w, r, http.StatusOK, &RESTResponse{Allowed: true})
return
}
h.d.Writer().WriteCode(w, r, http.StatusForbidden, &RESTResponse{Allowed: false})

@aeneasr
Copy link
Member

aeneasr commented Mar 11, 2022

Sorry, I meant error status codes. So everything >= 400

@zepatrik
Copy link
Member

Yes, that is why we should not return an error status code for the SDKs. Not sure what options we have to do that. Do the SDKs set a specific user agent? Or should we add a second route? We could also make it depend on a query parameter.

@aeneasr
Copy link
Member

aeneasr commented Mar 11, 2022

I'd suggest to make this as explicit. Interpreting 200 as "ok this passed" even though it didnt due to some obscure User-Agent behavior or other "auto-detect" feature (even query string) would be quite bad. Actually, I think we need to choose for this endpoint whether to walk back and go with the 200 route again.

@zepatrik
Copy link
Member

OK so having two endpoints makes more sense right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants