OAuth implementation - Important

[peske]

The key thing that I don't like here is decision to give up on the existing package and develop one from the scratch. I really don't like this. Especially since we are talking about package from the official Google repo, meaning very reliable source. And also especially because it is security-related part.

So I would strongly suggest going with the existing package. If the existing package does not implement everything that we need, we can extend it. So my suggestion is to do the following:

• Fork the official package repo
• Implement the feature(s) we are missing in a meaningful way, to fit the existing package design decisions.
• Create a PR upstream, in the official repo, and offer our extension
• Use forked version in eduvpn project until our PR isn't accepted.
• Switch to the official repo as soon as they accept our PR

The key benefits of such approach are:

• We don't have to worry about maintaining the code. Very reliable and knowledgeable people will do that for us, for free.
• Top-notch pros will deal with this security related piece, and we don't have to worry about that.
• If we make PR for them, we will get instant review of our code by top-notch engineers for free.

I'm not sure what feature is missing in the official package? I see that PKCE is implemented in https://github.com/golang/oauth2/commit/fd043fe589d2d1486b6af56f44a691e819752a23 . @jwijenbergh can you please explain?

[jwijenbergh]

Thanks for looking at the oauth part of the code. To my understanding, oauth PKCE is still not fully supported. See issue number 603 in the oauth2 library that you linked (I do not want to link to it directly to avoid spamming that repo). You'd still have to generate the verifier, challenge and fill in the challenge method yourself. Also note that the support of the PKCE parameters came after I started my own oauth library (after also using go /x/ oauth2 that you linked).

Furthermore ISS is not implemented in the oauth2 library as well https://datatracker.ietf.org/doc/rfc9207/, which we also use. Additionally, the oauth2 project also does not start a web server, so you'd also have to do that yourself.

We could use this library, but I do not see any compelling reason why we should. Just because something falls under the golang's github does not mean they will support it for years and years. If this library was built-in (without being /x/) I would use it in a heartbeat. See https://github.com/eduvpn/eduvpn-common/commit/5065de4cff907b70ea3446888a7bad243744a8ab for some history of where I previously used the library.

I do agree with you that maybe it's useful if I add the missing parts to the oauth2 library, but there will always be a maintenance burden. I do not want to blindly trust third party dependencies. I will see how feasible it is to implement my additions into the library and maintain a fork

[peske]

I understand that some things are not implemented there. But still I think that the library should be used for the things that are implemented. It is much safer this way. I disagree with some of your reasoning:

Just because something falls under the golang's github does not mean they will support it for years and years. If this library was built-in (without being /x/) I would use it in a heartbeat.

/x/ or not, it is still maintained by Google, which is probably the strongest quality guarantee you can get. It cannot be simply removed because OAuth is quite important. It may happen that it will be moved / replaced with something better, but even if it happens you will change the dependency.

I do not want to blindly trust third party dependencies.

I strongly disagree with this. I am not sure what you actually mean, but if you don't want to trust third party dependencies you will end up reinventing the wheel over and over. And this will lead not only to much more work, but also to less quality because you simply can't keep the pace with all the developers out there. Talking about this particular lib, guys who are working there are top of the top in the industry. I know some of them personally, and you can find them on YT giving lectures about Golang. One thing that I'm certain about is that they are better in Golang than both you and me.

But this is just my opinion. The decision is yours.

[jwijenbergh]

I feel like I have clearly stated my points here and the OAuth implementation has seen a great deal of improvements since this issue was opened. If you feel otherwise and have some specific code examples that are not good then we can reopen this.

Thanks for the concerns!