FriendsOfSymfony / friendsofsymfony.github.io

Description of what FriendsOfSymfony (aka FoS) is about.
http://friendsofsymfony.github.io
32 stars 4 forks source link

[Vote] Adding OAuth bundles/library, and arnaud-lb as member of FOS #28

Closed schmittjoh closed 12 years ago

schmittjoh commented 12 years ago

This is a result of arnaud-lb/AlbOAuth2ServerBundle#4

The vote concerns

I'd kindly ask you to cast your votes :)

rande commented 12 years ago

I am not a FOS member, but +1

stof commented 12 years ago

I'm +1 for adding them if the current contributors to these bundles keep contributing (pushing it to FOS and then stopping to work on them would mean a bad support for the bundle)

Regarding the discussion you linked, I think it makes sense to separate the server-side and the client-side OAuth bundles: the first one being the merge of AlbOAuth2ServerBundle and BazingaOAuthServerBundle, and the second one being based on @mazen's bundle if he wants to move it to FOS (it should probably not be an issue as he first suggested it as part of FOSUserBundle, which was not the good place for it).

Note that there is a fourth bundle dealing with OAuth so @benji07 may be interested by this discussion

Regarding the OAuth library, do you suggest writing a standalone library for OAuth and then using it in the bundles ?

willdurand commented 12 years ago

+1 for a OAuth 1/2 server bundle. Big -1 for mixing both server and client sides. +1 for adding @arnaud-lb as a FOS member.

I ever talked to him about merging both BazingaOAuthServerBundle and AlbOAuth2Bundle. I think it's a good start for a FOSOAuthServerBundle.

schmittjoh commented 12 years ago

Yeah the intention is keep things as decoupled, and re-usable as possible. So, one bundle for client, one for server. We don't necessarily need to add both right away, but I just wanted to see if FOS member generally see a need for adding oauth related repositories.

The library I was referring to is https://github.com/arnaud-lb/oauth2-php which is used inside the bundle.

tecbot commented 12 years ago

+1

stof commented 12 years ago

We discussed it a bit on IRC. Here is a summary of what we said:

server-side: FOSOAuthServerBundle created by merging the 2 existing bundles to support both OAuth 1 and 2.

client-side: improving EtcpasswordOAuthBundle to have a generic configurable client instead of one client for each possible provider, and then provider pre-configured versions for each weel-known providers (need to see if this is possible). the OAuth 1 support also needs to be added, and @benji07's code can eventually be reused here as it supports OAuth1 AFAIK. We also agreed that even if moving the bundle to FOS would increase its visibility, we don't want to create a duplicate of EtcpasswordOAuthBundle so it is either moving it to FOS or contributing to the original bundle depending of what @mazen prefers.

stof commented 12 years ago

and the client-side bundle should ideally be as decoupled from the Security component as possible so that an OAuth client can be built reusing the code when you don't use the Security component (Silex for instance). It would be a good place for a standalone library and a bundle integrating it.

Regarding @arnaud-lb's library, I'm wondering if it is necessary to have the client and the server code in the same library (same concern than the bundles: a client is not a server at the same time). But it can indeed be the starting point for a FOS library.

benji07 commented 12 years ago

+1 for 2 bundle

mazen commented 12 years ago

Since I have limited time at the moment to work on the OAuth Bundle myself I'd rather would like seeing it integrated as a FOS bundle so there is more activity. I won't have time to implement the OAuth 1 stuff myself in the next months so work on that would be appreciated.

willdurand commented 12 years ago

I think we have the solution for the client side: https://github.com/KnpLabs/KnpOAuthBundle

/cc @geoffrey

jmikola commented 12 years ago

:+1: on consolidating what sounds like a few bundles into a two FOS bundles for client and server.

ubermuda commented 12 years ago

My 2cts:

We are using EtcpasswdOAuthBundle in KnpBundles for Github integration, and I've been very disappointed by it:

Those reasons (and the fact that I wanted to learn the security components internals) drove me to start my own OAuth bundle (you can see it there: https://github.com/KnpLabs/KnpOAuthBundle, it's still a WIP, but it's already (in my "not so" humble opinion) superior to EtcpasswdOAuthBundle regarding the first two issues I mentionned).

That said, I'm totally +1 with decoupling the OAuth implementation (standalone lib + security component firewall + bundle), and I'm also willing to contribute to the community effort.

kriswallsmith commented 12 years ago

I don't think we should get into the OAuth1 space at all. Let's focus on energy on OAuth2 only.

merk commented 12 years ago

I agree with Kris - is there any point to OAuth1 support?

Otherwise +1

willdurand commented 12 years ago

OAuth2 is still a draft, but I agree we might forget OAuth 1.0a for the server side, but it's quite different for the client side as it really depends on the provider implementation.

You should watch the following presentation: http://www.slideshare.net/aaronpk/the-current-state-of-oauth-2.

schmittjoh commented 12 years ago

Ok, if I counted correctly, we got a majory in favor of this. I have added @arnaud-lb to FOS, welcome!

@arnaud-lb, could you move your repository to the organization (there should be an option somewhere on the Admin page), and do the namespace/naming changes to the code?

I have also reviewed the library (oauth2-php) a bit more closely, and I'm not sure how much sense it makes to keep it separate, or if we should directly integrated it into the bundle. The library would depend on Symfony's HttpFoundation component, would need several controllers, and also a persistence backend. @arnaud-lb, what do you think here?

arnaud-lb commented 12 years ago

Thanks !

@arnaud-lb, could you move your repository to the organization (there should be an option somewhere on the Admin page), and do the namespace/naming changes to the code?

Sure, I'll do

I have also reviewed the library (oauth2-php) a bit more closely, and I'm not sure how much sense it makes to keep it separate, or if we should directly integrated it into the bundle.

In its current state the library can still be re-used by other projects, with just a dependency on Symfony's HttpFoundation. It just implements the few rules defined by the RFC, leaving persistence and controllers implementation to the client, but the library still saves some time. I'm in favor of keeping it in a separate repository.

stof commented 12 years ago

@arnaud-lb once you move the repo to FOS, create a new team with the same name than the repo and with write access to the repo. As we are all owners, we all have access to all repos. The team is only there for notifications on the repo. Then, each FOS guy will be able to choose whether he wants to add himself in the team

stof commented 12 years ago

@kriswallsmith @merk for the server side, supporting only OAuth2 can be enough (to be discussed as we already have an OAuth1 implementation in the bundle mentionned as source). But for the client side, we definitely need to support both. Twitter uses OAuth1 for instance

stof commented 12 years ago

@arnaud-lb Please move the original repo (and eventually fork it again to your repo with the old name to keep the old url) instead of forking it to FOS. Having the main repo of a bundle marked as fork of an older repo is really confusing for users so we prefer avoiding it now.

willdurand commented 12 years ago

@arnaud-lb You have to transfer ownership. Btw I started to work on renaming on my branch yesterday but you didn't use it... The renaming is better as the bundle should be called FOSOAuthServerBundle. And we need to remove all OAuth prefixes, the namespace is enough. I also noticed an issue with how you manage vendors for the testsuite.

Last thing, we should not use a OAuth lib for that bundle.

arnaud-lb commented 12 years ago

Please move the original repo (and eventually fork it again to your repo with the old name to keep the old url) instead of forking it to FOS. Having the main repo of a bundle marked as fork of an older repo is really confusing for users so we prefer avoiding it now.

ok, done

Btw I started to work on renaming on my branch yesterday but you didn't use it...

Sorry, didn't seen you had done this. Could I let you merge this in the repo ?

Last thing, we should not use a OAuth lib for that bundle.

It's the only oauth2 library I've found for php, and it's not perfect, but maybe we could address problems with it instead of rewriting this code in the bundle ?

stof commented 12 years ago

@willdurand are you working on it ? Otherwise I can use your work and merge it.

willdurand commented 12 years ago

I won't be available until this evening :/

Envoyé de mon iPhone

Le 17 déc. 2011 à 16:06, Christophe Coevoetreply@reply.github.com a écrit :

@willdurand are you working on it ? Otherwise I can use your work and merge it.


Reply to this email directly or view it on GitHub: https://github.com/FriendsOfSymfony/friendsofsymfony.github.com/issues/28#issuecomment-3189740

stof commented 12 years ago

@arnaud-lb what's your plan for the oauth library ? making it a FOS library too or keeping the main repo on your account ?

arnaud-lb commented 12 years ago

For now I've just forked it on FOS, but I don't care if it's maintained on my account or if it become a FOS library.

schmittjoh commented 12 years ago

btw, which bundle should we base the client on? I used neither KnpOAuthBundle, nor EtcpasswdOAuthBundle. What @geoffrey said makes sense, but at the same time the OAuth2 specification allows custom extensions, so I guess we still need some way to allow provider specific logic in addition to the generic part.

Should we base it on KnpOAuthBundle (and adding @geoffrey as member), and merge the parts of EtcpasswdOAuthBundle that make sense, or vice-versa? Any opinions on this?

stof commented 12 years ago

@schmittjoh the points targeted by @geoffrey are points where I also thought the Etcpassword bundle was doing things wrong. So IMO the Knp bundle could be a better starting point as it designs the client in a better way. I suggest opening a separate issue for the client bundle as this one is closed now and contains the whole discussion about the server bundle.

stof commented 12 years ago

@marcw Which bundle are you using for the OAuth implementation in Sensio Connect ? one of the open source bundles mentionned above or another one ?

marcw commented 12 years ago

@stof I use my own bundle, which is not open-sourced

stof commented 12 years ago

if you have some ideas about the way to improve the FOSOAuthServerBundle based on your own work, feel free to open issues (or pull requests) on the bundle :) Our goal being to consolidate existing bundles to avoid duplication, having your inputs on it would be great too

marcw commented 12 years ago

Regarding OAuth1, it is really a complicated subject and I advise you not to lose time implementing those stuffs in a bundle. There's a pecl extension that does it very well (at least on the consumer side, for what I know).

stof commented 12 years ago

btw, are you using OAuth 1 or 2 for Sensio Connect ?

marcw commented 12 years ago

OAuth2. OAuth1 is a PITA, for the provider and the consumer.

willdurand commented 12 years ago

Which draft number ?

OAuth1 is not so complicated.. And it's stable.

Le 18 déc. 2011 à 15:02, Marc Weistroffreply@reply.github.com a écrit :

OAuth2. OAuth1 is a PITA, for the provider and the consumer.


Reply to this email directly or view it on GitHub: https://github.com/FriendsOfSymfony/friendsofsymfony.github.com/issues/28#issuecomment-3194804

marcw commented 12 years ago

Sure, we're not talking about launching a rocket to the moon, but compared to OAuth2, OAuth1is complicated, both server side and client side. Sure it's stable, but as far as I'm concerned, OAuth2, for basic needs, is pretty stable too.