fix: prevent overwriting URL struct

[MattNotarangelo]

I noticed that OIDC authentication was broken with the message {"code":13,"message":"handling OIDC callback: Provider.Exchange: authentication request state and authorization state are not equal: invalid parameter","details":[]}, beginning with the changes from #1754.

This fix stops the http.Request.URL struct from being overwritten to preserve the query data.

EDIT: Just realised that this is the same change that @GeorgeMac suggested (#1754 (comment))

[GeorgeMac]

Thank you @MattNotarangelo 🙏 Great catch.
Just coming online, I will try and get this released this AM (UK time).

I feel like this should be breaking more than just oauth state.
Going to take a quick minute see if I can fill any missing holes in our integration tests.
(the irony of my suggestion in that original PR coming back to haunt me).

[codecov]

Codecov Report

Merging #1784 (1f12382) into main (e1b4ea0) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1784      +/-   ##
==========================================
+ Coverage   70.43%   70.45%   +0.01%     
==========================================
  Files          58       58              
  Lines        5570     5567       -3     
==========================================
- Hits         3923     3922       -1     
+ Misses       1422     1421       -1     
+ Partials      225      224       -1     

Impacted Files	Coverage Δ
internal/cmd/http.go	3.14% <100.00%> (-0.56%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[GeorgeMac]

OK I found something more interesting here. This piece of code never actually made it into any release.
This is on main, but not in 1.23.1, 1.23.0 or 1.22.*.

So I think it might be not the root cause of your oauth issue @MattNotarangelo
I've seen that oauth error before, usually when restarting Flipt locally and having old cookie state cruft lying around in my browser. I think there may be a subtle issue still there somewhere. I will dig a little further.

All that is to say, your change here is still good and valid. Just trying to figure out why the original issue isn't causing more problem :D

[GeorgeMac]

Yeah when I built Flipt from main I get what I suspected would happen from what you're fixing here:

➜  curl --silent http://localhost:8080/api/v1/flags\?limit=2 | jq '.flags | length'
50

All query parameters getting stripped throughout Flipt.
We have integration tests which should be catching this, so going to check those.
(Thank you again for bringing this up)

Update: There is a hole in our integration tests which this bug found its way around.
I am updating them now.

[MattNotarangelo]

Thanks @GeorgeMac, I appreciate the prompt response 🙏

[GeorgeMac]

Hey @MattNotarangelo qq. were you using an official release of Flipt? or were you running on e.g. nightly?

The bug you shared does look like it would've been caused by the state parameter being dropped altogether on the callback. But I can't see this code in any of our tagged releases. Wondering if you were on nightly or a build off main?

[MattNotarangelo]

Hey, I'm currently running v1.20 and wanted to play around with some of the newer features since that release. I used the latest commit on main and noticed the issue

[GeorgeMac]

Thanks @MattNotarangelo 🙏 I think that explains it as the trailing slash code was on main.

[markphelps]

@all-contributors please add @MattNotarangelo for code

[allcontributors]

@markphelps

I've put up a pull request to add @MattNotarangelo! 🎉