Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider moving to a template generation model #46

Closed
kevinchalet opened this issue Jan 10, 2016 · 5 comments
Closed

Consider moving to a template generation model #46

kevinchalet opened this issue Jan 10, 2016 · 5 comments

Comments

@kevinchalet
Copy link
Member

Currently, OpenIddict.Mvc exposes a special (generic) controller and a few embedded assets/views to handle the entire authorization flow. Though really convenient for people who don't want to customize the controller or the view models, this approach offers a less flexible experience than the generated controller/models used by Identity.

The question is: should we move to a template generation model (probably based on yeoman) or should we keep the existing approach?

/cc @damccull @ilmax @Bartmax

@Bartmax
Copy link
Member

Bartmax commented Jan 10, 2016

There isn't a really and standard way of managing controllers/view. That's something I would expect to change soon since is a request for various authors, and even more now that everything is more "composable".

That being said, I think the normal workflow for someone is to :

  • install package
  • quickly make it work and get a feel of it
  • customize little
  • use it lightly
  • customize heavily.

If we fail to #2 or #5 we put user satisfaction in danger.

I'm not sure if everything can be done, here some ideas:

Create a separate package. OpenIddict.Frontend with a dependency on core. But instead of having everything embedded and compiled to install the actual controllers / views in the project. (This is not very upgrade able friendly)

create a template for all the controllers/views. (Identity does this for file new, but file new is an awful experience, if we can do like right click add new item select openiddict client views and controller and then pick some info (like dbcontext, user entity) that would be great.

Yeoman have some limitations (Don't know exactly) and I think that's one reason why aspnet didn't go heavily with it.

We also need to think cross platform solution so maybe we can do a "command" like EntityFramework to create/download the assets.

For client side normally one goes with npm/bower. We can explore if those solution can be useful. Right now there is no npm not bower package that includes .cshtml but doesn't mean they won't exist. We would need some task for grunt/gulp to actually copy this files in the correct location so looks like a lot of investment but I won't discard without taking a look at it.

Can we have a post restore command on package? So that after installing the dll can execute something that creates/download the assets? Can this be optional?

We can also ask the asp.net for help and guidance, they are aware of authors of packages with views/controllers needs. While they don't have a solution yet, they may have some ideas.

@atrauzzi
Copy link
Contributor

Here's my first round of thoughts on the matter... I'm sure additional information could influence how I feel, so do help me out if I have something wrong here :)

In my travels creating packages for a different framework and language (although with similar challenges and structures to ASP.NET & C#), I landed on the practice of including controllers in my package, but not trying to wire them into the routing system.

If the project dev wanted to use my defaults, I'd offer a facility that quickly sets up all the routes to point to my default controllers. In C#, this would likely done as an optional extension method on the MVC route building interface.

If however they didn't want to use my defaults, they could either create their own controllers and integrate with the library manually, or they could even extend my default controllers and override specific parts of functionality.

This approach resulted in maximum flexibility. Devs could slide up and down the scale based on their needs without being restricted at any point. It also obviated scaffolding for the most part and played nicely with the framework, allowing DI to continue to shine.

Openiddict seems close to this, although I wonder if it could just be rolled in with the main OpenIdConnectServer repository at that point. The main thing that would have to change is that it would have to not use attributes for routing and middleware/filtering.

tldr;

  • I use nodejs stuff for front-end, but I don't like having to use yeoman for ASP.NET
  • Don't use attribute routing
  • Rely on the language and DI to offer extension points
  • Based on the last 2 items, Merge openiddict with OpenIdConnect

@ilmax
Copy link
Contributor

ilmax commented May 18, 2016

This is a hard one :)

Security is hard, and so provide maximum flexibility yet ensuring the correct behavior of the system should be hard as well.

I completely agree with @Bartmax for the typical user workflow, so we have to get up & running very quickly (e.g. provide a meta package that brings in all required dependencies)

We need to figure out which pieces of the puzzle are the most frequently changed/replaced by the end user and factor that pieces out.

IMO the UI is the part that will probably be replaced most of the time, but there are a discrete amount of interaction between front-end and back-end that are not so obvious to the average developer (read "not a security guy that loves reading RFCs" 😄 ) so we need to document all those interactions to provide all necessary guidance.

I think that template generation can be a nice approach, I'm not sure however if a VS template can be installed via nuget (a VS plug-in would be less than ideal IMO), and we should also keep in mind cross platform as well as mentioned by @Bartmax.

I like the cli commands approach but I have no experience on this subject and looking through EF issues seems to be not so easy to implement it right.

@ilmax
Copy link
Contributor

ilmax commented May 24, 2016

See related issues

#123

@kevinchalet
Copy link
Member Author

Closing, as OpenIddict.Mvc and OpenIddict.Assets have been removed and people are now encouraged to create their own controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants