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

Clarification Needed on TrustedOrigins Variable Usage #177

Open
1 task done
kokoichi206 opened this issue Jul 27, 2024 · 1 comment
Open
1 task done

Clarification Needed on TrustedOrigins Variable Usage #177

kokoichi206 opened this issue Jul 27, 2024 · 1 comment
Labels

Comments

@kokoichi206
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Based on my understanding and testing, it appears that the variable should actually contain Host values (e.g., github.com) rather than full Origin values (e.g., https://github.com).

csrf/csrf.go

Lines 261 to 262 in a009743

for _, trustedOrigin := range cs.opts.TrustedOrigins {
if referer.Host == trustedOrigin {

This distinction was not immediately clear from the documentation or the code comments (This is stated in the only README).

Expected Behavior

To avoid potential confusion, would it be possible to consider renaming the variable to something more indicative of its intended content, such as TrustedHosts?

If renaming is not feasible for backward compatibility, perhaps adding a more explicit explanation or comment in the code to clarify the expected format of values could be helpful.

csrf/options.go

Lines 123 to 128 in a009743

// TrustedOrigins configures a set of origins (Referers) that are considered as trusted.
// This will allow cross-domain CSRF use-cases - e.g. where the front-end is served
// from a different domain than the API server - to correctly pass a CSRF check.
//
// You should only provide origins you own or have full control over.
func TrustedOrigins(origins []string) Option {

Steps To Reproduce

No response

Anything else?

No response

@kokoichi206
Copy link
Author

This ticket is marked as a bug, but it's more of a suggestion for clarification rather than an issue with the functionality itself.

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

No branches or pull requests

1 participant