search

knpuniversity / oauth2-client-bundle

Easily talk to an OAuth2 server for social functionality in Symfony
https://symfonycasts.com
MIT License
771 stars 146 forks source link

symfony recipe #112

Open ckrack opened 6 years ago

ckrack commented 6 years ago

Hi all, i've created a recipe for the symfony/recipe-contrip repo.

Please take a look at my pull request and let me know if i should change anything: https://github.com/symfony/recipes-contrib/pull/451

The guidelines say that one should not include empty configs. I opted to use a commented-out client, as I'd need to load an example and it's dependencies otherwise. I sticked to the facebook example, as that is used in the docs here, too. Another option would be to add a generic client and add the necessary example code into the bundle, however i believe that would not be the most common use case and could lead to confusion. We could also add a guard and controller to the code, which would be an updated version of what is in the readme here. What do you think?

It'd be great to get a regular symfony core merger to approve the pull request, if everything is alright. I think @weaverryan would qualify?

weaverryan commented 6 years ago

Hey @ckrack!

Awesome! Thanks for doing that!

sticked to the facebook example, as that is used in the docs here, too. Another option would be to add a generic client and add the necessary example code into the bundle, however i believe that would not be the most common use case and could lead to confusion.

I agree. As I said in my review, I think we just shouldn't have any example there - not needed

We could also add a guard and controller to the code, which would be an updated version of what is in the readme here.

Hmm, I don't think we should do this. I like it... but not everyone needs this. And, more importantly, we don't know what client they'll configure, so we can't really generate decent code.

One idea (not sure if it's a good one yet) would be to embed a make: command into this bundle - e.g. make:oauth2-client. This might help them configure their client and generate the controller & Guard stuff. That would be a fairly advanced (but cool) make command :).

One more thing: would you mind creating a pull request to update the README.md file for this repository to be written for Symfony4/Flex (and also, you can assume the recipe will exist)? I think that is very important.

Cheers!

ckrack commented 6 years ago

One idea (not sure if it's a good one yet) would be to embed a make: command into this bundle - e.g. make:oauth2-client. This might help them configure their client and generate the controller & Guard stuff. That would be a fairly advanced (but cool) make command :).

That would be kind of complicated. The maker would have to know about the specific configs of the oauth providers, it would need to modify the yaml config and create opinionated Authenticators/UserProviders. It would also require a pack that includes maker, this bundle, etc.. A copy/paste of the clients and the composer require is a bit easier.

One more thing: would you mind creating a pull request to update the README.md file for this repository to be written for Symfony4/Flex (and also, you can assume the recipe will exist)? I think that is very important.

Did this to my best for the time available right now. I would love to include a provider specific to the client and show what needs to be done with the user entity. However, i think we should split up the docs for this from the readme. I have set up a skeleton project, maybe we could also refer to this instead of copying all the code into markdown.

weaverryan commented 6 years ago

Did this to my best for the time available right now.

You did a great job :).

I would love to include a provider specific to the client and show what needs to be done with the user entity. However, i think we should split up the docs for this from the readme.

Can you explain a bit further what you mean? Perhaps link to the files in the skeleton project you're referring to?

I have set up a skeleton project, maybe we could also refer to this instead of copying all the code into markdown.

Yes... maybe. Having the code inside 1 readable docs is really convenient for users. But, I also always create projects... then copy the code into the README. And, it's not obvious over time if that code gets out of date or has mistakes. So, depending on what you want to document (my previous question), this may indeed be a good idea.

ckrack commented 5 years ago

Here we go with an example: https://github.com/ckrack/symfony-knpu-oauth2-example

Please take a look at src/Security/Authenticator/ and src/Security/Provider/