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

401 response on OPTIONS #24

Closed
inventivejon opened this issue Oct 28, 2022 · 4 comments
Closed

401 response on OPTIONS #24

inventivejon opened this issue Oct 28, 2022 · 4 comments

Comments

@inventivejon
Copy link

inventivejon commented Oct 28, 2022

During testing this plugin we noticed having issues when trying to perform a GET Operation from Browser Chrome. Chrome was execution a preflight operation where it is first requesting OPTIONS to the endpoint before executing a GET. Since OPTIONS does not contain the password in case it was sent within the Headers, OPTIONS is refused with a 401. This can cause a CORS issue on the GET operation.

We found a workaround by explicitely excluding OPTIONS from the jwt-check. But we wonder if this should be respected within this plugin itself :).

This is the example for the workaround:

<your-domain> {
	@exclude-options {
		not method OPTIONS
	}

	route @exclude-options {
		jwtauth {
			sign_key "
<JWTAUTH>
    "
			from_header X-Api-Token
			issuer_whitelist <Your Issuer>
			audience_whitelist <audience>
		}
	}
	respond "hello world"
}
@ggicci
Copy link
Owner

ggicci commented Oct 29, 2022

Thanks so much @inventivejon for the feedback. After reading the specification of OPTIONS requests, I decided not to handle it in this plugin. Because the logic of how to respond to an OPTIONS request can be complex at times. And it can be customized through Caddy directives.

@inventivejon
Copy link
Author

I understand your point. Do you think it would help to add it to your README? I guess it will be something a lot of people stumble over. Or do you think the problem is solved the wrong way by the workaround?

@ggicci ggicci assigned ggicci and unassigned ggicci Oct 30, 2022
@ggicci
Copy link
Owner

ggicci commented Oct 30, 2022

Sure, it would be great helpful. I also experienced the same issue before. Which cost me some time doing research on how to deal with OPTIONS requests with Caddy.

The above workaround looks good to me. But I prefer handling OPTIONS requests separately in the same route before any other directives:

myapp.localhost {
  @options {
    method OPTIONS
  }

  route /protected/* {
    # CORS
    header {
      access-control-allow-origin      "https://myapp.localhost"
      access-control-allow-headers     "content-type,x-myapp-client"
      access-control-allow-methods     "OPTIONS,HEAD,GET,POST,PUT,PATCH,DELETE"
      access-control-allow-credentials "true"
    }

    # respond 204 to OPTIONS requests
    respond @options 204

    jwtauth {
      # ...
    }

    respond "protected resource"
  }

}

@ggicci
Copy link
Owner

ggicci commented Oct 31, 2022

Hi @inventivejon, I'm closing this issue since I've already added a link to this issue in the FAQ section in the README.

@ggicci ggicci closed this as completed Oct 31, 2022
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

No branches or pull requests

2 participants