Test failures need investigation. I think I was quite thorough with this removal,rg csrf returns clean.

Unexpectedly, the fix in1eb058d worked to fix the last issue. I think there may be a underlying bug why this test is sensitive to aGET request, but as far as this PR is concerned, its ok.

⚠️ BREAKING⚠️

The_crsf cookie will no longer be set and is no longer necessary to be sent. GET requests that were used to retrieve it are no longer neccessary.

"breaking" usually means end users or site admins must do something, or experience something totally different.

But for this one, I think it isn't really "breaking". WDYT?

And, the TODOs are not necessary (or, we shouldn't "add additional trusted origins", there is no such use case)

And, the TODOs are not necessary (or, we shouldn't "add additional trusted origins", there is no such use case)

Yeah it's more of a "nice to have" thing. No known use case exists for adding more origins.

TestOAuth2Provider/AuthorizeLoginRedirect still needs some fix it seems, it was passing earlier.

TestOAuth2Provider/AuthorizeLoginRedirect still needs some fix it seems, it was passing earlier.

Because this review seems wrong#36183 (comment)

And I made a mistake for the middlewares, now onlyreqSignIn is used as the first commit.

Fixes:#35107

I think this PR doesn't fix 35107. 35107 needs an API

I wonder if it's even necessary to disable COP on any endpoints. It's only active for browsers and if browser headers are not present, the protection will just allow the request which means any API client will be unaffected.

Requests without Sec-Fetch-Site or Origin headers are currently assumed to be either same-origin or non-browser requests, and are allowed.

Fixes:#35107

I think this PR doesn't fix 35107. 35107 needs an API

It does fix the immediate issue, but I agree, what the autor really wants is a API to set up a mirror repo. Maybe we already have one?

I wonder if it's even necessary to disable COP on any endpoints. It's only active for browsers and if browser headers are not present, the protection will just allow the request which means any API client will be unaffected.

I also consider that's unnecessary to disable CrossOriginProtection. So I think we can removeoptSignInAnyOrigin and only useoptSignIn

Not right, see below

Yeah I think we can go strict and have it enabled on all endpoints. If issues arise, we can think about solutions.

Done inda28335, now it's always enabled unconditionally.

Done inda28335, now it's always enabled unconditionally.

Wait, it will cause problems with CORS, according to AI:

If your server implements a policy that rejects Sec-Fetch-Site: cross-site for certain endpoints, the server can block it (e.g., respond 403) even if CORS would otherwise allow it.

So we still need to disable CrossOriginProtection for the CORS endpoints.

For the record, if there are issues, COP offers 2 ways to disable it conditionally:

• https://pkg.go.dev/net/http#CrossOriginProtection.AddInsecureBypassPattern
• https://pkg.go.dev/net/http#CrossOriginProtection.AddTrustedOrigin

Both could be made configurable, if needed.

So we still need to disable CrossOriginProtection for the CORS endpoints.

I'm not sure but yes sounds plausible that CORS is an exception.

I've re-addedoptSignInAnyOrigin and added it anywhere whereoptionsCorsHandler is active. Maybe the option could also be combined intooptionsCorsHandler but I have no idea how to do that.

I think combining these two options is still not good because those route groups also match on non-CORS requests but we probably only want to disable the protection when the request was identified as CORS, e.g. whencorsHandler is non-nil inside theoptionsCorsHandler callback.

I think the cors handler could indicate in the request context that it had a match with a boolean flag, and then the COP handler could act on that flag and be disabled.

I'm not sure your AI answer is right. Go's protection relies onSec-Fetch-Site orOrigin headers and I think one, if not both headers will be present on CORS request, at least for the unsafe methods. There is even aSec-Fetch-Mode: cors, which further reinforces my assumption.

So I'd lean towards having all endpoints protected, without exception. Restrictions can be relaxed later if issues are demonstrated. I will push again the full strict settings.